----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103715/ -----------------------------------------------------------
Review request for Amarok, Bart Cerneels and Ralf Engels. Description ------- MetaFile: guess album artist as in SQL collection scanner, compilations Call albumArtist = ArtistHelper::bestGuessAlbumArtist( albumArtist, artist, genre, composer ); afer the data are read from file tags. This makes album artist field consistent for tracks in Local collection and tracks in file browser or drag & dropped to Amarok playlist, but it has some downsides: * Amarok will essentially lie about what album artist is assigned to a track, most notably will replace empty album artist by track artist and will replace "Various Artists" by empty artist. * Editing album artist will be strange in corner cases: in particular if user sets album artist to "Various Artists", it will be written to file tags as-is, but displayed as empty album artist. Other case would be when user sets album artist to empty string, but this is currently prevented due to a bug/feature in TagDialog. (every occurrence of "Various Artists" also accepts localized version of the string) Above change is needed for upcoming "MemoryCollection: use also album artist as identifying key in album map" change for UmsCollection, but if someone opposes, similar change can be made on UmsCollection level. Also, implement MetaFile::FileAlbum isCompilation() which is now easy as it can return albumArtist.isEmpty() DIGEST: unify album artist handling in Amarok -------------------------------------------------------------------- MemoryCollection: use also album artist as identifying key in album map This has some far-reaching consequences which I believe are only positive, most notably Amarok is able to differentiate between 2 albums with same name but different album artists in USB Mass Storage collection and in "new" iPod collection. This unifies album artist handling across MemoryCollection and SqlCollection, 2 major collection storage implementations in Amarok. Technically, AlbumMap is changed from: typedef QMap<QString, Meta::AlbumPtr> AlbumMap; to: class AlbumMap : public QMap<Meta::AlbumKey, Meta::AlbumPtr> with insert(), remove(), value(), contains() QMap methods hidden and reimplemented for convenience and to prevent coding mistakes. (you no longer can add album under wrong key) This change is tested to work with new iPod collection, UMS collection (depends on previous "MetaFile: guess album artist as in SQL collection scanner, compilations"), all tests still pass. Only area that received just very basic testing are online service collections that are also affected by this change. DIGEST: Albums with same name but different album artist are now correctly separated in USB Mass Storage, iPod and various online service collections. Diffs ----- ChangeLog ed4e955deeebf6a506e87ca8973645b9eeadc67b src/core-impl/collections/daap/daapreader/Reader.cpp 9f749c5fe892f0cbcbacd8454d8fc80940f6bb43 src/core-impl/collections/ipodcollection/handler/IpodHandler.h ceccd7cc3028051d9b93201caea180a10af15ed3 src/core-impl/collections/ipodcollection/handler/IpodHandler.cpp 83c5d27839dcb5f18a4233daa5367fb18962c7af src/core-impl/collections/mediadevicecollection/MediaDeviceMeta.cpp e892952ba0ede760a66e71ffa9e43c123b74a6d7 src/core-impl/collections/mediadevicecollection/handler/MediaDeviceHandler.cpp 46000b3f1950cc64feef8321bcf698980c2742fe src/core-impl/collections/playdarcollection/PlaydarCollection.cpp 1e598f0d20ea88bef4c60877eefb71a8b934218d src/core-impl/collections/support/MemoryCollection.h e2b28dea72393456f5c62ecf06ce7832b49e2e4f src/core-impl/collections/support/MemoryMatcher.cpp 624d6d4c0eb63745293d083982b936e5c050ff0a src/core-impl/collections/support/MemoryMeta.h 45eefb7a4c085fa32478799e296579eef117072b src/core-impl/collections/support/MemoryMeta.cpp PRE-CREATION src/core-impl/collections/upnpcollection/UpnpCache.h da00fd1b11306ef49e10b703f601147a5762e18e src/core-impl/collections/upnpcollection/UpnpCache.cpp 58130795298911ab1849fdc64314b8ed128df3d4 src/core-impl/meta/file/File_p.h af2dbf670e23357dca0efc48421c7d0d6f884f32 src/core/meta/support/MetaKeys.h f4ffa0eb9889787b1cebb9ea4781930fbcf3fd01 src/core/meta/support/MetaKeys.cpp 3e8f0fa8587badf044c6a00806230adfe257a6fd src/services/ampache/AmpacheServiceQueryMaker.cpp 72862b917fb397b02187d14cd97c1c2c5fa4f79d tests/TestTrackOrganizer.cpp 75faf04b332e9e9c0ff279037ae66053e1011cd4 tests/browsers/TestSingleCollectionTreeItemModel.cpp 99ec36f7eae0560024669e63ca5102829d610748 tests/synchronization/TestMasterSlaveSynchronizationJob.cpp 707e578d042c3b13ecea7069c1d8b496896a3c5c tests/synchronization/TestOneWaySynchronizationJob.cpp 058611fd8fdde491e6b2eb62c5895e20324613b1 tests/synchronization/TestUnionJob.cpp 591d695d1ea0641951e5aa7188a697f98b988b5c Diff: http://git.reviewboard.kde.org/r/103715/diff/diff Testing ------- This change is tested to work with new iPod collection, UMS collection, all tests still pass. Only area that received just very basic testing are online service collections that are also affected by this change. Screenshots ----------- amarok-ums-collections-and-album-artists.png http://git.reviewboard.kde.org/r/103715/s/416/ Thanks, Matěj Laitl
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel