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@kde.org
CCMAIL: Alberto Villa <avi...@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;
_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel
