Review request: MemoryMeta changes and new iPod collection

Bart Cerneels bart.cerneels at kde.org
Wed Jan 11 10:45:59 UTC 2012


On Tue, Jan 10, 2012 at 14:55, Matěj Laitl <laitlmat at fjfi.cvut.cz> wrote:
> Hi list and mainly Bart,
> please review memorymeta-tweaks branch [1] in my personal git clone which
> contains changes that I needed for iPod collection rewrite.
>
> [1]
> http://quickgit.kde.org/?p=clones%2Famarok%2Flaitl%2Famarok.git&a=shortlog&h=refs/heads/memorymeta-
> tweaks
>
> UmsCollection would get support for following for free is memorymeta-tweaks is
> merged:
>  * album artists (was hard-coded to track artists previously)
>  * less memory leaks due to circular referencing removal
>  * capability forwarding: editing tags thanks to EditCapability.
>
> ChangeLog is added for above entries.
>
> With a small bit of work, UmsCollection could get support for:
>  * album covers if MetaFile::FileAlbum::{image(),hasImage()} is implemented
>  * compilations if MetaFile::FileAlbum::isCompilation() is implemented (see
> ArtistHelper::bestGuessAlbumArtist())
>  * track removing reflected in collection browser, call
> MapChanger::removeTrack() perhaps somewhere in
> UmsCollectionLocation::removeUrlsFromCollection()
>  * track metadata changes reflected in collection browser if you observe tracks
> in UmsCollection and then call MapChanger::trackChanged() in
> metadataChanged(). You should emit updated() when trackChanged() returns true.
> Beware that it cannot currently cope with changes to track uidUrl().
>
> ChangeLog is not updated with above entries as the changes are not user-
> visible anywhere in Amarok yet.

It all looks completely sane and functional to me. Nice touch
preventing possible threading issues in MemoryMeta.


>
> All of the MemoryMeta changes are already used & tested in iPod collection
> rewrite that can be found in the ipod-rewrite branch [2], which is nearly
> merge-ready.
>
> [2]
> http://quickgit.kde.org/?p=clones%2Famarok%2Flaitl%2Famarok.git&a=shortlog&h=refs/heads/ipod-
> rewrite
>

I'll test this later.


More information about the Amarok-devel mailing list