From 8fd32aba4f1972625e16924446ea34a7dffc5f28 Mon Sep 17 00:00:00 2001 From: Jonas Kvinge Date: Thu, 10 Jun 2021 23:13:03 +0200 Subject: [PATCH] Improve resume playback on startup, re-request stream URL when unpausing Fixes #270 --- src/core/mainwindow.cpp | 18 +------- src/core/mainwindow.h | 1 - src/core/player.cpp | 90 +++++++++++++++++++++++++++++++------ src/core/player.h | 27 ++++++----- src/core/song.cpp | 1 + src/core/song.h | 1 + src/engine/enginebase.cpp | 14 +++--- src/engine/enginebase.h | 2 +- src/playlist/playlistview.h | 2 +- 9 files changed, 106 insertions(+), 50 deletions(-) diff --git a/src/core/mainwindow.cpp b/src/core/mainwindow.cpp index 4c249386..29e4d8aa 100644 --- a/src/core/mainwindow.cpp +++ b/src/core/mainwindow.cpp @@ -766,7 +766,7 @@ MainWindow::MainWindow(Application *app, std::shared_ptr tray_ic #ifdef HAVE_GLOBALSHORTCUTS // Global shortcuts - QObject::connect(globalshortcuts_manager_, &GlobalShortcutsManager::Play, app_->player(), &Player::Play); + QObject::connect(globalshortcuts_manager_, &GlobalShortcutsManager::Play, app_->player(), &Player::PlayHelper); QObject::connect(globalshortcuts_manager_, &GlobalShortcutsManager::Pause, app_->player(), &Player::Pause); QObject::connect(globalshortcuts_manager_, &GlobalShortcutsManager::PlayPause, ui_->action_play_pause, &QAction::trigger); QObject::connect(globalshortcuts_manager_, &GlobalShortcutsManager::Stop, ui_->action_stop, &QAction::trigger); @@ -1446,13 +1446,7 @@ void MainWindow::ResumePlayback() { app_->player()->PlayPause(); }); } - // Seek after we got song length. - std::shared_ptr connection = std::make_shared(); - *connection = QObject::connect(track_position_timer_, &QTimer::timeout, this, [this, connection, playback_position]() { - QObject::disconnect(*connection); - ResumePlaybackSeek(playback_position); - }); - app_->player()->Play(); + app_->player()->Play(playback_position * kNsecPerSec); } // Reset saved playback status so we don't resume again from the same position. @@ -1464,14 +1458,6 @@ void MainWindow::ResumePlayback() { } -void MainWindow::ResumePlaybackSeek(const int playback_position) { - - if (app_->player()->engine()->length_nanosec() > 0) { - app_->player()->SeekTo(playback_position); - } - -} - void MainWindow::PlayIndex(const QModelIndex &idx, Playlist::AutoScroll autoscroll) { if (!idx.isValid()) return; diff --git a/src/core/mainwindow.h b/src/core/mainwindow.h index e8813dc1..efbfdde2 100644 --- a/src/core/mainwindow.h +++ b/src/core/mainwindow.h @@ -242,7 +242,6 @@ class MainWindow : public QMainWindow, public PlatformInterface { void SavePlaybackStatus(); void LoadPlaybackStatus(); void ResumePlayback(); - void ResumePlaybackSeek(const int playback_position); void Exit(); void DoExit(); diff --git a/src/core/player.cpp b/src/core/player.cpp index 542b8d7a..c59e9be1 100644 --- a/src/core/player.cpp +++ b/src/core/player.cpp @@ -90,7 +90,8 @@ Player::Player(Application *app, QObject *parent) greyout_(true), menu_previousmode_(BehaviourSettingsPage::PreviousBehaviour_DontRestart), seek_step_sec_(10), - volume_control_(true) + volume_control_(true), + play_offset_nanosec_(0) { settings_.beginGroup(kSettingsGroup); @@ -333,9 +334,10 @@ void Player::HandleLoadResult(const UrlHandler::LoadResult &result) { } if (is_current) { - qLog(Debug) << "Playing song" << item->Metadata().title() << result.stream_url_; - engine_->Play(result.stream_url_, result.original_url_, stream_change_type_, song.has_cue(), song.beginning_nanosec(), song.end_nanosec()); + qLog(Debug) << "Playing song" << item->Metadata().title() << result.stream_url_ << "position" << play_offset_nanosec_; + engine_->Play(result.stream_url_, result.original_url_, stream_change_type_, song.has_cue(), song.beginning_nanosec(), song.end_nanosec(), play_offset_nanosec_); current_item_ = item; + play_offset_nanosec_ = 0; } else if (is_next) { qLog(Debug) << "Preloading next song" << next_item->Metadata().title() << result.stream_url_; @@ -359,6 +361,9 @@ void Player::Next() { NextInternal(Engine::Manual, Playlist::AutoScroll_Always); void Player::NextInternal(const Engine::TrackChangeFlags change, const Playlist::AutoScroll autoscroll) { + pause_time_ = QDateTime(); + play_offset_nanosec_ = 0; + if (HandleStopAfter(autoscroll)) return; NextItem(change, autoscroll); @@ -367,6 +372,9 @@ void Player::NextInternal(const Engine::TrackChangeFlags change, const Playlist: void Player::NextItem(const Engine::TrackChangeFlags change, const Playlist::AutoScroll autoscroll) { + pause_time_ = QDateTime(); + play_offset_nanosec_ = 0; + Playlist *active_playlist = app_->playlist_manager()->active(); // If we received too many errors in auto change, with repeat enabled, we stop @@ -402,7 +410,11 @@ void Player::PlayPlaylist(const QString &playlist_name) { PlayPlaylistInternal(Engine::Manual, Playlist::AutoScroll_Always, playlist_name); } -void Player::PlayPlaylistInternal(Engine::TrackChangeFlags change, const Playlist::AutoScroll autoscroll, const QString &playlist_name) { +void Player::PlayPlaylistInternal(const Engine::TrackChangeFlags change, const Playlist::AutoScroll autoscroll, const QString &playlist_name) { + + pause_time_ = QDateTime(); + play_offset_nanosec_ = 0; + Playlist *playlist = nullptr; for (Playlist *p : app_->playlist_manager()->GetAllPlaylists()) { if (playlist_name == app_->playlist_manager()->GetPlaylistName(p->id())) { @@ -425,6 +437,7 @@ void Player::PlayPlaylistInternal(Engine::TrackChangeFlags change, const Playlis if (i == -1) i = 0; PlayAt(i, change, autoscroll, true); + } @@ -442,6 +455,7 @@ bool Player::HandleStopAfter(const Playlist::AutoScroll autoscroll) { Stop(true); return true; } + return false; } @@ -458,11 +472,13 @@ void Player::TrackEnded() { } -void Player::PlayPause(Playlist::AutoScroll autoscroll) { +void Player::PlayPause(const quint64 offset_nanosec, const Playlist::AutoScroll autoscroll) { + + play_offset_nanosec_ = offset_nanosec; switch (engine_->state()) { case Engine::Paused: - engine_->Unpause(); + UnPause(); emit Resumed(); break; @@ -471,6 +487,8 @@ void Player::PlayPause(Playlist::AutoScroll autoscroll) { Stop(); } else { + pause_time_ = QDateTime::currentDateTime(); + play_offset_nanosec_ = engine_->position_nanosec(); engine_->Pause(); } break; @@ -479,6 +497,7 @@ void Player::PlayPause(Playlist::AutoScroll autoscroll) { case Engine::Empty: case Engine::Error: case Engine::Idle: { + pause_time_ = QDateTime(); app_->playlist_manager()->SetActivePlaylist(app_->playlist_manager()->current_id()); if (app_->playlist_manager()->active()->rowCount() == 0) break; int i = app_->playlist_manager()->active()->current_row(); @@ -491,19 +510,45 @@ void Player::PlayPause(Playlist::AutoScroll autoscroll) { } +void Player::UnPause() { + + if (current_item_ && pause_time_.isValid()) { + const Song &song = current_item_->Metadata(); + if (url_handlers_.contains(song.url().scheme()) && song.stream_url_can_expire()) { + const quint64 time = QDateTime::currentDateTime().toSecsSinceEpoch() - pause_time_.toSecsSinceEpoch(); + if (time >= 30) { // Stream URL might be expired. + qLog(Debug) << "Re-requesting stream URL for" << song.url(); + HandleLoadResult(url_handlers_[song.url().scheme()]->StartLoading(song.url())); + return; + } + } + } + + pause_time_ = QDateTime(); + play_offset_nanosec_ = 0; + + engine_->Unpause(); + +} + void Player::RestartOrPrevious() { + pause_time_ = QDateTime(); + play_offset_nanosec_ = 0; + if (engine_->position_nanosec() < 8 * kNsecPerSec) return Previous(); SeekTo(0); } -void Player::Stop(bool stop_after) { +void Player::Stop(const bool stop_after) { engine_->Stop(stop_after); app_->playlist_manager()->active()->set_current_row(-1); current_item_.reset(); + pause_time_ = QDateTime(); + play_offset_nanosec_ = 0; } @@ -515,12 +560,16 @@ bool Player::PreviousWouldRestartTrack() const { // Check if it has been over two seconds since previous button was pressed return menu_previousmode_ == BehaviourSettingsPage::PreviousBehaviour_Restart && last_pressed_previous_.isValid() && last_pressed_previous_.secsTo(QDateTime::currentDateTime()) >= 2; + } void Player::Previous() { PreviousItem(Engine::Manual); } void Player::PreviousItem(const Engine::TrackChangeFlags change) { + pause_time_ = QDateTime(); + play_offset_nanosec_ = 0; + const bool ignore_repeat_track = change & Engine::Manual; if (menu_previousmode_ == BehaviourSettingsPage::PreviousBehaviour_Restart) { @@ -557,9 +606,13 @@ void Player::EngineStateChanged(const Engine::State state) { switch (state) { case Engine::Paused: + pause_time_ = QDateTime::currentDateTime(); + play_offset_nanosec_ = engine_->position_nanosec(); emit Paused(); break; case Engine::Playing: + pause_time_ = QDateTime(); + play_offset_nanosec_ = 0; emit Playing(); break; case Engine::Error: @@ -567,9 +620,12 @@ void Player::EngineStateChanged(const Engine::State state) { // fallthrough case Engine::Empty: case Engine::Idle: + pause_time_ = QDateTime(); + play_offset_nanosec_ = 0; emit Stopped(); break; } + last_state_ = state; } @@ -605,6 +661,8 @@ void Player::PlayAt(const int index, Engine::TrackChangeFlags change, const Play app_->playlist_manager()->active()->set_current_row(index, autoscroll, false, force_inform); if (app_->playlist_manager()->active()->current_row() == -1) { // Maybe index didn't exist in the playlist. + pause_time_ = QDateTime(); + play_offset_nanosec_ = 0; return; } @@ -613,15 +671,19 @@ void Player::PlayAt(const int index, Engine::TrackChangeFlags change, const Play if (url_handlers_.contains(url.scheme())) { // It's already loading - if (loading_async_.contains(url)) return; + if (loading_async_.contains(url)) { + return; + } stream_change_type_ = change; autoscroll_ = autoscroll; HandleLoadResult(url_handlers_[url.scheme()]->StartLoading(url)); } else { - qLog(Debug) << "Playing song" << current_item_->Metadata().title() << url; - engine_->Play(url, current_item_->Url(), change, current_item_->Metadata().has_cue(), current_item_->effective_beginning_nanosec(), current_item_->effective_end_nanosec()); + qLog(Debug) << "Playing song" << current_item_->Metadata().title() << url << "position" << play_offset_nanosec_; + engine_->Play(url, current_item_->Url(), change, current_item_->Metadata().has_cue(), current_item_->effective_beginning_nanosec(), current_item_->effective_end_nanosec(), play_offset_nanosec_); + pause_time_ = QDateTime(); + play_offset_nanosec_ = 0; } } @@ -717,17 +779,17 @@ void Player::Mute() { void Player::Pause() { engine_->Pause(); } -void Player::Play() { +void Player::Play(const quint64 offset_nanosec) { switch (GetState()) { case Engine::Playing: - SeekTo(0); + SeekTo(offset_nanosec); break; case Engine::Paused: - engine_->Unpause(); + UnPause(); break; default: - PlayPause(); + PlayPause(offset_nanosec); break; } diff --git a/src/core/player.h b/src/core/player.h index f8d4f3fb..484f21e0 100644 --- a/src/core/player.h +++ b/src/core/player.h @@ -64,7 +64,7 @@ class PlayerInterface : public QObject { virtual int GetVolume() const = 0; virtual PlaylistItemPtr GetCurrentItem() const = 0; - virtual PlaylistItemPtr GetItemAt(int pos) const = 0; + virtual PlaylistItemPtr GetItemAt(const int pos) const = 0; virtual void RegisterUrlHandler(UrlHandler *handler) = 0; virtual void UnregisterUrlHandler(UrlHandler *handler) = 0; @@ -76,7 +76,7 @@ class PlayerInterface : public QObject { virtual void PlayAt(const int index, Engine::TrackChangeFlags change, const Playlist::AutoScroll autoscroll, const bool reshuffle, const bool force_inform = false) = 0; // If there's currently a song playing, pause it, otherwise play the track that was playing last, or the first one on the playlist - virtual void PlayPause(Playlist::AutoScroll autoscroll = Playlist::AutoScroll_Always) = 0; + virtual void PlayPause(const quint64 offset_nanosec = 0, const Playlist::AutoScroll autoscroll = Playlist::AutoScroll_Always) = 0; virtual void PlayPauseHelper() = 0; virtual void RestartOrPrevious() = 0; @@ -97,8 +97,9 @@ class PlayerInterface : public QObject { virtual void Mute() = 0; virtual void Pause() = 0; - virtual void Stop(bool stop_after = false) = 0; - virtual void Play() = 0; + virtual void Stop(const bool stop_after = false) = 0; + virtual void Play(const quint64 offset_nanosec = 0) = 0; + virtual void PlayHelper() = 0; virtual void ShowOSD() = 0; signals: @@ -143,7 +144,7 @@ class Player : public PlayerInterface { int GetVolume() const override; PlaylistItemPtr GetCurrentItem() const override { return current_item_; } - PlaylistItemPtr GetItemAt(int pos) const override; + PlaylistItemPtr GetItemAt(const int pos) const override; void RegisterUrlHandler(UrlHandler *handler) override; void UnregisterUrlHandler(UrlHandler *handler) override; @@ -159,8 +160,8 @@ class Player : public PlayerInterface { void ReloadSettings() override; void PlayAt(const int index, Engine::TrackChangeFlags change, const Playlist::AutoScroll autoscroll, const bool reshuffle, const bool force_inform = false) override; - void PlayPause(Playlist::AutoScroll autoscroll = Playlist::AutoScroll_Always) override; - void PlayPauseHelper() override { PlayPause(); } + void PlayPause(const quint64 offset_nanosec = 0, const Playlist::AutoScroll autoscroll = Playlist::AutoScroll_Always) override; + void PlayPauseHelper() override { PlayPause(play_offset_nanosec_); } void RestartOrPrevious() override; void Next() override; void Previous() override; @@ -176,9 +177,10 @@ class Player : public PlayerInterface { void Mute() override; void Pause() override; - void Stop(bool stop_after = false) override; + void Stop(const bool stop_after = false) override; void StopAfterCurrent(); - void Play() override; + void Play(const quint64 offset_nanosec = 0) override; + void PlayHelper() override { Play(); } void ShowOSD() override; void TogglePrettyOSD(); @@ -197,7 +199,7 @@ class Player : public PlayerInterface { void PreviousItem(const Engine::TrackChangeFlags change); void NextInternal(const Engine::TrackChangeFlags, const Playlist::AutoScroll autoscroll); - void PlayPlaylistInternal(Engine::TrackChangeFlags, const Playlist::AutoScroll autoscroll, const QString &playlist_name); + void PlayPlaylistInternal(const Engine::TrackChangeFlags, const Playlist::AutoScroll autoscroll, const QString &playlist_name); void FatalError(); void ValidSongRequested(const QUrl&); @@ -210,6 +212,8 @@ class Player : public PlayerInterface { // Returns true if we were supposed to stop after this track. bool HandleStopAfter(const Playlist::AutoScroll autoscroll); + void UnPause(); + private: Application *app_; std::unique_ptr engine_; @@ -241,6 +245,9 @@ class Player : public PlayerInterface { bool volume_control_; + QDateTime pause_time_; + quint64 play_offset_nanosec_; + }; #endif // PLAYER_H diff --git a/src/core/song.cpp b/src/core/song.cpp index c5ebdf6b..04724e10 100644 --- a/src/core/song.cpp +++ b/src/core/song.cpp @@ -379,6 +379,7 @@ bool Song::is_metadata_good() const { return !d->url_.isEmpty() && !d->artist_.i bool Song::is_stream() const { return d->source_ == Source_Stream || d->source_ == Source_Tidal || d->source_ == Source_Subsonic || d->source_ == Source_Qobuz; } bool Song::is_cdda() const { return d->source_ == Source_CDDA; } bool Song::is_compilation() const { return (d->compilation_ || d->compilation_detected_ || d->compilation_on_) && !d->compilation_off_; } +bool Song::stream_url_can_expire() const { return d->source_ == Song::Source_Tidal || d->source_ == Song::Source_Qobuz; } bool Song::art_automatic_is_valid() const { return !d->art_automatic_.isEmpty() && diff --git a/src/core/song.h b/src/core/song.h index d433864a..4624c756 100644 --- a/src/core/song.h +++ b/src/core/song.h @@ -263,6 +263,7 @@ class Song { bool art_manual_is_valid() const; bool has_valid_art() const; bool is_compilation() const; + bool stream_url_can_expire() const; // Playlist views are special because you don't want to fill in album artists automatically for compilations, but you do for normal albums: const QString &playlist_albumartist() const; diff --git a/src/engine/enginebase.cpp b/src/engine/enginebase.cpp index d130b2b7..4bad6b35 100644 --- a/src/engine/enginebase.cpp +++ b/src/engine/enginebase.cpp @@ -42,8 +42,6 @@ Engine::Base::Base() beginning_nanosec_(0), end_nanosec_(0), scope_(kScopeSize), - output_(""), - device_(QVariant()), rg_enabled_(false), rg_mode_(0), rg_preamp_(0.0), @@ -66,7 +64,7 @@ Engine::Base::Base() Engine::Base::~Base() {} -bool Engine::Base::Load(const QUrl &stream_url, const QUrl &original_url, TrackChangeFlags, bool force_stop_at_end, quint64 beginning_nanosec, qint64 end_nanosec) { +bool Engine::Base::Load(const QUrl &stream_url, const QUrl &original_url, const TrackChangeFlags, const bool force_stop_at_end, const quint64 beginning_nanosec, const qint64 end_nanosec) { Q_UNUSED(force_stop_at_end); @@ -76,16 +74,18 @@ bool Engine::Base::Load(const QUrl &stream_url, const QUrl &original_url, TrackC end_nanosec_ = end_nanosec; about_to_end_emitted_ = false; + return true; } -bool Engine::Base::Play(const QUrl &stream_url, const QUrl &original_url, TrackChangeFlags flags, bool force_stop_at_end, quint64 beginning_nanosec, qint64 end_nanosec) { +bool Engine::Base::Play(const QUrl &stream_url, const QUrl &original_url, const TrackChangeFlags flags, const bool force_stop_at_end, const quint64 beginning_nanosec, const qint64 end_nanosec, const quint64 offset_nanosec) { - if (!Load(stream_url, original_url, flags, force_stop_at_end, beginning_nanosec, end_nanosec)) + if (!Load(stream_url, original_url, flags, force_stop_at_end, beginning_nanosec, end_nanosec)) { return false; + } - return Play(0); + return Play(offset_nanosec); } @@ -96,7 +96,7 @@ void Engine::Base::SetVolume(const uint value) { } -uint Engine::Base::MakeVolumeLogarithmic(uint volume) { +uint Engine::Base::MakeVolumeLogarithmic(const uint volume) { // We're using a logarithmic function to make the volume ramp more natural. return static_cast( 100 - 100.0 * std::log10( ( 100 - volume ) * 0.09 + 1.0 ) ); } diff --git a/src/engine/enginebase.h b/src/engine/enginebase.h index 50538e60..e85eddd3 100644 --- a/src/engine/enginebase.h +++ b/src/engine/enginebase.h @@ -96,7 +96,7 @@ class Base : public QObject { // Plays a media stream represented with the URL 'u' from the given 'beginning' to the given 'end' (usually from 0 to a song's length). // Both markers should be passed in nanoseconds. 'end' can be negative, indicating that the real length of 'u' stream is unknown. - bool Play(const QUrl &stream_url, const QUrl &original_url, const TrackChangeFlags flags, const bool force_stop_at_end, const quint64 beginning_nanosec, const qint64 end_nanosec); + bool Play(const QUrl &stream_url, const QUrl &original_url, const TrackChangeFlags flags, const bool force_stop_at_end, const quint64 beginning_nanosec, const qint64 end_nanosec, const quint64 offset_nanosec); void SetVolume(const uint value); static uint MakeVolumeLogarithmic(const uint volume); diff --git a/src/playlist/playlistview.h b/src/playlist/playlistview.h index d4c2f2ca..b9384016 100644 --- a/src/playlist/playlistview.h +++ b/src/playlist/playlistview.h @@ -126,7 +126,7 @@ class PlaylistView : public QTreeView { signals: void PlayItem(QModelIndex idx, Playlist::AutoScroll autoscroll); - void PlayPause(Playlist::AutoScroll autoscroll = Playlist::AutoScroll_Never); + void PlayPause(const quint64 offset_nanosec = 0, Playlist::AutoScroll autoscroll = Playlist::AutoScroll_Never); void RightClicked(QPoint global_pos, QModelIndex idx); void SeekForward(); void SeekBackward();