Review Request: Refactoring of MusicBrainz finder subsystem. Improved support of MPC and MP4 files. "Album Artist" field support.

[Ralf Engels]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100084/#review192
-----------------------------------------------------------


I am still a little bit concerned about the changes to SqlMeta.cpp

The owner is not really needed as you can determine that from the uidUrl. It is also quite future proof. 
Allowing multiple uids would be nice, but that is clearly out of this scope.

Also changing the album artist is a change that should be thought about. 
I would rather move the track into a new album with the new album artist instead of changing the current album.
I just realized: Do you check if there is already another album with the same title and album artist?

For the rest of the MusicBrainz changes I trust in you. You are the expert.

- Ralf


On 2010-10-24 07:07:16, Sergey Ivanov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100084/
> -----------------------------------------------------------
> 
> (Updated 2010-10-24 07:07:16)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> -------
> 
> Rewrite (a bit) MusicBrainz tagger to use QVariantMap instead of specific classes to store tracks data. Search result threshold.
> Add code for reading/writing MusicBrainz and AFT (used as default if no uid_owner specified) UIDs from/to MPC and MP4 files.
> Add support of MPC and MP4 formats for AFTTagger utility.
> Some try to handle "Album Artist" data (ability to store/load It in/from file). Support of updating/setting "Album Artist" for albums in SQLCollection.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 45a9493 
>   src/core-impl/capabilities/timecode/TimecodeEditCapability.h c9f3e73 
>   src/core-impl/capabilities/timecode/TimecodeEditCapability.cpp 4bddd84 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceMeta.h fa57e0a 
>   src/core-impl/collections/mediadevicecollection/MediaDeviceMeta.cpp f277a40 
>   src/core-impl/collections/proxycollection/ProxyCollectionMeta.cpp f800987 
>   src/core-impl/collections/sqlcollection/CapabilityDelegateImpl.cpp 78fd02b 
>   src/core-impl/collections/sqlcollection/SqlMeta.h b92e351 
>   src/core-impl/collections/sqlcollection/SqlMeta.cpp 6b29b6e 
>   src/core-impl/meta/file/File.h c791ccd 
>   src/core-impl/meta/file/File.cpp b807cba 
>   src/core-impl/meta/file/File_p.h 4f120a9 
>   src/core-impl/meta/file/TagLibUtils.cpp ad1a07e 
>   src/core-impl/meta/proxy/MetaProxy.h 32f2f76 
>   src/core-impl/meta/proxy/MetaProxy.cpp 8cc01a1 
>   src/core/capabilities/EditCapability.h 2406c1e 
>   src/core/meta/support/MetaConstants.h 40cad34 
>   src/core/meta/support/MetaUtility.cpp 0bb29db 
>   src/dialogs/MusicBrainzTagger.h d053e88 
>   src/dialogs/MusicBrainzTagger.cpp 6f18606 
>   src/dialogs/TagDialog.cpp 4a0e94f 
>   src/musicbrainz/MusicBrainzFinder.h 6fe3a47 
>   src/musicbrainz/MusicBrainzFinder.cpp f65e25d 
>   src/musicbrainz/MusicBrainzMeta.h PRE-CREATION 
>   src/musicbrainz/MusicBrainzMetaClasses.h b9f7d0c 
>   src/musicbrainz/MusicBrainzMetaClasses.cpp da83c16 
>   src/musicbrainz/MusicBrainzTagsModel.h cc1552e 
>   src/musicbrainz/MusicBrainzTagsModel.cpp f1dd54a 
>   src/musicbrainz/MusicBrainzTrackListModel.cpp 442e662 
>   src/musicbrainz/MusicBrainzXmlParser.h b0e9089 
>   src/musicbrainz/MusicBrainzXmlParser.cpp b57baad 
>   src/musicbrainz/MusicDNSFinder.h 474c998 
>   src/musicbrainz/MusicDNSFinder.cpp c026a8e 
>   src/musicbrainz/MusicDNSXmlParser.h a9b67e0 
>   tests/core-impl/collections/proxycollection/TestProxyCollectionMeta.cpp 6fadb72 
>   tests/core-impl/collections/sqlcollection/TestAlbumCompilationChange.h daa35c0 
>   tests/core-impl/collections/sqlcollection/TestAlbumCompilationChange.cpp 84a0164 
>   utilities/afttagger/AFTTagger.h 8921875 
>   utilities/afttagger/AFTTagger.cpp 2aa9aef 
>   utilities/collectionscanner/AFTUtility.cpp fde3bfa 
> 
> Diff: http://git.reviewboard.kde.org/r/100084/diff
> 
> 
> Testing
> -------
> 
> Tested as I could. Works fine.
> Found out that I have ~5 different "Best Of" albums of different artists, but not the only 1 as I thought before. :)
> 
> 
> Thanks,
> 
> Sergey
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20101025/e71a954b/attachment.htm