Commit a8f1da1a authored by Matěj Laitl's avatar Matěj Laitl
Browse files

media device collection: more consistent handling of compilations

This makes handling of compilation albums consistent across:
 * Amarok collection
 * iPod viewed in Amarok
 * iPod viewed on itself

The code used to determine best guess album artist in
ScanResultProcessor was factored into ArtistHelper for reusability,
ArtistHelper is now exported in amaroklib.

This should definitely fix bug 232072. (or better - I'm not able to
reproduce it with this patch applied)

BUG: 232072
FIXED-IN: 2.6
DIGEST: compilation handling on iPod improved
parent 2e73dd37
......@@ -12,6 +12,7 @@ Version 2.6-Beta 1
* libgpod requirement raised from 0.7.0 to 0.7.93 for better maintainability.
BUGFIXES:
* Compilations are handled more consistently on iPods. (BR 232072)
Version 2.5
FEATURES:
......
......@@ -312,6 +312,7 @@ endif( TAGLIB_FOUND )
# COLLECTION
#####################################################################
set(collection_SRCS
core-impl/collections/support/ArtistHelper.cpp
core-impl/collections/support/CollectionManager.cpp
core-impl/collections/support/CollectionLocationDelegateImpl.cpp
core-impl/collections/support/MemoryCustomValue.cpp
......
......@@ -253,27 +253,11 @@ ScanResultProcessor::commitPlaylist( CollectionScanner::Playlist *playlist )
CollectionScanner::Album*
ScanResultProcessor::sortTrack( CollectionScanner::Track *track )
{
// -- try to find a sensible album artist
QString albumArtist( track->albumArtist() );
// - for classical tracks it's the composer
if( albumArtist.isEmpty() &&
(track->genre().compare( i18nc( "The genre name for classical music", "Classical" ), Qt::CaseInsensitive ) == 0 ||
track->genre().compare( QLatin1String( "Classical" ), Qt::CaseInsensitive ) == 0 ) )
albumArtist = ArtistHelper::realTrackArtist( track->composer() );
// - for "normal" tracks it's the track artist
if( albumArtist.isEmpty() )
albumArtist = ArtistHelper::realTrackArtist( track->artist() );
// - "Various Artists" is the same as no artist
if( albumArtist.compare( i18n( "Various Artists" ), Qt::CaseInsensitive ) == 0 ||
albumArtist.compare( QLatin1String( "Various Artists" ), Qt::CaseInsensitive ) == 0 )
albumArtist.clear();
QString albumArtist = ArtistHelper::bestGuessAlbumArtist( track->albumArtist(),
track->artist(), track->genre(), track->composer() );
if( track->album().isEmpty() && albumArtist.isEmpty() )
return 0;
return 0; // unknown album from various artists
return sortTrack( track, track->album(), albumArtist );
}
......
......@@ -32,7 +32,6 @@ set(amarok_collection-sqlcollection_SRCS
SqlWriteLabelCapability.cpp
SqlScanResultProcessor.cpp
${CMAKE_SOURCE_DIR}/shared/FileType.cpp
${CMAKE_SOURCE_DIR}/src/core-impl/collections/support/ArtistHelper.cpp
${extra_src}
)
......
......@@ -18,16 +18,14 @@
#include "MediaDeviceHandler.h"
#include "MediaDeviceCollection.h"
#include "MediaDeviceHandlerCapability.h"
#include "core/interfaces/Logger.h"
#include "core/support/Components.h"
#include "core-impl/collections/mediadevicecollection/MediaDeviceCollection.h"
#include "core-impl/collections/support/ArtistHelper.h"
#include "playlist/MediaDeviceUserPlaylistProvider.h"
#include "playlistmanager/PlaylistManager.h"
#include "playlist/MediaDevicePlaylist.h"
#include "playlistmanager/PlaylistManager.h"
#include <KMessageBox>
#include <threadweaver/ThreadWeaver.h>
......@@ -140,25 +138,51 @@ MediaDeviceHandler::setBasicMediaDeviceTrackInfo( const Meta::TrackPtr& srcTrack
return;
m_wc->libSetTitle( destTrack, srcTrack->name() );
QString albumArtist;
bool isCompilation = false;
if ( srcTrack->album() )
{
AlbumPtr album = srcTrack->album();
m_wc->libSetAlbum( destTrack, album->name() );
m_wc->libSetIsCompilation( destTrack, album->isCompilation() );
isCompilation = album->isCompilation();
m_wc->libSetIsCompilation( destTrack, isCompilation );
if( album->hasAlbumArtist() )
m_wc->libSetAlbumArtist( destTrack, album->albumArtist()->name() );
albumArtist = album->albumArtist()->name();
if( album->hasImage() )
m_wc->libSetCoverArt( destTrack, album->image() );
}
QString trackArtist;
if ( srcTrack->artist() )
m_wc->libSetArtist( destTrack, srcTrack->artist()->name() );
{
trackArtist = srcTrack->artist()->name();
m_wc->libSetArtist( destTrack, trackArtist );
}
QString composer;
if ( srcTrack->composer() )
m_wc->libSetComposer( destTrack, srcTrack->composer()->name() );
{
composer = srcTrack->composer()->name();
m_wc->libSetComposer( destTrack, composer );
}
QString genre;
if ( srcTrack->genre() )
m_wc->libSetGenre( destTrack, srcTrack->genre()->name() );
{
genre = srcTrack->genre()->name();
m_wc->libSetGenre( destTrack, genre );
}
if( isCompilation && albumArtist.isEmpty() )
// iPod doesn't handle empy album artist well for compilation albums (splits these albums)
albumArtist = i18n( "Various Artists" );
else
albumArtist = ArtistHelper::bestGuessAlbumArtist( albumArtist, trackArtist, genre, composer );
m_wc->libSetAlbumArtist( destTrack, albumArtist );
if ( srcTrack->year() )
m_wc->libSetYear( destTrack, srcTrack->year()->name() );
m_wc->libSetLength( destTrack, srcTrack->length() );
......@@ -721,10 +745,9 @@ MediaDeviceHandler::slotDatabaseWritten( bool success )
void
MediaDeviceHandler::setupArtistMap( Meta::MediaDeviceTrackPtr track, ArtistMap& artistMap )
MediaDeviceHandler::setupArtistMap( Meta::MediaDeviceTrackPtr track, ArtistMap &artistMap )
{
const QString artist( m_rcb->libGetArtist( track ) );
const QString albumArtist( m_rcb->libGetAlbumArtist( track ) );
MediaDeviceArtistPtr artistPtr;
if( artistMap.contains( artist ) )
......@@ -737,21 +760,13 @@ MediaDeviceHandler::setupArtistMap( Meta::MediaDeviceTrackPtr track, ArtistMap&
artistPtr->addTrack( track );
track->setArtist( artistPtr );
if( !albumArtist.isEmpty() && albumArtist != artist &&
!artistMap.contains( albumArtist ) )
{
artistPtr = MediaDeviceArtistPtr( new MediaDeviceArtist( albumArtist ) );
artistMap.insert( albumArtist, ArtistPtr::staticCast( artistPtr ) );
}
}
void
MediaDeviceHandler::setupAlbumMap( Meta::MediaDeviceTrackPtr track, AlbumMap& albumMap, const ArtistMap &artistMap )
MediaDeviceHandler::setupAlbumMap( Meta::MediaDeviceTrackPtr track, AlbumMap& albumMap, ArtistMap &artistMap )
{
const QString album( m_rcb->libGetAlbum( track ) );
const QString artist( m_rcb->libGetArtist( track ) );
const QString albumArtist( m_rcb->libGetAlbumArtist( track ) );
QString albumArtist( m_rcb->libGetAlbumArtist( track ) );
MediaDeviceAlbumPtr albumPtr;
if ( albumMap.contains( album ) )
......@@ -771,17 +786,28 @@ MediaDeviceHandler::setupAlbumMap( Meta::MediaDeviceTrackPtr track, AlbumMap& al
isCompilation |= m_rc->libIsCompilation( track );
albumPtr->setIsCompilation( isCompilation );
MediaDeviceArtistPtr artistPtr;
if( !albumArtist.isEmpty() && artistMap.contains( artist ) )
artistPtr = MediaDeviceArtistPtr::staticCast( artistMap.value( albumArtist ) );
else if( !artist.isEmpty() && artistMap.contains( artist ) )
artistPtr = MediaDeviceArtistPtr::staticCast( artistMap.value( artist ) );
if( albumArtist.compare( "Various Artists", Qt::CaseInsensitive ) == 0 ||
albumArtist.compare( i18n( "Various Artists" ), Qt::CaseInsensitive ) == 0 )
{
albumArtist.clear();
}
if( albumArtist.isEmpty() )
{
// set compilation flag, otherwise the album would be invisible in collection
// browser if "Album Artist / Album" view is selected.
albumPtr->setIsCompilation( true );
return; // nothing more to do - this is an album with no album artist
}
if( !artistPtr.isNull() )
MediaDeviceArtistPtr albumArtistPtr;
if( artistMap.contains( albumArtist ) )
albumArtistPtr = MediaDeviceArtistPtr::staticCast( artistMap.value( albumArtist ) );
else
{
albumPtr->setAlbumArtist( artistPtr );
albumArtistPtr = MediaDeviceArtistPtr( new MediaDeviceArtist( albumArtist ) );
artistMap.insert( albumArtist, ArtistPtr::staticCast( albumArtistPtr ) );
}
albumPtr->setAlbumArtist( albumArtistPtr );
}
void
......
......@@ -323,7 +323,7 @@ private:
*/
void setupArtistMap( Meta::MediaDeviceTrackPtr track, ArtistMap &artistMap );
void setupAlbumMap( Meta::MediaDeviceTrackPtr track, AlbumMap &albumMap, const ArtistMap &artistMap );
void setupAlbumMap( Meta::MediaDeviceTrackPtr track, AlbumMap &albumMap, ArtistMap &artistMap );
void setupGenreMap( Meta::MediaDeviceTrackPtr track, GenreMap &genreMap );
void setupComposerMap( Meta::MediaDeviceTrackPtr track, ComposerMap &composerMap );
void setupYearMap( Meta::MediaDeviceTrackPtr track, YearMap &yearMap );
......
......@@ -16,8 +16,34 @@
#include "ArtistHelper.h"
#include <KLocalizedString>
#include <QStringList>
QString
ArtistHelper::bestGuessAlbumArtist( const QString &albumArtist, const QString &trackArtist,
const QString &genre, const QString &composer)
{
QString best( albumArtist );
// - for classical tracks it's the composer
if( best.isEmpty() &&
(genre.compare( i18nc( "The genre name for classical music", "Classical" ), Qt::CaseInsensitive ) == 0 ||
genre.compare( QLatin1String( "Classical" ), Qt::CaseInsensitive ) == 0 ) )
best = ArtistHelper::realTrackArtist( composer );
// - for "normal" tracks it's the track artist
if( best.isEmpty() )
best = ArtistHelper::realTrackArtist( trackArtist );
// - "Various Artists" is the same as no artist
if( best.compare( i18n( "Various Artists" ), Qt::CaseInsensitive ) == 0 ||
best.compare( QLatin1String( "Various Artists" ), Qt::CaseInsensitive ) == 0 )
best.clear();
return best;
}
QString
ArtistHelper::realTrackArtist( const QString &trackArtistTag )
{
......
......@@ -17,16 +17,28 @@
#ifndef ARTISTHELPER_H
#define ARTISTHELPER_H
#include "amarok_export.h"
#include <QString>
namespace ArtistHelper
{
/**
* Return the best guess of the album artist.
*
* Guessing algorithm includes extracting artist from composer for classical tracks,
* falling back to trackArtist if albumArtist is empty etc. Returns empty string for
* tracks that are believed to belong to belong into a compilation.
*/
AMAROK_EXPORT QString bestGuessAlbumArtist( const QString &albumArtist, const QString &trackArtist,
const QString &genre, const QString &composer );
/**
* This helper function will determine the actual track artist.
* It takes into account "A featuring B" strings, in which case
* the actual track artist would be A.
*/
QString realTrackArtist( const QString &trackArtistTag );
AMAROK_EXPORT QString realTrackArtist( const QString &trackArtistTag );
}
#endif
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment