From 99dffe216cef4769cfa1f8f260f2b5b3e62bc836 Mon Sep 17 00:00:00 2001 From: Mark Furneaux Date: Mon, 18 May 2015 12:53:07 -0400 Subject: [PATCH 1/3] Fix Last.fm scrobbling after seek Fixes #4836 Last.fm defines a scrobble should be sent if: -the track is longer than 30 seconds. -the track has been played for at least half its duration, or for 4 minutes (whichever occurs earlier.) Clementine has treated this as seconds from the start of the track, and if any seeking occurs, it nullifies the scrobble. This IMO is incorrect. If I skip the first 10 seconds of a song, but listen to the rest (still meeting the time requirements), I should still be able to scrobble the play. This change moves the scrobble point with every seek, requiring continuous playback from any point that satisfies the time criteria. --- src/core/player.cpp | 10 +++------- src/playlist/playlist.cpp | 21 ++++++++++++++++----- src/playlist/playlist.h | 2 +- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/core/player.cpp b/src/core/player.cpp index ec5038c3b..f601cdcfd 100644 --- a/src/core/player.cpp +++ b/src/core/player.cpp @@ -436,13 +436,9 @@ void Player::SeekTo(int seconds) { qBound(0ll, qint64(seconds) * kNsecPerSec, length_nanosec); engine_->Seek(nanosec); - // If we seek the track we don't want to submit it to last.fm - qLog(Info) << "Track seeked to" << nanosec << "ns - not scrobbling"; - if (app_->playlist_manager()->active()->get_lastfm_status() == - Playlist::LastFM_New) { - app_->playlist_manager()->active()->set_lastfm_status( - Playlist::LastFM_Seeked); - } + // If we seek the track we need to move the scrobble point + qLog(Info) << "Track seeked to" << nanosec << "ns - updating srobble point"; + app_->playlist_manager()->active()->UpdateScrobblePoint(nanosec); emit Seeked(nanosec / 1000); } diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index 0fbd5e5e0..23542e4a8 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -1705,14 +1705,25 @@ Song Playlist::current_item_metadata() const { return current_item()->Metadata(); } -void Playlist::UpdateScrobblePoint() { +void Playlist::UpdateScrobblePoint(qint64 seek_point) { const qint64 length = current_item_metadata().length_nanosec(); - if (length == 0) { - scrobble_point_ = 240ll * kNsecPerSec; // 4 minutes + if (seek_point == 0) { + if (length == 0) { + scrobble_point_ = 240ll * kNsecPerSec; // 4 minutes + } else { + scrobble_point_ = + qBound(31ll * kNsecPerSec, length / 2, 240ll * kNsecPerSec); + } } else { - scrobble_point_ = - qBound(31ll * kNsecPerSec, length / 2, 240ll * kNsecPerSec); + if (length == 0) { + // current time + 4 minutes + scrobble_point_ = seek_point + (240ll * kNsecPerSec); + } else { + scrobble_point_ = + qBound(seek_point + (31ll * kNsecPerSec), seek_point + (length / 2), + seek_point + (240ll * kNsecPerSec)); + } } set_lastfm_status(LastFM_New); diff --git a/src/playlist/playlist.h b/src/playlist/playlist.h index a3a80cbb7..cafa9454f 100644 --- a/src/playlist/playlist.h +++ b/src/playlist/playlist.h @@ -227,6 +227,7 @@ class Playlist : public QAbstractListModel { } void set_lastfm_status(LastFMStatus status) { lastfm_status_ = status; } void set_have_incremented_playcount() { have_incremented_playcount_ = true; } + void UpdateScrobblePoint(qint64 seek_point = 0); // Changing the playlist void InsertItems(const PlaylistItemList& items, int pos = -1, @@ -351,7 +352,6 @@ signals: private: void SetCurrentIsPaused(bool paused); - void UpdateScrobblePoint(); int NextVirtualIndex(int i, bool ignore_repeat_track) const; int PreviousVirtualIndex(int i, bool ignore_repeat_track) const; bool FilterContainsVirtualIndex(int i) const; From 50ff5f5b0e88943ea91b7e479784c648b5cb434b Mon Sep 17 00:00:00 2001 From: Mark Furneaux Date: Mon, 18 May 2015 13:03:44 -0400 Subject: [PATCH 2/3] Spelling --- src/core/player.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/player.cpp b/src/core/player.cpp index f601cdcfd..bd0a107cd 100644 --- a/src/core/player.cpp +++ b/src/core/player.cpp @@ -437,7 +437,7 @@ void Player::SeekTo(int seconds) { engine_->Seek(nanosec); // If we seek the track we need to move the scrobble point - qLog(Info) << "Track seeked to" << nanosec << "ns - updating srobble point"; + qLog(Info) << "Track seeked to" << nanosec << "ns - updating scrobble point"; app_->playlist_manager()->active()->UpdateScrobblePoint(nanosec); emit Seeked(nanosec / 1000); From fde8ae0f3085acfb0a91bd1c042dd218586e0447 Mon Sep 17 00:00:00 2001 From: Mark Furneaux Date: Mon, 18 May 2015 13:22:55 -0400 Subject: [PATCH 3/3] Add constants and variable units Scrobble point constants are now defined as the minimum and maximum time last.fm requires for a scrobble to be valid. --- src/playlist/playlist.cpp | 19 +++++++++++-------- src/playlist/playlist.h | 5 ++++- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index 23542e4a8..564cfb57a 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -95,6 +95,9 @@ const char* Playlist::kWriteMetadata = "write_metadata"; const int Playlist::kUndoStackSize = 20; const int Playlist::kUndoItemLimit = 500; +const qint64 Playlist::kMinScrobblePointNsecs = 31ll * kNsecPerSec; +const qint64 Playlist::kMaxScrobblePointNsecs = 240ll * kNsecPerSec; + Playlist::Playlist(PlaylistBackend* backend, TaskManager* task_manager, LibraryBackend* library, int id, const QString& special_type, bool favorite, QObject* parent) @@ -1705,24 +1708,24 @@ Song Playlist::current_item_metadata() const { return current_item()->Metadata(); } -void Playlist::UpdateScrobblePoint(qint64 seek_point) { +void Playlist::UpdateScrobblePoint(qint64 seek_point_nanosec) { const qint64 length = current_item_metadata().length_nanosec(); - if (seek_point == 0) { + if (seek_point_nanosec == 0) { if (length == 0) { - scrobble_point_ = 240ll * kNsecPerSec; // 4 minutes + scrobble_point_ = kMaxScrobblePointNsecs; // 4 minutes } else { scrobble_point_ = - qBound(31ll * kNsecPerSec, length / 2, 240ll * kNsecPerSec); + qBound(kMinScrobblePointNsecs, length / 2, kMaxScrobblePointNsecs); } } else { if (length == 0) { // current time + 4 minutes - scrobble_point_ = seek_point + (240ll * kNsecPerSec); + scrobble_point_ = seek_point_nanosec + kMaxScrobblePointNsecs; } else { - scrobble_point_ = - qBound(seek_point + (31ll * kNsecPerSec), seek_point + (length / 2), - seek_point + (240ll * kNsecPerSec)); + scrobble_point_ = qBound(seek_point_nanosec + kMinScrobblePointNsecs, + seek_point_nanosec + (length / 2), + seek_point_nanosec + kMaxScrobblePointNsecs); } } diff --git a/src/playlist/playlist.h b/src/playlist/playlist.h index cafa9454f..e82d3cf18 100644 --- a/src/playlist/playlist.h +++ b/src/playlist/playlist.h @@ -160,6 +160,9 @@ class Playlist : public QAbstractListModel { static const int kUndoStackSize; static const int kUndoItemLimit; + static const qint64 kMinScrobblePointNsecs; + static const qint64 kMaxScrobblePointNsecs; + static bool CompareItems(int column, Qt::SortOrder order, PlaylistItemPtr a, PlaylistItemPtr b); @@ -227,7 +230,7 @@ class Playlist : public QAbstractListModel { } void set_lastfm_status(LastFMStatus status) { lastfm_status_ = status; } void set_have_incremented_playcount() { have_incremented_playcount_ = true; } - void UpdateScrobblePoint(qint64 seek_point = 0); + void UpdateScrobblePoint(qint64 seek_point_nanosec = 0); // Changing the playlist void InsertItems(const PlaylistItemList& items, int pos = -1,