Refactor the way Player gets tracks to play from RadioPlaylistItems. They can now return synchronously with a URL, asynchronously via a signal, or with an error. This properly fixes the problem of preloading a last.fm stream when the track before is about to end.

src/core/player.cpp

@@ -142,8 +142,36 @@ void Player::ReloadSettings() {
   engine_->ReloadSettings();
 }
 
-void Player::RadioStreamFinished() {
-  NextItem(Engine::Auto);
+void Player::HandleSpecialLoad(const PlaylistItem::SpecialLoadResult &result) {
+  switch (result.type_) {
+  case PlaylistItem::SpecialLoadResult::NoMoreTracks:
+    loading_async_ = QUrl();
+    NextItem(Engine::Auto);
+    break;
+
+  case PlaylistItem::SpecialLoadResult::TrackAvailable: {
+    // Might've been an async load, so check we're still on the same item
+    int current_index = playlist_->current_index();
+    if (current_index == -1)
+      return;
+
+    shared_ptr<PlaylistItem> item = playlist_->item_at(current_index);
+    if (!item || item->Url() != result.original_url_)
+      return;
+
+    engine_->Play(result.media_url_, stream_change_type_);
+
+    current_item_ = item->Metadata();
+    current_item_options_ = item->options();
+    loading_async_ = QUrl();
+    break;
+  }
+
+  case PlaylistItem::SpecialLoadResult::WillLoadAsynchronously:
+    // We'll get called again later with either NoMoreTracks or TrackAvailable
+    loading_async_ = result.original_url_;
+    break;
+  }
 }
 
 void Player::Next() {
@@ -152,8 +180,12 @@ void Player::Next() {
 
 void Player::NextInternal(Engine::TrackChangeType change) {
   if (playlist_->current_item_options() & PlaylistItem::ContainsMultipleTracks) {
+    // The next track is already being loaded
+    if (playlist_->current_item()->Url() == loading_async_)
+      return;
+
     stream_change_type_ = change;
-    playlist_->current_item()->LoadNext();
+    HandleSpecialLoad(playlist_->current_item()->LoadNext());
     return;
   }
 
@@ -267,9 +299,15 @@ void Player::PlayAt(int index, Engine::TrackChangeType change, bool reshuffle) {
   current_item_options_ = item->options();
   current_item_ = item->Metadata();
 
-  if (item->options() & PlaylistItem::SpecialPlayBehaviour)
-    item->StartLoading();
+  if (item->options() & PlaylistItem::SpecialPlayBehaviour) {
+    // It's already loading
+    if (item->Url() == loading_async_)
+      return;
+
+    HandleSpecialLoad(item->StartLoading());
+  }
   else {
+    loading_async_ = QUrl();
     engine_->Play(item->Url(), change);
 
     if (lastfm_->IsScrobblingEnabled())
@@ -279,21 +317,6 @@ void Player::PlayAt(int index, Engine::TrackChangeType change, bool reshuffle) {
   emit CapsChange(GetCaps());
 }
 
-void Player::StreamReady(const QUrl& original_url, const QUrl& media_url) {
-  int current_index = playlist_->current_index();
-  if (current_index == -1)
-    return;
-
-  shared_ptr<PlaylistItem> item = playlist_->item_at(current_index);
-  if (!item || item->Url() != original_url)
-    return;
-
-  engine_->Play(media_url, stream_change_type_);
-
-  current_item_ = item->Metadata();
-  current_item_options_ = item->options();
-}
-
 void Player::CurrentMetadataChanged(const Song &metadata) {
   lastfm_->NowPlaying(metadata);
   current_item_ = metadata;
@@ -547,11 +570,16 @@ void Player::TrackAboutToEnd() {
     if (!item)
       return;
 
+    QUrl url = item->Url();
+
     // Get the actual track URL rather than the stream URL.
     if (item->options() & PlaylistItem::ContainsMultipleTracks) {
-      item->LoadNext();
-      return;
+      PlaylistItem::SpecialLoadResult result = item->LoadNext();
+      if (result.type_ != PlaylistItem::SpecialLoadResult::TrackAvailable)
+        return;
+
+      url = result.media_url_;
     }
-    engine_->StartPreloading(item->Url());
+    engine_->StartPreloading(url);
   }
 }

src/core/player.h

@@ -89,16 +89,13 @@ class Player : public QObject {
   // Skips this track.  Might load more of the current radio station.
   void Next();
 
-  // Jumps to the next actual item on the playlist, with an automatic change
-  void RadioStreamFinished();
-
   void Previous();
   void SetVolume(int value);
   void Seek(int seconds);
   void SeekForward() { Seek(+5); }
   void SeekBackward() { Seek(-5); }
 
-  void StreamReady(const QUrl& original_url, const QUrl& media_url);
+  void HandleSpecialLoad(const PlaylistItem::SpecialLoadResult& result);
   void CurrentMetadataChanged(const Song& metadata);
 
   void PlaylistChanged();
@@ -177,6 +174,8 @@ class Player : public QObject {
 
   EngineBase* engine_;
   Engine::TrackChangeType stream_change_type_;
+
+  QUrl loading_async_;
 };
 
 #endif // PLAYER_H

src/playlist/playlistitem.cpp

@@ -22,6 +22,12 @@
 
 #include <QtDebug>
 
+PlaylistItem::SpecialLoadResult::SpecialLoadResult(
+    Type type, const QUrl& original_url, const QUrl& media_url)
+      : type_(type), original_url_(original_url), media_url_(media_url)
+{
+}
+
 PlaylistItem* PlaylistItem::NewFromType(const QString& type) {
   if (type == "Library")
     return new LibraryPlaylistItem(type);

src/playlist/playlistitem.h

@@ -36,13 +36,55 @@ class PlaylistItem {
   enum Option {
     Default = 0x00,
 
+    // The URL returned by Url() isn't the actual URL of the music - the
+    // item needs to do something special before it can get an actual URL.
+    // Causes StartLoading() to get called when the user wants to play.
     SpecialPlayBehaviour = 0x01,
+
+    // This item might be able to provide another track after one finishes, for
+    // example in a radio stream.  Causes LoadNext() to get called when the
+    // next URL is required.
     ContainsMultipleTracks = 0x02,
+
+    // Disables the "pause" action.
     PauseDisabled = 0x04,
+
+    // Enables the last.fm "ban" action.
     LastFMControls = 0x08,
   };
   Q_DECLARE_FLAGS(Options, Option);
 
+  // Returned by StartLoading() and LoadNext(), indicates what the player
+  // should do when it wants to load a playlist item that is marked
+  // SpecialPlayBehaviour or ContainsMultipleTracks.
+  struct SpecialLoadResult {
+    enum Type {
+      // There wasn't a track available, and the player should move on to the
+      // next playlist item.
+      NoMoreTracks,
+
+      // There might be another track available, something will call the
+      // player's HandleSpecialLoad() slot later with the same original_url.
+      WillLoadAsynchronously,
+
+      // There was a track available.  Its url is in media_url.
+      TrackAvailable,
+    };
+
+    SpecialLoadResult(Type type = NoMoreTracks,
+                      const QUrl& original_url = QUrl(),
+                      const QUrl& media_url = QUrl());
+
+    Type type_;
+
+    // The url that the playlist items has in Url().
+    // Might be something unplayable like lastfm://...
+    QUrl original_url_;
+
+    // The actual url to something that gstreamer can play.
+    QUrl media_url_;
+  };
+
   virtual QString type() const { return type_; }
 
   virtual Options options() const { return Default; }
@@ -52,17 +94,15 @@ class PlaylistItem {
   virtual void Reload() {}
 
   virtual Song Metadata() const = 0;
-
-  // If the item needs to do anything special before it can play (eg. start
-  // streaming the radio stream), then it should implement StartLoading() and
-  // return true.  If it returns false then the URL from Url() will be passed
-  // directly to xine instead.
-  virtual void StartLoading() {}
   virtual QUrl Url() const = 0;
 
-  // If the item is a radio station that can play another song after one has
-  // finished then it should do so and return true
-  virtual void LoadNext() {}
+  // Called by the Player if SpecialPlayBehaviour is set - gives the playlist
+  // item a chance to do something clever to get a playable track.
+  virtual SpecialLoadResult StartLoading() { return SpecialLoadResult(); }
+
+  // Called by the player if ContainsMultipleTracks is set - gives the playlist
+  // item a chance to get another track to play.
+  virtual SpecialLoadResult LoadNext() { return SpecialLoadResult(); }
 
   virtual void SetTemporaryMetadata(const Song& metadata) {Q_UNUSED(metadata)}
   virtual void ClearTemporaryMetadata() {}

src/radio/lastfmservice.cpp

@@ -284,23 +284,25 @@ QString LastFMService::TitleForItem(const RadioItem* item) const {
   return QString();
 }
 
-void LastFMService::StartLoading(const QUrl& url) {
+PlaylistItem::SpecialLoadResult LastFMService::StartLoading(const QUrl& url) {
   if (url.scheme() != "lastfm")
-    return;
+    return PlaylistItem::SpecialLoadResult();
   if (!IsAuthenticated())
-    return;
+    return PlaylistItem::SpecialLoadResult();
 
   emit TaskStarted(MultiLoadingIndicator::LoadingLastFM);
 
   last_url_ = url;
   initial_tune_ = true;
   Tune(lastfm::RadioStation(url));
+
+  return PlaylistItem::SpecialLoadResult(
+      PlaylistItem::SpecialLoadResult::WillLoadAsynchronously, url);
 }
 
-void LastFMService::LoadNext(const QUrl &) {
+PlaylistItem::SpecialLoadResult LastFMService::LoadNext(const QUrl &) {
   if (playlist_.empty()) {
-    emit StreamFinished();
-    return;
+    return PlaylistItem::SpecialLoadResult();
   }
 
   lastfm::MutableTrack track = playlist_.dequeue();
@@ -313,7 +315,9 @@ void LastFMService::LoadNext(const QUrl &) {
 
   next_metadata_ = track;
   StreamMetadataReady();
-  emit StreamReady(last_url_, last_track_.url());
+
+  return PlaylistItem::SpecialLoadResult(
+      PlaylistItem::SpecialLoadResult::TrackAvailable, last_url_, last_track_.url());
 }
 
 void LastFMService::StreamMetadataReady() {
@@ -334,7 +338,8 @@ void LastFMService::TunerError(lastfm::ws::Error error) {
   emit TaskFinished(MultiLoadingIndicator::LoadingLastFM);
 
   if (error == lastfm::ws::NotEnoughContent) {
-    emit StreamFinished();
+    emit AsyncLoadFinished(PlaylistItem::SpecialLoadResult(
+        PlaylistItem::SpecialLoadResult::NoMoreTracks, last_url_));
     return;
   }
 
@@ -372,7 +377,7 @@ QString LastFMService::ErrorString(lastfm::ws::Error error) const {
 
 void LastFMService::TunerTrackAvailable() {
   if (initial_tune_) {
-    LoadNext(last_url_);
+    emit AsyncLoadFinished(LoadNext(last_url_));
     initial_tune_ = false;
   }
 }
@@ -609,6 +614,8 @@ void LastFMService::FetchMoreTracksFinished() {
   QNetworkReply* reply = qobject_cast<QNetworkReply*>(sender());
   if (!reply) {
     qWarning() << "Invalid reply on radio.getPlaylist";
+    emit AsyncLoadFinished(PlaylistItem::SpecialLoadResult(
+        PlaylistItem::SpecialLoadResult::NoMoreTracks, reply->url()));
     return;
   }
   reply->deleteLater();
@@ -665,9 +672,18 @@ void LastFMService::TuneFinished() {
   QNetworkReply* reply = qobject_cast<QNetworkReply*>(sender());
   if (!reply) {
     qWarning() << "Invalid reply on radio.tune";
+    emit AsyncLoadFinished(PlaylistItem::SpecialLoadResult(
+        PlaylistItem::SpecialLoadResult::NoMoreTracks, reply->url()));
     return;
   }
 
   FetchMoreTracks();
   reply->deleteLater();
 }
+
+PlaylistItem::Options LastFMService::playlistitem_options() const {
+  return PlaylistItem::SpecialPlayBehaviour |
+         PlaylistItem::LastFMControls |
+         PlaylistItem::PauseDisabled |
+         PlaylistItem::ContainsMultipleTracks;
+}

src/radio/lastfmservice.h

@@ -84,11 +84,10 @@ class LastFMService : public RadioService {
   void ShowContextMenu(RadioItem *item, const QModelIndex& index,
                        const QPoint &global_pos);
 
-  void StartLoading(const QUrl& url);
-  void LoadNext(const QUrl& url);
+  PlaylistItem::SpecialLoadResult StartLoading(const QUrl& url);
+  PlaylistItem::SpecialLoadResult LoadNext(const QUrl& url);
 
-  bool IsPauseAllowed() const { return false; }
-  bool ShowLastFmControls() const { return true; }
+  PlaylistItem::Options playlistitem_options() const;
 
   void ReloadSettings();
 

src/radio/magnatuneservice.cpp

@@ -102,10 +102,6 @@ void MagnatuneService::LazyPopulate(RadioItem *item) {
   item->lazy_loaded = true;
 }
 
-void MagnatuneService::StartLoading(const QUrl& url) {
-  emit StreamReady(url, url);
-}
-
 void MagnatuneService::ReloadDatabase() {
   QNetworkRequest request = QNetworkRequest(QUrl(kDatabaseUrl));
   request.setRawHeader("User-Agent", QString("%1 %2").arg(

src/radio/magnatuneservice.h

@@ -44,8 +44,6 @@ class MagnatuneService : public RadioService {
   RadioItem* CreateRootItem(RadioItem* parent);
   void LazyPopulate(RadioItem* item);
 
-  void StartLoading(const QUrl &url);
-
   void ShowContextMenu(RadioItem* item, const QModelIndex& index,
                        const QPoint& global_pos);
 

src/radio/radiomodel.cpp

@@ -51,8 +51,7 @@ void RadioModel::AddService(RadioService *service) {
 
   connect(service, SIGNAL(TaskStarted(MultiLoadingIndicator::TaskType)), SIGNAL(TaskStarted(MultiLoadingIndicator::TaskType)));
   connect(service, SIGNAL(TaskFinished(MultiLoadingIndicator::TaskType)), SIGNAL(TaskFinished(MultiLoadingIndicator::TaskType)));
-  connect(service, SIGNAL(StreamReady(QUrl,QUrl)), SIGNAL(StreamReady(QUrl,QUrl)));
-  connect(service, SIGNAL(StreamFinished()), SIGNAL(StreamFinished()));
+  connect(service, SIGNAL(AsyncLoadFinished(PlaylistItem::SpecialLoadResult)), SIGNAL(AsyncLoadFinished(PlaylistItem::SpecialLoadResult)));
   connect(service, SIGNAL(StreamError(QString)), SIGNAL(StreamError(QString)));
   connect(service, SIGNAL(StreamMetadataFound(QUrl,Song)), SIGNAL(StreamMetadataFound(QUrl,Song)));
   connect(service, SIGNAL(AddItemToPlaylist(RadioItem*)), SIGNAL(AddItemToPlaylist(RadioItem*)));

src/radio/radiomodel.h

@@ -64,8 +64,7 @@ class RadioModel : public SimpleTreeModel<RadioItem> {
  signals:
   void TaskStarted(MultiLoadingIndicator::TaskType);
   void TaskFinished(MultiLoadingIndicator::TaskType);
-  void StreamReady(const QUrl& original_url, const QUrl& media_url);
-  void StreamFinished();
+  void AsyncLoadFinished(const PlaylistItem::SpecialLoadResult& result);
   void StreamError(const QString& message);
   void StreamMetadataFound(const QUrl& original_url, const Song& song);
 

src/radio/radioplaylistitem.cpp

@@ -90,14 +90,16 @@ Song RadioPlaylistItem::Metadata() const {
   return metadata_;
 }
 
-void RadioPlaylistItem::StartLoading() {
-  if (service_)
-    service_->StartLoading(url_);
+PlaylistItem::SpecialLoadResult RadioPlaylistItem::StartLoading() {
+  if (!service_)
+    return SpecialLoadResult();
+  return service_->StartLoading(url_);
 }
 
-void RadioPlaylistItem::LoadNext() {
-  if (service_)
-    service_->LoadNext(url_);
+PlaylistItem::SpecialLoadResult RadioPlaylistItem::LoadNext() {
+  if (!service_)
+    return SpecialLoadResult();
+  return service_->LoadNext(url_);
 }
 
 QUrl RadioPlaylistItem::Url() const {
@@ -105,18 +107,9 @@ QUrl RadioPlaylistItem::Url() const {
 }
 
 PlaylistItem::Options RadioPlaylistItem::options() const {
-  PlaylistItem::Options ret = SpecialPlayBehaviour;
-
-  if (service_) {
-    ret |= ContainsMultipleTracks;
-
-    if (!service_->IsPauseAllowed())
-      ret |= PauseDisabled;
-    if (service_->ShowLastFmControls())
-      ret |= LastFMControls;
-  }
-
-  return ret;
+  if (!service_)
+    return Default;
+  return service_->playlistitem_options();
 }
 
 void RadioPlaylistItem::SetTemporaryMetadata(const Song& metadata) {

src/radio/radioplaylistitem.h

@@ -36,11 +36,10 @@ class RadioPlaylistItem : public PlaylistItem {
   void BindToQuery(QSqlQuery *query) const;
 
   Song Metadata() const;
-
-  void StartLoading();
   QUrl Url() const;
 
-  void LoadNext();
+  SpecialLoadResult StartLoading();
+  SpecialLoadResult LoadNext();
 
   void SetTemporaryMetadata(const Song& metadata);
   void ClearTemporaryMetadata();

src/radio/radioservice.cpp

@@ -36,6 +36,11 @@ QString RadioService::ArtistForItem(const RadioItem* item) const {
   return item->artist;
 }
 
-void RadioService::LoadNext(const QUrl&) {
-  emit StreamFinished();
+PlaylistItem::SpecialLoadResult RadioService::StartLoading(const QUrl &url) {
+  return PlaylistItem::SpecialLoadResult(
+      PlaylistItem::SpecialLoadResult::TrackAvailable, url, url);
+}
+
+PlaylistItem::SpecialLoadResult RadioService::LoadNext(const QUrl&) {
+  return PlaylistItem::SpecialLoadResult();
 }

src/radio/radioservice.h

@@ -50,11 +50,10 @@ class RadioService : public QObject {
                                const QPoint& global_pos) {
     Q_UNUSED(item); Q_UNUSED(index); Q_UNUSED(global_pos); }
 
-  virtual void StartLoading(const QUrl& url) = 0;
-  virtual void LoadNext(const QUrl& url);
+  virtual PlaylistItem::SpecialLoadResult StartLoading(const QUrl& url);
+  virtual PlaylistItem::SpecialLoadResult LoadNext(const QUrl& url);
 
-  virtual bool IsPauseAllowed() const { return true; }
-  virtual bool ShowLastFmControls() const { return false; }
+  virtual PlaylistItem::Options playlistitem_options() const { return PlaylistItem::Default; }
 
   virtual bool SetupLibraryFilter(LibraryFilterWidget*) const { return false; }
 
@@ -66,8 +65,7 @@ class RadioService : public QObject {
   void TaskStarted(MultiLoadingIndicator::TaskType);
   void TaskFinished(MultiLoadingIndicator::TaskType);
 
-  void StreamReady(const QUrl& original_url, const QUrl& media_url);
-  void StreamFinished();
+  void AsyncLoadFinished(const PlaylistItem::SpecialLoadResult& result);
   void StreamError(const QString& message);
   void StreamMetadataFound(const QUrl& original_url, const Song& song);
 

src/radio/savedradio.cpp

@@ -102,10 +102,6 @@ void SavedRadio::Remove() {
   SaveStreams();
 }
 
-void SavedRadio::StartLoading(const QUrl& url) {
-  emit StreamReady(url, url);
-}
-
 void SavedRadio::AddToPlaylist() {
   emit AddItemToPlaylist(context_item_);
 }

src/radio/savedradio.h

@@ -41,8 +41,6 @@ class SavedRadio : public RadioService {
   void ShowContextMenu(RadioItem* item, const QModelIndex& index,
                        const QPoint& global_pos);
 
-  void StartLoading(const QUrl& url);
-
   void Add(const QUrl& url);
 
  signals:

src/radio/somafmservice.cpp

@@ -74,7 +74,7 @@ void SomaFMService::ShowContextMenu(RadioItem* item, const QModelIndex&,
   context_menu_->popup(global_pos);
 }
 
-void SomaFMService::StartLoading(const QUrl& url) {
+PlaylistItem::SpecialLoadResult SomaFMService::StartLoading(const QUrl& url) {
   // Load the playlist
   QNetworkRequest request = QNetworkRequest(url);
   request.setRawHeader("User-Agent", QString("%1 %2").arg(
@@ -84,15 +84,22 @@ void SomaFMService::StartLoading(const QUrl& url) {
   connect(reply, SIGNAL(finished()), SLOT(LoadPlaylistFinished()));
 
   emit TaskStarted(MultiLoadingIndicator::LoadingStream);
+
+  return PlaylistItem::SpecialLoadResult(
+      PlaylistItem::SpecialLoadResult::WillLoadAsynchronously, url);
 }
 
 void SomaFMService::LoadPlaylistFinished() {
   QNetworkReply* reply = qobject_cast<QNetworkReply*>(sender());
   emit TaskFinished(MultiLoadingIndicator::LoadingStream);
 
+  QUrl original_url(reply->url());
+
   if (reply->error() != QNetworkReply::NoError) {
     // TODO: Error handling
     qDebug() << reply->errorString();
+    emit AsyncLoadFinished(PlaylistItem::SpecialLoadResult(
+        PlaylistItem::SpecialLoadResult::NoMoreTracks, original_url));
     return;
   }
 
@@ -105,7 +112,9 @@ void SomaFMService::LoadPlaylistFinished() {
   QSettings s(temp_file.fileName(), QSettings::IniFormat);
   s.beginGroup("playlist");
 
-  emit StreamReady(reply->url().toString(), s.value("File1").toString());
+  emit AsyncLoadFinished(PlaylistItem::SpecialLoadResult(
+      PlaylistItem::SpecialLoadResult::TrackAvailable,
+      original_url, s.value("File1").toString()));
 }
 
 void SomaFMService::RefreshChannels() {
@@ -206,3 +215,8 @@ void SomaFMService::Homepage() {
 void SomaFMService::AddToPlaylist() {
   emit AddItemToPlaylist(context_item_);
 }
+
+PlaylistItem::Options SomaFMService::playlistitem_options() const {
+  return PlaylistItem::SpecialPlayBehaviour |
+         PlaylistItem::PauseDisabled;
+}

src/radio/somafmservice.h

@@ -46,7 +46,8 @@ class SomaFMService : public RadioService {
 
   void ShowContextMenu(RadioItem* item, const QModelIndex& index, const QPoint& global_pos);
 
-  void StartLoading(const QUrl& url);
+  PlaylistItem::Options playlistitem_options() const;
+  PlaylistItem::SpecialLoadResult StartLoading(const QUrl& url);
 
  private slots:
   void RefreshChannels();

src/ui/mainwindow.cpp

@@ -288,8 +288,7 @@ MainWindow::MainWindow(NetworkAccessManager* network, Engine::Type engine, QWidg
   connect(radio_model_, SIGNAL(TaskStarted(MultiLoadingIndicator::TaskType)), multi_loading_indicator_, SLOT(TaskStarted(MultiLoadingIndicator::TaskType)));
   connect(radio_model_, SIGNAL(TaskFinished(MultiLoadingIndicator::TaskType)), multi_loading_indicator_, SLOT(TaskFinished(MultiLoadingIndicator::TaskType)));
   connect(radio_model_, SIGNAL(StreamError(QString)), SLOT(ReportError(QString)));
-  connect(radio_model_, SIGNAL(StreamFinished()), player_, SLOT(RadioStreamFinished()));
-  connect(radio_model_, SIGNAL(StreamReady(QUrl,QUrl)), player_, SLOT(StreamReady(QUrl,QUrl)));
+  connect(radio_model_, SIGNAL(AsyncLoadFinished(PlaylistItem::SpecialLoadResult)), player_, SLOT(HandleSpecialLoad(PlaylistItem::SpecialLoadResult)));
   connect(radio_model_, SIGNAL(StreamMetadataFound(QUrl,Song)), playlist_, SLOT(SetStreamMetadata(QUrl,Song)));
   connect(radio_model_, SIGNAL(AddItemToPlaylist(RadioItem*)), SLOT(InsertRadioItem(RadioItem*)));
   connect(radio_model_, SIGNAL(AddItemsToPlaylist(PlaylistItemList)), SLOT(InsertRadioItems(PlaylistItemList)));