diff --git a/src/musicbrainz/acoustidclient.cpp b/src/musicbrainz/acoustidclient.cpp index c98d2de5a..ba29dc060 100644 --- a/src/musicbrainz/acoustidclient.cpp +++ b/src/musicbrainz/acoustidclient.cpp @@ -19,6 +19,7 @@ #include #include +#include #include @@ -45,7 +46,7 @@ void AcoustidClient::Start(int id, const QString& fingerprint, QList 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 pairs + QList 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 id_list; + for (const IdSource& is : id_source_list) { + id_list << is.id_; + } + + emit Finished(request_id, id_list); } diff --git a/src/musicbrainz/acoustidclient.h b/src/musicbrainz/acoustidclient.h index 36b3bc50c..1e68431f5 100644 --- a/src/musicbrainz/acoustidclient.h +++ b/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); diff --git a/src/musicbrainz/musicbrainzclient.cpp b/src/musicbrainz/musicbrainzclient.cpp index 4b1d5a566..30bf5b77e 100644 --- a/src/musicbrainz/musicbrainzclient.cpp +++ b/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 Param; - QList parameters; - parameters << Param("inc", "artists+releases+media"); + int request_number = 0; + for (const QString& mbid : mbid_list) { + QList 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 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::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; } diff --git a/src/musicbrainz/musicbrainzclient.h b/src/musicbrainz/musicbrainzclient.h index e17065343..61d37397a 100644 --- a/src/musicbrainz/musicbrainzclient.h +++ b/src/musicbrainz/musicbrainzclient.h @@ -19,7 +19,7 @@ #define MUSICBRAINZCLIENT_H #include -#include +#include #include #include @@ -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 requests_; + QMultiMap requests_; + // Results we received so far, kept here until all the replies are finished + QMap> pending_results_; }; inline uint qHash(const MusicBrainzClient::Result& result) { diff --git a/src/musicbrainz/tagfetcher.cpp b/src/musicbrainz/tagfetcher.cpp index 17309ca4f..900a47b14 100644 --- a/src/musicbrainz/tagfetcher.cpp +++ b/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, diff --git a/src/musicbrainz/tagfetcher.h b/src/musicbrainz/tagfetcher.h index 18bb90ef9..a066b18e7 100644 --- a/src/musicbrainz/tagfetcher.h +++ b/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: