LyricsFetcherSearch: Improve handling timeout

src/lyrics/lyricsfetchersearch.cpp

@@ -1,6 +1,6 @@
 /*
  * Strawberry Music Player
- * Copyright 2018-2021, Jonas Kvinge <jonas@jkvinge.net>
+ * Copyright 2018-2025, Jonas Kvinge <jonas@jkvinge.net>
  *
  * Strawberry is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -36,7 +36,7 @@
 using namespace Qt::Literals::StringLiterals;
 
 namespace {
-constexpr int kSearchTimeoutMs = 3000;
+constexpr int kSearchTimeoutMs = 5000;
 constexpr int kGoodLyricsLength = 60;
 constexpr float kHighScore = 2.5;
 }  // namespace
@@ -45,19 +45,23 @@ LyricsFetcherSearch::LyricsFetcherSearch(const quint64 id, const LyricsSearchReq
     : QObject(parent),
       id_(id),
       request_(request),
-      cancel_requested_(false) {
+      timer_search_timeout_(new QTimer(this)),
+      cancel_requested_(false),
+      finished_(false) {
 
-  QTimer::singleShot(kSearchTimeoutMs, this, &LyricsFetcherSearch::TerminateSearch);
+  QObject::connect(timer_search_timeout_, &QTimer::timeout, this, &LyricsFetcherSearch::SearchTimeout);
+
+  timer_search_timeout_->setSingleShot(true);
+  timer_search_timeout_->setInterval(kSearchTimeoutMs);
 
 }
 
-void LyricsFetcherSearch::TerminateSearch() {
+void LyricsFetcherSearch::CancelRequests() {
 
   const QList<int> keys = pending_requests_.keys();
   for (const int id : keys) {
     pending_requests_.take(id)->CancelSearchAsync(id);
   }
-  AllProvidersFinished();
 
 }
 
@@ -65,7 +69,7 @@ void LyricsFetcherSearch::Start(SharedPtr<LyricsProviders> lyrics_providers) {
 
   // Ignore Radio Paradise "commercial" break.
   if (request_.artist.compare("commercial-free"_L1, Qt::CaseInsensitive) == 0 && request_.title.compare("listener-supported"_L1, Qt::CaseInsensitive) == 0) {
-    TerminateSearch();
+    FinishSearch();
     return;
   }
 
@@ -82,7 +86,12 @@ void LyricsFetcherSearch::Start(SharedPtr<LyricsProviders> lyrics_providers) {
     }
   }
 
-  if (pending_requests_.isEmpty()) TerminateSearch();
+  if (pending_requests_.isEmpty()) {
+    FinishSearch();
+    return;
+  }
+
+  timer_search_timeout_->start();
 
 }
 
@@ -118,32 +127,50 @@ void LyricsFetcherSearch::ProviderSearchFinished(const int id, const LyricsSearc
   if (!pending_requests_.isEmpty()) {
     if (!results_.isEmpty() && higest_score >= kHighScore) {  // Highest score, no need to wait for other providers.
       qLog(Debug) << "Got lyrics with high score from" << results_.last().provider << "for" << request_.artist << request_.title << "score" << results_.last().score << "finishing search.";
-      TerminateSearch();
+      FinishSearch();
-    }
-    else {
-      return;
     }
     return;
   }
 
-  AllProvidersFinished();
+  FinishSearch();
 
 }
 
-void LyricsFetcherSearch::AllProvidersFinished() {
+void LyricsFetcherSearch::SearchTimeout() {
 
-  if (cancel_requested_) return;
+  if (finished_) return;
 
-  if (!results_.isEmpty()) {
+  qLog(Debug) << "Lyrics search for" << request_.artist << request_.album << request_.title << "timed out for remaining" << pending_requests_.count() << "providers";
+
+  FinishSearch();
+
+}
+
+void LyricsFetcherSearch::FinishSearch() {
+
+  if (cancel_requested_ || finished_) return;
+
+  if (!pending_requests_.isEmpty()) {
+    CancelRequests();
+  }
+
+  if (timer_search_timeout_->isActive()) {
+    timer_search_timeout_->stop();
+  }
+
+  if (results_.isEmpty()) {
+    qLog(Debug) << "No lyrics for" << request_.artist << request_.title;
+    Q_EMIT LyricsFetched(id_);
+  }
+  else {
     qLog(Debug) << "Using lyrics from" << results_.last().provider << "for" << request_.artist << request_.title << "with score" << results_.last().score;
     Q_EMIT LyricsFetched(id_, results_.constLast().provider, results_.constLast().lyrics);
   }
-  else {
-    Q_EMIT LyricsFetched(id_, QString(), QString());
-  }
 
   Q_EMIT SearchFinished(id_, results_);
 
+  finished_ = true;
+
 }
 
 void LyricsFetcherSearch::Cancel() {
@@ -151,7 +178,7 @@ void LyricsFetcherSearch::Cancel() {
   cancel_requested_ = true;
 
   if (!pending_requests_.isEmpty()) {
-    TerminateSearch();
+    CancelRequests();
   }
 
 }

src/lyrics/lyricsfetchersearch.h

@@ -1,6 +1,6 @@
 /*
  * Strawberry Music Player
- * Copyright 2018-2021, Jonas Kvinge <jonas@jkvinge.net>
+ * Copyright 2018-2025, Jonas Kvinge <jonas@jkvinge.net>
  *
  * Strawberry is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -31,6 +31,7 @@
 #include "lyricssearchrequest.h"
 #include "lyricssearchresult.h"
 
+class QTimer;
 class LyricsProvider;
 class LyricsProviders;
 
@@ -45,23 +46,26 @@ class LyricsFetcherSearch : public QObject {
 
  Q_SIGNALS:
   void SearchFinished(const quint64 id, const LyricsSearchResults &results);
-  void LyricsFetched(const quint64 id, const QString &provider, const QString &lyrics);
+  void LyricsFetched(const quint64 id, const QString &provider = QString(), const QString &lyrics = QString());
 
  private Q_SLOTS:
   void ProviderSearchFinished(const int id, const LyricsSearchResults &results);
-  void TerminateSearch();
+  void SearchTimeout();
 
  private:
-  void AllProvidersFinished();
+  void CancelRequests();
+  void FinishSearch();
   static bool ProviderCompareOrder(LyricsProvider *a, LyricsProvider *b);
   static bool LyricsSearchResultCompareScore(const LyricsSearchResult &a, const LyricsSearchResult &b);
 
  private:
   quint64 id_;
   const LyricsSearchRequest request_;
+  QTimer *timer_search_timeout_;
   LyricsSearchResults results_;
   QMap<int, LyricsProvider*> pending_requests_;
   bool cancel_requested_;
+  bool finished_;
 };
 
 #endif  // LYRICSFETCHERSEARCH_H