Delay play until playlists have finished loading

Fixes #1465

src/core/mainwindow.cpp

@@ -622,6 +622,7 @@ MainWindow::MainWindow(Application *app, SharedPtr<SystemTrayIcon> tray_icon, OS
   QObject::connect(&*app_->player(), &Player::VolumeChanged, ui_->volume, &VolumeSlider::SetValue);
   QObject::connect(&*app_->player(), &Player::ForceShowOSD, this, &MainWindow::ForceShowOSD);
 
+  QObject::connect(&*app_->playlist_manager(), &PlaylistManager::AllPlaylistsLoaded, &*app->player(), &Player::PlaylistsLoaded);
   QObject::connect(&*app_->playlist_manager(), &PlaylistManager::CurrentSongChanged, this, &MainWindow::SongChanged);
   QObject::connect(&*app_->playlist_manager(), &PlaylistManager::CurrentSongChanged, &*app_->player(), &Player::CurrentMetadataChanged);
   QObject::connect(&*app_->playlist_manager(), &PlaylistManager::EditingFinished, this, &MainWindow::PlaylistEditFinished);
@@ -1024,9 +1025,6 @@ MainWindow::MainWindow(Application *app, SharedPtr<SystemTrayIcon> tray_icon, OS
 
   CommandlineOptionsReceived(options);
 
-  if (!options.contains_play_options()) {
-    LoadPlaybackStatus();
-  }
   if (app_->scrobbler()->enabled() && !app_->scrobbler()->offline()) {
     app_->scrobbler()->Submit();
   }
@@ -1281,8 +1279,8 @@ void MainWindow::RefreshStyleSheet() {
 void MainWindow::SaveSettings() {
 
   SaveGeometry();
-  SavePlaybackStatus();
   app_->player()->SaveVolume();
+  app_->player()->SavePlaybackStatus();
   ui_->tabs->SaveSettings(QLatin1String(kSettingsGroup));
   ui_->playlist->view()->SaveSettings();
   app_->scrobbler()->WriteCache();
@@ -1549,78 +1547,6 @@ void MainWindow::SaveGeometry() {
 
 }
 
-void MainWindow::SavePlaybackStatus() {
-
-  Settings s;
-
-  s.beginGroup(Player::kSettingsGroup);
-  s.setValue("playback_state", static_cast<int>(app_->player()->GetState()));
-  if (app_->player()->GetState() == EngineBase::State::Playing || app_->player()->GetState() == EngineBase::State::Paused) {
-    s.setValue("playback_playlist", app_->playlist_manager()->active()->id());
-    s.setValue("playback_position", app_->player()->engine()->position_nanosec() / kNsecPerSec);
-  }
-  else {
-    s.setValue("playback_playlist", -1);
-    s.setValue("playback_position", 0);
-  }
-
-  s.endGroup();
-
-}
-
-void MainWindow::LoadPlaybackStatus() {
-
-  Settings s;
-
-  s.beginGroup(BehaviourSettingsPage::kSettingsGroup);
-  const bool resume_playback = s.value("resumeplayback", false).toBool();
-  s.endGroup();
-
-  s.beginGroup(Player::kSettingsGroup);
-  const EngineBase::State playback_state = static_cast<EngineBase::State>(s.value("playback_state", static_cast<int>(EngineBase::State::Empty)).toInt());
-  s.endGroup();
-
-  if (resume_playback && (playback_state == EngineBase::State::Playing || playback_state == EngineBase::State::Paused)) {
-    SharedPtr<QMetaObject::Connection> connection = make_shared<QMetaObject::Connection>();
-    *connection = QObject::connect(&*app_->playlist_manager(), &PlaylistManager::AllPlaylistsLoaded, this, [this, connection]() {
-      QObject::disconnect(*connection);
-      QTimer::singleShot(400ms, this, &MainWindow::ResumePlayback);
-    });
-  }
-
-}
-
-void MainWindow::ResumePlayback() {
-
-  qLog(Debug) << "Resuming playback";
-
-  Settings s;
-  s.beginGroup(Player::kSettingsGroup);
-  const EngineBase::State playback_state = static_cast<EngineBase::State>(s.value("playback_state", static_cast<int>(EngineBase::State::Empty)).toInt());
-  const int playback_playlist = s.value("playback_playlist", -1).toInt();
-  const int playback_position = s.value("playback_position", 0).toInt();
-  s.endGroup();
-
-  if (playback_playlist == app_->playlist_manager()->current()->id()) {
-    // Set active to current to resume playback on correct playlist.
-    app_->playlist_manager()->SetActiveToCurrent();
-    if (playback_state == EngineBase::State::Playing) {
-      app_->player()->Play(playback_position * kNsecPerSec);
-    }
-    else if (playback_state == EngineBase::State::Paused) {
-      app_->player()->PlayWithPause(playback_position * kNsecPerSec);
-    }
-  }
-
-  // Reset saved playback status so we don't resume again from the same position.
-  s.beginGroup(Player::kSettingsGroup);
-  s.setValue("playback_state", static_cast<int>(EngineBase::State::Empty));
-  s.setValue("playback_playlist", -1);
-  s.setValue("playback_position", 0);
-  s.endGroup();
-
-}
-
 void MainWindow::PlayIndex(const QModelIndex &idx, Playlist::AutoScroll autoscroll) {
 
   if (!idx.isValid()) return;

src/core/mainwindow.h

@@ -238,9 +238,6 @@ class MainWindow : public QMainWindow, public PlatformInterface {
   void ToggleSidebar(const bool checked);
   void ToggleSearchCoverAuto(const bool checked);
   void SaveGeometry();
-  void SavePlaybackStatus();
-  void LoadPlaybackStatus();
-  void ResumePlayback();
 
   void Exit();
   void DoExit();

src/core/player.cpp

@@ -85,6 +85,8 @@ Player::Player(Application *app, QObject *parent)
       analyzer_(nullptr),
       equalizer_(nullptr),
       timer_save_volume_(new QTimer(this)),
+      playlists_loaded_(false),
+      play_requested_(false),
       stream_change_type_(EngineBase::TrackChangeType::First),
       autoscroll_(Playlist::AutoScroll::Maybe),
       last_state_(EngineBase::State::Empty),
@@ -245,6 +247,80 @@ void Player::SaveVolume() {
 
 }
 
+void Player::SavePlaybackStatus() {
+
+  Settings s;
+
+  s.beginGroup(kSettingsGroup);
+  s.setValue("playback_state", static_cast<int>(app_->player()->GetState()));
+  if (app_->player()->GetState() == EngineBase::State::Playing || app_->player()->GetState() == EngineBase::State::Paused) {
+    s.setValue("playback_playlist", app_->playlist_manager()->active()->id());
+    s.setValue("playback_position", app_->player()->engine()->position_nanosec() / kNsecPerSec);
+  }
+  else {
+    s.setValue("playback_playlist", -1);
+    s.setValue("playback_position", 0);
+  }
+  s.endGroup();
+
+}
+
+void Player::PlaylistsLoaded() {
+
+  playlists_loaded_ = true;
+
+  Settings s;
+
+  s.beginGroup(BehaviourSettingsPage::kSettingsGroup);
+  const bool resume_playback = s.value("resumeplayback", false).toBool();
+  s.endGroup();
+
+  s.beginGroup(Player::kSettingsGroup);
+  const EngineBase::State playback_state = static_cast<EngineBase::State>(s.value("playback_state", static_cast<int>(EngineBase::State::Empty)).toInt());
+  s.endGroup();
+
+  if (resume_playback && (playback_state == EngineBase::State::Playing || playback_state == EngineBase::State::Paused)) {
+    ResumePlayback();
+  }
+  else if (play_requested_) {
+    Play();
+  }
+
+  play_requested_ = false;
+
+}
+
+void Player::ResumePlayback() {
+
+  qLog(Debug) << "Resuming playback";
+
+  Settings s;
+  s.beginGroup(kSettingsGroup);
+  const EngineBase::State playback_state = static_cast<EngineBase::State>(s.value("playback_state", static_cast<int>(EngineBase::State::Empty)).toInt());
+  const int playback_playlist = s.value("playback_playlist", -1).toInt();
+  const int playback_position = s.value("playback_position", 0).toInt();
+  s.endGroup();
+
+  if (playback_playlist == app_->playlist_manager()->current()->id()) {
+    // Set active to current to resume playback on correct playlist.
+    app_->playlist_manager()->SetActiveToCurrent();
+    if (playback_state == EngineBase::State::Playing) {
+      Play(playback_position * kNsecPerSec);
+    }
+    else if (playback_state == EngineBase::State::Paused) {
+      PlayWithPause(playback_position * kNsecPerSec);
+    }
+  }
+
+  // Reset saved playback status so we don't resume again from the same position.
+  s.beginGroup(kSettingsGroup);
+  s.setValue("playback_state", static_cast<int>(EngineBase::State::Empty));
+  s.setValue("playback_playlist", -1);
+  s.setValue("playback_position", 0);
+  s.endGroup();
+
+}
+
 void Player::HandleLoadResult(const UrlHandler::LoadResult &result) {
 
   if (loading_async_.contains(result.media_url_)) {
@@ -852,6 +928,11 @@ void Player::Pause() { engine_->Pause(); }
 
 void Player::Play(const quint64 offset_nanosec) {
 
+  if (!playlists_loaded_) {
+    play_requested_ = true;
+    return;
+  }
+
   switch (GetState()) {
     case EngineBase::State::Playing:
       SeekTo(offset_nanosec);

src/core/player.h

@@ -68,6 +68,9 @@ class PlayerInterface : public QObject {
   virtual void ReloadSettings() = 0;
   virtual void LoadVolume() = 0;
   virtual void SaveVolume() = 0;
+  virtual void SavePlaybackStatus() = 0;
+
+  virtual void PlaylistsLoaded() = 0;
 
   // Manual track change to the specified track
   virtual void PlayAt(const int index, const bool pause, const quint64 offset_nanosec, EngineBase::TrackChangeFlags change, const Playlist::AutoScroll autoscroll, const bool reshuffle, const bool force_inform = false) = 0;
@@ -159,6 +162,8 @@ class Player : public PlayerInterface {
   void ReloadSettings() override;
   void LoadVolume() override;
   void SaveVolume() override;
+  void SavePlaybackStatus() override;
+  void PlaylistsLoaded();
 
   void PlayAt(const int index, const bool pause, const quint64 offset_nanosec, EngineBase::TrackChangeFlags change, const Playlist::AutoScroll autoscroll, const bool reshuffle, const bool force_inform = false) override;
   void PlayPause(const quint64 offset_nanosec = 0, const Playlist::AutoScroll autoscroll = Playlist::AutoScroll::Always) override;
@@ -213,6 +218,8 @@ class Player : public PlayerInterface {
   void HandleLoadResult(const UrlHandler::LoadResult &result);
 
  private:
+  void ResumePlayback();
+
   // Returns true if we were supposed to stop after this track.
   bool HandleStopAfter(const Playlist::AutoScroll autoscroll);
 
@@ -228,6 +235,9 @@ class Player : public PlayerInterface {
   SharedPtr<Equalizer> equalizer_;
   QTimer *timer_save_volume_;
 
+  bool playlists_loaded_;
+  bool play_requested_;
+
   PlaylistItemPtr current_item_;
 
   bool pause_;