tags 556414 + patch thanks Bug is fixed in git://gitorious.org/~stuffcorpse/amarok/stuffcorpse-clone.git b354b2212e7c269f11b15c835994f5f24d658776 195a2810e1abea09e1af78fd3af549a93f1e4995 da0b5e0a28816e3fa3d520609e2230b56d86b333
A complete rebuild of the collection is needed to insert the new uniqueid's into the database.
From 195a2810e1abea09e1af78fd3af549a93f1e4995 Mon Sep 17 00:00:00 2001 From: Rick W. Chen <[email protected]> Date: Thu, 17 Dec 2009 19:20:46 +1300 Subject: [PATCH 2/3] Fix broken uid parsing in dynamic playlists When converting a QString uid to QByteArray, a QString::toAscii() is enough; no need to invoke QByteArray::fromHex(). This also fixes the bug where the dynamic playlist does not produce tracks tagged with MusicBrainz. MusicBrainz tagged tracks have uids that contain non-hex characters, e.g. "amarok-sqltrackuid://mb_cf9a810c-afa4-446c-9a5b-7284137c8651". Hyphens, underscores, and 'm' are not hex so they are parsed wrongly, causing failures in anything trying to get those tracks. --- ChangeLog | 1 + src/dynamic/Bias.cpp | 8 ++++---- src/dynamic/BiasSolver.cpp | 8 ++++---- src/dynamic/biases/EchoNest.cpp | 12 ++++++------ src/services/lastfm/biases/SimilarArtistsBias.cpp | 12 ++++++------ src/services/lastfm/biases/WeeklyTopBias.cpp | 2 +- 6 files changed, 22 insertions(+), 21 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6f057e0..dcfa5fc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -124,6 +124,7 @@ VERSION 2.2.2-beta1 * Remove broken right click menu from the Info applet. (BR 206642) * Fixed problems with loading tracks tagged by MusicBrainz in the XSPFPlaylist (full rescan required). + * Fixed problems with loading tracks tagged by MusicBrainz in dynamic playlists. * Fixed several bugs handling changing compilations. Patch by Morten Sjøgren <[email protected]> -- thanks! * Fixed bug with panel header icon size. (BR 208616) diff --git a/src/dynamic/Bias.cpp b/src/dynamic/Bias.cpp index 98cd37d..26d8fc5 100644 --- a/src/dynamic/Bias.cpp +++ b/src/dynamic/Bias.cpp @@ -354,9 +354,9 @@ bool Dynamic::GlobalBias::trackSatisfies( Meta::TrackPtr t ) { QMutexLocker locker( &m_mutex ); - // we only wan't the uid part: - QString uidString = t->uidUrl().mid( t->uidUrl().lastIndexOf( '/' ) ); - QByteArray uid = QByteArray::fromHex( uidString.toAscii() ); + // we only want the uid part: + const QString uidString = t->uidUrl().mid( t->uidUrl().lastIndexOf( '/' ) ); + const QByteArray uid = uidString.toAscii(); return m_property.contains( uid ); } @@ -388,7 +388,7 @@ Dynamic::GlobalBias::updateReady( QString collectionId, QStringList uids ) QByteArray uid; foreach( const QString &uidString, uids ) { - uid = QByteArray::fromHex( uidString.mid(protocolLength).toAscii() ); + uid = uidString.mid( protocolLength ).toAscii(); m_property.insert( uid ); } } diff --git a/src/dynamic/BiasSolver.cpp b/src/dynamic/BiasSolver.cpp index 695e42e..d5ca420 100644 --- a/src/dynamic/BiasSolver.cpp +++ b/src/dynamic/BiasSolver.cpp @@ -765,7 +765,8 @@ Dynamic::BiasSolver::getMutation() Meta::TrackPtr Dynamic::BiasSolver::trackForUid( const QByteArray& uid ) { - return s_universeCollection->trackForUrl(s_universeCollection->uidUrlProtocol() + "://" + QString(uid.toHex()) ); + const KUrl url = s_universeCollection->uidUrlProtocol() + "://" + QString( uid ); + return s_universeCollection->trackForUrl( url ); } @@ -881,10 +882,9 @@ Dynamic::BiasSolver::universeResults( QString collectionId, QStringList uids ) // for some reason we sometimes get uids without the protocol part if( uidString.at( s_uidUrlProtocolPrefixLength - 1 ) != '/' ) - uid = QByteArray::fromHex( uidString.toAscii() ); + uid = uidString.toAscii(); else - uid = QByteArray::fromHex( uidString.mid(s_uidUrlProtocolPrefixLength).toAscii() ); - + uid = uidString.mid( s_uidUrlProtocolPrefixLength ).toAscii(); if( !uid.isEmpty() ) s_universe += uid; diff --git a/src/dynamic/biases/EchoNest.cpp b/src/dynamic/biases/EchoNest.cpp index c0a82aa..02820dd 100644 --- a/src/dynamic/biases/EchoNest.cpp +++ b/src/dynamic/biases/EchoNest.cpp @@ -381,7 +381,7 @@ Dynamic::EchoNestBias::updateReady( QString collectionId, QStringList uids ) QByteArray uid; foreach( const QString &uidString, uids ) { - uid = QByteArray::fromHex( uidString.mid(protocolLength).toAscii() ); + uid = uidString.mid( protocolLength ).toAscii(); m_savedArtists[ key ].insert( uid ); } } @@ -394,8 +394,8 @@ Dynamic::EchoNestBias::trackSatisfies( const Meta::TrackPtr track ) QMutexLocker locker( &m_mutex ); //debug() << "checking if " << track->name() << "by" << track->artist()->name() << "is in suggested:" << m_savedArtists[ m_currentArtist ] << "of" << m_currentArtist; - QString uidString = track->uidUrl().mid( track->uidUrl().lastIndexOf( '/' ) ); - QByteArray uid = QByteArray::fromHex( uidString.toAscii() ); + const QString uidString = track->uidUrl().mid( track->uidUrl().lastIndexOf( '/' ) ); + const QByteArray uid = uidString.toAscii(); QString key; if( m_currentOnly ) @@ -429,10 +429,10 @@ Dynamic::EchoNestBias::numTracksThatSatisfy( const Meta::TrackList& tracks ) { foreach( const Meta::TrackPtr track, tracks ) { - QString uidString = track->uidUrl().mid( track->uidUrl().lastIndexOf( '/' ) ); - QByteArray uid = QByteArray::fromHex( uidString.toAscii() ); + const QString uidString = track->uidUrl().mid( track->uidUrl().lastIndexOf( '/' ) ); + const QByteArray uid = uidString.toAscii(); - if( m_savedArtists[ key ].contains(uid ) ) + if( m_savedArtists[ key ].contains( uid ) ) satisfy++; } diff --git a/src/services/lastfm/biases/SimilarArtistsBias.cpp b/src/services/lastfm/biases/SimilarArtistsBias.cpp index a9a657a..a17885d 100644 --- a/src/services/lastfm/biases/SimilarArtistsBias.cpp +++ b/src/services/lastfm/biases/SimilarArtistsBias.cpp @@ -237,7 +237,7 @@ Dynamic::SimilarArtistsBias::updateReady( QString collectionId, QStringList uids QByteArray uid; foreach( const QString &uidString, uids ) { - uid = QByteArray::fromHex( uidString.mid(protocolLength).toAscii() ); + uid = uidString.mid( protocolLength ).toAscii(); m_savedArtists[ m_currentArtist ].insert( uid ); } } @@ -250,8 +250,8 @@ Dynamic::SimilarArtistsBias::trackSatisfies( const Meta::TrackPtr track ) QMutexLocker locker( &m_mutex ); //debug() << "checking if " << track->name() << "by" << track->artist()->name() << "is in suggested:" << m_savedArtists[ m_currentArtist ] << "of" << m_currentArtist; - QString uidString = track->uidUrl().mid( track->uidUrl().lastIndexOf( '/' ) ); - QByteArray uid = QByteArray::fromHex( uidString.toAscii() ); + const QString uidString = track->uidUrl().mid( track->uidUrl().lastIndexOf( '/' ) ); + const QByteArray uid = uidString.toAscii(); if( m_savedArtists.keys().contains( m_currentArtist ) ) { @@ -275,10 +275,10 @@ Dynamic::SimilarArtistsBias::numTracksThatSatisfy( const Meta::TrackList& tracks { foreach( const Meta::TrackPtr track, tracks ) { - QString uidString = track->uidUrl().mid( track->uidUrl().lastIndexOf( '/' ) ); - QByteArray uid = QByteArray::fromHex( uidString.toAscii() ); + const QString uidString = track->uidUrl().mid( track->uidUrl().lastIndexOf( '/' ) ); + const QByteArray uid = uidString.toAscii(); - if( m_savedArtists[ m_currentArtist ].contains(uid ) ) + if( m_savedArtists[ m_currentArtist ].contains( uid ) ) satisfy++; } diff --git a/src/services/lastfm/biases/WeeklyTopBias.cpp b/src/services/lastfm/biases/WeeklyTopBias.cpp index bf1ca4b..e8ce9cc 100644 --- a/src/services/lastfm/biases/WeeklyTopBias.cpp +++ b/src/services/lastfm/biases/WeeklyTopBias.cpp @@ -292,7 +292,7 @@ Dynamic::WeeklyTopBias::updateReady( QString collectionId, QStringList uids ) QByteArray uid; foreach( const QString &uidString, uids ) { - uid = QByteArray::fromHex( uidString.mid(protocolLength).toAscii() ); + uid = uidString.mid( protocolLength ).toAscii(); m_trackList.insert( uid ); } -- 1.6.5.7
From da0b5e0a28816e3fa3d520609e2230b56d86b333 Mon Sep 17 00:00:00 2001 From: Rick W. Chen <[email protected]> Date: Fri, 25 Dec 2009 00:52:05 +1300 Subject: [PATCH 3/3] Fix MusicBrainz uid prefix for Qt 4.6 Due to changes to QUrl in 4.6, underscores are no longer allowed in the hostname: - QUrl's parser is more strict when for hostnames in URLs. QUrl now enforces STD 3 rules: * each individual hostname section (between dots) must be at most 63 ASCII characters in length; * only letters, digits, and the hyphen character are allowed in the ASCII range; letters outside the ASCII range follow the normal IDN rules That means QUrl no longer accepts some URLs that were invalid before, but weren't interpreted as such. http://qt.nokia.com/developer/changes/changes-4.6.0 --- src/services/lastfm/ScrobblerAdapter.cpp | 4 ++-- utilities/collectionscanner/AFTUtility.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/services/lastfm/ScrobblerAdapter.cpp b/src/services/lastfm/ScrobblerAdapter.cpp index 7f6c5e6..6b88ff4 100644 --- a/src/services/lastfm/ScrobblerAdapter.cpp +++ b/src/services/lastfm/ScrobblerAdapter.cpp @@ -81,9 +81,9 @@ ScrobblerAdapter::engineNewTrackPlaying() m_current.setAlbum( track->album()->name() ); QString uid = track->uidUrl(); - if( uid.startsWith( "amarok-sqltrackuid://mb_" ) ) + if( uid.startsWith( "amarok-sqltrackuid://mb-" ) ) { - uid.remove( "amarok-sqltrackuid://mb_" ); + uid.remove( "amarok-sqltrackuid://mb-" ); m_current.setMbid( lastfm::Mbid( uid ) ); } diff --git a/utilities/collectionscanner/AFTUtility.cpp b/utilities/collectionscanner/AFTUtility.cpp index e929647..55761c4 100644 --- a/utilities/collectionscanner/AFTUtility.cpp +++ b/utilities/collectionscanner/AFTUtility.cpp @@ -77,7 +77,7 @@ AFTUtility::readEmbeddedUniqueId( const TagLib::FileRef &fileref ) } } if( !storedMBId.isEmpty() && ( storedMBId != mbDefaultUUID ) ) - return QString( "mb_" ) + storedMBId; + return QString( "mb-" ) + storedMBId; } //from here below assumes a file with a XiphComment; put non-conforming formats up above... TagLib::Ogg::XiphComment *comment = 0; @@ -107,7 +107,7 @@ AFTUtility::readEmbeddedUniqueId( const TagLib::FileRef &fileref ) { QString identifier = TStringToQString( comment->fieldListMap()[Qt4QStringToTString(mbId.toUpper())].front()).toLower(); if( !identifier.isEmpty() && ( identifier != mbDefaultUUID ) ) - return QString( "mb_" ) + identifier; + return QString( "mb-" ) + identifier; } return QString(); -- 1.6.5.7
From b354b2212e7c269f11b15c835994f5f24d658776 Mon Sep 17 00:00:00 2001 From: Rick W. Chen <[email protected]> Date: Tue, 15 Dec 2009 13:28:49 +1300 Subject: [PATCH 1/3] Change MusicBrainz collection uid prefix to lowercase When a QString containing a uid gets converted to QUrl/KUrl, it is automatically converted to lowercase. From Qt docs: Note that the case folding rules in Nameprep, which QUrl conforms to, require host names to always be converted to lower case, regardless of the Qt::FormattingOptions used. The result is that whenever such a conversion occurs, the corresponding track is no longer valid, causing it to be dropped. This happens, for example, when loading the XSPF playlist and when the dynamic playlist searches for a track. Dragging tracks with MBIDs from the album applet or the collection browser works because it does not go through the same path flow. Consequently, the dynamic playlist will never load a track that was tagged using MusicBrainz; a current.xspf with such tracks saved (could happen if you drag them to the playlist and then close Amarok) will not show those tracks when Amarok is reopened. Instead of going through and manage these conversions wherever they pop up I decided to change the prefix directly. Addendum: it turns out the issue with dynamic playlists is plagued by another bug. This is addressed in a separate patch. --- ChangeLog | 4 +++- src/services/lastfm/ScrobblerAdapter.cpp | 4 ++-- utilities/collectionscanner/AFTUtility.cpp | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2e8a56e..6f057e0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -122,7 +122,9 @@ VERSION 2.2.2-beta1 BUGFIXES: * Fixed two bookmark actions having the same text description. (BR 214716) * Remove broken right click menu from the Info applet. (BR 206642) - * Fix several bugs handling changing compilations. Patch by Morten Sjøgren + * Fixed problems with loading tracks tagged by MusicBrainz in the + XSPFPlaylist (full rescan required). + * Fixed several bugs handling changing compilations. Patch by Morten Sjøgren <[email protected]> -- thanks! * Fixed bug with panel header icon size. (BR 208616) * Fixed bug with OSD not being shown by dbus call. (BR 208424) diff --git a/src/services/lastfm/ScrobblerAdapter.cpp b/src/services/lastfm/ScrobblerAdapter.cpp index 74d29a1..7f6c5e6 100644 --- a/src/services/lastfm/ScrobblerAdapter.cpp +++ b/src/services/lastfm/ScrobblerAdapter.cpp @@ -81,9 +81,9 @@ ScrobblerAdapter::engineNewTrackPlaying() m_current.setAlbum( track->album()->name() ); QString uid = track->uidUrl(); - if( uid.startsWith( "amarok-sqltrackuid://MB_" ) ) + if( uid.startsWith( "amarok-sqltrackuid://mb_" ) ) { - uid.remove( "amarok-sqltrackuid://MB_" ); + uid.remove( "amarok-sqltrackuid://mb_" ); m_current.setMbid( lastfm::Mbid( uid ) ); } diff --git a/utilities/collectionscanner/AFTUtility.cpp b/utilities/collectionscanner/AFTUtility.cpp index c139a15..e929647 100644 --- a/utilities/collectionscanner/AFTUtility.cpp +++ b/utilities/collectionscanner/AFTUtility.cpp @@ -77,7 +77,7 @@ AFTUtility::readEmbeddedUniqueId( const TagLib::FileRef &fileref ) } } if( !storedMBId.isEmpty() && ( storedMBId != mbDefaultUUID ) ) - return QString( "MB_" ) + storedMBId; + return QString( "mb_" ) + storedMBId; } //from here below assumes a file with a XiphComment; put non-conforming formats up above... TagLib::Ogg::XiphComment *comment = 0; @@ -107,7 +107,7 @@ AFTUtility::readEmbeddedUniqueId( const TagLib::FileRef &fileref ) { QString identifier = TStringToQString( comment->fieldListMap()[Qt4QStringToTString(mbId.toUpper())].front()).toLower(); if( !identifier.isEmpty() && ( identifier != mbDefaultUUID ) ) - return QString( "MB_" ) + identifier; + return QString( "mb_" ) + identifier; } return QString(); -- 1.6.5.7
