Merge pull request #6323 from jbroadus/async-load-errors

Add error handling path for async song loading.

src/core/songloader.cpp

@@ -130,9 +130,12 @@ SongLoader::Result SongLoader::Load(const QUrl& url) {
   return BlockingLoadRequired;
 }
 
-void SongLoader::LoadFilenamesBlocking() {
+SongLoader::Result SongLoader::LoadFilenamesBlocking() {
   if (preload_func_) {
-    preload_func_();
+    return preload_func_();
+  } else {
+    qLog(Error) << "Preload function was not set for blocking operation";
+    return Error;
   }
 }
 
@@ -207,18 +210,21 @@ SongLoader::Result SongLoader::LoadLocal(const QString& filename) {
   return BlockingLoadRequired;
 }
 
-void SongLoader::LoadLocalAsync(const QString& filename) {
+SongLoader::Result 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;
+    return Success;
   }
 
   // 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;
+  if (!file.open(QIODevice::ReadOnly)) {
+    qLog(Error) << "Could not open file " << filename;
+    return Error;
+  }
   QByteArray data(file.read(PlaylistParser::kMagicSize));
 
   ParserBase* parser = playlist_parser_->ParserForMagic(data);
@@ -234,7 +240,7 @@ void SongLoader::LoadLocalAsync(const QString& filename) {
 
     // It's a playlist!
     LoadPlaylist(parser, filename);
-    return;
+    return Success;
   }
 
   // Check if it's a cue file
@@ -249,7 +255,7 @@ void SongLoader::LoadLocalAsync(const QString& filename) {
     for (Song song : song_list) {
       if (song.is_valid()) songs_ << song;
     }
-    return;
+    return Success;
   }
 
   // Assume it's just a normal file
@@ -257,6 +263,9 @@ void SongLoader::LoadLocalAsync(const QString& filename) {
   song.InitFromFilePartial(filename);
   if (song.is_valid()) {
     songs_ << song;
+    return Success;
+  } else {
+    return Error;
   }
 }
 
@@ -374,7 +383,7 @@ void SongLoader::StopTypefind() {
   emit LoadRemoteFinished();
 }
 
-void SongLoader::LoadRemote() {
+SongLoader::Result 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
@@ -399,7 +408,7 @@ void SongLoader::LoadRemote() {
   if (!source) {
     qLog(Warning) << "Couldn't create gstreamer source element for"
                   << url_.toString();
-    return;
+    return Error;
   }
 
   // Create the other elements and link them up
@@ -430,6 +439,7 @@ void SongLoader::LoadRemote() {
 
   // Wait until loading is finished
   loop.exec();
+  return Success;
 }
 
 void SongLoader::TypeFound(GstElement*, uint, GstCaps* caps, void* self) {

src/core/songloader.h

@@ -72,7 +72,7 @@ class SongLoader : public QObject {
   // Loads the files with only filenames. When finished, songs() contains a
   // complete list of all Song objects, but without metadata. This method is
   // blocking, do not call it from the UI thread.
-  void LoadFilenamesBlocking();
+  Result LoadFilenamesBlocking();
   // Completely load songs previously loaded with LoadFilenamesBlocking(). When
   // finished, the Song objects in songs() contain metadata now. This method is
   // blocking, do not call it from the UI thread.
@@ -101,7 +101,7 @@ signals:
   };
 
   Result LoadLocal(const QString& filename);
-  void LoadLocalAsync(const QString& filename);
+  Result LoadLocalAsync(const QString& filename);
   void EffectiveSongLoad(Song* song);
   Result LoadLocalPartial(const QString& filename);
   void LoadLocalDirectory(const QString& filename);
@@ -109,7 +109,7 @@ signals:
 
   void AddAsRawStream();
 
-  void LoadRemote();
+  Result LoadRemote();
   bool LoadRemotePlaylist(const QUrl& url);
 
   // GStreamer callbacks
@@ -138,7 +138,7 @@ signals:
   CueParser* cue_parser_;
 
   // For async loads
-  std::function<void()> preload_func_;
+  std::function<Result()> preload_func_;
   int timeout_;
   State state_;
   bool success_;

src/playlist/songloaderinserter.cpp

@@ -134,15 +134,23 @@ void SongLoaderInserter::AsyncLoad() {
   int async_load_id = task_manager_->StartTask(tr("Loading tracks"));
   task_manager_->SetTaskProgress(async_load_id, async_progress,
                                  pending_.count());
+  bool first_loaded = false;
   for (int i = 0; i < pending_.count(); ++i) {
     SongLoader* loader = pending_[i];
-    loader->LoadFilenamesBlocking();
+    SongLoader::Result res = loader->LoadFilenamesBlocking();
+
     task_manager_->SetTaskProgress(async_load_id, ++async_progress);
-    if (i == 0) {
+
+    if (res == SongLoader::Error) {
+      emit Error(tr("Error loading %1").arg(loader->url().toString()));
+      continue;
+    }
+    if (!first_loaded) {
       // Load everything from the first song.  It'll start playing as soon as
       // we emit PreloadFinished, so it needs to have the duration set to show
       // properly in the UI.
       loader->LoadMetadataBlocking();
+      first_loaded = true;
     }
     songs_ << loader->songs();
   }