Version: 2.4-GIT (using KDE 4.4.2) OS: Linux When scrobbling, if the track has a composer tag set, the desired behaviour is almost certainly to submit the name of the composer as the "artist". / src / services / lastfm / ScrobblerAdapter.cpp 135 m_current.setArtist( track->artist()->name() ); I suggest m_current.setArtist ( track->composer()->name() ? track->composer()->name() : track->artist()->name() ); or something similar. I will make and test a real patch if you will take it. Reproducible: Always
Please submit your patch at http://git.reviewboard.kde.org (register at http:77identity.kde.org first)
Done. http://git.reviewboard.kde.org/r/100089/
commit c5b16718cc86474afb11a7de6e7da99c0f107f07 branch master Author: Leo Franchi <lfranchi@kde.org> Date: Tue Oct 19 22:41:58 2010 -0400 Patch to submit Composer as Artist for last.fm scrobbles when there is no artist but only composer field. Thanks to Nicholas Wilson for the patch! BUG: 254580 diff --git a/src/services/lastfm/ScrobblerAdapter.cpp b/src/services/lastfm/ScrobblerAdapter.cpp index 47df3e3..8a546b6 100644 --- a/src/services/lastfm/ScrobblerAdapter.cpp +++ b/src/services/lastfm/ScrobblerAdapter.cpp @@ -77,8 +77,9 @@ ScrobblerAdapter::engineNewTrackPlaying() m_current.setTitle( track->name() ); m_current.setDuration( track->length() / 1000 ); - if( track->artist() ) - m_current.setArtist( track->artist()->name() ); + if( track->artist() || track->composer() ) + m_current.setArtist( + track->composer() ? track->composer()->name() : track->artist()->name() ); if( track->album() ) m_current.setAlbum( track->album()->name() ); @@ -122,7 +123,7 @@ ScrobblerAdapter::engineNewMetaData( const QHash<qint64, QString> &newMetaData, Meta::TrackPtr track = The::engineController()->currentTrack(); if( track && ( track->type() == "stream" && ( !track->name().isEmpty() - && track->artist() ) ) ) // got a stream, and it has enough info to be a new track + && ( track->artist() || track->composer() ) ) ) ) // got a stream, and it has enough info to be a new track { // don't use checkScrobble as we don't need to check timestamps, it is a stream debug() << "scrobble: " << m_current.artist() << " - " << m_current.album() << " - " << m_current.title(); @@ -132,7 +133,7 @@ ScrobblerAdapter::engineNewMetaData( const QHash<qint64, QString> &newMetaData, resetVariables(); m_current.setTitle( track->name() ); - m_current.setArtist( track->artist()->name() ); + m_current.setArtist( track->composer() ? track->composer()->name() : track->artist()->name() ); m_current.stamp(); m_current.setSource( lastfm::Track::NonPersonalisedBroadcast ); @@ -210,8 +211,8 @@ ScrobblerAdapter::loveTrack( Meta::TrackPtr track ) // slot { lastfm::MutableTrack trackInfo; trackInfo.setTitle( track->name() ); - if( track->artist() ) - trackInfo.setArtist( track->artist()->name() ); + if( track->artist() || track->composer() ) + trackInfo.setArtist( track->composer() ? track->composer()->name() : track->artist()->name() ); if( track->album() ) trackInfo.setAlbum( track->album()->name() );
From the patch, it looks to me that it scrobbles the composer as artist if a composer is set (whether or not the artist is empty). I'd prefer that if it is this way round - rather than that in the Leo Franchi comment to the patch - to a user settable option. If I have 5 versions of Brahms' Symphony No 4 I'd prefer the artist to be the conductor - if one is set. Also (probably should be another bug entry) - in this case the opposite to the above if the composer is set then the Lyrics search ought always to use that field
True. I am not sure about making this a user option on the basis of the tiny number of people who want that much control. In my mind, the main point is that the data sent to last.fm has to be enough to identify the piece, so at least the composer should be sent it it were available. I have nine 'Symphony 5's in my collection, so sending just the album and conductor is not a good option. Somehow you want a way to shoe-horn two bits of information into one field. Currently, I tag my collection to duplicate the main performer in the album field when I have two copies of the same piece, so I have: Beethoven, Ludwig van Violin Concerto in D Major, op. 61 (Mutter) Violin Concerto in D Major, op. 61 (Schneiderhan) Violin Sonata No. 1 in D Major op. 12 No. 1 [no need to distinguish recordings] Given that the conductor has to be sent, how do you want the performers' data to be sent? We would have to munge two fields into one. Sending artist as "<composer> (<artist>)" is probably not right, because works by one composer do not get grouped together. Sending album as "<work> (<performers>)" seems to me to be preferable, but then I would want a pref to turn it off for myself, given the way I have tagged my collection. Any comment?
Tricky isn't it! Maybe we should petition last.fm to support the composer field! Unfortunately I've gone the opposite way and have the title as "[composer] work/movement name" so I don't think we're going to get a common view here.
Woah! This is really bad! Composer being submitted as Artist? Scrobbling a "Pearl Jam" track with proper tags now scrobbles as "Eddie Vedder/Mike McCready/Stone Gossard/Dave Abbruzzese/Jeff Ament - [track_name]". Please, please revert this.
Reopened, as it seems to submit the composer field despite the artist field being set.
Changed priority
The patch is great for classical music and the like. However, as Comment #7 pointed out, it doesn't work well for tracks that are composed by specific members in a band, which is often the case. This depends too much on the collection and user preference; not everyone would want composer to be submitted. At least with the artist it has been the convention and that's what most people would expect to be used. If adding a config interface is not an option, I propose a revert.
Comment #7 brings up a good point. I don't mind reverting either, as this patch never affected me either way :)
Anyone else has issue with reverting?
If it did what the checkin comment suggested - use composer only if artist is not set (so wants some !track->artist()) then - although not perfect for classical users- would be of some use. Otherwise if a user settable option isn't possible then IMHO drop it
It doesn't actually (commit c5b16718cc86474afb11a7de6e7da99c0f107f07). Composer is used no matter what if it exists.
That was my point, I think the setArtist calls should read if( track->artist() m_current.setArtist( track->artist() ); else if(track->composer() ) m_current.setArtist( track->composer()->name() ); but, for preference, a user settable option to determine which the usage priority if it exists
I would very much like to make this optional, not all use cases are the same, and I bet quite a few people would not like to scrobble that way. BTW, it still doesn't work correctly, could we please revert this until it is really tested?
OK, so, it's been a while. I got back to this today and made a bigger patch adding a config setting for this. As far as I can tell, people either have important composer tags, which need to be scrobbled to identify the piece (like in my classical collection), or do not want them scrobbled. The config option I added toggles between the old behaviour and the new. The people who will fall down the gaps are those with mixed collections of popular and classical music. I can't a way around that problem, really; you can't test against the genre, because it might be "Classical" or "Baroque" or "Classíca" or "Classique" or any number of variations. Please comment. Thanks for help! https://git.reviewboard.kde.org/r/100806/
This was committed a while ago as cd8f7865fd93bfcb4612c3f19124b47f0298142f.