[kde-doc-english] [amarok] /: Drop support for treating MusicBrainz ids as track unique ids

Matěj Laitl matej at laitl.cz
Thu Feb 21 12:10:02 UTC 2013


Git commit 216c18bdaf18acf28e9ca98b115623934c9b4401 by Matěj Laitl.
Committed on 21/02/2013 at 01:30.
Pushed by laitl into branch 'master'.

Drop support for treating MusicBrainz ids as track unique ids

No comments on my mail inquiry, which I treat as a consent. Since 2.6
the SqlScanResultProcessor should cope with changed track ids just fine.

CHANGES:
 * Problematic support for treating MusicBrainz ids as track unique ids
   was dropped; should avoid surprising "Duplicate Tracks Found" errors.

@Alberto, you'll probably need to rebase your patch as I've touched
MusicBrainzFinder.cpp

BUG: 315329
GUI: Mentions of MusicBrainz ids being useful for AFT (Amarok File Tracking)
     in various documentation need to be removed
CCMAIL: amarok-devel at kde.org
CCMAIL: Alberto Villa <avilla at FreeBSD.org>

M  +2    -0    ChangeLog
M  +0    -6    shared/tag_helpers/APETagHelper.cpp
M  +0    -6    shared/tag_helpers/ASFTagHelper.cpp
M  +0    -6    shared/tag_helpers/ID3v2TagHelper.cpp
M  +0    -6    shared/tag_helpers/MP4TagHelper.cpp
M  +1    -9    shared/tag_helpers/TagHelper.cpp
M  +0    -1    shared/tag_helpers/TagHelper.h
M  +0    -7    shared/tag_helpers/VorbisCommentTagHelper.cpp
M  +5    -4    src/core/meta/Meta.h
M  +0    -2    src/musicbrainz/MusicBrainzFinder.cpp
M  +0    -5    src/services/lastfm/ScrobblerAdapter.cpp
M  +1    -1    tests/core-impl/collections/db/sql/TestSqlTrack.cpp

http://commits.kde.org/amarok/216c18bdaf18acf28e9ca98b115623934c9b4401

diff --git a/ChangeLog b/ChangeLog
index 1561f76..3a1a6d2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -12,6 +12,8 @@ VERSION 2.8-Beta 1
    * Added Ctrl+H shortcut to randomize playlist, patch by Harsh Gupta. (BR 208061)
 
   CHANGES:
+   * Problematic support for treating MusicBrainz ids as track unique ids was dropped;
+     should avoid surprising "Duplicate Tracks Found" errors. (BR 315329)
    * Fancy behavior of some context menus showing different actions when Shift
      key is held has been reverted. All entries are now shown all the time.
    * The dynamic playlist behaviour has changed. It will not longer
diff --git a/shared/tag_helpers/APETagHelper.cpp b/shared/tag_helpers/APETagHelper.cpp
index e74cc08..c628694 100644
--- a/shared/tag_helpers/APETagHelper.cpp
+++ b/shared/tag_helpers/APETagHelper.cpp
@@ -40,7 +40,6 @@ APETagHelper::APETagHelper( TagLib::Tag *tag, TagLib::APE::Tag *apeTag, Amarok::
     m_fieldMap.insert( Meta::valScore,       TagLib::String( "FMPS_RATING_AMAROK_SCORE" ) );
 
     m_uidFieldMap.insert( UIDAFT,            TagLib::String( "Amarok 2 AFTv1 - amarok.kde.org" ) );
-    m_uidFieldMap.insert( UIDMusicBrainz,    TagLib::String( "MusicBrainz Track Id" ) );
 }
 
 Meta::FieldHash
@@ -64,11 +63,6 @@ APETagHelper::tags() const
         }
         else if( it->first == uidFieldName( UIDAFT ) && isValidUID( value, UIDAFT ) )
             data.insert( Meta::valUniqueId, value );
-        else if( it->first == uidFieldName( UIDMusicBrainz ) && isValidUID( value, UIDMusicBrainz ) )
-        {
-            if( !data.contains( Meta::valUniqueId ) ) // we prefer AFT uids
-                data.insert( Meta::valUniqueId, value.prepend( "mb-" ) );
-        }
     }
 
     return data;
diff --git a/shared/tag_helpers/ASFTagHelper.cpp b/shared/tag_helpers/ASFTagHelper.cpp
index 4956330..966c298 100644
--- a/shared/tag_helpers/ASFTagHelper.cpp
+++ b/shared/tag_helpers/ASFTagHelper.cpp
@@ -43,7 +43,6 @@ ASFTagHelper::ASFTagHelper( TagLib::Tag *tag, TagLib::ASF::Tag *asfTag, Amarok::
     m_fieldMap.insert( Meta::valScore,       TagLib::String( "FMPS/Rating_Amarok_Score" ) );
 
     m_uidFieldMap.insert( UIDAFT,            TagLib::String( "Amarok/AFTv1" ) );
-    m_uidFieldMap.insert( UIDMusicBrainz,    TagLib::String( "MusicBrainz/Track Id" ) );
 }
 
 Meta::FieldHash
@@ -94,11 +93,6 @@ ASFTagHelper::tags() const
         }
         else if( it->first == uidFieldName( UIDAFT ) && isValidUID( strValue, UIDAFT ) )
             data.insert( Meta::valUniqueId, strValue );
-        else if( it->first == uidFieldName( UIDMusicBrainz ) && isValidUID( strValue, UIDMusicBrainz ) )
-        {
-            if( !data.contains( Meta::valUniqueId ) ) // we prefer AFT uids
-                data.insert( Meta::valUniqueId, strValue.prepend( "mb-" ) );
-        }
     }
 
     return data;
diff --git a/shared/tag_helpers/ID3v2TagHelper.cpp b/shared/tag_helpers/ID3v2TagHelper.cpp
index 1baab1e..7d50ca6 100644
--- a/shared/tag_helpers/ID3v2TagHelper.cpp
+++ b/shared/tag_helpers/ID3v2TagHelper.cpp
@@ -55,7 +55,6 @@ ID3v2TagHelper::ID3v2TagHelper( TagLib::Tag *tag, TagLib::ID3v2::Tag *id3v2Tag,
     m_fmpsFieldMap.insert( FMPSScore,        TagLib::String( "FMPS_Rating_Amarok_Score" ) );
 
     m_uidFieldMap.insert( UIDAFT,            TagLib::String( "Amarok 2 AFTv1 - amarok.kde.org" ) );
-    m_uidFieldMap.insert( UIDMusicBrainz,    TagLib::String( "http://musicbrainz.org" ) );
 }
 
 Meta::FieldHash
@@ -85,11 +84,6 @@ ID3v2TagHelper::tags() const
 
                 if( frame->owner() == uidFieldName( UIDAFT ) && isValidUID( identifier, UIDAFT ) )
                     data.insert( Meta::valUniqueId, identifier );
-                else if( frame->owner() == uidFieldName( UIDMusicBrainz ) && isValidUID( identifier, UIDMusicBrainz ) )
-                {
-                    if( !data.contains( Meta::valUniqueId ) ) // we prefer AFT uids
-                        data.insert( Meta::valUniqueId, identifier.prepend( "mb-" ) );
-                }
                 continue;
             }
             else if( field == Meta::valHasCover )
diff --git a/shared/tag_helpers/MP4TagHelper.cpp b/shared/tag_helpers/MP4TagHelper.cpp
index e14b43e..37fcc17 100644
--- a/shared/tag_helpers/MP4TagHelper.cpp
+++ b/shared/tag_helpers/MP4TagHelper.cpp
@@ -46,7 +46,6 @@ MP4TagHelper::MP4TagHelper( TagLib::Tag *tag, TagLib::MP4::Tag *mp4Tag, Amarok::
     m_fieldMap.insert( Meta::valScore,       TagLib::String( "----:com.apple.iTunes:FMPS_Rating_Amarok_Score" ) );
 
     m_uidFieldMap.insert( UIDAFT,            TagLib::String( "----:com.apple.iTunes:Amarok 2 AFTv1 - amarok.kde.org" ) );
-    m_uidFieldMap.insert( UIDMusicBrainz,    TagLib::String( "----:com.apple.iTunes:MusicBrainz Track Id" ) );
 }
 
 Meta::FieldHash
@@ -92,11 +91,6 @@ MP4TagHelper::tags() const
         }
         else if( it->first == uidFieldName( UIDAFT ) && isValidUID( value, UIDAFT ) )
             data.insert( Meta::valUniqueId, value );
-        else if( it->first == uidFieldName( UIDMusicBrainz ) && isValidUID( value, UIDMusicBrainz ) )
-        {
-            if( !data.contains( Meta::valUniqueId ) ) // we prefer AFT uids
-                data.insert( Meta::valUniqueId, value.prepend( "mb-" ) );
-        }
     }
 
     return data;
diff --git a/shared/tag_helpers/TagHelper.cpp b/shared/tag_helpers/TagHelper.cpp
index 4985a4e..b8b96c4 100644
--- a/shared/tag_helpers/TagHelper.cpp
+++ b/shared/tag_helpers/TagHelper.cpp
@@ -179,13 +179,7 @@ TagHelper::splitUID( const QString &uidUrl ) const
     if( uid.startsWith( "amarok-" ) )
         uid = uid.remove( QRegExp( "^(amarok-\\w+://).+$" ) );
 
-    if( uid.startsWith( "mb-" ) )
-    {
-        uid = uid.mid( 3 );
-        if( isValidUID( uid, UIDMusicBrainz ) )
-            type = UIDMusicBrainz;
-    }
-    else if( isValidUID( uid, UIDAFT ) )
+    if( isValidUID( uid, UIDAFT ) )
         type = UIDAFT;
 
     return qMakePair( type, uid );
@@ -224,8 +218,6 @@ TagHelper::isValidUID( const QString &uid, const TagHelper::UIDType type ) const
 
     if( type == UIDAFT )
         regexp.setPattern( "^[0-9a-fA-F]{32}$" );
-    else if( type == UIDMusicBrainz )
-        regexp.setPattern( "^[0-9a-fA-F]{8}(-[0-9a-fA-F]{4}){3}-[0-9a-fA-F]{12}$" );
 
     return regexp.exactMatch( uid );
 }
diff --git a/shared/tag_helpers/TagHelper.h b/shared/tag_helpers/TagHelper.h
index 4af13cb..898a76b 100644
--- a/shared/tag_helpers/TagHelper.h
+++ b/shared/tag_helpers/TagHelper.h
@@ -43,7 +43,6 @@ namespace Meta
                 enum UIDType
                 {
                     UIDInvalid     = 0,
-                    UIDMusicBrainz = 1,
                     UIDAFT         = 3
                 };
 
diff --git a/shared/tag_helpers/VorbisCommentTagHelper.cpp b/shared/tag_helpers/VorbisCommentTagHelper.cpp
index dbec695..520e32e 100644
--- a/shared/tag_helpers/VorbisCommentTagHelper.cpp
+++ b/shared/tag_helpers/VorbisCommentTagHelper.cpp
@@ -51,7 +51,6 @@ VorbisCommentTagHelper::VorbisCommentTagHelper( TagLib::Tag *tag, TagLib::Ogg::X
     m_fieldMap.insert( Meta::valScore,       TagLib::String( "FMPS_RATING_AMAROK_SCORE" ) );
 
     m_uidFieldMap.insert( UIDAFT,            TagLib::String( "AMAROK 2 AFTV1 - AMAROK.KDE.ORG" ) );
-    m_uidFieldMap.insert( UIDMusicBrainz,    TagLib::String( "MUSICBRAINZ_TRACKID" ) );
 }
 
 VorbisCommentTagHelper::VorbisCommentTagHelper( TagLib::Tag *tag, TagLib::Ogg::XiphComment *commentsTag, TagLib::FLAC::File *file, Amarok::FileType fileType )
@@ -69,7 +68,6 @@ VorbisCommentTagHelper::VorbisCommentTagHelper( TagLib::Tag *tag, TagLib::Ogg::X
     m_fieldMap.insert( Meta::valScore,       TagLib::String( "FMPS_RATING_AMAROK_SCORE" ) );
 
     m_uidFieldMap.insert( UIDAFT,            TagLib::String( "AMAROK 2 AFTV1 - AMAROK.KDE.ORG" ) );
-    m_uidFieldMap.insert( UIDMusicBrainz,    TagLib::String( "MUSICBRAINZ_TRACKID" ) );
 }
 
 static inline bool
@@ -135,11 +133,6 @@ VorbisCommentTagHelper::tags() const
         }
         else if( it->first == uidFieldName( UIDAFT ) && isValidUID( value, UIDAFT ) )
             data.insert( Meta::valUniqueId, value );
-        else if( it->first == uidFieldName( UIDMusicBrainz ) && isValidUID( value, UIDMusicBrainz ) )
-        {
-            if( !data.contains( Meta::valUniqueId ) ) // we prefer AFT uids
-                data.insert( Meta::valUniqueId, value.prepend( "mb-" ) );
-        }
         else if( it->first == VORBIS_PICTURE_TAG )
         {
             if( parsePictureBlock( it->second ) )
diff --git a/src/core/meta/Meta.h b/src/core/meta/Meta.h
index 23fe796..d0fea20 100644
--- a/src/core/meta/Meta.h
+++ b/src/core/meta/Meta.h
@@ -204,10 +204,11 @@ namespace Meta
             virtual KUrl playableUrl() const = 0;
             /** an url for display purposes */
             virtual QString prettyUrl() const = 0;
-            /** an url which is unique for this track. Use this if you need a key for the track.
-                Actually this is not guaranteed to be an url at all and could be something like
-                mb-f5a3456bb0 for a MusicBrainz id.
-            */
+
+            /**
+             * A fake url which is unique for this track. Use this if you need a key for
+             * the track.
+             */
             virtual QString uidUrl() const = 0;
 
             /** Returns whether playableUrl() will return a playable Url
diff --git a/src/musicbrainz/MusicBrainzFinder.cpp b/src/musicbrainz/MusicBrainzFinder.cpp
index cb6d122..724c04d 100644
--- a/src/musicbrainz/MusicBrainzFinder.cpp
+++ b/src/musicbrainz/MusicBrainzFinder.cpp
@@ -409,8 +409,6 @@ MusicBrainzFinder::sendTrack( const Meta::TrackPtr track, const QVariantMap &inf
         }
     }
 
-    tags.insert( Meta::Field::UNIQUEID, tags.value( MusicBrainz::TRACKID ).toString().prepend( "mb-" ) );
-
     //Clean metadata from unused fields
     tags.remove( Meta::Field::LENGTH );
     tags.remove( Meta::Field::SCORE );
diff --git a/src/services/lastfm/ScrobblerAdapter.cpp b/src/services/lastfm/ScrobblerAdapter.cpp
index f8bf890..b1a09f8 100644
--- a/src/services/lastfm/ScrobblerAdapter.cpp
+++ b/src/services/lastfm/ScrobblerAdapter.cpp
@@ -217,11 +217,6 @@ ScrobblerAdapter::copyTrackMetadata( lastfm::MutableTrack &to, const Meta::Track
     if( track->trackNumber() >= 0 )
         to.setTrackNumber( track->trackNumber() );
 
-    static const QString mbidUrlStart( "amarok-sqltrackuid://mb-" );
-    QString uid = track->uidUrl();
-    if( uid.startsWith( mbidUrlStart ) )
-        to.setMbid( lastfm::Mbid( uid.mid( mbidUrlStart.length() ) ) );
-
     lastfm::Track::Source source = lastfm::Track::Player;
     if( track->type() == "stream/lastfm" )
         source = lastfm::Track::LastFmRadio;
diff --git a/tests/core-impl/collections/db/sql/TestSqlTrack.cpp b/tests/core-impl/collections/db/sql/TestSqlTrack.cpp
index ac4a0ee..3064daa 100644
--- a/tests/core-impl/collections/db/sql/TestSqlTrack.cpp
+++ b/tests/core-impl/collections/db/sql/TestSqlTrack.cpp
@@ -268,7 +268,7 @@ TestSqlTrack::testSetAllValuesSingleNotExisting()
 {
     {
         // get a new track
-        Meta::TrackPtr track1 = m_collection->registry()->getTrack( -1, "./IamANewTrack.mp3", 0, "mb-1e34fb213489" );
+        Meta::TrackPtr track1 = m_collection->registry()->getTrack( -1, "./IamANewTrack.mp3", 0, "1e34fb213489" );
 
         QSignalSpy spy( m_collection, SIGNAL(updated()));
         MetaNotificationSpy metaSpy;



More information about the kde-doc-english mailing list