[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 Amarok-devel
mailing list