From 1d0d03ed830f29fc9e8bbb5fee06c0cdca2c3507 Mon Sep 17 00:00:00 2001 From: Jonas Kvinge Date: Sun, 5 Oct 2025 21:42:14 +0200 Subject: [PATCH] Rewrite MusicBrainzClient Use Json instead of XML, make Disc ID requests respect rate limiting, handle sort names. --- src/device/cddasongloader.cpp | 70 ++- src/device/cddasongloader.h | 17 +- src/dialogs/trackselectiondialog.cpp | 15 +- src/dialogs/trackselectiondialog.h | 4 +- src/dialogs/trackselectiondialog.ui | 6 +- src/musicbrainz/musicbrainzclient.cpp | 726 +++++++++++++------------- src/musicbrainz/musicbrainzclient.h | 201 ++++--- src/musicbrainz/tagfetcher.cpp | 11 +- 8 files changed, 542 insertions(+), 508 deletions(-) diff --git a/src/device/cddasongloader.cpp b/src/device/cddasongloader.cpp index 59c3ac7d0..1325fee57 100644 --- a/src/device/cddasongloader.cpp +++ b/src/device/cddasongloader.cpp @@ -53,11 +53,17 @@ using namespace Qt::Literals::StringLiterals; CDDASongLoader::CDDASongLoader(const QUrl &url, QObject *parent) : QObject(parent), url_(url), - network_(make_shared()) { + network_(make_shared()), +#ifdef HAVE_MUSICBRAINZ + musicbrainz_client_(new MusicBrainzClient(network_, this)), +#endif + whatever_(false) { #ifdef HAVE_MUSICBRAINZ QObject::connect(this, &CDDASongLoader::LoadTagsFromMusicBrainz, this, &CDDASongLoader::LoadTagsFromMusicBrainzSlot); + QObject::connect(musicbrainz_client_, &MusicBrainzClient::DiscIdFinished, this, &CDDASongLoader::LoadTagsFromMusicBrainzFinished); #endif // HAVE_MUSICBRAINZ + } CDDASongLoader::~CDDASongLoader() { @@ -349,7 +355,7 @@ void CDDASongLoader::LoadSongsFromCDDA() { } else { qLog(Info) << "MusicBrainz Disc ID:" << musicbrainz_discid; - Q_EMIT LoadTagsFromMusicBrainz(musicbrainz_discid); + Q_EMIT LoadTagsFromMusicBrainz(musicbrainz_discid, songs); } #else Q_EMIT LoadingFinished(); @@ -360,18 +366,23 @@ void CDDASongLoader::LoadSongsFromCDDA() { #ifdef HAVE_MUSICBRAINZ -void CDDASongLoader::LoadTagsFromMusicBrainzSlot(const QString &musicbrainz_discid) const { +void CDDASongLoader::LoadTagsFromMusicBrainzSlot(const QString &musicbrainz_discid, const QMap &songs) { - MusicBrainzClient *musicbrainz_client = new MusicBrainzClient(network_); - QObject::connect(musicbrainz_client, &MusicBrainzClient::DiscIdFinished, this, &CDDASongLoader::LoadTagsFromMusicBrainzFinished); - musicbrainz_client->StartDiscIdRequest(musicbrainz_discid); + musicbrainz_discid_ = musicbrainz_discid; + musicbrainz_songs_ = songs; + musicbrainz_client_->StartDiscIdRequest(musicbrainz_discid); } -void CDDASongLoader::LoadTagsFromMusicBrainzFinished(const QString &artist, const QString &album, const MusicBrainzClient::ResultList &results, const QString &error) { +void CDDASongLoader::LoadTagsFromMusicBrainzFinished(const QString &musicbrainz_discid, const MusicBrainzClient::ResultList &results, const QString &error) { - MusicBrainzClient *musicbrainz_client = qobject_cast(sender()); - musicbrainz_client->deleteLater(); + if (musicbrainz_discid != musicbrainz_discid_) { + return; + } + + QMap songs = musicbrainz_songs_; + musicbrainz_discid_.clear(); + musicbrainz_songs_.clear(); if (!error.isEmpty()) { Error(error); @@ -383,27 +394,32 @@ void CDDASongLoader::LoadTagsFromMusicBrainzFinished(const QString &artist, cons return; } - SongList songs; - songs.reserve(results.count()); - int track_number = 0; for (const MusicBrainzClient::Result &result : results) { - ++track_number; - Song song(Song::Source::CDDA); - song.set_artist(artist); - song.set_album(album); - song.set_title(result.title_); - song.set_length_nanosec(result.duration_msec_ * kNsecPerMsec); - song.set_track(track_number); - song.set_year(result.year_); - song.set_id(track_number); - song.set_filetype(Song::FileType::CDDA); - song.set_valid(true); - // We need to set URL, that's how playlist will find the correct item to update - song.set_url(GetUrlFromTrack(track_number)); - songs << song; + if (songs.contains(result.track_)) { + Song &song = songs[result.track_]; + song.set_valid(true); + song.set_id(result.track_); + song.set_track(result.track_); + song.set_artist(result.artist_); + song.set_artistsort(result.sort_artist_); + song.set_album(result.album_); + song.set_title(result.title_); + song.set_track(result.track_); + song.set_year(result.year_); + song.set_url(GetUrlFromTrack(song.track())); + if (song.length_nanosec() <= 0) { + song.set_length_nanosec(result.duration_msec_ * kNsecPerMsec); + } + if (!result.album_artist_.isEmpty() && result.album_artist_ != result.artist_) { + song.set_albumartist(result.album_artist_); + } + if (!result.sort_album_artist_.isEmpty() && result.sort_album_artist_ != result.sort_artist_) { + song.set_albumartistsort(result.sort_album_artist_); + } + } } - Q_EMIT SongsUpdated(songs); + Q_EMIT SongsUpdated(songs.values()); Q_EMIT LoadingFinished(); } diff --git a/src/device/cddasongloader.h b/src/device/cddasongloader.h index 9de17a452..42c12a2c9 100644 --- a/src/device/cddasongloader.h +++ b/src/device/cddasongloader.h @@ -29,9 +29,10 @@ #include #include +#include +#include #include #include -#include #include "includes/shared_ptr.h" #include "core/song.h" @@ -62,19 +63,27 @@ class CDDASongLoader : public QObject { void SongsUpdated(const SongList &songs); void LoadError(const QString &error); void LoadingFinished(); - void LoadTagsFromMusicBrainz(const QString &musicbrainz_discid); + void LoadTagsFromMusicBrainz(const QString &musicbrainz_discid, const QMap &songs); private Q_SLOTS: #ifdef HAVE_MUSICBRAINZ - void LoadTagsFromMusicBrainzSlot(const QString &musicbrainz_discid) const; - void LoadTagsFromMusicBrainzFinished(const QString &artist, const QString &album, const MusicBrainzClient::ResultList &results, const QString &error); + void LoadTagsFromMusicBrainzSlot(const QString &musicbrainz_discid, const QMap &songs); + void LoadTagsFromMusicBrainzFinished(const QString &musicbrainz_discid, const MusicBrainzClient::ResultList &results, const QString &error); #endif private: const QUrl url_; SharedPtr network_; +#ifdef HAVE_MUSICBRAINZ + MusicBrainzClient *musicbrainz_client_; +#endif QMutex mutex_load_; QFuture loading_future_; +#ifdef HAVE_MUSICBRAINZ + QString musicbrainz_discid_; + QMap musicbrainz_songs_; +#endif + bool whatever_; }; #endif // CDDASONGLOADER_H diff --git a/src/dialogs/trackselectiondialog.cpp b/src/dialogs/trackselectiondialog.cpp index 8bf2d9d81..13584d03b 100644 --- a/src/dialogs/trackselectiondialog.cpp +++ b/src/dialogs/trackselectiondialog.cpp @@ -2,7 +2,7 @@ * Strawberry Music Player * This file was part of Clementine. * Copyright 2010, David Sansome - * Copyright 2019-2021, Jonas Kvinge + * Copyright 2019-2025, Jonas Kvinge * * Strawberry is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -89,6 +89,7 @@ TrackSelectionDialog::TrackSelectionDialog(const SharedPtr tagr ui_->results->setColumnWidth(2, 160); // Title column ui_->results->setColumnWidth(3, 160); // Artist column ui_->results->setColumnWidth(4, 160); // Album column + ui_->results->setColumnWidth(5, 160); // Album artist column } @@ -228,10 +229,15 @@ void TrackSelectionDialog::AddDivider(const QString &text, QTreeWidget *parent) } -void TrackSelectionDialog::AddSong(const Song &song, int result_index, QTreeWidget *parent) { +void TrackSelectionDialog::AddSong(const Song &song, const int result_index, QTreeWidget *parent) { QStringList values; - values << ((song.track() > 0) ? QString::number(song.track()) : QString()) << ((song.year() > 0) ? QString::number(song.year()) : QString()) << song.title() << song.artist() << song.album(); + values << ((song.track() > 0) ? QString::number(song.track()) : QString()) + << ((song.year() > 0) ? QString::number(song.year()) : QString()) + << song.title() + << song.artist() + << song.album() + << song.albumartist(); QTreeWidgetItem *item = new QTreeWidgetItem(parent, values); item->setData(0, Qt::UserRole, result_index); @@ -275,6 +281,9 @@ void TrackSelectionDialog::SaveData(const QList &_data) const { Song copy(ref.original_song_); copy.set_title(new_metadata.title()); copy.set_artist(new_metadata.artist()); + copy.set_artistsort(new_metadata.artistsort()); + copy.set_albumartist(new_metadata.albumartist()); + copy.set_albumartistsort(new_metadata.albumartistsort()); copy.set_album(new_metadata.album()); copy.set_track(new_metadata.track()); copy.set_year(new_metadata.year()); diff --git a/src/dialogs/trackselectiondialog.h b/src/dialogs/trackselectiondialog.h index 3ddb8bfe8..07ee430c9 100644 --- a/src/dialogs/trackselectiondialog.h +++ b/src/dialogs/trackselectiondialog.h @@ -2,7 +2,7 @@ * Strawberry Music Player * This file was part of Clementine. * Copyright 2010, David Sansome - * Copyright 2019-2021, Jonas Kvinge + * Copyright 2019-2025, Jonas Kvinge * * Strawberry is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -83,7 +83,7 @@ class TrackSelectionDialog : public QDialog { }; void AddDivider(const QString &text, QTreeWidget *parent) const; - static void AddSong(const Song &song, int result_index, QTreeWidget *parent); + static void AddSong(const Song &song, const int result_index, QTreeWidget *parent); void SetLoading(const QString &message); void SaveData(const QList &data) const; diff --git a/src/dialogs/trackselectiondialog.ui b/src/dialogs/trackselectiondialog.ui index 593315cca..98de65ca9 100644 --- a/src/dialogs/trackselectiondialog.ui +++ b/src/dialogs/trackselectiondialog.ui @@ -210,6 +210,11 @@ Title + + + Album artist + + Artist @@ -260,7 +265,6 @@ - diff --git a/src/musicbrainz/musicbrainzclient.cpp b/src/musicbrainz/musicbrainzclient.cpp index c4efaff78..35ba18918 100644 --- a/src/musicbrainz/musicbrainzclient.cpp +++ b/src/musicbrainz/musicbrainzclient.cpp @@ -1,7 +1,5 @@ /* * Strawberry Music Player - * This file was part of Clementine. - * Copyright 2010, David Sansome * Copyright 2019-2025, Jonas Kvinge * * Strawberry is free software: you can redistribute it and/or modify @@ -32,21 +30,20 @@ #include #include #include -#include #include #include #include #include #include #include -#include +#include #include #include "includes/shared_ptr.h" #include "core/logging.h" #include "core/networkaccessmanager.h" #include "core/networktimeouts.h" -#include "utilities/xmlutils.h" +#include "core/jsonbaserequest.h" #include "musicbrainzclient.h" using namespace Qt::Literals::StringLiterals; @@ -61,7 +58,7 @@ constexpr int kMaxRequestPerTrack = 3; } // namespace MusicBrainzClient::MusicBrainzClient(SharedPtr network, QObject *parent) - : QObject(parent), + : JsonBaseRequest(network, parent), network_(network), timeouts_(new NetworkTimeouts(kDefaultTimeout, this)), timer_flush_requests_(new QTimer(this)) { @@ -78,73 +75,83 @@ MusicBrainzClient::~MusicBrainzClient() { } -QByteArray MusicBrainzClient::GetReplyData(QNetworkReply *reply, QString &error) { +void MusicBrainzClient::FlushRequests() { - QByteArray data; + FlushMbIdRequests(); + FlushDiscIdRequests(); - if (reply->error() == QNetworkReply::NoError && reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() == 200) { - data = reply->readAll(); - } - else { - if (reply->error() != QNetworkReply::NoError && reply->error() < 200) { - // This is a network error, there is nothing more to do. - Error(QStringLiteral("%1 (%2)").arg(reply->errorString()).arg(reply->error())); - } - else { - // See if there is Json data containing "error" - then use that instead. - data = reply->readAll(); - QJsonParseError json_error; - QJsonDocument json_doc = QJsonDocument::fromJson(data, &json_error); - if (json_error.error == QJsonParseError::NoError && json_doc.isObject()) { - QJsonObject json_obj = json_doc.object(); - if (!json_obj.isEmpty() && json_obj.contains("error"_L1)) { - error = json_obj["error"_L1].toString(); - } - } - if (error.isEmpty()) { - if (reply->error() != QNetworkReply::NoError) { - error = QStringLiteral("%1 (%2)").arg(reply->errorString()).arg(reply->error()); - } - else { - error = QStringLiteral("Received HTTP code %1").arg(reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt()); - } - Error(error, data); - } - else Error(error); - } - return QByteArray(); - } - - return data; - -} - -void MusicBrainzClient::Cancel(int id) { - - while (!requests_.isEmpty() && requests_.contains(id)) { - QNetworkReply *reply = requests_.take(id); - QObject::disconnect(reply, nullptr, this, nullptr); - if (reply->isRunning()) reply->abort(); - reply->deleteLater(); + if (pending_mbid_requests_.isEmpty() && pending_discid_requests_.isEmpty() && timer_flush_requests_->isActive()) { + timer_flush_requests_->stop(); } } void MusicBrainzClient::CancelAll() { - qDeleteAll(requests_); - requests_.clear(); + qDeleteAll(mbid_requests_); + mbid_requests_.clear(); + + qDeleteAll(discid_requests_); + discid_requests_.clear(); } -void MusicBrainzClient::Start(const int id, const QStringList &mbid_list) { +JsonBaseRequest::JsonObjectResult MusicBrainzClient::ParseJsonObject(QNetworkReply *reply) { + + if (reply->error() != QNetworkReply::NoError && reply->error() < 200) { + return ReplyDataResult(ErrorCode::NetworkError, QStringLiteral("%1 (%2)").arg(reply->errorString()).arg(reply->error())); + } + + JsonObjectResult result(ErrorCode::Success); + result.network_error = reply->error(); + if (reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).isValid()) { + result.http_status_code = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); + } + + const QByteArray data = reply->readAll(); + if (!data.isEmpty()) { + QJsonParseError json_parse_error; + const QJsonDocument json_document = QJsonDocument::fromJson(data, &json_parse_error); + if (json_parse_error.error == QJsonParseError::NoError) { + const QJsonObject json_object = json_document.object(); + if (json_object.contains("error"_L1)) { + result.error_code = ErrorCode::APIError; + result.error_message = json_object["error"_L1].toString(); + } + else { + result.json_object = json_document.object(); + } + } + else { + result.error_code = ErrorCode::ParseError; + result.error_message = json_parse_error.errorString(); + } + } + + if (result.error_code != ErrorCode::APIError) { + if (reply->error() != QNetworkReply::NoError) { + result.error_code = ErrorCode::NetworkError; + result.error_message = QStringLiteral("%1 (%2)").arg(reply->errorString()).arg(reply->error()); + } + else if (result.http_status_code != 200) { + result.error_code = ErrorCode::HttpError; + result.error_message = QStringLiteral("Received HTTP code %1").arg(result.http_status_code); + } + } + + return result; + +} + +void MusicBrainzClient::StartMbIdRequest(const int id, const QStringList &mbid_list) { + + qLog(Debug) << "Starting MusicBrainz MBID request for" << mbid_list; int request_number = 0; for (const QString &mbid : mbid_list) { ++request_number; if (request_number > kMaxRequestPerTrack) break; - Request request(id, mbid, request_number); - requests_pending_.insert(id, request); + pending_mbid_requests_.insert(id, MbIdRequest(id, mbid, request_number)); } if (!timer_flush_requests_->isActive()) { @@ -153,373 +160,366 @@ void MusicBrainzClient::Start(const int id, const QStringList &mbid_list) { } -void MusicBrainzClient::StartDiscIdRequest(const QString &discid) { +void MusicBrainzClient::CancelMbIdRequest(const int id) { - const ParamList params = ParamList() << Param(u"inc"_s, u"artists+recordings"_s); + while (!mbid_requests_.isEmpty() && mbid_requests_.contains(id)) { + QNetworkReply *reply = mbid_requests_.take(id); + replies_.removeAll(reply); + QObject::disconnect(reply, nullptr, this, nullptr); + if (reply->isRunning()) reply->abort(); + reply->deleteLater(); + } + +} + +void MusicBrainzClient::FlushMbIdRequests() { + + if (!mbid_requests_.isEmpty() || pending_mbid_requests_.isEmpty()) return; + + SendMbIdRequest(pending_mbid_requests_.take(pending_mbid_requests_.firstKey())); + +} + +void MusicBrainzClient::SendMbIdRequest(const MbIdRequest &request) { QUrlQuery url_query; - url_query.setQueryItems(params); - QUrl url(QString::fromLatin1(kDiscUrl) + discid); - url.setQuery(url_query); + url_query.addQueryItem(u"inc"_s, u"releases+media+artists"_s); + url_query.addQueryItem(u"status"_s, u"official"_s); + url_query.addQueryItem(u"fmt"_s, u"json"_s); - QNetworkRequest network_request(url); - network_request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::NoLessSafeRedirectPolicy); - QNetworkReply *reply = network_->get(network_request); - QObject::connect(reply, &QNetworkReply::finished, this, [this, discid, reply]() { DiscIdRequestFinished(discid, reply); }); + QNetworkReply *reply = CreateGetRequest(QUrl(QString::fromLatin1(kTrackUrl) + request.mbid), url_query); + QObject::connect(reply, &QNetworkReply::finished, this, [this, reply, request]() { MbIdRequestFinished(reply, request.id, request.number); }); + mbid_requests_.insert(request.id, reply); timeouts_->AddReply(reply); } -void MusicBrainzClient::FlushRequests() { - - if (!requests_.isEmpty() || requests_pending_.isEmpty()) return; - - const Request request = requests_pending_.take(requests_pending_.firstKey()); - - const ParamList params = ParamList() << Param(u"inc"_s, u"artists+releases+media"_s); - - QUrlQuery url_query; - url_query.setQueryItems(params); - QUrl url(QString::fromLatin1(kTrackUrl) + request.mbid); - url.setQuery(url_query); - - QNetworkRequest network_request(url); - network_request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::NoLessSafeRedirectPolicy); - QNetworkReply *reply = network_->get(network_request); - QObject::connect(reply, &QNetworkReply::finished, this, [this, reply, request]() { RequestFinished(reply, request.id, request.number); }); - requests_.insert(request.id, reply); - - timeouts_->AddReply(reply); - -} - -void MusicBrainzClient::RequestFinished(QNetworkReply *reply, const int id, const int request_number) { +void MusicBrainzClient::MbIdRequestFinished(QNetworkReply *reply, const int id, const int request_number) { + if (replies_.contains(reply)) { + replies_.removeAll(reply); + } QObject::disconnect(reply, nullptr, this, nullptr); reply->deleteLater(); - const qint64 nb_removed = requests_.remove(id, reply); - if (nb_removed != 1) { - qLog(Debug) << "MusicBrainz: Unknown reply received:" << nb_removed << "requests removed, while only one was supposed to be removed"; + if (mbid_requests_.contains(id, reply)) { + mbid_requests_.remove(id, reply); } - if (!timer_flush_requests_->isActive() && requests_.isEmpty() && !requests_pending_.isEmpty()) { + if (!timer_flush_requests_->isActive() && mbid_requests_.isEmpty() && !pending_mbid_requests_.isEmpty()) { timer_flush_requests_->start(); } - QString error; - QByteArray data = GetReplyData(reply, error); - if (!data.isEmpty()) { - QXmlStreamReader reader(data); - ResultList res; - while (!reader.atEnd()) { - if (reader.readNext() == QXmlStreamReader::StartElement && reader.name().toString() == "recording"_L1) { - const ResultList tracks = ParseTrack(&reader); - for (const Result &track : tracks) { - if (!track.title_.isEmpty()) { - res << track; - } - } - } + const JsonObjectResult json_object_result = ParseJsonObject(reply); + if (!json_object_result.success()) { + Q_EMIT MbIdFinished(id, ResultList(), json_object_result.error_message); + return; + } + const QJsonObject object_root = json_object_result.json_object; + + if (object_root.contains("releases"_L1) && object_root.value("releases"_L1).isArray()) { + const ReleaseList releases = ParseReleases(object_root.value("releases"_L1).toArray()); + if (!releases.isEmpty()) { + pending_results_[id] << PendingResults(request_number, ResultListFromReleases(releases)); } - pending_results_[id] << PendingResults(request_number, res); } - // No more pending requests for this id: emit the results we have. - if (!requests_.contains(id) && !requests_pending_.contains(id)) { - // Merge the results we have - ResultList ret; - QList result_list_list = pending_results_.take(id); - std::sort(result_list_list.begin(), result_list_list.end()); - for (const PendingResults &result_list : std::as_const(result_list_list)) { - ret << result_list.results_; + if (!mbid_requests_.contains(id) && !pending_mbid_requests_.contains(id)) { + QList pending_results_list = pending_results_.take(id); + std::sort(pending_results_list.begin(), pending_results_list.end()); + ResultList results; + for (const PendingResults &pending_results : std::as_const(pending_results_list)) { + results << pending_results.results_; } - Q_EMIT Finished(id, UniqueResults(ret, UniqueResultsSortOption::KeepOriginalOrder), error); + Q_EMIT MbIdFinished(id, UniqueResults(results, UniqueResultsSortOption::KeepOriginalOrder)); } } -void MusicBrainzClient::DiscIdRequestFinished(const QString &discid, QNetworkReply *reply) { +void MusicBrainzClient::StartDiscIdRequest(const QString &disc_id) { + qLog(Debug) << "Starting MusicBrainz Disc ID request for" << disc_id; + + pending_discid_requests_ << disc_id; + + if (!timer_flush_requests_->isActive()) { + timer_flush_requests_->start(); + } + +} + +void MusicBrainzClient::CancelDiscIdRequest(const QString &disc_id) { + + while (!discid_requests_.isEmpty() && discid_requests_.contains(disc_id)) { + QNetworkReply *reply = discid_requests_.take(disc_id); + replies_.removeAll(reply); + QObject::disconnect(reply, nullptr, this, nullptr); + if (reply->isRunning()) reply->abort(); + reply->deleteLater(); + } + +} + +void MusicBrainzClient::FlushDiscIdRequests() { + + if (!discid_requests_.isEmpty() || pending_discid_requests_.isEmpty()) return; + + SendDiscIdRequest(pending_discid_requests_.takeFirst()); + +} + +void MusicBrainzClient::SendDiscIdRequest(const QString &disc_id) { + + QUrlQuery url_query; + url_query.addQueryItem(u"inc"_s, u"recordings+artists"_s); + url_query.addQueryItem(u"fmt"_s, u"json"_s); + + QNetworkReply *reply = CreateGetRequest(QUrl(QString::fromLatin1(kDiscUrl) + disc_id), url_query); + QObject::connect(reply, &QNetworkReply::finished, this, [this, disc_id, reply]() { DiscIdRequestFinished(disc_id, reply); }); + + timeouts_->AddReply(reply); + +} + +void MusicBrainzClient::DiscIdRequestFinished(const QString &disc_id, QNetworkReply *reply) { + + if (replies_.contains(reply)) { + replies_.removeAll(reply); + } QObject::disconnect(reply, nullptr, this, nullptr); reply->deleteLater(); - ResultList ret; - QString artist; - QString album; - int year = 0; + if (discid_requests_.contains(disc_id)) { + discid_requests_.remove(disc_id); + } - QString error; - QByteArray data = GetReplyData(reply, error); - if (data.isEmpty()) { - Q_EMIT DiscIdFinished(artist, album, ret, error); + if (!timer_flush_requests_->isActive() && discid_requests_.isEmpty() && !pending_discid_requests_.isEmpty()) { + timer_flush_requests_->start(); + } + + const JsonObjectResult json_object_result = ParseJsonObject(reply); + if (!json_object_result.success()) { + Q_EMIT DiscIdFinished(disc_id, ResultList(), json_object_result.error_message); return; } + const QJsonObject object_root = json_object_result.json_object; - // Parse xml result: - // -get title - // -get artist - // -get year - // -get all the tracks' tags - // Note: If there are multiple releases for the discid, the first - // release is chosen. - QXmlStreamReader reader(data); - while (!reader.atEnd()) { - QXmlStreamReader::TokenType type = reader.readNext(); - if (type == QXmlStreamReader::StartElement) { - QString name = reader.name().toString(); - if (name == "title"_L1) { - album = reader.readElementText(); - } - else if (name == "date"_L1) { - QRegularExpression regex(QString::fromLatin1(kDateRegex)); - QRegularExpressionMatch re_match = regex.match(reader.readElementText()); - if (re_match.capturedStart() == 0) { - year = re_match.captured(0).toInt(); - } - } - else if (name == "artist-credit"_L1) { - ParseArtist(&reader, &artist); - } - else if (name == "medium-list"_L1) { - break; - } + ResultList results; + if (object_root.contains("releases"_L1) && object_root.value("releases"_L1).isArray()) { + const ReleaseList releases = ParseReleases(object_root.value("releases"_L1).toArray()); + if (!releases.isEmpty()) { + results = ResultListFromReleases(ReleaseList() << releases.first(), disc_id); } } - while (!reader.atEnd()) { - QXmlStreamReader::TokenType token = reader.readNext(); - QString name = reader.name().toString(); - if (token == QXmlStreamReader::StartElement && name == "medium"_L1) { - // Get the medium with a matching discid. - if (MediumHasDiscid(discid, &reader)) { - const ResultList tracks = ParseMedium(&reader); - for (const Result &track : tracks) { - if (!track.title_.isEmpty()) { - ret << track; - } - } - } - else { - Utilities::ConsumeCurrentElement(&reader); - } - } - else if (token == QXmlStreamReader::EndElement && name == "medium-list"_L1) { - break; - } - } - - // If we parsed a year, copy it to the tracks. - if (year > 0) { - for (ResultList::iterator it = ret.begin(); it != ret.end(); ++it) { - it->year_ = year; - } - } - - Q_EMIT DiscIdFinished(artist, album, UniqueResults(ret, UniqueResultsSortOption::SortResults)); + Q_EMIT DiscIdFinished(disc_id, UniqueResults(results, UniqueResultsSortOption::SortResults)); } -bool MusicBrainzClient::MediumHasDiscid(const QString &discid, QXmlStreamReader *reader) { +MusicBrainzClient::ReleaseList MusicBrainzClient::ParseReleases(const QJsonArray &array_releases) { - while (!reader->atEnd()) { - QXmlStreamReader::TokenType type = reader->readNext(); - QString name = reader->name().toString(); - - if (type == QXmlStreamReader::StartElement && name == "disc"_L1 && reader->attributes().value("id"_L1).toString() == discid) { - return true; - } - if (type == QXmlStreamReader::EndElement && name == "disc-list"_L1) { - return false; - } + ReleaseList releases; + for (const QJsonValue &value_release : array_releases) { + if (!value_release.isObject()) continue; + releases << ParseRelease(value_release.toObject()); } - qLog(Debug) << "Reached end of xml stream without encountering "; - return false; + + return releases; } -MusicBrainzClient::ResultList MusicBrainzClient::ParseMedium(QXmlStreamReader *reader) { +MusicBrainzClient::Release MusicBrainzClient::ParseRelease(const QJsonObject &object_release) { - ResultList ret; - while (!reader->atEnd()) { - QXmlStreamReader::TokenType type = reader->readNext(); - QString name = reader->name().toString(); + Release release; - if (type == QXmlStreamReader::StartElement) { - if (name == "track"_L1) { + if (object_release.contains("artist-credit"_L1) && object_release.value("artist-credit"_L1).isArray()) { + release.artist_ = ParseArtistCredit(object_release.value("artist-credit"_L1).toArray()); + } + if (object_release.contains("title"_L1) && object_release.value("title"_L1).isString()) { + release.album_ = object_release.value("title"_L1).toString(); + } + if (object_release.contains("date"_L1) && object_release.value("date"_L1).isString()) { + const QString year_str = ParseDate(object_release.value("date"_L1).toString()); + if (!year_str.isEmpty()) { + release.year_ = year_str.toInt(); + } + } + if (object_release.contains("media"_L1) && object_release.value("media"_L1).isArray()) { + release.media_ = ParseMediaList(object_release.value("media"_L1).toArray()); + } + + return release; + +} + +MusicBrainzClient::MediaList MusicBrainzClient::ParseMediaList(const QJsonArray &array_media_list) { + + MediaList media_list; + for (const QJsonValue &value_media : array_media_list) { + if (!value_media.isObject()) continue; + media_list << ParseMedia(value_media.toObject()); + } + + return media_list; + +} + +MusicBrainzClient::Media MusicBrainzClient::ParseMedia(const QJsonObject &object_media) { + + Media media; + + if (object_media.contains("discs"_L1) && object_media.value("discs"_L1).isArray()) { + media.disc_ids_ = ParseDiscIds(object_media.value("discs"_L1).toArray()); + } + + if (object_media.contains("tracks"_L1) && object_media.value("tracks"_L1).isArray()) { + media.tracks_ = ParseTracks(object_media.value("tracks"_L1).toArray()); + } + + return media; + +} + +MusicBrainzClient::Artist MusicBrainzClient::ParseArtistCredit(const QJsonArray &array_artist_credits) { + + Artist artist; + for (const QJsonValue &value_artist_credit : array_artist_credits) { + if (!value_artist_credit.isObject()) continue; + const QJsonObject object_artist_credit = value_artist_credit.toObject(); + QString name; + QString sort_name; + QString join_phrase; + if (object_artist_credit.contains("name"_L1) && object_artist_credit.value("name"_L1).isString()) { + name = object_artist_credit.value("name"_L1).toString(); + } + if (object_artist_credit.contains("artist"_L1) && object_artist_credit.value("artist"_L1).isObject()) { + const QJsonObject object_artist = object_artist_credit.value("artist"_L1).toObject(); + if (object_artist.contains("name"_L1) && object_artist.value("name"_L1).isString()) { + name = object_artist.value("name"_L1).toString(); + } + if (object_artist.contains("sort-name"_L1) && object_artist.value("sort-name"_L1).isString()) { + sort_name = object_artist.value("sort-name"_L1).toString(); + } + } + if (object_artist_credit.contains("joinphrase"_L1) && object_artist_credit.value("joinphrase"_L1).isString()) { + join_phrase = object_artist_credit.value("joinphrase"_L1).toString(); + } + if (!name.isEmpty()) { + artist.name_ += name + join_phrase; + } + if (!sort_name.isEmpty()) { + artist.sort_name_ += sort_name + join_phrase; + } + } + + return artist; + +} + +QString MusicBrainzClient::ParseDate(const QString &date_str) { + + static QRegularExpression regex(QString::fromLatin1(kDateRegex)); + const QRegularExpressionMatch match = regex.match(date_str); + if (match.capturedStart() == 0) { + return match.captured(0); + } + + return QString(); + +} + +QStringList MusicBrainzClient::ParseDiscIds(const QJsonArray &array_discs) { + + QStringList disc_ids; + for (const QJsonValue &value_disc : array_discs) { + if (!value_disc.isObject()) continue; + const QJsonObject object_disc = value_disc.toObject(); + if (object_disc.contains("id"_L1)) { + disc_ids << object_disc.value("id"_L1).toString(); + } + } + + return disc_ids; + +} + +MusicBrainzClient::TrackList MusicBrainzClient::ParseTracks(const QJsonArray &array_tracks) { + + TrackList tracks; + for (const QJsonValue &value_track : array_tracks) { + if (!value_track.isObject()) continue; + tracks << ParseTrack(value_track.toObject()); + } + + return tracks; + +} + +MusicBrainzClient::Track MusicBrainzClient::ParseTrack(const QJsonObject &object_track) { + + Track track; + if (object_track.contains("position"_L1) && object_track.value("position"_L1).isDouble()) { + track.number_ = object_track.value("position"_L1).toInt(); + } + if (object_track.contains("title"_L1) && object_track.value("title"_L1).isString()) { + track.title_ = object_track.value("title"_L1).toString(); + } + if (object_track.contains("length"_L1) && object_track.value("length"_L1).isDouble()) { + track.duration_msec_ = object_track.value("length"_L1).toInt(); + } + if (object_track.contains("artist-credit"_L1) && object_track.value("artist-credit"_L1).isArray()) { + track.artist_ = ParseArtistCredit(object_track.value("artist-credit"_L1).toArray()); + } + + return track; + +} + +MusicBrainzClient::ResultList MusicBrainzClient::ResultListFromReleases(const ReleaseList &releases, const QString &disc_id) { + + ResultList results; + for (const Release &release : releases) { + for (const Media &media : release.media_) { + if (!disc_id.isEmpty() && !media.disc_ids_.contains(disc_id)) { + continue; + } + for (const Track &track : media.tracks_) { Result result; - result = ParseTrackFromDisc(reader); - ret << result; + result.year_ = release.year_; + result.album_artist_ = release.artist_.name_; + result.sort_album_artist_ = release.artist_.sort_name_; + result.album_ = release.album_; + result.title_ = track.title_; + result.artist_ = track.artist_.name_; + result.sort_artist_ = track.artist_.sort_name_; + result.track_ = track.number_; + result.duration_msec_ = track.duration_msec_; + results << result; } } - - if (type == QXmlStreamReader::EndElement && name == "track-list"_L1) { - break; - } } - return ret; + return results; } -MusicBrainzClient::Result MusicBrainzClient::ParseTrackFromDisc(QXmlStreamReader *reader) { +MusicBrainzClient::ResultList MusicBrainzClient::UniqueResults(const ResultList &results, const UniqueResultsSortOption opt) { - Result result; - - while (!reader->atEnd()) { - QXmlStreamReader::TokenType type = reader->readNext(); - QString name = reader->name().toString(); - - if (type == QXmlStreamReader::StartElement) { - if (name == "position"_L1) { - result.track_ = reader->readElementText().toInt(); - } - else if (name == "length"_L1) { - result.duration_msec_ = reader->readElementText().toInt(); - } - else if (name == "title"_L1) { - result.title_ = reader->readElementText(); - } - } - - if (type == QXmlStreamReader::EndElement && name == "track"_L1) { - break; - } - } - - return result; - -} - -MusicBrainzClient::ResultList MusicBrainzClient::ParseTrack(QXmlStreamReader *reader) { - - Result result; - QList releases; - - while (!reader->atEnd()) { - QXmlStreamReader::TokenType type = reader->readNext(); - QString name = reader->name().toString(); - - if (type == QXmlStreamReader::StartElement) { - - if (name == "title"_L1) { - result.title_ = reader->readElementText(); - } - else if (name == "length"_L1) { - result.duration_msec_ = reader->readElementText().toInt(); - } - else if (name == "artist-credit"_L1) { - ParseArtist(reader, &result.artist_); - } - else if (name == "release"_L1) { - releases << ParseRelease(reader); - } - } - - if (type == QXmlStreamReader::EndElement && name == "recording"_L1) { - break; - } - } - - ResultList ret; - if (releases.isEmpty()) { - ret << result; + ResultList unique_results; + if (opt == UniqueResultsSortOption::SortResults) { + unique_results = QSet(results.begin(), results.end()).values(); + std::sort(unique_results.begin(), unique_results.end()); } else { - std::stable_sort(releases.begin(), releases.end()); - ret.reserve(releases.count()); - for (const Release &release : std::as_const(releases)) { - ret << release.CopyAndMergeInto(result); - } - } - - return ret; - -} - -// Parse the artist. Multiple artists are joined together with the joinphrase from musicbrainz. -void MusicBrainzClient::ParseArtist(QXmlStreamReader *reader, QString *artist) { - - QString join_phrase; - - while (!reader->atEnd()) { - QXmlStreamReader::TokenType type = reader->readNext(); - QString name = reader->name().toString(); - if (type == QXmlStreamReader::StartElement && name == "name-credit"_L1) { - join_phrase = reader->attributes().value("joinphrase"_L1).toString(); - } - - if (type == QXmlStreamReader::StartElement && name == "name"_L1) { - *artist += reader->readElementText() + join_phrase; - } - - if (type == QXmlStreamReader::EndElement && name == "artist-credit"_L1) { - return; - } - } -} - -MusicBrainzClient::Release MusicBrainzClient::ParseRelease(QXmlStreamReader *reader) { - - Release ret; - - while (!reader->atEnd()) { - QXmlStreamReader::TokenType type = reader->readNext(); - QString name = reader->name().toString(); - - if (type == QXmlStreamReader::StartElement) { - if (name == "title"_L1) { - ret.album_ = reader->readElementText(); - } - else if (name == "status"_L1) { - ret.SetStatusFromString(reader->readElementText()); - } - else if (name == "date"_L1) { - QRegularExpression regex(QString::fromLatin1(kDateRegex)); - QRegularExpressionMatch re_match = regex.match(reader->readElementText()); - if (re_match.capturedStart() == 0) { - ret.year_ = re_match.captured(0).toInt(); - } - } - else if (name == "track-list"_L1) { - ret.track_ = reader->attributes().value("offset"_L1).toString().toInt() + 1; - Utilities::ConsumeCurrentElement(reader); - } - } - - if (type == QXmlStreamReader::EndElement && name == "release"_L1) { - break; - } - } - - return ret; - -} - -MusicBrainzClient::ResultList MusicBrainzClient::UniqueResults(const ResultList &results, UniqueResultsSortOption opt) { - - ResultList ret; - if (opt == UniqueResultsSortOption::SortResults) { - ret = QSet(results.begin(), results.end()).values(); - std::sort(ret.begin(), ret.end()); - } - else { // KeepOriginalOrder - // Qt doesn't provide an ordered set (QSet "stores values in an unspecified order" according to Qt documentation). - // We might use std::set instead, but it's probably faster to use ResultList directly to avoid converting from one structure to another. - for (const Result &res : results) { - if (!ret.contains(res)) { - ret << res; + for (const Result &result : results) { + if (!unique_results.contains(result)) { + unique_results << result; } } } - return ret; + + return unique_results; } -void MusicBrainzClient::Error(const QString &error, const QVariant &debug) { - - qLog(Error) << "MusicBrainz:" << error; - if (debug.isValid()) qLog(Debug) << debug; - -} diff --git a/src/musicbrainz/musicbrainzclient.h b/src/musicbrainz/musicbrainzclient.h index 340bc443c..8c36e44d2 100644 --- a/src/musicbrainz/musicbrainzclient.h +++ b/src/musicbrainz/musicbrainzclient.h @@ -1,7 +1,5 @@ /* * Strawberry Music Player - * This file was part of Clementine. - * Copyright 2010, David Sansome * Copyright 2019-2025, Jonas Kvinge * * Strawberry is free software: you can redistribute it and/or modify @@ -24,6 +22,8 @@ #include "config.h" +#include + #include #include #include @@ -34,48 +34,44 @@ #include #include "includes/shared_ptr.h" +#include "core/jsonbaserequest.h" class QNetworkReply; class QTimer; -class QXmlStreamReader; +class QJsonValue; +class QJsonObject; +class QJsonArray; class NetworkAccessManager; class NetworkTimeouts; -class MusicBrainzClient : public QObject { +class MusicBrainzClient : public JsonBaseRequest { Q_OBJECT - // Gets metadata for a particular MBID. - // An MBID is created from a fingerprint using MusicDnsClient. - // You can create one MusicBrainzClient and make multiple requests using it. - // IDs are provided by the caller when a request is started and included in the Finished signal - they have no meaning to MusicBrainzClient. - public: - // The second argument allows for specifying a custom network access manager. - // It is used in tests. The ownership of network is not transferred. explicit MusicBrainzClient(SharedPtr network, QObject *parent = nullptr); ~MusicBrainzClient() override; + virtual QString service_name() const override { return QLatin1String("MusicBrainz"); } + virtual bool authentication_required() const override { return false; } + virtual bool authenticated() const override { return true; } + virtual bool use_authorization_header() const override { return false; } + virtual QByteArray authorization_header() const override { return QByteArray(); } + struct Result { + public: Result() : duration_msec_(0), track_(0), year_(-1) {} bool operator<(const Result &other) const { -#define cmp(field) \ - if ((field) < other.field) return true; \ - if ((field) > other.field) return false; - - cmp(track_); - cmp(year_); - cmp(title_); - cmp(artist_); - return false; - -#undef cmp + return std::tie(title_, artist_, sort_artist_, album_artist_, sort_album_artist_, album_, duration_msec_, track_, year_) + < std::tie(other.title_, other.artist_, other.sort_artist_, other.album_artist_, other.sort_album_artist_, other.album_, other.duration_msec_, other.track_, other.year_); } bool operator==(const Result &other) const { - return - title_ == other.title_ && + return title_ == other.title_ && artist_ == other.artist_ && + sort_artist_ == other.sort_artist_ && + album_artist_ == other.album_artist_ && + sort_album_artist_ == other.sort_album_artist_ && album_ == other.album_ && duration_msec_ == other.duration_msec_ && track_ == other.track_ && @@ -84,6 +80,9 @@ class MusicBrainzClient : public QObject { QString title_; QString artist_; + QString sort_artist_; + QString album_artist_; + QString sort_album_artist_; QString album_; int duration_msec_; int track_; @@ -91,97 +90,76 @@ class MusicBrainzClient : public QObject { }; using ResultList = QList; - // Starts a request and returns immediately. Finished() will be emitted later with the same ID. - void Start(const int id, const QStringList &mbid); + void StartMbIdRequest(const int id, const QStringList &mbid); + void CancelMbIdRequest(const int id); + void StartDiscIdRequest(const QString &discid); + void CancelDiscIdRequest(const QString &disc_id); - // Cancels the request with the given ID. Finished() will never be emitted for that ID. Does nothing if there is no request with the given ID. - void Cancel(int id); - - // Cancels all requests. Finished() will never be emitted for any pending requests. void CancelAll(); Q_SIGNALS: - // Finished signal emitted when feching songs tags - void Finished(const int id, const MusicBrainzClient::ResultList &result, const QString &error = QString()); - // Finished signal emitted when fechting album's songs tags using DiscId - void DiscIdFinished(const QString &artist, const QString &album, const MusicBrainzClient::ResultList &result, const QString &error = QString()); + void MbIdFinished(const int id, const MusicBrainzClient::ResultList &result, const QString &error = QString()); + void DiscIdFinished(const QString &disc_id, const MusicBrainzClient::ResultList &result, const QString &error = QString()); private Q_SLOTS: void FlushRequests(); - // id identifies the track, and request_number means it's the 'request_number'th request for this track - void RequestFinished(QNetworkReply *reply, const int id, const int request_number); + // ID identifies the track, and request_number means it's the 'request_number'th request for this track + void MbIdRequestFinished(QNetworkReply *reply, const int id, const int request_number); void DiscIdRequestFinished(const QString &discid, QNetworkReply *reply); private: - using Param = QPair; - using ParamList = QList; - - struct Request { - Request() : id(0), number(0) {} - Request(const int _id, const QString &_mbid, const int _number) : id(_id), mbid(_mbid), number(_number) {} + class MbIdRequest { + public: + MbIdRequest() : id(0), number(0) {} + MbIdRequest(const int _id, const QString &_mbid, const int _number) : id(_id), mbid(_mbid), number(_number) {} int id; QString mbid; int number; }; - // Used as parameter for UniqueResults enum class UniqueResultsSortOption { SortResults = 0, KeepOriginalOrder }; - struct Release { - - enum class Status { - Unknown = 0, - PseudoRelease, - Bootleg, - Promotional, - Official - }; - - Release() : track_(0), year_(0), status_(Status::Unknown) {} - - Result CopyAndMergeInto(const Result &orig) const { - Result ret(orig); - ret.album_ = album_; - ret.track_ = track_; - ret.year_ = year_; - return ret; - } - - void SetStatusFromString(const QString &s) { - if (s.compare(QLatin1String("Official"), Qt::CaseInsensitive) == 0) { - status_ = Status::Official; - } - else if (s.compare(QLatin1String("Promotion"), Qt::CaseInsensitive) == 0) { - status_ = Status::Promotional; - } - else if (s.compare(QLatin1String("Bootleg"), Qt::CaseInsensitive) == 0) { - status_ = Status::Bootleg; - } - else if (s.compare(QLatin1String("Pseudo-release"), Qt::CaseInsensitive) == 0) { - status_ = Status::PseudoRelease; - } - else { - status_ = Status::Unknown; - } - } - - bool operator<(const Release &other) const { - // Compare status so that "best" status (e.g. Official) will be first when sorting a list of releases. - return status_ > other.status_; - } - - QString album_; - int track_; - int year_; - Status status_; + class Artist { + public: + QString name_; + QString sort_name_; }; - struct PendingResults { - PendingResults(int sort_id, const ResultList &results) : sort_id_(sort_id), results_(results) {} + class Track { + public: + Track() : number_(0), duration_msec_(0) {} + int number_; + QString title_; + Artist artist_; + int duration_msec_; + }; + using TrackList = QList; + + class Media { + public: + QList tracks_; + QStringList disc_ids_; + }; + using MediaList = QList; + + class Release { + public: + Release() : year_(0) {} + + Artist artist_; + QString album_; + int year_; + QList media_; + }; + using ReleaseList = QList; + + class PendingResults { + public: + PendingResults(const int sort_id, const ResultList &results) : sort_id_(sort_id), results_(results) {} bool operator<(const PendingResults &other) const { return sort_id_ < other.sort_id_; @@ -191,29 +169,40 @@ class MusicBrainzClient : public QObject { ResultList results_; }; - static QByteArray GetReplyData(QNetworkReply *reply, QString &error); - 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, UniqueResultsSortOption opt = UniqueResultsSortOption::SortResults); - static void Error(const QString &error, const QVariant &debug = QVariant()); + JsonObjectResult ParseJsonObject(QNetworkReply *reply); + void FlushMbIdRequests(); + void FlushDiscIdRequests(); + void SendMbIdRequest(const MbIdRequest &request); + void SendDiscIdRequest(const QString &disc_id); + + static ReleaseList ParseReleases(const QJsonArray &array_releases); + static Release ParseRelease(const QJsonObject &object_release); + static MediaList ParseMediaList(const QJsonArray &array_media_list); + static Media ParseMedia(const QJsonObject &object_media); + static Artist ParseArtistCredit(const QJsonArray &array_artist_credits); + static QStringList ParseDiscIds(const QJsonArray &array_discs); + static TrackList ParseTracks(const QJsonArray &array_tracks); + static Track ParseTrack(const QJsonObject &object_track); + static QString ParseDate(const QString &date_str); + static ResultList ResultListFromReleases(const ReleaseList &releases, const QString &disc_id = QString()); + static ResultList UniqueResults(const ResultList &results, const UniqueResultsSortOption opt = UniqueResultsSortOption::SortResults); private: SharedPtr network_; NetworkTimeouts *timeouts_; - QMultiMap requests_pending_; - QMultiMap requests_; - // Results we received so far, kept here until all the replies are finished + + QMultiMap pending_mbid_requests_; + QList pending_discid_requests_; + + QMultiMap mbid_requests_; + QMultiMap discid_requests_; + QMap> pending_results_; QTimer *timer_flush_requests_; - }; inline size_t qHash(const MusicBrainzClient::Result &result) { - return qHash(result.album_) ^ qHash(result.artist_) ^ result.duration_msec_ ^ qHash(result.title_) ^ result.track_ ^ result.year_; + return qHash(result.album_) ^ qHash(result.artist_) ^ qHash(result.sort_artist_) ^ qHash(result.album_artist_) ^ qHash(result.sort_album_artist_) ^ result.duration_msec_ ^ qHash(result.title_) ^ result.track_ ^ result.year_; } #endif // MUSICBRAINZCLIENT_H diff --git a/src/musicbrainz/tagfetcher.cpp b/src/musicbrainz/tagfetcher.cpp index 82273a36a..6f67ea959 100644 --- a/src/musicbrainz/tagfetcher.cpp +++ b/src/musicbrainz/tagfetcher.cpp @@ -45,7 +45,7 @@ TagFetcher::TagFetcher(SharedPtr network, QObject *parent) musicbrainz_client_(new MusicBrainzClient(network, this)) { QObject::connect(acoustid_client_, &AcoustidClient::Finished, this, &TagFetcher::PuidsFound); - QObject::connect(musicbrainz_client_, &MusicBrainzClient::Finished, this, &TagFetcher::TagsFetched); + QObject::connect(musicbrainz_client_, &MusicBrainzClient::MbIdFinished, this, &TagFetcher::TagsFetched); } @@ -130,7 +130,7 @@ void TagFetcher::PuidsFound(const int index, const QStringList &puid_list, const } Q_EMIT Progress(song, tr("Downloading metadata")); - musicbrainz_client_->Start(index, puid_list); + musicbrainz_client_->StartMbIdRequest(index, puid_list); } @@ -146,8 +146,15 @@ void TagFetcher::TagsFetched(const int index, const MusicBrainzClient::ResultLis for (const MusicBrainzClient::Result &result : results) { Song song; song.Init(result.title_, result.artist_, result.album_, result.duration_msec_ * kNsecPerMsec); + song.set_artistsort(result.sort_artist_); song.set_track(result.track_); song.set_year(result.year_); + if (!result.album_artist_.isEmpty() && result.album_artist_ != result.artist_) { + song.set_albumartist(result.album_artist_); + } + if (!result.sort_album_artist_.isEmpty() && result.sort_album_artist_ != result.sort_artist_) { + song.set_albumartistsort(result.sort_album_artist_); + } songs_guessed << song; }