From 938ee20f1fe9fabfc18b87d963bfaa7dd81fba0f Mon Sep 17 00:00:00 2001 From: Jonas Kvinge Date: Tue, 29 Sep 2020 22:40:43 +0200 Subject: [PATCH] Make sure song changed is only called once --- src/core/mainwindow.cpp | 16 +++------ src/core/player.cpp | 26 ++++++--------- src/core/player.h | 1 - src/playlist/playlist.cpp | 70 +++++++++++++++++++-------------------- src/playlist/playlist.h | 9 +++-- 5 files changed, 54 insertions(+), 68 deletions(-) diff --git a/src/core/mainwindow.cpp b/src/core/mainwindow.cpp index 91778e191..c53c6980d 100644 --- a/src/core/mainwindow.cpp +++ b/src/core/mainwindow.cpp @@ -571,7 +571,6 @@ MainWindow::MainWindow(Application *app, SystemTrayIcon *tray_icon, OSDBase *osd connect(app_->player(), SIGNAL(VolumeChanged(int)), osd_, SLOT(VolumeChanged(int))); connect(app_->player(), SIGNAL(VolumeChanged(int)), ui_->volume, SLOT(setValue(int))); connect(app_->player(), SIGNAL(ForceShowOSD(Song, bool)), SLOT(ForceShowOSD(Song, bool))); - connect(app_->player(), SIGNAL(SendNowPlaying()), SLOT(SendNowPlaying())); connect(app_->playlist_manager(), SIGNAL(CurrentSongChanged(Song)), SLOT(SongChanged(Song))); connect(app_->playlist_manager(), SIGNAL(CurrentSongChanged(Song)), app_->player(), SLOT(CurrentMetadataChanged(Song))); @@ -1263,21 +1262,14 @@ void MainWindow::MediaPlaying() { track_slider_timer_->start(); UpdateTrackPosition(); - if (app_->playlist_manager()->active()) app_->playlist_manager()->active()->set_nowplaying(false); - SendNowPlaying(); - } void MainWindow::SendNowPlaying() { - PlaylistItemPtr item(app_->player()->GetCurrentItem()); - if (!item) return; - // Send now playing to scrobble services Playlist *playlist = app_->playlist_manager()->active(); - if (app_->scrobbler()->IsEnabled() && playlist && !playlist->nowplaying() && item->Metadata().is_metadata_good()) { - app_->scrobbler()->UpdateNowPlaying(item->Metadata()); - playlist->set_nowplaying(true); + if (app_->scrobbler()->IsEnabled() && playlist && playlist->current_item() && playlist->current_item()->Metadata().is_metadata_good()) { + app_->scrobbler()->UpdateNowPlaying(playlist->current_item()->Metadata()); ui_->action_love->setEnabled(true); ui_->button_love->setEnabled(true); if (tray_icon_) tray_icon_->LoveStateChanged(true); @@ -1297,6 +1289,8 @@ void MainWindow::SongChanged(const Song &song) { setWindowTitle(song.PrettyTitleWithArtist()); if (tray_icon_) tray_icon_->SetProgress(0); + SendNowPlaying(); + } void MainWindow::TrackSkipped(PlaylistItemPtr item) { @@ -1568,7 +1562,7 @@ void MainWindow::UpdateTrackPosition() { // Send Scrobble if (app_->scrobbler()->IsEnabled() && item->Metadata().is_metadata_good()) { Playlist *playlist = app_->playlist_manager()->active(); - if (playlist && playlist->nowplaying() && !playlist->scrobbled()) { + if (playlist && !playlist->scrobbled()) { const int scrobble_point = (playlist->scrobble_point_nanosec() / kNsecPerSec); if (position >= scrobble_point) { app_->scrobbler()->Scrobble(item->Metadata(), scrobble_point); diff --git a/src/core/player.cpp b/src/core/player.cpp index 052b867dd..9d8d30a79 100644 --- a/src/core/player.cpp +++ b/src/core/player.cpp @@ -237,10 +237,11 @@ void Player::HandleLoadResult(const UrlHandler::LoadResult &result) { if (!item) { return; } - const bool has_next_row = app_->playlist_manager()->active()->next_row() != -1; + int next_row = app_->playlist_manager()->active()->next_row(); + const bool has_next_row = next_row != -1; PlaylistItemPtr next_item; if (has_next_row) { - next_item = app_->playlist_manager()->active()->item_at(app_->playlist_manager()->active()->next_row()); + next_item = app_->playlist_manager()->active()->item_at(next_row); } bool is_current(false); @@ -322,11 +323,12 @@ void Player::HandleLoadResult(const UrlHandler::LoadResult &result) { if (update) { if (is_current) { item->SetTemporaryMetadata(song); - app_->playlist_manager()->active()->InformOfCurrentSongChange(autoscroll_); + app_->playlist_manager()->active()->InformOfCurrentSongChange(autoscroll_, true); + app_->playlist_manager()->active()->UpdateScrobblePoint(); } else if (is_next) { next_item->SetTemporaryMetadata(song); - app_->playlist_manager()->active()->ItemChanged(next_item); + app_->playlist_manager()->active()->ItemChanged(next_row); } } @@ -599,15 +601,6 @@ void Player::CurrentMetadataChanged(const Song &metadata) { // Those things might have changed (especially when a previously invalid song was reloaded) so we push the latest version into Engine engine_->RefreshMarkers(metadata.beginning_nanosec(), metadata.end_nanosec()); - // Send now playing to scrobble services - if (app_->scrobbler()->IsEnabled() && engine_->state() == Engine::Playing) { - Playlist *playlist = app_->playlist_manager()->active(); - current_item_ = playlist->current_item(); - if (playlist && current_item_ && !playlist->nowplaying() && current_item_->Metadata() == metadata && current_item_->Metadata().is_metadata_good()) { - emit SendNowPlaying(); - } - } - } void Player::SeekTo(const int seconds) { @@ -649,13 +642,14 @@ void Player::EngineMetadataReceived(const Engine::SimpleMetaBundle &bundle) { return; } - if (app_->playlist_manager()->active()->next_row() != -1) { - PlaylistItemPtr next_item = app_->playlist_manager()->active()->item_at(app_->playlist_manager()->active()->next_row()); + int next_row = app_->playlist_manager()->active()->next_row(); + if (next_row != -1) { + PlaylistItemPtr next_item = app_->playlist_manager()->active()->item_at(next_row); if (bundle.url == next_item->Url()) { Song song = next_item->Metadata(); song.MergeFromSimpleMetaBundle(bundle); next_item->SetTemporaryMetadata(song); - app_->playlist_manager()->active()->ItemChanged(next_item); + app_->playlist_manager()->active()->ItemChanged(next_row); } } diff --git a/src/core/player.h b/src/core/player.h index 4d3093032..e5e91f0fb 100644 --- a/src/core/player.h +++ b/src/core/player.h @@ -122,7 +122,6 @@ class PlayerInterface : public QObject { // The toggle parameter is true when user requests to toggle visibility for Pretty OSD void ForceShowOSD(Song, bool toggle); - void SendNowPlaying(); void Authenticated(); }; diff --git a/src/playlist/playlist.cpp b/src/playlist/playlist.cpp index 89efeaf24..e33185f18 100644 --- a/src/playlist/playlist.cpp +++ b/src/playlist/playlist.cpp @@ -134,7 +134,6 @@ Playlist::Playlist(PlaylistBackend *backend, TaskManager *task_manager, Collecti special_type_(special_type), cancel_restore_(false), scrobbled_(false), - nowplaying_(false), scrobble_point_(-1), editing_(-1) { @@ -599,11 +598,12 @@ void Playlist::set_current_row(const int i, const AutoScroll autoscroll, const b if (new_current_item_index != current_item_index_) ClearStreamMetadata(); - if (next_row() != -1 && next_row() != i) { - PlaylistItemPtr next_item = item_at(next_row()); + int nextrow = next_row(); + if (nextrow != -1 && nextrow != i) { + PlaylistItemPtr next_item = item_at(nextrow); if (next_item) { next_item->ClearTemporaryMetadata(); - emit dataChanged(index(next_row(), 0), index(next_row(), ColumnCount - 1)); + emit dataChanged(index(nextrow, 0), index(nextrow, ColumnCount - 1)); } } @@ -644,7 +644,7 @@ void Playlist::set_current_row(const int i, const AutoScroll autoscroll, const b } if (current_item_index_.isValid() && !is_stopping) { - InformOfCurrentSongChange(autoscroll); + InformOfCurrentSongChange(autoscroll, false); } // The structure of a dynamic playlist is as follows: @@ -1596,29 +1596,10 @@ void Playlist::SetStreamMetadata(const QUrl &url, const Song &song, const bool m if (!current_item() || current_item()->Url() != url) return; - //qLog(Debug) << "Setting temporary metadata for" << url; - bool update_scrobble_point = song.length_nanosec() != current_item_metadata().length_nanosec(); - current_item()->SetTemporaryMetadata(song); - - if (minor) { - if (editing_ != current_item_index_.row()) { - emit dataChanged(index(current_item_index_.row(), 0), index(current_item_index_.row(), ColumnCount - 1)); - } - // if the song is invalid, we won't play it - there's no point in informing anybody about the change - const Song metadata(current_item_metadata()); - if (metadata.is_valid()) { - emit SongMetadataChanged(metadata); - } - } - else { - update_scrobble_point = true; - nowplaying_ = false; - InformOfCurrentSongChange(AutoScroll_Never); - } - if (update_scrobble_point) UpdateScrobblePoint(); + InformOfCurrentSongChange(AutoScroll_Never, minor); } @@ -1737,10 +1718,16 @@ void Playlist::ReloadItems(const QList &rows) { for (int row : rows) { PlaylistItemPtr item = item_at(row); + Song old_metadata = item->Metadata(); + item->Reload(); if (row == current_row()) { - InformOfCurrentSongChange(AutoScroll_Never); + const bool minor = old_metadata.title() == item->Metadata().title() && + old_metadata.albumartist() == item->Metadata().albumartist() && + old_metadata.artist() == item->Metadata().artist() && + old_metadata.album() == item->Metadata().album(); + InformOfCurrentSongChange(AutoScroll_Never, minor); } else { emit dataChanged(index(row, 0), index(row, ColumnCount - 1)); @@ -1946,28 +1933,41 @@ void Playlist::QueueLayoutChanged() { } +void Playlist::ItemChanged(const int row) { + + QModelIndex idx = index(row, ColumnCount - 1); + if (idx.isValid()) { + emit dataChanged(index(row, 0), index(row, ColumnCount - 1)); + } + +} + void Playlist::ItemChanged(PlaylistItemPtr item) { for (int row = 0; row < items_.count(); ++row) { if (items_[row] == item) { - QModelIndex idx = index(row, ColumnCount - 1); - if (idx.isValid()) { - emit dataChanged(index(row, 0), index(row, ColumnCount - 1)); - } + ItemChanged(row); } } } -void Playlist::InformOfCurrentSongChange(const AutoScroll autoscroll) { - - emit dataChanged(index(current_item_index_.row(), 0), index(current_item_index_.row(), ColumnCount - 1)); +void Playlist::InformOfCurrentSongChange(const AutoScroll autoscroll, const bool minor) { // if the song is invalid, we won't play it - there's no point in informing anybody about the change const Song metadata(current_item_metadata()); if (metadata.is_valid()) { - emit CurrentSongChanged(metadata); - emit MaybeAutoscroll(autoscroll); + if (minor) { + emit SongMetadataChanged(metadata); + if (editing_ != current_item_index_.row()) { + emit dataChanged(index(current_item_index_.row(), 0), index(current_item_index_.row(), ColumnCount - 1)); + } + } + else { + emit CurrentSongChanged(metadata); + emit MaybeAutoscroll(autoscroll); + emit dataChanged(index(current_item_index_.row(), 0), index(current_item_index_.row(), ColumnCount - 1)); + } } } diff --git a/src/playlist/playlist.h b/src/playlist/playlist.h index 6037b9134..7623f377b 100644 --- a/src/playlist/playlist.h +++ b/src/playlist/playlist.h @@ -232,9 +232,7 @@ class Playlist : public QAbstractListModel { QUndoStack *undo_stack() const { return undo_stack_; } bool scrobbled() const { return scrobbled_; } - bool nowplaying() const { return nowplaying_; } void set_scrobbled(const bool state) { scrobbled_ = state; } - void set_nowplaying(const bool state) { nowplaying_ = state; } void set_editing(const int row) { editing_ = row; } qint64 scrobble_point_nanosec() const { return scrobble_point_; } void UpdateScrobblePoint(const qint64 seek_point_nanosec = 0); @@ -262,7 +260,7 @@ class Playlist : public QAbstractListModel { void StopAfter(const int row); void ReloadItems(const QList &rows); - void InformOfCurrentSongChange(const AutoScroll autoscroll); + void InformOfCurrentSongChange(const AutoScroll autoscroll, const bool minor); // Registers an object which will get notifications when new songs are about to be inserted into this playlist. void AddSongInsertVetoListener(SongInsertVetoListener *listener); @@ -290,6 +288,9 @@ class Playlist : public QAbstractListModel { static bool ComparePathDepths(Qt::SortOrder, PlaylistItemPtr, PlaylistItemPtr); + void ItemChanged(PlaylistItemPtr item); + void ItemChanged(const int row); + // Changes rating of a song to the given value asynchronously void RateSong(const QModelIndex &idx, const double rating); void RateSongs(const QModelIndexList &index_list, const double rating); @@ -303,7 +304,6 @@ class Playlist : public QAbstractListModel { void ClearStreamMetadata(); void SetStreamMetadata(const QUrl &url, const Song &song, const bool minor); - void ItemChanged(PlaylistItemPtr item); void UpdateItems(SongList songs); void Clear(); @@ -424,7 +424,6 @@ class Playlist : public QAbstractListModel { bool cancel_restore_; bool scrobbled_; - bool nowplaying_; qint64 scrobble_point_; int editing_;