Merge pull request #4263 from abika/master

Load all songs that require disc-read non-blocking.

src/core/songloader.cpp

@@ -17,7 +17,6 @@
 
 #include "songloader.h"
 
-#include <functional>
 #include <memory>
 
 #include <QBuffer>
@@ -113,8 +112,14 @@ SongLoader::Result SongLoader::Load(const QUrl& url) {
 
   url_ = PodcastUrlLoader::FixPodcastUrl(url_);
 
-  timeout_timer_->start(timeout_);
+  preload_func_ = std::bind(&SongLoader::LoadRemote, this);
-  return LoadRemote();
+  return BlockingLoadRequired;
+}
+
+void SongLoader::LoadFilenamesBlocking() {
+  if (preload_func_) {
+    preload_func_();
+  }
 }
 
 SongLoader::Result SongLoader::LoadLocalPartial(const QString& filename) {
@@ -238,104 +243,91 @@ void SongLoader::AudioCDTagsLoaded(
     song.set_url(QUrl(QString("cdda://%1").arg(track_number++)));
     songs_ << song;
   }
-  emit LoadFinished(true);
+  emit LoadAudioCDFinished(true);
 }
 
 SongLoader::Result SongLoader::LoadLocal(const QString& filename) {
   qLog(Debug) << "Loading local file" << filename;
 
-  // First check to see if it's a directory - if so we can load all the songs
+  // Search in the database.
-  // inside right away.
-  if (QFileInfo(filename).isDir()) {
-    ConcurrentRun::Run<void>(
-        &thread_pool_,
-        std::bind(&SongLoader::LoadLocalDirectoryAndEmit, this, filename));
-    return WillLoadAsync;
-  }
-
-  // It's a local file, so check if it looks like a playlist.
-  // Read the first few bytes.
-  QFile file(filename);
-  if (!file.open(QIODevice::ReadOnly)) return Error;
-  QByteArray data(file.read(PlaylistParser::kMagicSize));
-
-  ParserBase* parser = playlist_parser_->ParserForMagic(data);
-  if (!parser) {
-    // Check the file extension as well, maybe the magic failed, or it was a
-    // basic M3U file which is just a plain list of filenames.
-    parser = playlist_parser_->ParserForExtension(QFileInfo(filename).suffix());
-  }
-
-  if (parser) {
-    qLog(Debug) << "Parsing using" << parser->name();
-
-    // It's a playlist!
-    ConcurrentRun::Run<void>(
-        &thread_pool_,
-        std::bind(&SongLoader::LoadPlaylistAndEmit, this, parser, filename));
-    return WillLoadAsync;
-  }
-
-  // Not a playlist, so just assume it's a song
   QUrl url = QUrl::fromLocalFile(filename);
 
   LibraryQuery query;
   query.SetColumnSpec("%songs_table.ROWID, " + Song::kColumnSpec);
   query.AddWhere("filename", url.toEncoded());
 
-  SongList song_list;
-
   if (library_->ExecQuery(&query) && query.Next()) {
     // we may have many results when the file has many sections
     do {
       Song song;
       song.InitFromQuery(query, true);
 
-      song_list << song;
+      if (song.is_valid()) {
+        songs_ << song;
+      }
     } while (query.Next());
-  } else {
-    QString matching_cue = filename.section('.', 0, -2) + ".cue";
 
+    return Success;
+  }
+
+  // It's not in the database, load it asynchronously.
+  preload_func_ =
+      std::bind(&SongLoader::LoadLocalAsync, this, filename);
+  return BlockingLoadRequired;
+}
+
+void SongLoader::LoadLocalAsync(const QString& filename) {
+
+  // First check to see if it's a directory - if so we will load all the songs
+  // inside right away.
+  if (QFileInfo(filename).isDir()) {
+    LoadLocalDirectory(filename);
+    return;
+  }
+
+  // It's a local file, so check if it looks like a playlist.
+  // Read the first few bytes.
+  QFile file(filename);
+  if (!file.open(QIODevice::ReadOnly)) return;
+  QByteArray data(file.read(PlaylistParser::kMagicSize));
+
+  ParserBase* parser = playlist_parser_->ParserForMagic(data);
+  if (!parser) {
+    // Check the file extension as well, maybe the magic failed, or it was a
+    // basic M3U file which is just a plain list of filenames.
+    parser = playlist_parser_->ParserForExtension(QFileInfo(filename).suffix().toLower());
+  }
+
+  if (parser) {
+    qLog(Debug) << "Parsing using" << parser->name();
+
+    // It's a playlist!
+    LoadPlaylist(parser, filename);
+    return;
+  }
+
+  // Check if it's a cue file
+  QString matching_cue = filename.section('.', 0, -2) + ".cue";
   if (QFile::exists(matching_cue)) {
     // it's a cue - create virtual tracks
     QFile cue(matching_cue);
     cue.open(QIODevice::ReadOnly);
 
-      song_list = cue_parser_->Load(&cue, matching_cue,
+    SongList song_list = cue_parser_->Load(&cue, matching_cue,
                                            QDir(filename.section('/', 0, -2)));
-    } else {
+    for (Song song: song_list){
-      // it's a normal media file, load it asynchronously.
+      if (song.is_valid()) songs_ << song;
-      TagReaderReply* reply = TagReaderClient::Instance()->ReadFile(filename);
-      NewClosure(reply, SIGNAL(Finished(bool)), this,
-                 SLOT(LocalFileLoaded(TagReaderReply*)), reply);
-
-      return WillLoadAsync;
     }
+    return;
   }
 
-  for (const Song& song : song_list) {
+  // Assume it's just a normal file
-    if (song.is_valid()) {
-      songs_ << song;
-    }
-  }
-
-  return Success;
-}
-
-void SongLoader::LocalFileLoaded(TagReaderReply* reply) {
-  reply->deleteLater();
-
   Song song;
-  song.InitFromProtobuf(reply->message().read_file_response().metadata());
+  song.InitFromFilePartial(filename);
-
+  if (song.is_valid()) songs_ << song;
-  if (song.is_valid()) {
-    songs_ << song;
 }
 
-  emit LoadFinished(true);
+void SongLoader::LoadMetadataBlocking() {
-}
-
-void SongLoader::EffectiveSongsLoad() {
   for (int i = 0; i < songs_.size(); i++) {
     EffectiveSongLoad(&songs_[i]);
   }
@@ -360,12 +352,6 @@ void SongLoader::EffectiveSongLoad(Song* song) {
   }
 }
 
-void SongLoader::LoadPlaylistAndEmit(ParserBase* parser,
-                                     const QString& filename) {
-  LoadPlaylist(parser, filename);
-  emit LoadFinished(true);
-}
-
 void SongLoader::LoadPlaylist(ParserBase* parser, const QString& filename) {
   QFile file(filename);
   file.open(QIODevice::ReadOnly);
@@ -385,11 +371,6 @@ static bool CompareSongs(const Song& left, const Song& right) {
   return left.url() < right.url();
 }
 
-void SongLoader::LoadLocalDirectoryAndEmit(const QString& filename) {
-  LoadLocalDirectory(filename);
-  emit LoadFinished(true);
-}
-
 void SongLoader::LoadLocalDirectory(const QString& filename) {
   QDirIterator it(filename, QDir::Files | QDir::NoDotAndDotDot | QDir::Readable,
                   QDirIterator::Subdirectories);
@@ -457,10 +438,10 @@ void SongLoader::StopTypefind() {
     AddAsRawStream();
   }
 
-  emit LoadFinished(success_);
+  emit LoadRemoteFinished();
 }
 
-SongLoader::Result SongLoader::LoadRemote() {
+void SongLoader::LoadRemote() {
   qLog(Debug) << "Loading remote file" << url_;
 
   // It's not a local file so we have to fetch it to see what it is.  We use
@@ -473,6 +454,8 @@ SongLoader::Result SongLoader::LoadRemote() {
   // If the magic succeeds then we know for sure it's a playlist - so read the
   // rest of the file, parse the playlist and return success.
 
+  timeout_timer_->start(timeout_);
+
   // Create the pipeline - it gets unreffed if it goes out of scope
   std::shared_ptr<GstElement> pipeline(gst_pipeline_new(nullptr),
                                        std::bind(&gst_object_unref, _1));
@@ -483,7 +466,7 @@ SongLoader::Result SongLoader::LoadRemote() {
   if (!source) {
     qLog(Warning) << "Couldn't create gstreamer source element for"
                   << url_.toString();
-    return Error;
+    return;
   }
 
   // Create the other elements and link them up
@@ -505,10 +488,15 @@ SongLoader::Result SongLoader::LoadRemote() {
   gst_pad_add_buffer_probe(pad, G_CALLBACK(DataReady), this);
   gst_object_unref(pad);
 
+  QEventLoop loop;
+  loop.connect(this, SIGNAL(LoadRemoteFinished()), SLOT(quit()));
+
   // Start "playing"
   gst_element_set_state(pipeline.get(), GST_STATE_PLAYING);
   pipeline_ = pipeline;
-  return WillLoadAsync;
+
+  // Wait until loading is finished
+  loop.exec();
 }
 
 void SongLoader::TypeFound(GstElement*, uint, GstCaps* caps, void* self) {

src/core/songloader.h

@@ -18,6 +18,7 @@
 #ifndef SONGLOADER_H
 #define SONGLOADER_H
 
+#include <functional>
 #include <memory>
 
 #include <gst/gst.h>
@@ -44,7 +45,11 @@ class SongLoader : public QObject {
              QObject* parent = nullptr);
   ~SongLoader();
 
-  enum Result { Success, Error, WillLoadAsync, };
+  enum Result {
+    Success,
+    Error,
+    BlockingLoadRequired,
+  };
 
   static const int kDefaultTimeout;
 
@@ -54,42 +59,42 @@ class SongLoader : public QObject {
   int timeout() const { return timeout_; }
   void set_timeout(int msec) { timeout_ = msec; }
 
+  // If Success is returned the songs are fully loaded. If BlockingLoadRequired
+  // is returned LoadFilenamesBlocking() needs to be called next.
   Result Load(const QUrl& url);
-  // To effectively load the songs:
+  // Loads the files with only filenames. When finished, songs() contains a
-  // when we call Load() on a directory, it will return WillLoadAsync, load the
+  // complete list of all Song objects, but without metadata. This method is
-  // files with only filenames and emit LoadFinished(). When LoadFinished() is
+  // blocking, do not call it from the UI thread.
-  // received by songloaderinserter, it will insert songs (incompletely loaded)
+  void LoadFilenamesBlocking();
-  // in playlist, and call EffectiveSongsLoad() in a background thread to
+  // Completely load songs previously loaded with LoadFilenamesBlocking(). When
-  // perform the real load of the songs. Next, UpdateItems() will be called on
+  // finished, the Song objects in songs() contain metadata now. This method is
-  // playlist and replace the partially-loaded items by the new ones, fully
+  // blocking, do not call it from the UI thread.
-  // loaded.
+  void LoadMetadataBlocking();
-  void EffectiveSongsLoad();
-  void EffectiveSongLoad(Song* song);
   Result LoadAudioCD();
 
 signals:
-  void LoadFinished(bool success);
+  void LoadAudioCDFinished(bool success);
+  void LoadRemoteFinished();
 
  private slots:
   void Timeout();
   void StopTypefind();
   void AudioCDTagsLoaded(const QString& artist, const QString& album,
                          const MusicBrainzClient::ResultList& results);
-  void LocalFileLoaded(TagReaderReply* reply);
 
  private:
   enum State { WaitingForType, WaitingForMagic, WaitingForData, Finished, };
 
   Result LoadLocal(const QString& filename);
+  void LoadLocalAsync(const QString& filename);
+  void EffectiveSongLoad(Song* song);
   Result LoadLocalPartial(const QString& filename);
   void LoadLocalDirectory(const QString& filename);
   void LoadPlaylist(ParserBase* parser, const QString& filename);
-  void LoadLocalDirectoryAndEmit(const QString& filename);
-  void LoadPlaylistAndEmit(ParserBase* parser, const QString& filename);
 
   void AddAsRawStream();
 
-  Result LoadRemote();
+  void LoadRemote();
 
   // GStreamer callbacks
   static void TypeFound(GstElement* typefind, uint probability, GstCaps* caps,
@@ -116,6 +121,7 @@ signals:
   CueParser* cue_parser_;
 
   // For async loads
+  std::function<void()> preload_func_;
   int timeout_;
   State state_;
   bool success_;

src/playlist/playlistmanager.cpp

@@ -158,37 +158,19 @@ void PlaylistManager::New(const QString& name, const SongList& songs,
 }
 
 void PlaylistManager::Load(const QString& filename) {
-  QUrl url = QUrl::fromLocalFile(filename);
-  SongLoader* loader = new SongLoader(library_backend_, app_->player(), this);
-  connect(loader, SIGNAL(LoadFinished(bool)), SLOT(LoadFinished(bool)));
-  SongLoader::Result result = loader->Load(url);
   QFileInfo info(filename);
 
-  if (result == SongLoader::Error ||
+  int id = playlist_backend_->CreatePlaylist(info.baseName(), QString());
-      (result == SongLoader::Success && loader->songs().isEmpty())) {
+
-    app_->AddError(tr("The playlist '%1' was empty or could not be loaded.")
+  if (id == -1) {
-                       .arg(info.completeBaseName()));
+    emit Error(tr("Couldn't create playlist"));
-    delete loader;
     return;
   }
 
-  if (result == SongLoader::Success) {
+  Playlist* playlist = AddPlaylist(id, info.baseName(), QString(), QString(), false);
-    New(info.baseName(), loader->songs());
-    delete loader;
-  }
-}
 
-void PlaylistManager::LoadFinished(bool success) {
+  QList<QUrl> urls;
-  SongLoader* loader = qobject_cast<SongLoader*>(sender());
+  playlist->InsertUrls(urls << QUrl::fromLocalFile(filename));
-  loader->deleteLater();
-  QString localfile = loader->url().toLocalFile();
-  QFileInfo info(localfile);
-  if (!success || loader->songs().isEmpty()) {
-    app_->AddError(tr("The playlist '%1' was empty or could not be loaded.")
-                       .arg(info.completeBaseName()));
-  }
-
-  New(info.baseName(), loader->songs());
 }
 
 void PlaylistManager::Save(int id, const QString& filename) {

src/playlist/playlistmanager.h

@@ -229,7 +229,6 @@ class PlaylistManager : public PlaylistManagerInterface {
   void OneOfPlaylistsChanged();
   void UpdateSummaryText();
   void SongsDiscovered(const SongList& songs);
-  void LoadFinished(bool success);
   void ItemsLoadedForSavePlaylist(QFutureWatcher<Song>* watcher,
                                   const QString& filename);
 

src/playlist/songloaderinserter.cpp

@@ -31,15 +31,10 @@ SongLoaderInserter::SongLoaderInserter(TaskManager* task_manager,
       row_(-1),
       play_now_(true),
       enqueue_(false),
-      async_load_id_(0),
-      async_progress_(0),
       library_(library),
       player_(player) {}
 
-SongLoaderInserter::~SongLoaderInserter() {
+SongLoaderInserter::~SongLoaderInserter() { qDeleteAll(pending_); }
-  qDeleteAll(pending_);
-  qDeleteAll(pending_async_);
-}
 
 void SongLoaderInserter::Load(Playlist* destination, int row, bool play_now,
                               bool enqueue, const QList<QUrl>& urls) {
@@ -49,20 +44,17 @@ void SongLoaderInserter::Load(Playlist* destination, int row, bool play_now,
   enqueue_ = enqueue;
 
   connect(destination, SIGNAL(destroyed()), SLOT(DestinationDestroyed()));
+  connect(this, SIGNAL(PreloadFinished()), SLOT(InsertSongs()));
   connect(this, SIGNAL(EffectiveLoadFinished(const SongList&)), destination,
           SLOT(UpdateItems(const SongList&)));
 
   for (const QUrl& url : urls) {
     SongLoader* loader = new SongLoader(library_, player_, this);
 
-    // we're connecting this before we're even sure if this is an async load
-    // to avoid race conditions (signal emission before we're listening to it)
-    connect(loader, SIGNAL(LoadFinished(bool)),
-            SLOT(PendingLoadFinished(bool)));
     SongLoader::Result ret = loader->Load(url);
 
-    if (ret == SongLoader::WillLoadAsync) {
+    if (ret == SongLoader::BlockingLoadRequired) {
-      pending_.insert(loader);
+      pending_.append(loader);
       continue;
     }
 
@@ -73,19 +65,17 @@ void SongLoaderInserter::Load(Playlist* destination, int row, bool play_now,
     delete loader;
   }
 
-  if (pending_.isEmpty())
+  if (pending_.isEmpty()) {
-    Finished();
+    InsertSongs();
-  else {
+    deleteLater();
-    async_progress_ = 0;
+  } else {
-    async_load_id_ = task_manager_->StartTask(tr("Loading tracks"));
+    QtConcurrent::run(this, &SongLoaderInserter::AsyncLoad);
-    task_manager_->SetTaskProgress(async_load_id_, async_progress_,
-                                   pending_.count());
   }
 }
 
 // Load audio CD tracks:
 // First, we add tracks (without metadata) into the playlist
-// In the meantine, MusicBrainz will be queried to get songs' metadata.
+// In the meantime, MusicBrainz will be queried to get songs' metadata.
 // AudioCDTagsLoaded will be called next, and playlist's items will be updated.
 void SongLoaderInserter::LoadAudioCD(Playlist* destination, int row,
                                      bool play_now, bool enqueue) {
@@ -95,15 +85,16 @@ void SongLoaderInserter::LoadAudioCD(Playlist* destination, int row,
   enqueue_ = enqueue;
 
   SongLoader* loader = new SongLoader(library_, player_, this);
-  connect(loader, SIGNAL(LoadFinished(bool)), SLOT(AudioCDTagsLoaded(bool)));
+  connect(loader, SIGNAL(LoadAudioCDFinished(bool)), SLOT(AudioCDTagsLoaded(bool)));
   qLog(Info) << "Loading audio CD...";
   SongLoader::Result ret = loader->LoadAudioCD();
   if (ret == SongLoader::Error) {
     emit Error(tr("Error while loading audio CD"));
     delete loader;
-  }
+  } else {
     songs_ = loader->songs();
-  PartiallyFinished();
+    InsertSongs();
+  }
 }
 
 void SongLoaderInserter::DestinationDestroyed() { destination_ = nullptr; }
@@ -119,30 +110,7 @@ void SongLoaderInserter::AudioCDTagsLoaded(bool success) {
   deleteLater();
 }
 
-void SongLoaderInserter::PendingLoadFinished(bool success) {
+void SongLoaderInserter::InsertSongs() {
-  SongLoader* loader = qobject_cast<SongLoader*>(sender());
-  if (!loader || !pending_.contains(loader)) return;
-  pending_.remove(loader);
-  pending_async_.insert(loader);
-
-  if (success)
-    songs_ << loader->songs();
-  else
-    emit Error(tr("Error loading %1").arg(loader->url().toString()));
-
-  task_manager_->SetTaskProgress(async_load_id_, ++async_progress_);
-  if (pending_.isEmpty()) {
-    task_manager_->SetTaskFinished(async_load_id_);
-    async_progress_ = 0;
-    async_load_id_ = task_manager_->StartTask(tr("Loading tracks info"));
-    task_manager_->SetTaskProgress(async_load_id_, async_progress_,
-                                   pending_async_.count());
-    PartiallyFinished();
-    QtConcurrent::run(this, &SongLoaderInserter::EffectiveLoad);
-  }
-}
-
-void SongLoaderInserter::PartiallyFinished() {
   // Insert songs (that haven't been completelly loaded) to allow user to see
   // and play them while not loaded completely
   if (destination_) {
@@ -150,23 +118,35 @@ void SongLoaderInserter::PartiallyFinished() {
   }
 }
 
-void SongLoaderInserter::EffectiveLoad() {
+void SongLoaderInserter::AsyncLoad() {
+
+  // First, quick load raw songs.
+  int async_progress = 0;
+  int async_load_id = task_manager_->StartTask(tr("Loading tracks"));
+  task_manager_->SetTaskProgress(async_load_id, async_progress,
+                                 pending_.count());
+  for (SongLoader* loader : pending_) {
+    loader->LoadFilenamesBlocking();
+    task_manager_->SetTaskProgress(async_load_id, ++async_progress);
+    songs_ << loader->songs();
+  }
+  task_manager_->SetTaskFinished(async_load_id);
+  emit PreloadFinished();
+
+  // Songs are inserted in playlist, now load them completely.
+  async_progress = 0;
+  async_load_id = task_manager_->StartTask(tr("Loading tracks info"));
+  task_manager_->SetTaskProgress(async_load_id, async_progress, songs_.count());
   SongList songs;
-  for (SongLoader* loader : pending_async_) {
+  for (SongLoader* loader : pending_) {
-    loader->EffectiveSongsLoad();
+    loader->LoadMetadataBlocking();
-    task_manager_->SetTaskProgress(async_load_id_, ++async_progress_);
     songs << loader->songs();
+    task_manager_->SetTaskProgress(async_load_id, songs.count());
   }
+  task_manager_->SetTaskFinished(async_load_id);
+
+  // Replace the partially-loaded items by the new ones, fully loaded.
   emit EffectiveLoadFinished(songs);
-  task_manager_->SetTaskFinished(async_load_id_);
-
-  deleteLater();
-}
-
-void SongLoaderInserter::Finished() {
-  if (destination_) {
-    destination_->InsertSongsOrLibraryItems(songs_, row_, play_now_, enqueue_);
-  }
 
   deleteLater();
 }

src/playlist/songloaderinserter.h

@@ -18,8 +18,8 @@
 #ifndef SONGLOADERINSERTER_H
 #define SONGLOADERINSERTER_H
 
+#include <QList>
 #include <QObject>
-#include <QSet>
 #include <QUrl>
 
 #include "core/song.h"
@@ -45,17 +45,16 @@ class SongLoaderInserter : public QObject {
 
 signals:
   void Error(const QString& message);
+  void PreloadFinished();
   void EffectiveLoadFinished(const SongList& songs);
 
  private slots:
-  void PendingLoadFinished(bool success);
   void DestinationDestroyed();
   void AudioCDTagsLoaded(bool success);
+  void InsertSongs();
 
  private:
-  void PartiallyFinished();
+  void AsyncLoad();
-  void EffectiveLoad();
-  void Finished();
 
  private:
   TaskManager* task_manager_;
@@ -67,10 +66,7 @@ signals:
 
   SongList songs_;
 
-  QSet<SongLoader*> pending_;
+  QList<SongLoader*> pending_;
-  QSet<SongLoader*> pending_async_;
-  int async_load_id_;
-  int async_progress_;
   LibraryBackendInterface* library_;
   const Player* player_;
 };