Add better error handling for CDDA loader

src/core/songloader.cpp

@@ -185,7 +185,7 @@ SongLoader::Result SongLoader::LoadAudioCD() {
 #if defined(HAVE_AUDIOCD) && defined(HAVE_GSTREAMER)
   if (player_->engine()->type() == Engine::GStreamer) {
     CddaSongLoader *cdda_song_loader = new CddaSongLoader(QUrl(), this);
-    connect(cdda_song_loader, SIGNAL(SongsDurationLoaded(SongList)), this, SLOT(AudioCDTracksLoadedSlot(SongList)));
+    connect(cdda_song_loader, SIGNAL(SongsDurationLoaded(SongList, QString)), this, SLOT(AudioCDTracksLoadFinishedSlot(SongList, QString)));
     connect(cdda_song_loader, SIGNAL(SongsMetadataLoaded(SongList)), this, SLOT(AudioCDTracksTagsLoaded(SongList)));
     cdda_song_loader->LoadSongs();
     return Success;
@@ -201,16 +201,22 @@ SongLoader::Result SongLoader::LoadAudioCD() {
 }
 
 #if defined(HAVE_AUDIOCD) && defined(HAVE_GSTREAMER)
-void SongLoader::AudioCDTracksLoadedSlot(const SongList &songs) {
+
+void SongLoader::AudioCDTracksLoadFinishedSlot(const SongList &songs, const QString &error) {
+
   songs_ = songs;
-  emit AudioCDTracksLoaded();
+  errors_ << error;
+  emit AudioCDTracksLoadFinished();
+
 }
 
 void SongLoader::AudioCDTracksTagsLoaded(const SongList &songs) {
+
   CddaSongLoader *cdda_song_loader = qobject_cast<CddaSongLoader*>(sender());
   cdda_song_loader->deleteLater();
   songs_ = songs;
   emit LoadAudioCDFinished(true);
+
 }
 #endif
 

src/core/songloader.h

@@ -57,7 +57,7 @@ class CddaSongLoader;
 class SongLoader : public QObject {
   Q_OBJECT
  public:
-  SongLoader(CollectionBackendInterface *collection, const Player *player, QObject *parent = nullptr);
+  explicit SongLoader(CollectionBackendInterface *collection, const Player *player, QObject *parent = nullptr);
   ~SongLoader();
 
   enum Result {
@@ -87,15 +87,15 @@ class SongLoader : public QObject {
   QStringList errors() { return errors_; }
 
  signals:
-  void AudioCDTracksLoaded();
+  void AudioCDTracksLoadFinished();
-  void LoadAudioCDFinished(bool success);
+  void LoadAudioCDFinished(const bool success);
   void LoadRemoteFinished();
 
  private slots:
   void Timeout();
   void StopTypefind();
 #if defined(HAVE_AUDIOCD) && defined(HAVE_GSTREAMER)
-  void AudioCDTracksLoadedSlot(const SongList &songs);
+  void AudioCDTracksLoadFinishedSlot(const SongList &songs, const QString &error);
   void AudioCDTracksTagsLoaded(const SongList &songs);
 #endif  // HAVE_AUDIOCD && HAVE_GSTREAMER
 

src/device/cddasongloader.cpp

@@ -67,6 +67,7 @@ void CddaSongLoader::LoadSongs() {
   QMutexLocker locker(&mutex_load_);
   cdio_ = cdio_open(url_.path().toLocal8Bit().constData(), DRIVER_DEVICE);
   if (cdio_ == nullptr) {
+    Error("Unable to open CDIO device.");
     return;
   }
 
@@ -74,7 +75,7 @@ void CddaSongLoader::LoadSongs() {
   GError *error = nullptr;
   cdda_ = gst_element_make_from_uri(GST_URI_SRC, "cdda://", nullptr, &error);
   if (error) {
-    qLog(Error) << error->code << error->message;
+    Error(QString("%1: %2").arg(error->code).arg(error->message));
   }
   if (cdda_ == nullptr) {
     return;
@@ -84,13 +85,23 @@ void CddaSongLoader::LoadSongs() {
     g_object_set(cdda_, "device", g_strdup(url_.path().toLocal8Bit().constData()), nullptr);
   }
   if (g_object_class_find_property (G_OBJECT_GET_CLASS (cdda_), "paranoia-mode")) {
-    g_object_set (cdda_, "paranoia-mode", 0, nullptr);
+    g_object_set(cdda_, "paranoia-mode", 0, nullptr);
   }
 
   // Change the element's state to ready and paused, to be able to query it
-  if (gst_element_set_state(cdda_, GST_STATE_READY) == GST_STATE_CHANGE_FAILURE || gst_element_set_state(cdda_, GST_STATE_PAUSED) == GST_STATE_CHANGE_FAILURE) {
+  if (gst_element_set_state(cdda_, GST_STATE_READY) == GST_STATE_CHANGE_FAILURE) {
     gst_element_set_state(cdda_, GST_STATE_NULL);
     gst_object_unref(GST_OBJECT(cdda_));
+    cdda_ = nullptr;
+    Error(tr("Error while setting CDDA device to ready state."));
+    return;
+  }
+
+  if (gst_element_set_state(cdda_, GST_STATE_PAUSED) == GST_STATE_CHANGE_FAILURE) {
+    gst_element_set_state(cdda_, GST_STATE_NULL);
+    gst_object_unref(GST_OBJECT(cdda_));
+    cdda_ = nullptr;
+    Error(tr("Error while setting CDDA device to pause state."));
     return;
   }
 
@@ -98,9 +109,20 @@ void CddaSongLoader::LoadSongs() {
   GstFormat fmt = gst_format_get_by_nick("track");
   GstFormat out_fmt = fmt;
   gint64 num_tracks = 0;
-  if (!gst_element_query_duration(cdda_, out_fmt, &num_tracks) || out_fmt != fmt) {
+  if (!gst_element_query_duration(cdda_, out_fmt, &num_tracks)) {
-    qLog(Error) << "Error while querying cdda GstElement";
+    gst_element_set_state(cdda_, GST_STATE_NULL);
     gst_object_unref(GST_OBJECT(cdda_));
+    cdda_ = nullptr;
+    Error(tr("Error while querying CDDA tracks."));
+    return;
+  }
+
+  if (out_fmt != fmt) {
+    qLog(Error) << "Error while querying cdda GstElement (2).";
+    gst_element_set_state(cdda_, GST_STATE_NULL);
+    gst_object_unref(GST_OBJECT(cdda_));
+    cdda_ = nullptr;
+    Error(tr("Error while querying CDDA tracks."));
     return;
   }
 
@@ -232,3 +254,9 @@ bool CddaSongLoader::HasChanged() {
 
 }
 
+void CddaSongLoader::Error(const QString &error) {
+
+  qLog(Error) << error;
+  emit SongsDurationLoaded(SongList(), error);
+
+}

src/device/cddasongloader.h

@@ -44,19 +44,21 @@ class CddaSongLoader : public QObject {
   Q_OBJECT
 
  public:
-  CddaSongLoader(
+  explicit CddaSongLoader(const QUrl &url = QUrl(), QObject *parent = nullptr);
-      // Url of the CD device. Will use the default device if empty
-      const QUrl &url = QUrl(),
-      QObject *parent = nullptr);
   ~CddaSongLoader();
 
   // Load songs. Signals declared below will be emitted anytime new information will be available.
   void LoadSongs();
   bool HasChanged();
 
+ private:
+  void Error(const QString &error);
+  QUrl GetUrlFromTrack(const int track_number) const;
+
  signals:
+  void SongsLoadError(const QString &error);
   void SongsLoaded(const SongList &songs);
-  void SongsDurationLoaded(const SongList &songs);
+  void SongsDurationLoaded(const SongList &songs, const QString &error = QString());
   void SongsMetadataLoaded(const SongList &songs);
 
  private slots:
@@ -65,8 +67,6 @@ class CddaSongLoader : public QObject {
 #endif
 
  private:
-  QUrl GetUrlFromTrack(int track_number) const;
-
   QUrl url_;
   GstElement *cdda_;
   CdIo_t *cdio_;
@@ -74,4 +74,3 @@ class CddaSongLoader : public QObject {
 };
 
 #endif // CDDASONGLOADER_H
-

src/playlist/songloaderinserter.cpp

@@ -100,7 +100,7 @@ void SongLoaderInserter::LoadAudioCD(Playlist *destination, int row, bool play_n
   enqueue_next_ = enqueue_next;
 
   SongLoader *loader = new SongLoader(collection_, player_, this);
-  NewClosure(loader, SIGNAL(AudioCDTracksLoaded()), this, SLOT(AudioCDTracksLoaded(SongLoader*)), loader);
+  NewClosure(loader, SIGNAL(AudioCDTracksLoadFinished()), this, SLOT(AudioCDTracksLoadFinished(SongLoader*)), loader);
   connect(loader, SIGNAL(LoadAudioCDFinished(bool)), SLOT(AudioCDTagsLoaded(bool)));
   qLog(Info) << "Loading audio CD...";
   SongLoader::Result ret = loader->LoadAudioCD();
@@ -114,18 +114,27 @@ void SongLoaderInserter::LoadAudioCD(Playlist *destination, int row, bool play_n
     }
     delete loader;
   }
-  // Songs will be loaded later: see AudioCDTracksLoaded and AudioCDTagsLoaded slots
+  // Songs will be loaded later: see AudioCDTracksLoadFinished and AudioCDTagsLoaded slots
 
 }
 
 void SongLoaderInserter::DestinationDestroyed() { destination_ = nullptr; }
 
-void SongLoaderInserter::AudioCDTracksLoaded(SongLoader *loader) {
+void SongLoaderInserter::AudioCDTracksLoadFinished(SongLoader *loader) {
+
   songs_ = loader->songs();
-  InsertSongs();
+  if (songs_.isEmpty()) {
+    for (const QString &error : loader->errors()) {
+      emit Error(error);
+    }
+  }
+  else {
+    InsertSongs();
+  }
+
 }
 
-void SongLoaderInserter::AudioCDTagsLoaded(bool success) {
+void SongLoaderInserter::AudioCDTagsLoaded(const bool success) {
 
   SongLoader *loader = qobject_cast<SongLoader*>(sender());
   if (!loader || !destination_) return;
@@ -140,10 +149,12 @@ void SongLoaderInserter::AudioCDTagsLoaded(bool success) {
 }
 
 void SongLoaderInserter::InsertSongs() {
+
   // Insert songs (that haven't been completely loaded) to allow user to see and play them while not loaded completely
   if (destination_) {
     destination_->InsertSongsOrCollectionItems(songs_, row_, play_now_, enqueue_, enqueue_next_);
   }
+
 }
 
 void SongLoaderInserter::AsyncLoad() {

src/playlist/songloaderinserter.h

@@ -38,8 +38,9 @@ class Playlist;
 
 class SongLoaderInserter : public QObject {
   Q_OBJECT
+
  public:
-  SongLoaderInserter(TaskManager *task_manager, CollectionBackendInterface *collection, const Player *player);
+  explicit SongLoaderInserter(TaskManager *task_manager, CollectionBackendInterface *collection, const Player *player);
   ~SongLoaderInserter();
 
   void Load(Playlist *destination, int row, bool play_now, bool enqueue, bool enqueue_next, const QList<QUrl> &urls);
@@ -52,8 +53,8 @@ class SongLoaderInserter : public QObject {
 
  private slots:
   void DestinationDestroyed();
-  void AudioCDTracksLoaded(SongLoader *loader);
+  void AudioCDTracksLoadFinished(SongLoader *loader);
-  void AudioCDTagsLoaded(bool success);
+  void AudioCDTagsLoaded(const bool success);
   void InsertSongs();
 
  private: