Get more results from MusicBrainz, and sort them by relevance.

We were stopping at the first one in AcoustidClient::RequestFinished.
To make things worse, it seems that results we received weren't sorted, so we sometimes ignored the most relevant one.

src/musicbrainz/acoustidclient.cpp

@@ -19,6 +19,7 @@
 
 #include <QCoreApplication>
 #include <QNetworkReply>
+#include <QStringList>
 
 #include <qjson/parser.h>
 
@@ -45,7 +46,7 @@ void AcoustidClient::Start(int id, const QString& fingerprint,
   QList<Param> parameters;
   parameters << Param("format", "json") << Param("client", kClientId)
              << Param("duration", QString::number(duration_msec / kMsecPerSec))
-             << Param("meta", "recordingids")
+             << Param("meta", "recordingids+sources")
              << Param("fingerprint", fingerprint);
 
   QUrl url(kUrl);
@@ -67,13 +68,29 @@ void AcoustidClient::CancelAll() {
   requests_.clear();
 }
 
-void AcoustidClient::RequestFinished(QNetworkReply* reply, int id) {
+namespace {
+// Struct used when extracting results in RequestFinished
+struct IdSource {
+  IdSource(const QString& id, int source)
+    : id_(id), nb_sources_(source) {}
+
+  bool operator<(const IdSource& other) const {
+    // We want the items with more sources to be at the beginning of the list
+    return nb_sources_ > other.nb_sources_;
+  }
+
+  QString id_;
+  int nb_sources_;
+};
+}
+
+void AcoustidClient::RequestFinished(QNetworkReply* reply, int request_id) {
   reply->deleteLater();
-  requests_.remove(id);
+  requests_.remove(request_id);
 
   if (reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() !=
       200) {
-    emit Finished(id, QString());
+    emit Finished(request_id, QStringList());
     return;
   }
 
@@ -81,16 +98,26 @@ void AcoustidClient::RequestFinished(QNetworkReply* reply, int id) {
   bool ok = false;
   QVariantMap result = parser.parse(reply, &ok).toMap();
   if (!ok) {
-    emit Finished(id, QString());
+    emit Finished(request_id, QStringList());
     return;
   }
 
   QString status = result["status"].toString();
   if (status != "ok") {
-    emit Finished(id, QString());
+    emit Finished(request_id, QStringList());
     return;
   }
+
+  // Get the results:
+  // -in a first step, gather ids and their corresponding number of sources
+  // -then sort results by number of sources (the results are originally
+  //  unsorted but results with more sources are likely to be more accurate)
+  // -keep only the ids, as sources where useful only to sort the results
   QVariantList results = result["results"].toList();
+
+  // List of <id, nb of sources> pairs
+  QList<IdSource> id_source_list;
+
   for (const QVariant& v : results) {
     QVariantMap r = v.toMap();
     if (r.contains("recordings")) {
@@ -98,12 +125,18 @@ void AcoustidClient::RequestFinished(QNetworkReply* reply, int id) {
       for (const QVariant& recording : recordings) {
         QVariantMap o = recording.toMap();
         if (o.contains("id")) {
-          emit Finished(id, o["id"].toString());
-          return;
+          id_source_list << IdSource(o["id"].toString(), o["sources"].toInt());
         }
       }
     }
   }
 
-  emit Finished(id, QString());
+  qStableSort(id_source_list);
+
+  QList<QString> id_list;
+  for (const IdSource& is : id_source_list) {
+    id_list << is.id_;
+  }
+
+  emit Finished(request_id, id_list);
 }

src/musicbrainz/acoustidclient.h

@@ -56,7 +56,7 @@ class AcoustidClient : public QObject {
   void CancelAll();
 
 signals:
-  void Finished(int id, const QString& mbid);
+  void Finished(int id, const QStringList& mbid_list);
 
  private slots:
   void RequestFinished(QNetworkReply* reply, int id);

src/musicbrainz/musicbrainzclient.cpp

@@ -40,22 +40,26 @@ MusicBrainzClient::MusicBrainzClient(QObject* parent,
       network_(network ? network : new NetworkAccessManager(this)),
       timeouts_(new NetworkTimeouts(kDefaultTimeout, this)) {}
 
-void MusicBrainzClient::Start(int id, const QString& mbid) {
+void MusicBrainzClient::Start(int id, const QStringList& mbid_list) {
   typedef QPair<QString, QString> Param;
 
-  QList<Param> parameters;
-  parameters << Param("inc", "artists+releases+media");
+  int request_number = 0;
+  for (const QString& mbid : mbid_list) {
+    QList<Param> parameters;
+    parameters << Param("inc", "artists+releases+media");
 
-  QUrl url(kTrackUrl + mbid);
-  url.setQueryItems(parameters);
-  QNetworkRequest req(url);
+    QUrl url(kTrackUrl + mbid);
+    url.setQueryItems(parameters);
+    QNetworkRequest req(url);
 
-  QNetworkReply* reply = network_->get(req);
-  NewClosure(reply, SIGNAL(finished()), this,
-             SLOT(RequestFinished(QNetworkReply*, int)), reply, id);
-  requests_[id] = reply;
+    QNetworkReply* reply = network_->get(req);
+    NewClosure(reply, SIGNAL(finished()), this,
+               SLOT(RequestFinished(QNetworkReply*, int, int)),
+               reply, id, request_number++);
+    requests_.insert(id, reply);
 
-  timeouts_->AddReply(reply);
+    timeouts_->AddReply(reply);
+  }
 }
 
 void MusicBrainzClient::StartDiscIdRequest(const QString& discid) {
@@ -141,31 +145,46 @@ void MusicBrainzClient::DiscIdRequestFinished(const QString& discid,
   emit Finished(artist, album, UniqueResults(ret));
 }
 
-void MusicBrainzClient::RequestFinished(QNetworkReply* reply, int id) {
+void MusicBrainzClient::RequestFinished(QNetworkReply* reply, int id, int request_number) {
   reply->deleteLater();
-  requests_.remove(id);
-  ResultList ret;
 
-  if (reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() !=
-      200) {
-    emit Finished(id, ret);
-    return;
+  const int nb_removed = requests_.remove(id, reply);
+  if (nb_removed != 1) {
+    qLog(Error) << "Error: unknown reply received:" << nb_removed <<
+        "requests removed, while only one was supposed to be removed";
   }
 
-  QXmlStreamReader reader(reply);
-  while (!reader.atEnd()) {
-    if (reader.readNext() == QXmlStreamReader::StartElement &&
-        reader.name() == "recording") {
-      ResultList tracks = ParseTrack(&reader);
-      for (const Result& track : tracks) {
-        if (!track.title_.isEmpty()) {
-          ret << track;
+
+  if (reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() ==
+      200) {
+    QXmlStreamReader reader(reply);
+    ResultList res;
+    while (!reader.atEnd()) {
+      if (reader.readNext() == QXmlStreamReader::StartElement &&
+          reader.name() == "recording") {
+        ResultList tracks = ParseTrack(&reader);
+        for (const Result& track : tracks) {
+          if (!track.title_.isEmpty()) {
+            res << track;
+          }
         }
       }
     }
+    qSort(res);
+    pending_results_[id] << PendingResults(request_number, res);
   }
 
-  emit Finished(id, UniqueResults(ret));
+  // No more pending requests for this id: emit the results we have.
+  if (!requests_.contains(id)) {
+    // Merge the results we have
+    ResultList ret;
+    QList<PendingResults> result_list_list = pending_results_.take(id);
+    qSort(result_list_list);
+    for (const PendingResults& result_list : result_list_list) {
+      ret << result_list.results_;
+    }
+    emit Finished(id, UniqueResults(ret));
+  }
 }
 
 bool MusicBrainzClient::MediumHasDiscid(const QString& discid,
@@ -326,9 +345,8 @@ MusicBrainzClient::Release MusicBrainzClient::ParseRelease(
   return ret;
 }
 
-MusicBrainzClient::ResultList MusicBrainzClient::UniqueResults(
-    const ResultList& results) {
-  ResultList ret = QSet<Result>::fromList(results).toList();
-  qSort(ret);
-  return ret;
+MusicBrainzClient::ResultList& MusicBrainzClient::UniqueResults(ResultList& results) {
+  qStableSort(results);
+  results.erase(std::unique(results.begin(), results.end()), results.end());
+  return results;
 }

src/musicbrainz/musicbrainzclient.h

@@ -19,7 +19,7 @@
 #define MUSICBRAINZCLIENT_H
 
 #include <QHash>
-#include <QMap>
+#include <QMultiMap>
 #include <QObject>
 #include <QXmlStreamReader>
 
@@ -78,7 +78,7 @@ class MusicBrainzClient : public QObject {
 
   // Starts a request and returns immediately.  Finished() will be emitted
   // later with the same ID.
-  void Start(int id, const QString& mbid);
+  void Start(int id, const QStringList& mbid);
   void StartDiscIdRequest(const QString& discid);
 
   // Cancels the request with the given ID.  Finished() will never be emitted
@@ -97,7 +97,9 @@ signals:
                 const MusicBrainzClient::ResultList& result);
 
  private slots:
-  void RequestFinished(QNetworkReply* reply, int id);
+  // id identifies the track, and request_number means it's the
+  // 'request_number'th request for this track
+  void RequestFinished(QNetworkReply* reply, int id, int request_number);
   void DiscIdRequestFinished(const QString& discid, QNetworkReply* reply);
 
  private:
@@ -117,13 +119,26 @@ signals:
     int year_;
   };
 
+  struct PendingResults {
+    PendingResults(int sort_id, const ResultList& results)
+      : sort_id_(sort_id), results_(results) {}
+
+    bool operator<(const PendingResults& other) const {
+      return sort_id_ < other.sort_id_;
+    }
+
+    int sort_id_;
+    ResultList results_;
+  };
+
   static bool MediumHasDiscid(const QString& discid, QXmlStreamReader* reader);
   static ResultList ParseMedium(QXmlStreamReader* reader);
   static Result ParseTrackFromDisc(QXmlStreamReader* reader);
   static ResultList ParseTrack(QXmlStreamReader* reader);
   static void ParseArtist(QXmlStreamReader* reader, QString* artist);
   static Release ParseRelease(QXmlStreamReader* reader);
-  static ResultList UniqueResults(const ResultList& results);
+  // Remove duplicate from the list. Returns a reference to the input parameter
+  static ResultList& UniqueResults(ResultList& results);
 
  private:
   static const char* kTrackUrl;
@@ -133,7 +148,9 @@ signals:
 
   QNetworkAccessManager* network_;
   NetworkTimeouts* timeouts_;
-  QMap<int, QNetworkReply*> requests_;
+  QMultiMap<int, QNetworkReply*> requests_;
+  // Results we received so far, kept here until all the replies are finished
+  QMap<int, QList<PendingResults>> pending_results_;
 };
 
 inline uint qHash(const MusicBrainzClient::Result& result) {

src/musicbrainz/tagfetcher.cpp

@@ -32,8 +32,8 @@ TagFetcher::TagFetcher(QObject* parent)
       fingerprint_watcher_(nullptr),
       acoustid_client_(new AcoustidClient(this)),
       musicbrainz_client_(new MusicBrainzClient(this)) {
-  connect(acoustid_client_, SIGNAL(Finished(int, QString)),
-          SLOT(PuidFound(int, QString)));
+  connect(acoustid_client_, SIGNAL(Finished(int, QStringList)),
+          SLOT(PuidsFound(int, QStringList)));
   connect(musicbrainz_client_,
           SIGNAL(Finished(int, MusicBrainzClient::ResultList)),
           SLOT(TagsFetched(int, MusicBrainzClient::ResultList)));
@@ -92,20 +92,20 @@ void TagFetcher::FingerprintFound(int index) {
                           song.length_nanosec() / kNsecPerMsec);
 }
 
-void TagFetcher::PuidFound(int index, const QString& puid) {
+void TagFetcher::PuidsFound(int index, const QStringList& puid_list) {
   if (index >= songs_.count()) {
     return;
   }
 
   const Song& song = songs_[index];
 
-  if (puid.isEmpty()) {
+  if (puid_list.isEmpty()) {
     emit ResultAvailable(song, SongList());
     return;
   }
 
   emit Progress(song, tr("Downloading metadata"));
-  musicbrainz_client_->Start(index, puid);
+  musicbrainz_client_->Start(index, puid_list);
 }
 
 void TagFetcher::TagsFetched(int index,

src/musicbrainz/tagfetcher.h

@@ -47,7 +47,7 @@ signals:
 
  private slots:
   void FingerprintFound(int index);
-  void PuidFound(int index, const QString& puid);
+  void PuidsFound(int index, const QStringList& puid_list);
   void TagsFetched(int index, const MusicBrainzClient::ResultList& result);
 
  private: