Don't use Discogs if fetching all album covers because of trottling.

TODO

@@ -4,8 +4,8 @@ TODO
 
 - Fix freeze on exit caused by GStreamer
 - Fix Audio CD playback
-- Improve status/context
-- Fix crash when switching backend
+- Finalize / Improve status/context
+- Fix crash when switching backend while playing
 
 .
 

src/covermanager/albumcoverchoicecontroller.cpp

@@ -97,8 +97,6 @@ QList<QAction*> AlbumCoverChoiceController::GetAllActions() {
 
 QString AlbumCoverChoiceController::LoadCoverFromFile(Song *song) {
     
- //qLog(Debug) << __PRETTY_FUNCTION__;
-    
   QString cover = QFileDialog::getOpenFileName(this, tr("Load cover from disk"), GetInitialPathForFileDialog(*song, QString()), tr(kLoadImageFileFilter) + ";;" + tr(kAllFilesFilter));
 
   if (cover.isNull()) return QString();
@@ -172,8 +170,6 @@ QString AlbumCoverChoiceController::LoadCoverFromURL(Song *song) {
 
 QString AlbumCoverChoiceController::SearchForCover(Song *song) {
 
-  //qLog(Debug) << __PRETTY_FUNCTION__;
-
   QString album = song->effective_album();
   album = album.remove(QRegExp(" ?-? ?(\\(|\\[)(Disc|CD)? ?[0-9](\\)|\\])$"));
 
@@ -345,4 +341,3 @@ QString AlbumCoverChoiceController::SaveCover(Song *song, const QDropEvent *e) {
   return QString();
 
 }
-

src/covermanager/albumcoverfetcher.cpp

@@ -57,7 +57,9 @@ quint64 AlbumCoverFetcher::FetchAlbumCover(const QString &artist, const QString
 }
 
 quint64 AlbumCoverFetcher::SearchForCovers(const QString &artist, const QString &album) {
-    
+
+  fetchall_ = false;
+
   QString album2(album);
   album2 = album2.remove(QRegExp(" ?-? ?(\\(|\\[)(Disc|CD)? ?[0-9](\\)|\\])$"));
 
@@ -105,9 +107,9 @@ void AlbumCoverFetcher::StartRequests() {
 
     CoverSearchRequest request = queued_requests_.dequeue();
 
-    // search objects are this fetcher's children so worst case scenario - they get 
-    // deleted with it
+    // search objects are this fetcher's children so worst case scenario - they get deleted with it
     AlbumCoverFetcherSearch *search = new AlbumCoverFetcherSearch(request, network_, this);
+    search->fetchall_ = fetchall_;
     active_requests_.insert(request.id, search);
 
     connect(search, SIGNAL(SearchFinished(quint64, CoverSearchResults)), SLOT(SingleSearchFinished(quint64, CoverSearchResults)));

src/covermanager/albumcoverfetcher.h

@@ -40,8 +40,7 @@ class QString;
 class AlbumCoverFetcherSearch;
 class CoverProviders;
 
-// This class represents a single search-for-cover request. It identifies
-// and describes the request.
+// This class represents a single search-for-cover request. It identifies and describes the request.
 struct CoverSearchRequest {
   // an unique (for one AlbumCoverFetcher) request identifier
   quint64 id;
@@ -50,17 +49,14 @@ struct CoverSearchRequest {
   QString artist;
   QString album;
 
-  // is this only a search request or should we also fetch the first
-  // cover that's found?
+  // is this only a search request or should we also fetch the first cover that's found?
   bool search;
 };
 
 // This structure represents a single result of some album's cover search request.
-// It contains an URL that leads to a found cover plus its description (usually
-// the "artist - album" string).
+// It contains an URL that leads to a found cover plus its description (usually the "artist - album" string).
 struct CoverSearchResult {
-  // used for grouping in the user interface.  This is set automatically - don't
-  // set it manually in your cover provider.
+  // used for grouping in the user interface.  This is set automatically - don't set it manually in your cover provider.
   QString provider;
 
   // description of this result (we suggest using the "artist - album" format)
@@ -91,6 +87,8 @@ class AlbumCoverFetcher : public QObject {
   quint64 FetchAlbumCover(const QString &artist, const QString &album);
 
   void Clear();
+  
+  bool fetchall_ = false;
 
 signals:
   void AlbumCoverFetched(quint64, const QImage &cover, const CoverSearchStatistics &statistics);
@@ -112,6 +110,7 @@ signals:
   QHash<quint64, AlbumCoverFetcherSearch*> active_requests_;
 
   QTimer *request_starter_;
+  
 };
 
 #endif  // ALBUMCOVERFETCHER_H

src/covermanager/albumcoverfetchersearch.cpp

@@ -36,8 +36,8 @@
 #include "core/logging.h"
 #include "core/network.h"
 
-const int AlbumCoverFetcherSearch::kSearchTimeoutMs = 10000;
-const int AlbumCoverFetcherSearch::kImageLoadTimeoutMs = 2500;
+const int AlbumCoverFetcherSearch::kSearchTimeoutMs = 12000;
+const int AlbumCoverFetcherSearch::kImageLoadTimeoutMs = 3000;
 const int AlbumCoverFetcherSearch::kTargetSize = 500;
 const float AlbumCoverFetcherSearch::kGoodScore = 1.85;
 
@@ -48,23 +48,32 @@ AlbumCoverFetcherSearch::AlbumCoverFetcherSearch(
       image_load_timeout_(new NetworkTimeouts(kImageLoadTimeoutMs, this)),
       network_(network),
       cancel_requested_(false) {
-  // we will terminate the search after kSearchTimeoutMs miliseconds if we are
-  // not
-  // able to find all of the results before that point in time
+
+  // We will terminate the search after kSearchTimeoutMs miliseconds if we are not able to find all of the results before that point in time
   QTimer::singleShot(kSearchTimeoutMs, this, SLOT(TerminateSearch()));
+
 }
 
 void AlbumCoverFetcherSearch::TerminateSearch() {
+
   for (int id : pending_requests_.keys()) {
     pending_requests_.take(id)->CancelSearch(id);
   }
 
   AllProvidersFinished();
+
 }
 
 void AlbumCoverFetcherSearch::Start(CoverProviders *cover_providers) {
 
   for (CoverProvider *provider : cover_providers->List()) {
+
+    // Skip provider if it does not have fetchall set, and we are doing fetchall - "Fetch Missing Covers".
+    if ((provider->fetchall() == false) && (fetchall_ == true)) {
+	//qLog(Debug) << "Skipping provider" << provider->name();
+	continue;
+    }
+
     connect(provider, SIGNAL(SearchFinished(int, QList<CoverSearchResult>)), SLOT(ProviderSearchFinished(int, QList<CoverSearchResult>)));
     const int id = cover_providers->NextId();
     const bool success = provider->StartSearch(request_.artist, request_.album, id);

src/covermanager/albumcoverfetchersearch.h

@@ -44,7 +44,7 @@ class AlbumCoverFetcherSearch : public QObject {
  public:
   AlbumCoverFetcherSearch(const CoverSearchRequest &request, QNetworkAccessManager *network, QObject *parent);
 
-  void Start(CoverProviders* cover_providers);
+  void Start(CoverProviders *cover_providers);
 
   // Cancels all pending requests.  No Finished signals will be emitted, and it
   // is the caller's responsibility to delete the AlbumCoverFetcherSearch.
@@ -52,6 +52,8 @@ class AlbumCoverFetcherSearch : public QObject {
 
   CoverSearchStatistics statistics() const { return statistics_; }
 
+  bool fetchall_ = false;
+
 signals:
   // It's the end of search (when there was no fetch-me-a-cover request).
   void SearchFinished(quint64, const CoverSearchResults& results);
@@ -96,6 +98,7 @@ private:
   QNetworkAccessManager *network_;
 
   bool cancel_requested_;
+  
 };
 
 #endif  // ALBUMCOVERFETCHERSEARCH_H

src/covermanager/albumcovermanager.cpp

@@ -79,8 +79,6 @@ AlbumCoverManager::AlbumCoverManager(Application *app, CollectionBackend *collec
       abort_progress_(new QPushButton(this)),
       jobs_(0),
       collection_backend_(collection_backend) {
-	  
-  //qLog(Debug) << __PRETTY_FUNCTION__;
 
   ui_->setupUi(this);
   ui_->albums->set_cover_manager(this);
@@ -89,7 +87,7 @@ AlbumCoverManager::AlbumCoverManager(Application *app, CollectionBackend *collec
   ui_->action_fetch->setIcon(IconLoader::Load("download" ));
   ui_->export_covers->setIcon(IconLoader::Load("document-save" ));
   ui_->view->setIcon(IconLoader::Load("view-choose" ));
-  ui_->fetch->setIcon(IconLoader::Load("download" ));
+  ui_->button_fetch->setIcon(IconLoader::Load("download" ));
   ui_->action_add_to_playlist->setIcon(IconLoader::Load("media-play" ));
   ui_->action_load->setIcon(IconLoader::Load("media-play" ));
 
@@ -129,7 +127,6 @@ AlbumCoverManager::AlbumCoverManager(Application *app, CollectionBackend *collec
 
 AlbumCoverManager::~AlbumCoverManager() {
   CancelRequests();
-
   delete ui_;
 }
 
@@ -180,7 +177,7 @@ void AlbumCoverManager::Init() {
   connect(ui_->filter, SIGNAL(textChanged(QString)), SLOT(UpdateFilter()));
   connect(filter_group, SIGNAL(triggered(QAction*)), SLOT(UpdateFilter()));
   connect(ui_->view, SIGNAL(clicked()), ui_->view, SLOT(showMenu()));
-  connect(ui_->fetch, SIGNAL(clicked()), SLOT(FetchAlbumCovers()));
+  connect(ui_->button_fetch, SIGNAL(clicked()), SLOT(FetchAlbumCovers()));
   connect(ui_->export_covers, SIGNAL(clicked()), SLOT(ExportCovers()));
   connect(cover_fetcher_, SIGNAL(AlbumCoverFetched(quint64, QImage, CoverSearchStatistics)), SLOT(AlbumCoverFetched(quint64, QImage, CoverSearchStatistics)));
   connect(ui_->action_fetch, SIGNAL(triggered()), SLOT(FetchSingleCover()));
@@ -211,7 +208,7 @@ void AlbumCoverManager::showEvent(QShowEvent *) {
 }
 
 void AlbumCoverManager::closeEvent(QCloseEvent *e) {
-  
+
   if (!cover_fetching_tasks_.isEmpty()) {
     std::unique_ptr<QMessageBox> message_box(new QMessageBox(QMessageBox::Question, tr("Really cancel?"), tr("Closing this window will stop searching for album covers."), QMessageBox::Abort, this));
     message_box->addButton(tr("Don't stop!"), QMessageBox::AcceptRole);
@@ -276,12 +273,12 @@ void AlbumCoverManager::Reset() {
 }
 
 void AlbumCoverManager::EnableCoversButtons() {
-  ui_->fetch->setEnabled(app_->cover_providers()->HasAnyProviders());
+  ui_->button_fetch->setEnabled(app_->cover_providers()->HasAnyProviders());
   ui_->export_covers->setEnabled(true);
 }
 
 void AlbumCoverManager::DisableCoversButtons() {
-  ui_->fetch->setEnabled(false);
+  ui_->button_fetch->setEnabled(false);
   ui_->export_covers->setEnabled(false);
 }
 
@@ -419,6 +416,8 @@ bool AlbumCoverManager::ShouldHide(const QListWidgetItem &item, const QString &f
 
 void AlbumCoverManager::FetchAlbumCovers() {
 
+  cover_fetcher_->fetchall_ = true;
+
   for (int i = 0; i < ui_->albums->count(); ++i) {
     QListWidgetItem *item = ui_->albums->item(i);
     if (item->isHidden()) continue;
@@ -429,7 +428,7 @@ void AlbumCoverManager::FetchAlbumCovers() {
     jobs_++;
   }
 
-  if (!cover_fetching_tasks_.isEmpty()) ui_->fetch->setEnabled(false);
+  if (!cover_fetching_tasks_.isEmpty()) ui_->button_fetch->setEnabled(false);
 
   progress_bar_->setMaximum(jobs_);
   progress_bar_->show();
@@ -455,6 +454,7 @@ void AlbumCoverManager::AlbumCoverFetched(quint64 id, const QImage &image, const
 
   fetch_statistics_ += statistics;
   UpdateStatusText();
+
 }
 
 void AlbumCoverManager::UpdateStatusText() {
@@ -556,7 +556,9 @@ void AlbumCoverManager::ShowCover() {
 }
 
 void AlbumCoverManager::FetchSingleCover() {
-  
+
+  cover_fetcher_->fetchall_ = false;
+
   for (QListWidgetItem *item : context_menu_items_) {
     quint64 id = cover_fetcher_->FetchAlbumCover(EffectiveAlbumArtistName(*item), item->data(Role_AlbumName).toString());
     cover_fetching_tasks_[id] = item;
@@ -567,9 +569,8 @@ void AlbumCoverManager::FetchSingleCover() {
   progress_bar_->show();
   abort_progress_->show();
   UpdateStatusText();
-  
-}
 
+}
 
 void AlbumCoverManager::UpdateCoverInList(QListWidgetItem *item, const QString &cover) {
   
@@ -595,7 +596,7 @@ void AlbumCoverManager::LoadCoverFromFile() {
 }
 
 void AlbumCoverManager::SaveCoverToFile() {
-  
+
   Song song = GetSingleSelectionAsSong();
   if (!song.is_valid()) return;
 
@@ -622,7 +623,7 @@ void AlbumCoverManager::SaveCoverToFile() {
 }
 
 void AlbumCoverManager::LoadCoverFromURL() {
-  
+
   Song song = GetSingleSelectionAsSong();
   if (!song.is_valid()) return;
 
@@ -633,11 +634,11 @@ void AlbumCoverManager::LoadCoverFromURL() {
   if (!cover.isEmpty()) {
     UpdateCoverInList(item, cover);
   }
-  
+
 }
 
 void AlbumCoverManager::SearchForCover() {
-  
+
   Song song = GetFirstSelectedAsSong();
   if (!song.is_valid()) return;
 

src/covermanager/albumcovermanager.ui

@@ -172,7 +172,7 @@
              <number>0</number>
             </property>
             <item>
-             <widget class="QPushButton" name="fetch">
+             <widget class="QPushButton" name="button_fetch">
               <property name="text">
                <string>Fetch Missing Covers</string>
               </property>

src/covermanager/amazoncoverprovider.cpp

@@ -41,11 +41,9 @@ const char *AmazonCoverProvider::kAssociateTag = "jonas052-20";
 const char *AmazonCoverProvider::kAccessKeyB64 = "QUtJQUozQ1dIQ0RWSVlYN1JMTFE=";
 const char *AmazonCoverProvider::kSecretAccessKeyB64 = "TjFZU3F2c2hJZDVtUGxKVW1Ka0kvc2E1WkZESG9TYy9ZWkgxYWdJdQ==";
 
-AmazonCoverProvider::AmazonCoverProvider(QObject *parent) : CoverProvider("Amazon", parent), network_(new NetworkAccessManager(this)) {}
+AmazonCoverProvider::AmazonCoverProvider(QObject *parent) : CoverProvider("Amazon", true, parent), network_(new NetworkAccessManager(this)) {}
 
 bool AmazonCoverProvider::StartSearch(const QString &artist, const QString &album, int id) {
-    
-  //qLog(Debug) << __PRETTY_FUNCTION__;
   
   typedef QPair<QString, QString> Arg;
   typedef QList<Arg> ArgList;

src/covermanager/amazoncoverprovider.h

@@ -52,9 +52,8 @@ class AmazonCoverProvider : public CoverProvider {
  private:
   void ReadItem(QXmlStreamReader *reader, CoverSearchResults *results);
   void ReadLargeImage(QXmlStreamReader *reader, CoverSearchResults *results);
-
- private:
   QNetworkAccessManager *network_;
+
 };
 
 #endif  // AMAZONCOVERPROVIDER_H

src/covermanager/coverprovider.cpp

@@ -22,5 +22,5 @@
 
 #include "coverprovider.h"
 
-CoverProvider::CoverProvider(const QString &name, QObject *parent)
-    : QObject(parent), name_(name) {}
+CoverProvider::CoverProvider(const QString &name, const bool &fetchall, QObject *parent)
+    : QObject(parent), name_(name), fetchall_(fetchall) {}

src/covermanager/coverprovider.h

@@ -39,10 +39,11 @@ class CoverProvider : public QObject {
   Q_OBJECT
 
 public:
-  explicit CoverProvider(const QString& name, QObject* parent);
+  explicit CoverProvider(const QString &name, const bool &fetchall, QObject *parent);
 
   // A name (very short description) of this provider, like "last.fm".
   QString name() const { return name_; }
+  bool fetchall() const { return fetchall_; }
 
   // Starts searching for covers matching the given query text.  Returns true
   // if the query has been started, or false if an error occurred.  The provider
@@ -56,6 +57,8 @@ signals:
 
 private:
   QString name_;
+  bool fetchall_;
+
 };
 
 #endif // COVERPROVIDER_H

src/covermanager/coverproviders.h

@@ -31,8 +31,7 @@ class AlbumCoverFetcherSearch;
 class CoverProvider;
 
 // This is a repository for cover providers.
-// Providers are automatically unregistered from the repository when they are
-// deleted.  The class is thread safe.
+// Providers are automatically unregistered from the repository when they are deleted.  The class is thread safe.
 class CoverProviders : public QObject {
   Q_OBJECT
 
@@ -64,4 +63,3 @@ class CoverProviders : public QObject {
 };
 
 #endif  // COVERPROVIDERS_H
-

src/covermanager/discogscoverprovider.cpp

@@ -43,7 +43,7 @@ const char *DiscogsCoverProvider::kUrlReleases = "https://api.discogs.com/releas
 const char *DiscogsCoverProvider::kAccessKeyB64 = "dGh6ZnljUGJlZ1NEeXBuSFFxSVk=";
 const char *DiscogsCoverProvider::kSecretAccessKeyB64 = "ZkFIcmlaSER4aHhRSlF2U3d0bm5ZVmdxeXFLWUl0UXI=";
 
-DiscogsCoverProvider::DiscogsCoverProvider(QObject *parent) : CoverProvider("Discogs", parent), network_(new NetworkAccessManager(this)) {}
+DiscogsCoverProvider::DiscogsCoverProvider(QObject *parent) : CoverProvider("Discogs", false, parent), network_(new NetworkAccessManager(this)) {}
 
 bool DiscogsCoverProvider::StartSearch(const QString &artist, const QString &album, int s_id) {
 

src/covermanager/discogscoverprovider.h

@@ -91,7 +91,7 @@ class DiscogsCoverProvider : public CoverProvider {
   QNetworkAccessManager *network_;
   QHash<int, DiscogsCoverSearchContext*> requests_search_;
   QHash<int, DiscogsCoverReleaseContext*> requests_release_;
-  
+
   bool StartRelease(DiscogsCoverSearchContext *s_ctx, int r_id, QString resource_url);
 
   void SendSearchRequest(DiscogsCoverSearchContext *s_ctx);

src/covermanager/lastfmcoverprovider.cpp

@@ -45,7 +45,7 @@ Registered to		jonaskvinge
 const char *LastFmCoverProvider::kApiKey = "211990b4c96782c05d1536e7219eb56e";
 const char *LastFmCoverProvider::kSecret = "80fd738f49596e9709b1bf9319c444a8";
 
-LastFmCoverProvider::LastFmCoverProvider(QObject *parent) : CoverProvider("last.fm", parent), network_(new NetworkAccessManager(this)) {
+LastFmCoverProvider::LastFmCoverProvider(QObject *parent) : CoverProvider("last.fm", true, parent), network_(new NetworkAccessManager(this)) {
   lastfm::ws::ApiKey = kApiKey;
   lastfm::ws::SharedSecret = kSecret;
   lastfm::setNetworkAccessManager(network_);

src/covermanager/lastfmcoverprovider.h

@@ -43,7 +43,7 @@ public:
   explicit LastFmCoverProvider(QObject *parent = nullptr);
 
   bool StartSearch(const QString &artist, const QString &album, int id);
-  
+
   static const char *kApiKey;
   static const char *kSecret;
 
@@ -53,6 +53,7 @@ private slots:
 private:
   QNetworkAccessManager *network_;
   QMap <QNetworkReply *, int> pending_queries_;
+
 };
 
 #endif // LASTFMCOVERPROVIDER_H

src/covermanager/musicbrainzcoverprovider.cpp

@@ -28,6 +28,7 @@
 
 #include "core/closure.h"
 #include "core/network.h"
+#include "core/logging.h"
 
 #include "musicbrainzcoverprovider.h"
 
@@ -39,7 +40,7 @@ static const char *kReleaseSearchUrl = "https://musicbrainz.org/ws/2/release/";
 static const char *kAlbumCoverUrl = "https://coverartarchive.org/release/%1/front";
 }  // namespace
 
-MusicbrainzCoverProvider::MusicbrainzCoverProvider(QObject *parent): CoverProvider("MusicBrainz", parent), network_(new NetworkAccessManager(this)) {}
+MusicbrainzCoverProvider::MusicbrainzCoverProvider(QObject *parent): CoverProvider("MusicBrainz", true, parent), network_(new NetworkAccessManager(this)) {}
 
 bool MusicbrainzCoverProvider::StartSearch(const QString &artist, const QString &album, int id) {
 

src/covermanager/musicbrainzcoverprovider.h

@@ -47,6 +47,7 @@ class MusicbrainzCoverProvider : public CoverProvider {
   QNetworkAccessManager *network_;
   QMultiMap<int, QNetworkReply *> image_checks_;
   QMap<int, QString> cover_names_;
+
 };
 
 #endif  // MUSICBRAINZCOVERPROVIDER_H

src/device/cddasongloader.cpp

@@ -124,7 +124,8 @@ void CddaSongLoader::LoadSongs() {
     if (GST_MESSAGE_TYPE(msg) == GST_MESSAGE_TOC) {
       if (msg_toc) gst_message_unref(msg_toc); // Shouldn't happen, but just in case
       msg_toc = msg;
-    } else if (GST_MESSAGE_TYPE(msg) == GST_MESSAGE_TAG) {
+    }
+    else if (GST_MESSAGE_TYPE(msg) == GST_MESSAGE_TAG) {
       if (msg_tag) gst_message_unref(msg_tag);
       msg_tag = msg;
     }