From 540c901f873308227a34997c566a2f6f4f923019 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 9 Feb 2020 22:42:36 +0100 Subject: [PATCH 1/9] Add (Json)WebTask framework --- CMakeLists.txt | 2 + build/depends.py | 3 + src/network/httprequestmethod.h | 17 ++ src/network/httpstatuscode.h | 64 +++++++ src/network/jsonwebtask.cpp | 297 ++++++++++++++++++++++++++++++++ src/network/jsonwebtask.h | 104 +++++++++++ src/network/webtask.cpp | 267 ++++++++++++++++++++++++++++ src/network/webtask.h | 165 ++++++++++++++++++ 8 files changed, 919 insertions(+) create mode 100644 src/network/httprequestmethod.h create mode 100644 src/network/httpstatuscode.h create mode 100644 src/network/jsonwebtask.cpp create mode 100644 src/network/jsonwebtask.h create mode 100644 src/network/webtask.cpp create mode 100644 src/network/webtask.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 99c51f3ebdf..f13fceb559c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -432,6 +432,8 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/musicbrainz/musicbrainzclient.cpp src/musicbrainz/network.cpp src/musicbrainz/tagfetcher.cpp + src/network/jsonwebtask.cpp + src/network/webtask.cpp src/preferences/broadcastprofile.cpp src/preferences/broadcastsettings.cpp src/preferences/broadcastsettings_legacy.cpp diff --git a/build/depends.py b/build/depends.py index 812fdad8130..b64a36057f9 100644 --- a/build/depends.py +++ b/build/depends.py @@ -1128,6 +1128,9 @@ def sources(self, build): "src/library/trackloader.cpp", + "src/network/jsonwebtask.cpp", + "src/network/webtask.cpp", + "src/widget/wwaveformviewer.cpp", "src/waveform/sharedglcontext.cpp", diff --git a/src/network/httprequestmethod.h b/src/network/httprequestmethod.h new file mode 100644 index 00000000000..5aef0d16628 --- /dev/null +++ b/src/network/httprequestmethod.h @@ -0,0 +1,17 @@ +#pragma once + +namespace mixxx { + +namespace network { + +enum class HttpRequestMethod { + Get, + Put, + Post, + Patch, + Delete, +}; + +} // namespace network + +} // namespace mixxx diff --git a/src/network/httpstatuscode.h b/src/network/httpstatuscode.h new file mode 100644 index 00000000000..c88f5abe846 --- /dev/null +++ b/src/network/httpstatuscode.h @@ -0,0 +1,64 @@ +#pragma once + +#include + +namespace mixxx { + +namespace network { + +typedef int HttpStatusCode; + +const HttpStatusCode kHttpStatusCodeInvalid = -1; +const HttpStatusCode kHttpStatusCodeOk = 200; +const HttpStatusCode kHttpStatusCodeCreated = 201; +const HttpStatusCode kHttpStatusCodeAccepted = 202; +const HttpStatusCode kHttpStatusCodeNoContent = 204; + +inline bool HttpStatusCode_isInformational( + int statusCode) { + return statusCode >= 100 && statusCode < 200; +} + +inline bool HttpStatusCode_isSuccess( + int statusCode) { + return statusCode >= 200 && statusCode < 300; +} + +inline bool HttpStatusCode_isRedirection( + int statusCode) { + return statusCode >= 300 && statusCode < 400; +} + +inline bool HttpStatusCode_isClientError( + int statusCode) { + return statusCode >= 400 && statusCode < 500; +} + +inline bool HttpStatusCode_isServerError( + int statusCode) { + return statusCode >= 500 && statusCode < 600; +} + +inline bool HttpStatusCode_isCustomError( + int statusCode) { + return statusCode >= 900 && statusCode < 1000; +} + +inline bool HttpStatusCode_isError( + int statusCode) { + return HttpStatusCode_isClientError(statusCode) || + HttpStatusCode_isServerError(statusCode) || + HttpStatusCode_isCustomError(statusCode); +} + +inline bool HttpStatusCode_isValid( + int statusCode) { + return (statusCode >= 100 && statusCode < 600) || + HttpStatusCode_isCustomError(statusCode); +} + +} // namespace network + +} // namespace mixxx + +Q_DECLARE_METATYPE(mixxx::network::HttpStatusCode); diff --git a/src/network/jsonwebtask.cpp b/src/network/jsonwebtask.cpp new file mode 100644 index 00000000000..08530f66494 --- /dev/null +++ b/src/network/jsonwebtask.cpp @@ -0,0 +1,297 @@ +#include "network/jsonwebtask.h" + +#if QT_VERSION < QT_VERSION_CHECK(5, 8, 0) +#include +#endif +#include +#include +#include +#include +#include +#include // std::once_flag + +#include "util/assert.h" +#include "util/counter.h" +#include "util/logger.h" + +namespace mixxx { + +namespace network { + +namespace { + +const Logger kLogger("mixxx::network::JsonWebTask"); + +std::once_flag registerMetaTypesOnceFlag; + +void registerMetaTypesOnce() { + JsonWebResponse::registerMetaType(); +} + +const QString JSON_CONTENT_TYPE = "application/json"; + +const QMimeType JSON_MIME_TYPE = QMimeDatabase().mimeTypeForName(JSON_CONTENT_TYPE); + +QMimeType readContentType( + const QNetworkReply* reply) { + DEBUG_ASSERT(reply); + const QVariant contentTypeHeader = reply->header(QNetworkRequest::ContentTypeHeader); + if (!contentTypeHeader.isValid() || contentTypeHeader.isNull()) { + kLogger.warning() + << "Missing content type header"; + return QMimeType(); + } + const QString contentTypeString = contentTypeHeader.toString(); + const QString contentTypeWithoutParams = contentTypeString.left(contentTypeString.indexOf(';')); + const QMimeType contentType = QMimeDatabase().mimeTypeForName(contentTypeWithoutParams); + if (!contentType.isValid()) { + kLogger.warning() + << "Unknown content type" + << contentTypeWithoutParams; + } + return contentType; +} + +bool readJsonContent( + QNetworkReply* reply, + QJsonDocument* jsonContent) { + DEBUG_ASSERT(reply); + DEBUG_ASSERT(jsonContent); + DEBUG_ASSERT(JSON_MIME_TYPE.isValid()); + const auto contentType = readContentType(reply); + if (contentType == JSON_MIME_TYPE) { + *jsonContent = QJsonDocument::fromJson(reply->readAll()); + return true; + } else { + return false; + } +} + +// TODO: Allow to customize headers and attributes? +QNetworkRequest newRequest( + const QUrl& url) { + QNetworkRequest request(url); + request.setHeader(QNetworkRequest::ContentTypeHeader, JSON_CONTENT_TYPE); +#if QT_VERSION >= QT_VERSION_CHECK(5, 6, 0) + request.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true); +#endif + return request; +} + +} // anonymous namespace + +/*static*/ void JsonWebResponse::registerMetaType() { + qRegisterMetaType("mixxx::network::JsonWebResponse"); +} + +JsonWebTask::JsonWebTask( + QNetworkAccessManager* networkAccessManager, + QUrl baseUrl, + JsonWebRequest request, + QObject* parent) + : WebTask(networkAccessManager, parent), + m_baseUrl(std::move(baseUrl)), + m_request(std::move(request)), + m_pendingNetworkReply(nullptr) { + std::call_once(registerMetaTypesOnceFlag, registerMetaTypesOnce); + DEBUG_ASSERT(!m_baseUrl.isEmpty()); +} + +JsonWebTask::~JsonWebTask() { + if (m_pendingNetworkReply) { + m_pendingNetworkReply->deleteLater(); + } +} + +void JsonWebTask::onFinished( + JsonWebResponse response) { + kLogger.info() + << "Response received" + << response.replyUrl + << response.statusCode + << response.content; + deleteAfterFinished(); +} + +void JsonWebTask::onFinishedCustom( + CustomWebResponse response) { + kLogger.info() + << "Custom response received" + << response.replyUrl + << response.statusCode + << response.content; + deleteAfterFinished(); +} + +QNetworkReply* JsonWebTask::sendNetworkRequest( + QNetworkAccessManager* networkAccessManager, + HttpRequestMethod method, + const QUrl& url, + const QJsonDocument& content) { + switch (method) { + case HttpRequestMethod::Get: { + DEBUG_ASSERT(m_request.content.isEmpty()); + if (kLogger.debugEnabled()) { + kLogger.debug() + << "GET" + << url; + } + return networkAccessManager->get( + QNetworkRequest(url)); + } + case HttpRequestMethod::Put: { + const auto body = content.toJson(QJsonDocument::Compact); + if (kLogger.debugEnabled()) { + kLogger.debug() + << "PUT" + << url + << body; + } + return networkAccessManager->put( + newRequest(url), + body); + } + case HttpRequestMethod::Post: { + const auto body = content.toJson(QJsonDocument::Compact); + if (kLogger.debugEnabled()) { + kLogger.debug() + << "POST" + << url + << body; + } + return networkAccessManager->post( + newRequest(url), + body); + } + case HttpRequestMethod::Patch: { + const auto body = m_request.content.toJson(QJsonDocument::Compact); + if (kLogger.debugEnabled()) { + kLogger.debug() + << "PATCH" + << url + << body; + } +#if QT_VERSION >= QT_VERSION_CHECK(5, 8, 0) + return networkAccessManager->sendCustomRequest( + newRequest(url), + "PATCH", + body); +#else + auto* buffer = new QBuffer(this); + buffer->setData(body); + return networkAccessManager->sendCustomRequest( + newRequest(url), + "PATCH", + buffer); +#endif + } + case HttpRequestMethod::Delete: { + DEBUG_ASSERT(content.isEmpty()); + if (kLogger.debugEnabled()) { + kLogger.debug() + << "DELETE" + << url; + } + return networkAccessManager->deleteResource( + QNetworkRequest(url)); + } + } + DEBUG_ASSERT(!"unreachable"); + return nullptr; +} + +bool JsonWebTask::doStart( + QNetworkAccessManager* networkAccessManager, + int parentTimeoutMillis) { + Q_UNUSED(parentTimeoutMillis); + DEBUG_ASSERT(thread() == QThread::currentThread()); + DEBUG_ASSERT(networkAccessManager); + VERIFY_OR_DEBUG_ASSERT(!m_pendingNetworkReply) { + kLogger.warning() + << "Task has already been started"; + return false; + } + + DEBUG_ASSERT(m_baseUrl.isValid()); + QUrl url = m_baseUrl; + url.setPath(m_request.path); + if (!m_request.query.isEmpty()) { + url.setQuery(m_request.query); + } + DEBUG_ASSERT(url.isValid()); + + m_pendingNetworkReply = sendNetworkRequest( + networkAccessManager, + m_request.method, + url, + m_request.content); + VERIFY_OR_DEBUG_ASSERT(m_pendingNetworkReply) { + kLogger.warning() + << "Request not sent"; + return false; + } + + connect(m_pendingNetworkReply, + &QNetworkReply::finished, + this, + &JsonWebTask::slotNetworkReplyFinished, + Qt::UniqueConnection); + + return true; +} + +void JsonWebTask::doAbort() { + if (m_pendingNetworkReply) { + m_pendingNetworkReply->abort(); + } +} + +void JsonWebTask::slotNetworkReplyFinished() { + DEBUG_ASSERT(thread() == QThread::currentThread()); + const QPair networkReplyWithStatusCode = + receiveNetworkReply(); + auto* const networkReply = networkReplyWithStatusCode.first; + if (!networkReply) { + // already aborted + return; + } + const auto statusCode = networkReplyWithStatusCode.second; + VERIFY_OR_DEBUG_ASSERT(networkReply == m_pendingNetworkReply) { + return; + } + m_pendingNetworkReply = nullptr; + + QJsonDocument content; + if (statusCode != kHttpStatusCodeInvalid && + networkReply->bytesAvailable() > 0 && + !readJsonContent(networkReply, &content)) { + onFinishedCustom(CustomWebResponse{ + WebResponse{ + networkReply->url(), + statusCode}, + networkReply->readAll()}); + } else { + onFinished(JsonWebResponse{ + WebResponse{ + networkReply->url(), + statusCode}, + std::move(content)}); + } +} + +void JsonWebTask::emitFailed( + network::JsonWebResponse response) { + const auto signal = QMetaMethod::fromSignal( + &JsonWebTask::failed); + DEBUG_ASSERT(receivers(signal.name()) <= 1); // unique connection + if (isSignalConnected(signal)) { + emit failed( + std::move(response)); + } else { + deleteLater(); + } +} + +} // namespace network + +} // namespace mixxx diff --git a/src/network/jsonwebtask.h b/src/network/jsonwebtask.h new file mode 100644 index 00000000000..1a7fb0927aa --- /dev/null +++ b/src/network/jsonwebtask.h @@ -0,0 +1,104 @@ +#pragma once + +#include +#include + +#include "network/httprequestmethod.h" +#include "network/webtask.h" + +namespace mixxx { + +namespace network { + +struct JsonWebRequest final { + public: + JsonWebRequest() = delete; + JsonWebRequest(const JsonWebRequest&) = default; + JsonWebRequest(JsonWebRequest&&) = default; + + JsonWebRequest& operator=(const JsonWebRequest&) = default; + JsonWebRequest& operator=(JsonWebRequest&&) = default; + + HttpRequestMethod method; + QString path; + QUrlQuery query; + QJsonDocument content; +}; + +struct JsonWebResponse : public WebResponse { + public: + static void registerMetaType(); + + JsonWebResponse() = default; + JsonWebResponse( + WebResponse response, + QJsonDocument content) + : WebResponse(std::move(response)), + content(std::move(content)) { + } + JsonWebResponse(const JsonWebResponse&) = default; + JsonWebResponse(JsonWebResponse&&) = default; + ~JsonWebResponse() override = default; + + JsonWebResponse& operator=(const JsonWebResponse&) = default; + JsonWebResponse& operator=(JsonWebResponse&&) = default; + + QJsonDocument content; +}; + +class JsonWebTask : public WebTask { + Q_OBJECT + + public: + JsonWebTask( + QNetworkAccessManager* networkAccessManager, + QUrl baseUrl, + JsonWebRequest request, + QObject* parent = nullptr); + ~JsonWebTask() override; + + signals: + void failed( + network::JsonWebResponse response); + + private slots: + void slotNetworkReplyFinished(); + + protected: + // Customizable in derived classes + virtual QNetworkReply* sendNetworkRequest( + QNetworkAccessManager* networkAccessManager, + HttpRequestMethod method, + const QUrl& url, + const QJsonDocument& content); + + void emitFailed( + network::JsonWebResponse response); + + private: + // Handle the response and ensure that the task eventually + // gets deleted. The default implementation discards the + // response and deletes the task. + virtual void onFinished( + JsonWebResponse response); + virtual void onFinishedCustom( + CustomWebResponse response); + + bool doStart( + QNetworkAccessManager* networkAccessManager, + int parentTimeoutMillis) override; + void doAbort() override; + + // All member variables must only be accessed from + // the event loop thread!! + const QUrl m_baseUrl; + const JsonWebRequest m_request; + + QPointer m_pendingNetworkReply; +}; + +} // namespace network + +} // namespace mixxx + +Q_DECLARE_METATYPE(mixxx::network::JsonWebResponse); diff --git a/src/network/webtask.cpp b/src/network/webtask.cpp new file mode 100644 index 00000000000..c79b5438429 --- /dev/null +++ b/src/network/webtask.cpp @@ -0,0 +1,267 @@ +#include "network/webtask.h" + +#include +#include +#include +#include // std::once_flag + +#include "util/assert.h" +#include "util/counter.h" +#include "util/logger.h" + +namespace mixxx { + +namespace network { + +namespace { + +const Logger kLogger("mixxx::network::WebTask"); + +constexpr int kInvalidTimerId = -1; + +// count = even number (ctor + dtor) +// sum = 0 (no memory leaks) +Counter s_instanceCounter(QStringLiteral("mixxx::network::WebTask")); + +std::once_flag registerMetaTypesOnceFlag; + +void registerMetaTypesOnce() { + WebResponse::registerMetaType(); + CustomWebResponse::registerMetaType(); +} + +bool readStatusCode( + const QNetworkReply* reply, + int* statusCode) { + DEBUG_ASSERT(statusCode); + const QVariant statusCodeAttr = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute); + bool statusCodeValid = false; + const int statusCodeValue = statusCodeAttr.toInt(&statusCodeValid); + VERIFY_OR_DEBUG_ASSERT(statusCodeValid && HttpStatusCode_isValid(statusCodeValue)) { + kLogger.warning() + << "Invalid or missing status code attribute" + << statusCodeAttr; + } + else { + *statusCode = statusCodeValue; + } + return statusCodeValid; +} + +} // anonymous namespace + +/*static*/ void WebResponse::registerMetaType() { + qRegisterMetaType("mixxx::network::WebResponse"); +} + +/*static*/ void CustomWebResponse::registerMetaType() { + qRegisterMetaType("mixxx::network::CustomWebResponse"); +} + +WebTask::WebTask( + QNetworkAccessManager* networkAccessManager, + QObject* parent) + : QObject(parent), + m_networkAccessManager(networkAccessManager), + m_timeoutTimerId(kInvalidTimerId), + m_abort(false) { + std::call_once(registerMetaTypesOnceFlag, registerMetaTypesOnce); + DEBUG_ASSERT(m_networkAccessManager); + s_instanceCounter.increment(1); +} + +WebTask::~WebTask() { + s_instanceCounter.increment(-1); +} + +void WebTask::onAborted() { + DEBUG_ASSERT(m_abort); + const auto signal = QMetaMethod::fromSignal( + &WebTask::aborted); + if (isSignalConnected(signal)) { + emit aborted(); + } else { + kLogger.info() + << "Request aborted"; + deleteAfterFinished(); + } +} + +void WebTask::onNetworkError( + QUrl requestUrl, + QNetworkReply::NetworkError errorCode, + QString errorString, + QByteArray errorContent) { + const auto signal = QMetaMethod::fromSignal( + &WebTask::networkError); + if (isSignalConnected(signal)) { + emit networkError( + std::move(requestUrl), + errorCode, + std::move(errorString), + std::move(errorContent)); + } else { + kLogger.warning() + << "Network error" + << requestUrl + << errorCode + << errorString + << errorContent; + deleteAfterFinished(); + } +} + +void WebTask::invokeStart(int timeoutMillis) { + QMetaObject::invokeMethod( + this, +#if QT_VERSION < QT_VERSION_CHECK(5, 10, 0) + "slotStart", + Qt::AutoConnection, + Q_ARG(int, timeoutMillis) +#else + [this, timeoutMillis] { + this->slotStart(timeoutMillis); + } +#endif + ); +} + +void WebTask::invokeAbort() { + QMetaObject::invokeMethod( + this, +#if QT_VERSION < QT_VERSION_CHECK(5, 10, 0) + "slotAbort" +#else + [this] { + this->slotAbort(); + } +#endif + ); +} + +void WebTask::deleteBeforeFinished() { + // Might be called from any thread so we must not + // access any member variables! + // Do not disconnect any connections, because otherwise + // the destroyed() signal is not received! + invokeAbort(); + deleteLater(); +} + +void WebTask::deleteAfterFinished() { + // Might be called from any thread so we must not + // access any member variables! + // Do not disconnect any connections, because otherwise + // the destroyed() signal is not received! + deleteLater(); +} + +void WebTask::slotStart(int timeoutMillis) { + DEBUG_ASSERT(thread() == QThread::currentThread()); + VERIFY_OR_DEBUG_ASSERT(m_networkAccessManager) { + kLogger.warning() + << "No network access"; + return; + } + VERIFY_OR_DEBUG_ASSERT(!m_abort) { + kLogger.warning() + << "Task has already been aborted"; + return; + } + + kLogger.debug() + << "Starting..."; + if (!doStart(m_networkAccessManager, timeoutMillis)) { + kLogger.warning() + << "Start aborted"; + return; + } + // Task can only be started once + m_networkAccessManager = nullptr; + + if (m_abort) { + onAborted(); + return; + } + if (timeoutMillis > 0) { + m_timeoutTimerId = startTimer(timeoutMillis); + DEBUG_ASSERT(m_timeoutTimerId != kInvalidTimerId); + } +} + +void WebTask::slotAbort() { + DEBUG_ASSERT(thread() == QThread::currentThread()); + if (m_abort) { + return; + } + m_abort = true; + kLogger.debug() + << "Aborting..."; + doAbort(); +} + +void WebTask::timerEvent(QTimerEvent* event) { + DEBUG_ASSERT(thread() == QThread::currentThread()); + const auto timerId = event->timerId(); + DEBUG_ASSERT(timerId != kInvalidTimerId); + if (timerId != m_timeoutTimerId) { + // ignore + return; + } + killTimer(timerId); + m_timeoutTimerId = kInvalidTimerId; + kLogger.info() + << "Timed out"; + slotAbort(); +} + +QPair WebTask::receiveNetworkReply() { + DEBUG_ASSERT(thread() == QThread::currentThread()); + auto* const networkReply = qobject_cast(sender()); + HttpStatusCode statusCode = kHttpStatusCodeInvalid; + VERIFY_OR_DEBUG_ASSERT(networkReply) { + return qMakePair(nullptr, statusCode); + } + networkReply->deleteLater(); + + if (m_abort) { + onAborted(); + return qMakePair(nullptr, statusCode); + } + + if (networkReply->error() != QNetworkReply::NetworkError::NoError) { + onNetworkError( + networkReply->request().url(), + networkReply->error(), + networkReply->errorString(), + networkReply->readAll()); + return qMakePair(nullptr, statusCode); + } + + if (kLogger.debugEnabled()) { + if (networkReply->url() == networkReply->request().url()) { + kLogger.debug() + << "Received reply for request" + << networkReply->url(); + } else { + // Redirected + kLogger.debug() + << "Received reply for redirected request" + << networkReply->request().url() + << "->" + << networkReply->url(); + } + } + + DEBUG_ASSERT(statusCode == kHttpStatusCodeInvalid); + VERIFY_OR_DEBUG_ASSERT(readStatusCode(networkReply, &statusCode)) { + kLogger.warning() + << "Failed to read HTTP status code"; + } + + return qMakePair(networkReply, statusCode); +} + +} // namespace network + +} // namespace mixxx diff --git a/src/network/webtask.h b/src/network/webtask.h new file mode 100644 index 00000000000..42c448a0ff7 --- /dev/null +++ b/src/network/webtask.h @@ -0,0 +1,165 @@ +#pragma once + +#include +#include +#include +#include +#include + +#include "network/httpstatuscode.h" + +namespace mixxx { + +namespace network { + +struct WebResponse { + public: + static void registerMetaType(); + + WebResponse() + : statusCode(kHttpStatusCodeInvalid) { + } + explicit WebResponse( + QUrl replyUrl, + HttpStatusCode statusCode = kHttpStatusCodeInvalid) + : replyUrl(std::move(replyUrl)), + statusCode(statusCode) { + } + WebResponse(const WebResponse&) = default; + WebResponse(WebResponse&&) = default; + virtual ~WebResponse() = default; + + WebResponse& operator=(const WebResponse&) = default; + WebResponse& operator=(WebResponse&&) = default; + + bool isStatusCodeValid() const { + return HttpStatusCode_isValid(statusCode); + } + bool isStatusCodeSuccess() const { + return HttpStatusCode_isSuccess(statusCode); + } + bool isStatusCodeRedirection() const { + return HttpStatusCode_isRedirection(statusCode); + } + bool isStatusCodeClientError() const { + return HttpStatusCode_isClientError(statusCode); + } + bool isStatusCodeServerError() const { + return HttpStatusCode_isServerError(statusCode); + } + bool isStatusCodeCustomError() const { + return HttpStatusCode_isCustomError(statusCode); + } + bool isStatusCodeError() const { + return HttpStatusCode_isError(statusCode); + } + + QUrl replyUrl; + HttpStatusCode statusCode; +}; + +struct CustomWebResponse : public WebResponse { + public: + static void registerMetaType(); + + CustomWebResponse() = default; + CustomWebResponse( + WebResponse response, + QByteArray content) + : WebResponse(std::move(response)), + content(std::move(content)) { + } + CustomWebResponse(const CustomWebResponse&) = default; + CustomWebResponse(CustomWebResponse&&) = default; + ~CustomWebResponse() override = default; + + CustomWebResponse& operator=(const CustomWebResponse&) = default; + CustomWebResponse& operator=(CustomWebResponse&&) = default; + + QUrl replyUrl; + HttpStatusCode statusCode; + QByteArray content; +}; + +class WebTask : public QObject { + Q_OBJECT + + public: + explicit WebTask( + QNetworkAccessManager* networkAccessManager, + QObject* parent = nullptr); + ~WebTask() override; + + // timeoutMillis <= 0: No timeout (unlimited) + // timeoutMillis > 0: Implicitly aborted after timeout expired + void invokeStart( + int timeoutMillis = 0); + + // Cancel a pending request. + void invokeAbort(); + + // Abort the pending request while suppressing any signals + // and mark the task for deletion. + void deleteBeforeFinished(); + + // Disconnect from all signals after receiving a reply + // and mark the task for deletion. + void deleteAfterFinished(); + + public slots: + void slotStart( + int timeoutMillis); + void slotAbort(); + + signals: + // The receiver is responsible for deleting the task in the + // corresponding slot handler!! Otherwise the task will remain + // in memory as a dysfunctional zombie until its parent object + // is finally deleted. If no receiver is connected the task + // will be deleted implicitly. + void aborted(); + void networkError( + QUrl requestUrl, + QNetworkReply::NetworkError errorCode, + QString errorString, + QByteArray errorContent); + + protected: + void timerEvent(QTimerEvent* event) override; + + // Handle an aborted request and ensure that the task eventually + // gets deleted. The default implementation simply deletes the + // task. + virtual void onAborted(); + + // Handle the abort and ensure that the task eventually + // gets deleted. The default implementation logs a warning + // and deletes the task. + virtual void onNetworkError( + QUrl requestUrl, + QNetworkReply::NetworkError errorCode, + QString errorString, + QByteArray errorContent); + + QPair receiveNetworkReply(); + + private: + virtual bool doStart( + QNetworkAccessManager* networkAccessManager, + int parentTimeoutMillis) = 0; + virtual void doAbort() = 0; + + // All member variables must only be accessed from + // the event loop thread!! + QPointer m_networkAccessManager; + int m_timeoutTimerId; + bool m_abort; +}; + +} // namespace network + +} // namespace mixxx + +Q_DECLARE_METATYPE(mixxx::network::WebResponse); + +Q_DECLARE_METATYPE(mixxx::network::CustomWebResponse); From 745f336b614c14662f62f358317e61669259559b Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 23 Feb 2020 02:04:55 +0100 Subject: [PATCH 2/9] Add AcoustIdLookupTask --- CMakeLists.txt | 1 + build/depends.py | 1 + src/musicbrainz/web/acoustidlookuptask.cpp | 225 +++++++++++++++++++++ src/musicbrainz/web/acoustidlookuptask.h | 43 ++++ 4 files changed, 270 insertions(+) create mode 100644 src/musicbrainz/web/acoustidlookuptask.cpp create mode 100644 src/musicbrainz/web/acoustidlookuptask.h diff --git a/CMakeLists.txt b/CMakeLists.txt index f13fceb559c..73e853db720 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -432,6 +432,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/musicbrainz/musicbrainzclient.cpp src/musicbrainz/network.cpp src/musicbrainz/tagfetcher.cpp + src/musicbrainz/web/acoustidlookuptask.cpp src/network/jsonwebtask.cpp src/network/webtask.cpp src/preferences/broadcastprofile.cpp diff --git a/build/depends.py b/build/depends.py index b64a36057f9..8cc41c8d49f 100644 --- a/build/depends.py +++ b/build/depends.py @@ -1003,6 +1003,7 @@ def sources(self, build): "src/musicbrainz/acoustidclient.cpp", "src/musicbrainz/chromaprinter.cpp", "src/musicbrainz/musicbrainzclient.cpp", + "src/musicbrainz/web/acoustidlookuptask.cpp", "src/widget/wtracktableview.cpp", "src/widget/wtracktableviewheader.cpp", diff --git a/src/musicbrainz/web/acoustidlookuptask.cpp b/src/musicbrainz/web/acoustidlookuptask.cpp new file mode 100644 index 00000000000..1cb2d724f7c --- /dev/null +++ b/src/musicbrainz/web/acoustidlookuptask.cpp @@ -0,0 +1,225 @@ +#include "musicbrainz/web/acoustidlookuptask.h" + +#include +#include +#include + +#include "musicbrainz/gzip.h" +#include "util/assert.h" +#include "util/logger.h" + +namespace mixxx { + +namespace { + +const Logger kLogger("AcoustIdLookupTask"); + +// see API-KEY site here http://acoustid.org/application/496 +// I registered the KEY for version 1.12 -- kain88 (may 2013) +// See also: https://acoustid.org/webservice +const QString kClientApiKey = QStringLiteral("czKxnkyO"); + +const QUrl kBaseUrl = QStringLiteral("https://api.acoustid.org/"); + +const QString kRequestPath = QStringLiteral("/v2/lookup"); + +const QLatin1String kContentTypeHeaderValue("application/x-www-form-urlencoded"); + +const QByteArray kContentEncodingRawHeaderKey = "Content-Encoding"; +const QByteArray kContentEncodingRawHeaderValue = "gzip"; + +QUrlQuery lookupUrlQuery( + const QString& fingerprint, + int duration) { + DEBUG_ASSERT(!fingerprint.isEmpty()); + DEBUG_ASSERT(duration >= 0); + + QUrlQuery urlQuery; + urlQuery.addQueryItem( + QStringLiteral("format"), + QStringLiteral("json")); + urlQuery.addQueryItem( + QStringLiteral("client"), + kClientApiKey); + urlQuery.addQueryItem( + QStringLiteral("meta"), + QStringLiteral("recordingids")); + urlQuery.addQueryItem( + QStringLiteral("fingerprint"), + fingerprint); + urlQuery.addQueryItem( + QStringLiteral("duration"), + QString::number(duration)); + return urlQuery; +} + +network::JsonWebRequest lookupRequest() { + return network::JsonWebRequest{ + network::HttpRequestMethod::Post, + kRequestPath, + QUrlQuery(), // custom query + QJsonDocument(), // custom body + }; +} + +} // anonymous namespace + +AcoustIdLookupTask::AcoustIdLookupTask( + QNetworkAccessManager* networkAccessManager, + const QString& fingerprint, + int duration, + QObject* parent) + : network::JsonWebTask( + networkAccessManager, + kBaseUrl, + lookupRequest(), + parent), + m_urlQuery(lookupUrlQuery(fingerprint, duration)) { +} + +QNetworkReply* AcoustIdLookupTask::sendNetworkRequest( + QNetworkAccessManager* networkAccessManager, + network::HttpRequestMethod method, + const QUrl& url, + const QJsonDocument& content) { + Q_UNUSED(method); + DEBUG_ASSERT(method == network::HttpRequestMethod::Post); + Q_UNUSED(content); + DEBUG_ASSERT(content.isEmpty()); + + DEBUG_ASSERT(url.isValid()); + QNetworkRequest req(url); + req.setHeader( + QNetworkRequest::ContentTypeHeader, + kContentTypeHeaderValue); + req.setRawHeader( + kContentEncodingRawHeaderKey, + kContentEncodingRawHeaderValue); + + // application/x-www-form-urlencoded request bodies must be percent encoded. + DEBUG_ASSERT(!m_urlQuery.isEmpty()); + QByteArray body = gzipCompress( + m_urlQuery.query(QUrl::FullyEncoded).toLatin1()); + + if (kLogger.traceEnabled()) { + kLogger.trace() + << "POST" + << url + << body; + } + DEBUG_ASSERT(networkAccessManager); + return networkAccessManager->post(req, body); +} + +void AcoustIdLookupTask::onFinished( + network::JsonWebResponse response) { + if (!response.isStatusCodeSuccess()) { + kLogger.warning() + << "Request failed with HTTP status code" + << response.statusCode; + emitFailed(std::move(response)); + return; + } + VERIFY_OR_DEBUG_ASSERT(response.statusCode == network::kHttpStatusCodeOk) { + kLogger.warning() + << "Unexpected HTTP status code" + << response.statusCode; + emitFailed(std::move(response)); + return; + } + + VERIFY_OR_DEBUG_ASSERT(response.content.isObject()) { + kLogger.warning() + << "Invalid JSON content" + << response.content; + emitFailed(std::move(response)); + return; + } + const auto jsonObject = response.content.object(); + + const auto statusText = jsonObject.value(QStringLiteral("status")).toString(); + if (statusText != QStringLiteral("ok")) { + kLogger.warning() + << "Unexpected response status" + << statusText; + emitFailed(std::move(response)); + return; + } + + QList recordingIds; + DEBUG_ASSERT(jsonObject.value(QStringLiteral("results")).isArray()); + const QJsonArray results = jsonObject.value(QStringLiteral("results")).toArray(); + double maxScore = -1.0; // uninitialized (< 0) + // Results are expected to be ordered by score (descending) + for (const auto result : results) { + DEBUG_ASSERT(result.isObject()); + const auto resultObject = result.toObject(); + const auto resultId = + resultObject.value(QStringLiteral("id")).toString(); + DEBUG_ASSERT(!resultId.isEmpty()); + // The default score is 1.0 if missing + const double score = + resultObject.value(QStringLiteral("score")).toDouble(1.0); + DEBUG_ASSERT(score >= 0.0); + DEBUG_ASSERT(score <= 1.0); + if (maxScore < 0.0) { + // Initialize the maximum score + maxScore = score; + } + DEBUG_ASSERT(score <= maxScore); + if (score < maxScore && !recordingIds.isEmpty()) { + // Ignore all remaining results with lower values + // than the maximum score + break; + } + const auto recordings = result.toObject().value(QStringLiteral("recordings")); + if (recordings.isUndefined()) { + if (kLogger.debugEnabled()) { + kLogger.debug() + << "No recording(s) available for result" + << resultId + << "with score" + << score; + } + continue; + } else { + DEBUG_ASSERT(recordings.isArray()); + const QJsonArray recordingsArray = recordings.toArray(); + if (kLogger.debugEnabled()) { + kLogger.debug() + << "Found" + << recordingsArray.size() + << "recording(s) for result" + << resultId + << "with score" + << score; + } + for (const auto recording : recordingsArray) { + DEBUG_ASSERT(recording.isObject()); + const auto recordingObject = recording.toObject(); + const auto recordingId = + QUuid(recordingObject.value(QStringLiteral("id")).toString()); + VERIFY_OR_DEBUG_ASSERT(!recordingId.isNull()) { + continue; + } + recordingIds.append(recordingId); + } + } + } + emitSucceeded(std::move(recordingIds)); +} + +void AcoustIdLookupTask::emitSucceeded( + QList&& recordingIds) { + const auto signal = QMetaMethod::fromSignal( + &AcoustIdLookupTask::succeeded); + DEBUG_ASSERT(receivers(signal.name()) <= 1); // unique connection + if (isSignalConnected(signal)) { + emit succeeded( + std::move(recordingIds)); + } else { + deleteLater(); + } +} + +} // namespace mixxx diff --git a/src/musicbrainz/web/acoustidlookuptask.h b/src/musicbrainz/web/acoustidlookuptask.h new file mode 100644 index 00000000000..e8d0a2cce67 --- /dev/null +++ b/src/musicbrainz/web/acoustidlookuptask.h @@ -0,0 +1,43 @@ +#pragma once + +#include +#include +#include + +#include "network/jsonwebtask.h" + +namespace mixxx { + +class AcoustIdLookupTask : public network::JsonWebTask { + Q_OBJECT + + public: + AcoustIdLookupTask( + QNetworkAccessManager* networkAccessManager, + const QString& fingerprint, + int duration, + QObject* parent = nullptr); + ~AcoustIdLookupTask() override = default; + + signals: + void succeeded( + QList recordingIds); + + protected: + QNetworkReply* sendNetworkRequest( + QNetworkAccessManager* networkAccessManager, + network::HttpRequestMethod method, + const QUrl& url, + const QJsonDocument& content) override; + + private: + void onFinished( + network::JsonWebResponse response) override; + + void emitSucceeded( + QList&& recordingIds); + + const QUrlQuery m_urlQuery; +}; + +} // namespace mixxx From c4e5da752acb7e53f450ae0fd9224058826023e5 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 27 Feb 2020 16:24:47 +0100 Subject: [PATCH 3/9] Add MusicBrainzRecordingsTask --- CMakeLists.txt | 3 + build/depends.py | 3 + src/musicbrainz/musicbrainz.cpp | 26 + src/musicbrainz/musicbrainz.h | 51 ++ src/musicbrainz/musicbrainzxml.cpp | 508 ++++++++++++++++++ src/musicbrainz/musicbrainzxml.h | 30 ++ .../web/musicbrainzrecordingstask.cpp | 225 ++++++++ .../web/musicbrainzrecordingstask.h | 61 +++ 8 files changed, 907 insertions(+) create mode 100644 src/musicbrainz/musicbrainz.cpp create mode 100644 src/musicbrainz/musicbrainz.h create mode 100644 src/musicbrainz/musicbrainzxml.cpp create mode 100644 src/musicbrainz/musicbrainzxml.h create mode 100644 src/musicbrainz/web/musicbrainzrecordingstask.cpp create mode 100644 src/musicbrainz/web/musicbrainzrecordingstask.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 73e853db720..57d5a69a569 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -429,10 +429,13 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/musicbrainz/chromaprinter.cpp src/musicbrainz/crc.c src/musicbrainz/gzip.cpp + src/musicbrainz/musicbrainz.cpp + src/musicbrainz/musicbrainzxml.cpp src/musicbrainz/musicbrainzclient.cpp src/musicbrainz/network.cpp src/musicbrainz/tagfetcher.cpp src/musicbrainz/web/acoustidlookuptask.cpp + src/musicbrainz/web/musicbrainzrecordingstask.cpp src/network/jsonwebtask.cpp src/network/webtask.cpp src/preferences/broadcastprofile.cpp diff --git a/build/depends.py b/build/depends.py index 8cc41c8d49f..087aade6e57 100644 --- a/build/depends.py +++ b/build/depends.py @@ -1002,8 +1002,11 @@ def sources(self, build): "src/musicbrainz/crc.c", "src/musicbrainz/acoustidclient.cpp", "src/musicbrainz/chromaprinter.cpp", + "src/musicbrainz/musicbrainz.cpp", + "src/musicbrainz/musicbrainzxml.cpp", "src/musicbrainz/musicbrainzclient.cpp", "src/musicbrainz/web/acoustidlookuptask.cpp", + "src/musicbrainz/web/musicbrainzrecordingstask.cpp", "src/widget/wtracktableview.cpp", "src/widget/wtracktableviewheader.cpp", diff --git a/src/musicbrainz/musicbrainz.cpp b/src/musicbrainz/musicbrainz.cpp new file mode 100644 index 00000000000..874e3e9b55b --- /dev/null +++ b/src/musicbrainz/musicbrainz.cpp @@ -0,0 +1,26 @@ +#include "musicbrainz/musicbrainz.h" + +#include // std::once_flag/call_once + +namespace mixxx { + +namespace musicbrainz { + +namespace { + +std::once_flag s_registerMetaTypesOnceFlag; + +void registerMetaTypes() { + qRegisterMetaType(); + qRegisterMetaType>(); +} + +} // anonymous namespace + +void registerMetaTypesOnce() { + std::call_once(s_registerMetaTypesOnceFlag, registerMetaTypes); +} + +} // namespace musicbrainz + +} // namespace mixxx diff --git a/src/musicbrainz/musicbrainz.h b/src/musicbrainz/musicbrainz.h new file mode 100644 index 00000000000..a86d90df5c7 --- /dev/null +++ b/src/musicbrainz/musicbrainz.h @@ -0,0 +1,51 @@ +#pragma once + +#include +#include +#include + +#include "util/duration.h" + +namespace mixxx { + +namespace musicbrainz { + +struct TrackRelease final { + TrackRelease() = default; + TrackRelease(const TrackRelease&) = default; + TrackRelease(TrackRelease&&) = default; + TrackRelease& operator=(const TrackRelease&) = default; + TrackRelease& operator=(TrackRelease&&) = default; + ~TrackRelease() = default; + + QUuid artistId; + QUuid albumArtistId; + QUuid albumReleaseId; + QUuid recordingId; + QUuid releaseGroupId; + QUuid trackReleaseId; + + QString title; + QString artist; + QString albumTitle; + QString albumArtist; + QString releaseGroupType; + QString mediumFormat; + QString trackNumber; + QString trackTotal; + QString discNumber; + QString discTotal; + QString date; + + Duration duration; +}; + +void registerMetaTypesOnce(); + +} // namespace musicbrainz + +} // namespace mixxx + +Q_DECLARE_METATYPE(mixxx::musicbrainz::TrackRelease); + +Q_DECLARE_METATYPE(QList); diff --git a/src/musicbrainz/musicbrainzxml.cpp b/src/musicbrainz/musicbrainzxml.cpp new file mode 100644 index 00000000000..8a0f579105e --- /dev/null +++ b/src/musicbrainz/musicbrainzxml.cpp @@ -0,0 +1,508 @@ +#include "musicbrainz/musicbrainzxml.h" + +#include +#include +#include + +#include "util/logger.h" + + +namespace mixxx { + +namespace musicbrainz { + +namespace { + +Logger kLogger("MusicBrainz XML"); + +constexpr int kDefaultErrorCode = 0; + +bool finishElement( + const QXmlStreamReader& reader, + const QString& elementName = QString()) { + if (reader.tokenType() != QXmlStreamReader::EndElement) { + return false; + } + VERIFY_OR_DEBUG_ASSERT(elementName.isEmpty() || reader.name() == elementName) { + kLogger.warning() + << "Unexpected end element tag" + << reader.name() + << "instead of" + << elementName; + return false; + } + return true; +} + +bool continueReading(QXmlStreamReader& reader) { + return !(reader.atEnd() || reader.hasError()); +} + +void consumeCurrentElement(QXmlStreamReader& reader) { + DEBUG_ASSERT(reader.tokenType() == QXmlStreamReader::StartElement); + const auto elementName = reader.name().toString(); + int level = 0; + while (continueReading(reader)) { + switch (reader.readNext()) { + case QXmlStreamReader::StartElement: + if (kLogger.traceEnabled()) { + kLogger.trace() + << "Start skipping" + << reader.name(); + } + ++level; + break; + case QXmlStreamReader::EndElement: + if (level == 0) { + finishElement(reader, elementName); + return; + } + DEBUG_ASSERT(level > 0); + if (kLogger.traceEnabled()) { + kLogger.trace() + << "End skipping" + << reader.name(); + } + --level; + break; + default: + DEBUG_ASSERT(reader.tokenType() != QXmlStreamReader::NoToken); + DEBUG_ASSERT(reader.tokenType() != QXmlStreamReader::Invalid); + DEBUG_ASSERT(reader.tokenType() != QXmlStreamReader::StartDocument); + DEBUG_ASSERT(reader.tokenType() != QXmlStreamReader::EndDocument); + DEBUG_ASSERT(reader.tokenType() != QXmlStreamReader::DTD); + DEBUG_ASSERT(reader.tokenType() != QXmlStreamReader::EntityReference); + // ignore other token types + } + } +} + +void skipCurrentElement(QXmlStreamReader& reader) { + if (kLogger.traceEnabled()) { + kLogger.trace() + << "Skipping current element" + << reader.name(); + } + consumeCurrentElement(reader); +} + +bool parseDuration( + const QString& text, + Duration* pDuration = nullptr) { + bool ok = false; + int durationMillis = text.toInt(&ok); + if (ok && durationMillis > 0) { + if (pDuration) { + *pDuration = Duration::fromMillis(durationMillis); + } + return true; + } + kLogger.warning() + << "Invalid duration [ms]:" + << text; + return false; +} + +void readElementArtistCredit(QXmlStreamReader& reader, QString& artistName, QUuid& artistId) { + DEBUG_ASSERT(reader.name() == QStringLiteral("artist-credit")); + DEBUG_ASSERT(artistName.isEmpty()); + DEBUG_ASSERT(artistId.isNull()); + + QString nextJoinPhrase; + while (continueReading(reader)) { + const QXmlStreamReader::TokenType type = reader.readNext(); + if (type == QXmlStreamReader::StartElement) { + if (reader.name() == QStringLiteral("name-credit")) { + QString thisJoinPhrase = nextJoinPhrase; + nextJoinPhrase = reader.attributes().value(QStringLiteral("joinphrase")).toString(); + QString creditedName; + // Consume name-credit + while (continueReading(reader)) { + const QXmlStreamReader::TokenType type = reader.readNext(); + if (type == QXmlStreamReader::StartElement) { + if (reader.name() == QStringLiteral("name")) { + creditedName = reader.readElementText(); + } else if (reader.name() == QStringLiteral("artist")) { + // Consume artist + if (artistId.isNull()) { + artistId = QUuid(reader.attributes().value(QStringLiteral("id")).toString()); + } else { + kLogger.info() + << "Ignoring additional artist id" + << reader.attributes().value(QStringLiteral("id")); + } + DEBUG_ASSERT(!artistId.isNull()); + if (creditedName.isEmpty()) { + while (continueReading(reader)) { + const QXmlStreamReader::TokenType type = reader.readNext(); + if (type == QXmlStreamReader::StartElement) { + if (reader.name() == QStringLiteral("name")) { + creditedName = reader.readElementText(); + } else { + skipCurrentElement(reader); + } + } else if (finishElement(reader, QStringLiteral("artist"))) { + break; + } + } + } else { + // Already parsed from credited name + skipCurrentElement(reader); + } + } else { + skipCurrentElement(reader); + } + } else if (finishElement(reader, QStringLiteral("name-credit"))) { + break; + } + } + // Apply credited name + DEBUG_ASSERT(!creditedName.isEmpty()); + if (!artistName.isEmpty()) { + DEBUG_ASSERT(!thisJoinPhrase.isEmpty()); + artistName.append(thisJoinPhrase); + thisJoinPhrase = QString(); + } + artistName.append(creditedName); + } else { + skipCurrentElement(reader); + } + } else if (finishElement(reader, QStringLiteral("artist-credit"))) { + break; + } + } +} + +TrackRelease readElementRelease( + QXmlStreamReader& reader) { + DEBUG_ASSERT(reader.name() == QStringLiteral("release")); + + QString firstReleaseDate; + QString releaseGroupTitle; + QString releaseGroupArtist; + QUuid releaseGroupArtistId; + QString releaseTitleDisambiguation; + QString discNumber; + QString trackNumber; + TrackRelease trackRelease; + trackRelease.albumReleaseId = QUuid(reader.attributes().value(QStringLiteral("id")).toString()); + DEBUG_ASSERT(!trackRelease.albumReleaseId.isNull()); + while (continueReading(reader)) { + const QXmlStreamReader::TokenType type = reader.readNext(); + if (type == QXmlStreamReader::StartElement) { + if (reader.name() == QStringLiteral("title")) { + DEBUG_ASSERT(trackRelease.albumTitle.isNull()); + trackRelease.albumTitle = reader.readElementText(); + } else if (reader.name() == QStringLiteral("disambiguation")) { + DEBUG_ASSERT(releaseTitleDisambiguation.isNull()); + releaseTitleDisambiguation = reader.readElementText(); + } else if (reader.name() == QStringLiteral("artist-credit")) { + readElementArtistCredit(reader, trackRelease.albumArtist, trackRelease.albumArtistId); + } else if (reader.name() == QStringLiteral("date")) { + DEBUG_ASSERT(trackRelease.date.isNull()); + trackRelease.date = reader.readElementText(); + } else if (reader.name() == QStringLiteral("release-group")) { + DEBUG_ASSERT(trackRelease.releaseGroupId.isNull()); + trackRelease.releaseGroupId = QUuid(reader.attributes().value(QStringLiteral("id")).toString()); + DEBUG_ASSERT(!trackRelease.releaseGroupId.isNull()); + DEBUG_ASSERT(trackRelease.releaseGroupType.isNull()); + trackRelease.releaseGroupType = reader.attributes().value(QStringLiteral("type")).toString(); + // Consume release-group + while (continueReading(reader)) { + const QXmlStreamReader::TokenType type = reader.readNext(); + if (type == QXmlStreamReader::StartElement) { + if (reader.name() == QStringLiteral("first-release-date")) { + firstReleaseDate = reader.readElementText(); + } else if (reader.name() == QStringLiteral("title")) { + releaseGroupTitle = reader.readElementText(); + } else if (reader.name() == QStringLiteral("artist-credit")) { + readElementArtistCredit(reader, releaseGroupArtist, releaseGroupArtistId); + } else { + skipCurrentElement(reader); + } + } else if (finishElement(reader, QStringLiteral("release-group"))) { + break; + } + } + } else if (reader.name() == QStringLiteral("medium-list")) { + DEBUG_ASSERT(trackRelease.discTotal.isNull()); + trackRelease.discTotal = reader.attributes().value(QStringLiteral("count")).toString(); + // Consume medium-list (with a single medium element) + int mediumCount = 0; + while (continueReading(reader)) { + const QXmlStreamReader::TokenType type = reader.readNext(); + if (type == QXmlStreamReader::StartElement) { + if (reader.name() == QStringLiteral("medium")) { + // Consume medium + VERIFY_OR_DEBUG_ASSERT(++mediumCount == 1) { + kLogger.warning() + << "Ignoring additional medium" + << mediumCount + << "in medium-list"; + consumeCurrentElement(reader); + continue; + } + while (continueReading(reader)) { + const QXmlStreamReader::TokenType type = reader.readNext(); + if (type == QXmlStreamReader::StartElement) { + if (reader.name() == QStringLiteral("position")) { + DEBUG_ASSERT(trackRelease.discNumber.isNull()); + trackRelease.discNumber = reader.readElementText(); + } else if (reader.name() == QStringLiteral("number")) { + // Literal, non-numeric disc number + DEBUG_ASSERT(discNumber.isNull()); + discNumber = reader.readElementText(); + } else if (reader.name() == QStringLiteral("format")) { + DEBUG_ASSERT(trackRelease.mediumFormat.isNull()); + trackRelease.mediumFormat = reader.readElementText(); + } else if (reader.name() == QStringLiteral("track-list")) { + DEBUG_ASSERT(trackRelease.trackTotal.isNull()); + trackRelease.trackTotal = reader.attributes().value(QStringLiteral("count")).toString(); + // Consume track-list (with a single track element) + int trackCount = 0; + while (continueReading(reader)) { + const QXmlStreamReader::TokenType type = reader.readNext(); + if (type == QXmlStreamReader::StartElement) { + if (reader.name() == QStringLiteral("track")) { + // Consume track + VERIFY_OR_DEBUG_ASSERT(++trackCount == 1) { + kLogger.warning() + << "Ignoring additional track" + << trackCount + << "in track-list"; + consumeCurrentElement(reader); + continue; + } + DEBUG_ASSERT(trackRelease.trackReleaseId.isNull()); + trackRelease.trackReleaseId = QUuid(reader.attributes().value(QStringLiteral("id")).toString()); + DEBUG_ASSERT(!trackRelease.trackReleaseId.isNull()); + while (continueReading(reader)) { + const QXmlStreamReader::TokenType type = reader.readNext(); + if (type == QXmlStreamReader::StartElement) { + if (reader.name() == QStringLiteral("position")) { + DEBUG_ASSERT(trackRelease.trackNumber.isNull()); + trackRelease.trackNumber = reader.readElementText(); + } else if (reader.name() == QStringLiteral("number")) { + // Literal, non-numeric track number + DEBUG_ASSERT(trackNumber.isNull()); + trackNumber = reader.readElementText(); + } else if (reader.name() == QStringLiteral("title")) { + DEBUG_ASSERT(trackRelease.title.isNull()); + trackRelease.title = reader.readElementText(); + } else if (reader.name() == QStringLiteral("length")) { + DEBUG_ASSERT(trackRelease.duration == Duration::empty()); + parseDuration(reader.readElementText(), &trackRelease.duration); + } else { + skipCurrentElement(reader); + } + } else if (finishElement(reader, QStringLiteral("track"))) { + break; + } + } + } else { + skipCurrentElement(reader); + } + } else if (finishElement(reader, QStringLiteral("track-list"))) { + break; + } + } + } else { + skipCurrentElement(reader); + } + } else if (finishElement(reader, QStringLiteral("medium"))) { + break; + } + } + } else { + skipCurrentElement(reader); + } + } else if (finishElement(reader, QStringLiteral("medium-list"))) { + break; + } + } + } else { + skipCurrentElement(reader); + } + } else if (finishElement(reader, QStringLiteral("release"))) { + break; + } + } + + if (trackRelease.date.isEmpty()) { + trackRelease.date = firstReleaseDate; + } + if (trackRelease.albumTitle.isEmpty()) { + DEBUG_ASSERT(releaseTitleDisambiguation.isEmpty()); + trackRelease.albumTitle = releaseGroupTitle; + } else { + if (!releaseTitleDisambiguation.isEmpty()) { + trackRelease.albumTitle = QStringLiteral("%1 (%2)").arg(trackRelease.albumTitle, releaseTitleDisambiguation); + } + } + if (trackRelease.albumArtist.isEmpty()) { + trackRelease.albumArtist = releaseGroupArtist; + trackRelease.albumArtistId = releaseGroupArtistId; + } + if (trackRelease.discNumber.isEmpty()) { + // Use literal disc number only if numeric disc number not available + trackRelease.discNumber = discNumber; + } + if (trackRelease.trackNumber.isEmpty()) { + // Use literal track number only if numeric track number not available + trackRelease.trackNumber = trackNumber; + } + + return trackRelease; +} + +QPair, bool> readElementRecording(QXmlStreamReader& reader) { + DEBUG_ASSERT(reader.name() == QStringLiteral("recording")); + + QList trackReleases; + const auto recordingId = + QUuid(reader.attributes().value(QStringLiteral("id")).toString()); + DEBUG_ASSERT(!recordingId.isNull()); + + QString recordingTitle; + QString recordingArtist; + QUuid recordingArtistId; + QUuid releaseGroupId; + QString releaseGroupType; + Duration recordingDuration; + while (continueReading(reader)) { + const QXmlStreamReader::TokenType type = reader.readNext(); + if (type == QXmlStreamReader::StartElement) { + const QStringRef name = reader.name(); + if (reader.name() == QStringLiteral("title")) { + DEBUG_ASSERT(recordingTitle.isNull()); + recordingTitle = reader.readElementText(); + } else if (reader.name() == QStringLiteral("length")) { + DEBUG_ASSERT(recordingDuration == Duration::empty()); + parseDuration(reader.readElementText(), &recordingDuration); + } else if (reader.name() == QStringLiteral("artist-credit")) { + readElementArtistCredit(reader, recordingArtist, recordingArtistId); + } else if (reader.name() == QStringLiteral("release-list")) { + // Consume release-list + while (continueReading(reader)) { + const QXmlStreamReader::TokenType type = reader.readNext(); + if (type == QXmlStreamReader::StartElement) { + if (reader.name() == QStringLiteral("release")) { + trackReleases.append(readElementRelease(reader)); + } else if (reader.name() == QStringLiteral("release-group")) { + DEBUG_ASSERT(releaseGroupId.isNull()); + releaseGroupId = QUuid(reader.attributes().value(QStringLiteral("id")).toString()); + DEBUG_ASSERT(!releaseGroupId.isNull()); + DEBUG_ASSERT(releaseGroupType.isNull()); + releaseGroupType = reader.attributes().value(QStringLiteral("type")).toString(); + consumeCurrentElement(reader); + } else { + skipCurrentElement(reader); + } + } else if (finishElement(reader, QStringLiteral("release-list"))) { + break; + } + } + } else { + skipCurrentElement(reader); + } + } else if (finishElement(reader, QStringLiteral("recording"))) { + break; + } + } + + // Merge releases with common recording properties + for (auto&& trackRelease : trackReleases) { + DEBUG_ASSERT(trackRelease.recordingId.isNull()); + trackRelease.recordingId = recordingId; + if (trackRelease.releaseGroupId.isNull()) { + trackRelease.releaseGroupId = releaseGroupId; + } + if (trackRelease.releaseGroupType.isEmpty()) { + trackRelease.releaseGroupType = releaseGroupType; + } + if (trackRelease.title.isEmpty()) { + trackRelease.title = recordingTitle; + } + if (trackRelease.artist.isEmpty()) { + trackRelease.artist = recordingArtist; + } + if (trackRelease.artistId.isNull()) { + trackRelease.artistId = recordingArtistId; + } + if (trackRelease.duration == Duration::empty()) { + trackRelease.duration = recordingDuration; + } + } + + return qMakePair(trackReleases, true); +} + +} // anonymous namespace + +Error::Error() + : code(kDefaultErrorCode) { +} + +Error::Error( + QXmlStreamReader& reader) + : Error() { + while (continueReading(reader)) { + if (reader.readNext() == QXmlStreamReader::StartElement) { + const QStringRef name = reader.name(); + if (name == QStringLiteral("message")) { + DEBUG_ASSERT(message == Error().message); + message = reader.readElementText(); + } else if (name == QStringLiteral("code")) { + DEBUG_ASSERT(code == Error().code); + bool ok; + int val = reader.readElementText().toInt(&ok); + if (ok) { + code = val; + } + } + } + } +} + +QPair, bool> parseRecordings(QXmlStreamReader& reader) { + QStringRef codecName; + QList trackReleases; + while (continueReading(reader)) { + switch (reader.readNext()) { + case QXmlStreamReader::Invalid: + { + return qMakePair(trackReleases, false); + } + case QXmlStreamReader::StartDocument: + { + // The character encoding is always an ASCII string + codecName = reader.documentEncoding(); + break; + } + case QXmlStreamReader::StartElement: + { + if (reader.name() == QStringLiteral("recording")) { + auto recordingResult = readElementRecording(reader); + trackReleases += recordingResult.first; + if (!recordingResult.second) { + DEBUG_ASSERT(reader.hasError()); + kLogger.warning() + << "Aborting reading recording results after error" + << reader.errorString(); + return qMakePair(trackReleases, false); + } + } + break; + } + default: + { + // ignore any other token type + } + } + } + return qMakePair(trackReleases, true); +} + +} // namespace musicbrainz + +} // namespace mixxx diff --git a/src/musicbrainz/musicbrainzxml.h b/src/musicbrainz/musicbrainzxml.h new file mode 100644 index 00000000000..9afa7a0cfb1 --- /dev/null +++ b/src/musicbrainz/musicbrainzxml.h @@ -0,0 +1,30 @@ +#pragma once + +#include +#include +#include + +#include "musicbrainz/musicbrainz.h" + +namespace mixxx { + +namespace musicbrainz { + +struct Error final { + Error(); + explicit Error(QXmlStreamReader& reader); + Error(const Error&) = default; + Error(Error&&) = default; + Error& operator=(const Error&) = default; + Error& operator=(Error&&) = default; + ~Error() = default; + + int code; + QString message; +}; + +QPair, bool> parseRecordings(QXmlStreamReader& reader); + +} // namespace musicbrainz + +} // namespace mixxx diff --git a/src/musicbrainz/web/musicbrainzrecordingstask.cpp b/src/musicbrainz/web/musicbrainzrecordingstask.cpp new file mode 100644 index 00000000000..7c6d1d7df1b --- /dev/null +++ b/src/musicbrainz/web/musicbrainzrecordingstask.cpp @@ -0,0 +1,225 @@ +#include "musicbrainz/web/musicbrainzrecordingstask.h" + +#include +#include +#include + +#include "defs_urls.h" +#include "musicbrainz/gzip.h" +#include "musicbrainz/musicbrainzxml.h" +#include "network/httpstatuscode.h" +#include "util/assert.h" +#include "util/compatibility.h" +#include "util/logger.h" +#include "util/version.h" + +namespace mixxx { + +namespace { + +const Logger kLogger("MusicBrainzRecordingsTask"); + +const QUrl kBaseUrl = QStringLiteral("https://musicbrainz.org/"); + +const QString kRequestPath = QStringLiteral("/ws/2/recording/"); + +const QByteArray kUserAgentRawHeaderKey = "User-Agent"; + +QString userAgentRawHeaderValue() { + return Version::applicationName() + + QStringLiteral("/") + + Version::version() + + QStringLiteral(" ( ") + + QStringLiteral(MIXXX_WEBSITE_URL) + + QStringLiteral(" )"); +} + +QUrlQuery createUrlQuery() { + typedef QPair Param; + QList params; + params << Param("inc", "artists+artist-credits+releases+release-groups+media"); + + QUrlQuery query; + query.setQueryItems(params); + return query; +} + +QNetworkRequest createNetworkRequest( + const QUuid& recordingId) { + DEBUG_ASSERT(kBaseUrl.isValid()); + DEBUG_ASSERT(!recordingId.isNull()); + QUrl url = kBaseUrl; + url.setPath(kRequestPath + uuidToStringWithoutBraces(recordingId)); + url.setQuery(createUrlQuery()); + DEBUG_ASSERT(url.isValid()); + QNetworkRequest networkRequest(url); + // https://musicbrainz.org/doc/XML_Web_Service/Rate_Limiting#Provide_meaningful_User-Agent_strings + // HTTP request headers must be latin1. + networkRequest.setRawHeader( + kUserAgentRawHeaderKey, + userAgentRawHeaderValue().toLatin1()); + return networkRequest; +} + +} // anonymous namespace + +MusicBrainzRecordingsTask::MusicBrainzRecordingsTask( + QNetworkAccessManager* networkAccessManager, + QList&& recordingIds, + QObject* parent) + : network::WebTask( + networkAccessManager, + parent), + m_queuedRecordingIds(std::move(recordingIds)), + m_parentTimeoutMillis(0) { + musicbrainz::registerMetaTypesOnce(); +} + +bool MusicBrainzRecordingsTask::doStart( + QNetworkAccessManager* networkAccessManager, + int parentTimeoutMillis) { + m_parentTimeoutMillis = parentTimeoutMillis; + DEBUG_ASSERT(thread() == QThread::currentThread()); + DEBUG_ASSERT(networkAccessManager); + VERIFY_OR_DEBUG_ASSERT(!m_pendingNetworkReply) { + kLogger.warning() + << "Task has already been started"; + return false; + } + + VERIFY_OR_DEBUG_ASSERT(!m_queuedRecordingIds.isEmpty()) { + kLogger.warning() + << "Nothing to do"; + return false; + } + const auto recordingId = m_queuedRecordingIds.takeFirst(); + DEBUG_ASSERT(!recordingId.isNull()); + + const QNetworkRequest networkRequest = + createNetworkRequest(recordingId); + + if (kLogger.traceEnabled()) { + kLogger.trace() + << "GET" + << networkRequest.url(); + } + m_pendingNetworkReply = + networkAccessManager->get(networkRequest); + VERIFY_OR_DEBUG_ASSERT(m_pendingNetworkReply) { + kLogger.warning() + << "Request not sent"; + return false; + } + + connect(m_pendingNetworkReply, + &QNetworkReply::finished, + this, + &MusicBrainzRecordingsTask::slotNetworkReplyFinished, + Qt::UniqueConnection); + + return true; +} + +void MusicBrainzRecordingsTask::doAbort() { + if (m_pendingNetworkReply) { + m_pendingNetworkReply->abort(); + } +} + +void MusicBrainzRecordingsTask::slotNetworkReplyFinished() { + DEBUG_ASSERT(thread() == QThread::currentThread()); + const QPair + networkReplyWithStatusCode = receiveNetworkReply(); + auto* const networkReply = networkReplyWithStatusCode.first; + if (!networkReply) { + // already aborted + return; + } + const auto statusCode = networkReplyWithStatusCode.second; + VERIFY_OR_DEBUG_ASSERT(networkReply == m_pendingNetworkReply) { + return; + } + m_pendingNetworkReply = nullptr; + + const QByteArray body = networkReply->readAll(); + QXmlStreamReader reader(body); + + // HTTP status of successful results: + // 200: Found + // 301: Found, but UUID moved permanently in database + // 404: Not found in database, i.e. empty result + if (statusCode != 200 && statusCode != 301 && statusCode != 404) { + kLogger.info() + << "GET reply" + << "statusCode:" << statusCode + << "body:" << body; + const auto error = musicbrainz::Error(reader); + emitFailed( + network::WebResponse( + networkReply->url(), + statusCode), + error.code, + error.message); + return; + } + + auto recordingsResult = musicbrainz::parseRecordings(reader); + for (auto&& trackRelease : recordingsResult.first) { + // In case of a response with status 301 (Moved Permanently) + // the actual recording id might differ from the requested id. + // To avoid requesting recording ids twice we need to remember + // all recording ids. + m_finishedRecordingIds.insert(trackRelease.recordingId); + m_trackReleases.insert(trackRelease.trackReleaseId, trackRelease); + } + if (!recordingsResult.second) { + kLogger.warning() + << "Failed to parse XML response"; + slotAbort(); + return; + } + + if (m_queuedRecordingIds.isEmpty()) { + // Finished all recording ids + m_finishedRecordingIds.clear(); + auto trackReleases = m_trackReleases.values(); + m_trackReleases.clear(); + emitSucceeded(std::move(trackReleases)); + return; + } + + // Continue with next recording id + DEBUG_ASSERT(!m_queuedRecordingIds.isEmpty()); + slotStart(m_parentTimeoutMillis); +} + +void MusicBrainzRecordingsTask::emitSucceeded( + QList&& trackReleases) { + const auto signal = QMetaMethod::fromSignal( + &MusicBrainzRecordingsTask::succeeded); + DEBUG_ASSERT(receivers(signal.name()) <= 1); // unique connection + if (isSignalConnected(signal)) { + emit succeeded( + std::move(trackReleases)); + } else { + deleteLater(); + } +} +void MusicBrainzRecordingsTask::emitFailed( + network::WebResponse response, + int errorCode, + QString errorMessage) { + const auto signal = QMetaMethod::fromSignal( + &MusicBrainzRecordingsTask::succeeded); + DEBUG_ASSERT(receivers(signal.name()) <= 1); // unique connection + if (isSignalConnected(signal)) { + emit failed( + response, + errorCode, + errorMessage); + } else { + deleteLater(); + } +} + +} // namespace mixxx diff --git a/src/musicbrainz/web/musicbrainzrecordingstask.h b/src/musicbrainz/web/musicbrainzrecordingstask.h new file mode 100644 index 00000000000..a81f8923050 --- /dev/null +++ b/src/musicbrainz/web/musicbrainzrecordingstask.h @@ -0,0 +1,61 @@ +#pragma once + +#include +#include +#include +#include +#include + +#include "musicbrainz/musicbrainz.h" +#include "network/webtask.h" + +namespace mixxx { + +class MusicBrainzRecordingsTask : public network::WebTask { + Q_OBJECT + + public: + MusicBrainzRecordingsTask( + QNetworkAccessManager* networkAccessManager, + QList&& recordingIds, + QObject* parent = nullptr); + ~MusicBrainzRecordingsTask() override = default; + + signals: + void succeeded( + QList trackReleases); + void failed( + network::WebResponse response, + int errorCode, + QString errorMessage); + + private slots: + void slotNetworkReplyFinished(); + + private: + bool doStart( + QNetworkAccessManager* networkAccessManager, + int parentTimeoutMillis) override; + void doAbort() override; + + void emitSucceeded( + QList&& trackReleases); + void emitFailed( + network::WebResponse response, + int errorCode, + QString errorMessage); + + void continueWithNextRequest(); + + const QUrlQuery m_urlQuery; + + QList m_queuedRecordingIds; + QSet m_finishedRecordingIds; + + QMap m_trackReleases; + + QPointer m_pendingNetworkReply; + int m_parentTimeoutMillis; +}; + +} // namespace mixxx From daacb90fe5ca81bf88ea9decc729348f2157bd30 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 27 Feb 2020 16:25:31 +0100 Subject: [PATCH 4/9] Use web task framework and fetch additional MusicBrainz metadata --- CMakeLists.txt | 3 - build/depends.py | 3 - src/library/dlgtagfetcher.cpp | 142 +++++++++--- src/library/dlgtagfetcher.h | 2 - src/library/dlgtagfetcher.ui | 15 +- src/musicbrainz/acoustidclient.cpp | 205 ----------------- src/musicbrainz/acoustidclient.h | 64 ------ src/musicbrainz/musicbrainzclient.cpp | 303 -------------------------- src/musicbrainz/musicbrainzclient.h | 143 ------------ src/musicbrainz/network.cpp | 84 ------- src/musicbrainz/network.h | 50 ----- src/musicbrainz/tagfetcher.cpp | 266 +++++++++++++++------- src/musicbrainz/tagfetcher.h | 90 +++++--- src/track/track.h | 2 +- src/util/compatibility.h | 11 + 15 files changed, 369 insertions(+), 1014 deletions(-) delete mode 100644 src/musicbrainz/acoustidclient.cpp delete mode 100644 src/musicbrainz/acoustidclient.h delete mode 100644 src/musicbrainz/musicbrainzclient.cpp delete mode 100644 src/musicbrainz/musicbrainzclient.h delete mode 100644 src/musicbrainz/network.cpp delete mode 100644 src/musicbrainz/network.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 57d5a69a569..3cb65147709 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -425,14 +425,11 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/mixer/samplerbank.cpp src/mixxx.cpp src/mixxxapplication.cpp - src/musicbrainz/acoustidclient.cpp src/musicbrainz/chromaprinter.cpp src/musicbrainz/crc.c src/musicbrainz/gzip.cpp src/musicbrainz/musicbrainz.cpp src/musicbrainz/musicbrainzxml.cpp - src/musicbrainz/musicbrainzclient.cpp - src/musicbrainz/network.cpp src/musicbrainz/tagfetcher.cpp src/musicbrainz/web/acoustidlookuptask.cpp src/musicbrainz/web/musicbrainzrecordingstask.cpp diff --git a/build/depends.py b/build/depends.py index 087aade6e57..d8376c45b46 100644 --- a/build/depends.py +++ b/build/depends.py @@ -996,15 +996,12 @@ def sources(self, build): "src/widget/wsingletoncontainer.cpp", "src/widget/wmainmenubar.cpp", - "src/musicbrainz/network.cpp", "src/musicbrainz/tagfetcher.cpp", "src/musicbrainz/gzip.cpp", "src/musicbrainz/crc.c", - "src/musicbrainz/acoustidclient.cpp", "src/musicbrainz/chromaprinter.cpp", "src/musicbrainz/musicbrainz.cpp", "src/musicbrainz/musicbrainzxml.cpp", - "src/musicbrainz/musicbrainzclient.cpp", "src/musicbrainz/web/acoustidlookuptask.cpp", "src/musicbrainz/web/musicbrainzrecordingstask.cpp", diff --git a/src/library/dlgtagfetcher.cpp b/src/library/dlgtagfetcher.cpp index 59c844d21be..c1c0f7420c2 100644 --- a/src/library/dlgtagfetcher.cpp +++ b/src/library/dlgtagfetcher.cpp @@ -2,6 +2,36 @@ #include #include "library/dlgtagfetcher.h" +#include "track/tracknumbers.h" + +namespace { + +QStringList track2row(const Track& track) { + const auto trackNumbers = TrackNumbers::joinStrings( + track.getTrackNumber(), + track.getTrackTotal()); + QStringList row; + row.reserve(6); + row + << track.getYear() + << track.getAlbum() + << track.getAlbumArtist() + << trackNumbers + << track.getTitle() + << track.getArtist(); + return row; +} + +void addTrack( + const QStringList& trackRow, + int resultIndex, + QTreeWidget* parent) { + QTreeWidgetItem* item = new QTreeWidgetItem(parent, trackRow); + item->setData(0, Qt::UserRole, resultIndex); + item->setData(0, Qt::TextAlignmentRole, Qt::AlignRight); +} + +} // anonymous namespace DlgTagFetcher::DlgTagFetcher(QWidget *parent) : QDialog(parent), @@ -32,11 +62,12 @@ void DlgTagFetcher::init() { this, &DlgTagFetcher::slotNetworkResult); // Resize columns, this can't be set in the ui file - results->setColumnWidth(0, 50); // Track column - results->setColumnWidth(1, 50); // Year column - results->setColumnWidth(2, 160); // Title column - results->setColumnWidth(3, 160); // Artist column - results->setColumnWidth(4, 160); // Album column + results->setColumnWidth(0, 50); // Year column + results->setColumnWidth(1, 160); // Album column + results->setColumnWidth(2, 160); // Album artist column + results->setColumnWidth(3, 50); // Track (numbers) column + results->setColumnWidth(4, 160); // Title column + results->setColumnWidth(5, 160); // Artist column } void DlgTagFetcher::loadTrack(const TrackPointer& track) { @@ -71,23 +102,71 @@ void DlgTagFetcher::slotTrackChanged(TrackId trackId) { void DlgTagFetcher::apply() { int resultIndex = m_data.m_selectedResult; if (resultIndex > -1) { - if (!m_data.m_results[resultIndex]->getAlbum().isEmpty()) { - m_track->setAlbum(m_data.m_results[resultIndex]->getAlbum()); + mixxx::TrackMetadata importedMetadata; + m_data.m_results[resultIndex]->readTrackMetadata(&importedMetadata); + mixxx::TrackMetadata updatedMetadata; + m_track->readTrackMetadata(&updatedMetadata); + if (!importedMetadata.getTrackInfo().getArtist().isEmpty()) { + updatedMetadata.refTrackInfo().setArtist( + importedMetadata.getTrackInfo().getArtist()); + } + if (!importedMetadata.getTrackInfo().getTitle().isEmpty()) { + updatedMetadata.refTrackInfo().setTitle( + importedMetadata.getTrackInfo().getTitle()); + } + if (!importedMetadata.getTrackInfo().getTrackNumber().isEmpty()) { + updatedMetadata.refTrackInfo().setTrackNumber( + importedMetadata.getTrackInfo().getTrackNumber() + ); + } + if (!importedMetadata.getTrackInfo().getTrackTotal().isEmpty()) { + updatedMetadata.refTrackInfo().setTrackTotal( + importedMetadata.getTrackInfo().getTrackTotal() + ); + } + if (!importedMetadata.getTrackInfo().getYear().isEmpty()) { + updatedMetadata.refTrackInfo().setYear( + importedMetadata.getTrackInfo().getYear()); } - if (!m_data.m_results[resultIndex]->getArtist().isEmpty()) { - m_track->setArtist(m_data.m_results[resultIndex]->getArtist()); + if (!importedMetadata.getAlbumInfo().getArtist().isEmpty()) { + updatedMetadata.refAlbumInfo().setArtist( + importedMetadata.getAlbumInfo().getArtist()); } - if (!m_data.m_results[resultIndex]->getTitle().isEmpty()) { - m_track->setTitle(m_data.m_results[resultIndex]->getTitle()); + if (!importedMetadata.getAlbumInfo().getTitle().isEmpty()) { + updatedMetadata.refAlbumInfo().setTitle( + importedMetadata.getAlbumInfo().getTitle()); } - if (!m_data.m_results[resultIndex]->getYear().isEmpty() && - m_data.m_results[resultIndex]->getYear() != "0") { - m_track->setYear(m_data.m_results[resultIndex]->getYear()); +#if defined(__EXTRA_METADATA__) + if (!importedMetadata.getTrackInfo().getMusicBrainzArtistId().isNull()) { + updatedMetadata.refTrackInfo().setMusicBrainzArtistId( + importedMetadata.getTrackInfo().getMusicBrainzArtistId() + ); } - if (!m_data.m_results[resultIndex]->getTrackNumber().isEmpty() && - m_data.m_results[resultIndex]->getTrackNumber() != "0") { - m_track->setTrackNumber(m_data.m_results[resultIndex]->getTrackNumber()); + if (!importedMetadata.getTrackInfo().getMusicBrainzRecordingId().isNull()) { + updatedMetadata.refTrackInfo().setMusicBrainzRecordingId( + importedMetadata.getTrackInfo().getMusicBrainzRecordingId() + ); } + if (!importedMetadata.getTrackInfo().getMusicBrainzReleaseId().isNull()) { + updatedMetadata.refTrackInfo().setMusicBrainzReleaseId( + importedMetadata.getTrackInfo().getMusicBrainzReleaseId() + ); + } + if (!importedMetadata.getAlbumInfo().getMusicBrainzArtistId().isNull()) { + updatedMetadata.refAlbumInfo().setMusicBrainzArtistId( + importedMetadata.getAlbumInfo().getMusicBrainzArtistId() + ); + } + if (!importedMetadata.getAlbumInfo().getMusicBrainzReleaseId().isNull()) { + updatedMetadata.refAlbumInfo().setMusicBrainzReleaseId( + importedMetadata.getAlbumInfo().getMusicBrainzReleaseId()); + } + if (!importedMetadata.getAlbumInfo().getMusicBrainzReleaseGroupId().isNull()) { + updatedMetadata.refAlbumInfo().setMusicBrainzReleaseGroupId( + importedMetadata.getAlbumInfo().getMusicBrainzReleaseGroupId()); + } +#endif // __EXTRA_METADATA__ + m_track->importMetadata(std::move(updatedMetadata)); } } @@ -147,13 +226,21 @@ void DlgTagFetcher::updateStack() { results->clear(); addDivider(tr("Original tags"), results); - addTrack(m_track, -1, results); + addTrack(track2row(*m_track), -1, results); addDivider(tr("Suggested tags"), results); - - int trackIndex = 0; - foreach (const TrackPointer track, m_data.m_results) { - addTrack(track, trackIndex++, results); + { + int trackIndex = 0; + QSet trackRows; + foreach (const TrackPointer track, m_data.m_results) { + const auto trackRow = track2row(*track); + // Ignore duplicate results + if (!trackRows.contains(trackRow)) { + trackRows.insert(trackRow); + addTrack(trackRow, trackIndex, results); + } + ++trackIndex; + } } // Find the item that was selected last time @@ -167,17 +254,6 @@ void DlgTagFetcher::updateStack() { } } -void DlgTagFetcher::addTrack(const TrackPointer track, int resultIndex, - QTreeWidget* parent) const { - QStringList values; - values << track->getTrackNumber() << track->getYear() << track->getTitle() - << track->getArtist() << track->getAlbum(); - - QTreeWidgetItem* item = new QTreeWidgetItem(parent, values); - item->setData(0, Qt::UserRole, resultIndex); - item->setData(0, Qt::TextAlignmentRole, Qt::AlignRight); -} - void DlgTagFetcher::addDivider(const QString& text, QTreeWidget* parent) const { QTreeWidgetItem* item = new QTreeWidgetItem(parent); item->setFirstColumnSpanned(true); diff --git a/src/library/dlgtagfetcher.h b/src/library/dlgtagfetcher.h index 1f70e717ca9..58beb7f5d3d 100644 --- a/src/library/dlgtagfetcher.h +++ b/src/library/dlgtagfetcher.h @@ -36,8 +36,6 @@ class DlgTagFetcher : public QDialog, public Ui::DlgTagFetcher { private: void updateStack(); void addDivider(const QString& text, QTreeWidget* parent) const; - void addTrack(const TrackPointer track, int resultIndex, - QTreeWidget* parent) const; TagFetcher m_tagFetcher; diff --git a/src/library/dlgtagfetcher.ui b/src/library/dlgtagfetcher.ui index 99d1efcee9f..471061db12a 100644 --- a/src/library/dlgtagfetcher.ui +++ b/src/library/dlgtagfetcher.ui @@ -47,27 +47,32 @@ - Track + Year - Year + Album - Title + Album Artist - Artist + Track - Album + Title + + + + + Artist diff --git a/src/musicbrainz/acoustidclient.cpp b/src/musicbrainz/acoustidclient.cpp deleted file mode 100644 index 497e108f66b..00000000000 --- a/src/musicbrainz/acoustidclient.cpp +++ /dev/null @@ -1,205 +0,0 @@ -/***************************************************************************** - * Copyright © 2012 John Maguire * - * David Sansome * - * This work is free. You can redistribute it and/or modify it under the * - * terms of the Do What The Fuck You Want To Public License, Version 2, * - * as published by Sam Hocevar. * - * See http://www.wtfpl.net/ for more details. * - *****************************************************************************/ - -#include -#include -#include -#include -#include - -#include "musicbrainz/acoustidclient.h" -#include "musicbrainz/gzip.h" -#include "musicbrainz/network.h" - -#include "util/logger.h" - -namespace { - -mixxx::Logger kLogger("AcoustidClient"); - -// see API-KEY site here http://acoustid.org/application/496 -// I registered the KEY for version 1.12 -- kain88 (may 2013) -// See also: https://acoustid.org/webservice -const QString CLIENT_APIKEY = "czKxnkyO"; -const QString ACOUSTID_URL = "https://api.acoustid.org/v2/lookup"; -constexpr int kDefaultTimeout = 5000; // msec - -} // anonymous namespace - -AcoustidClient::AcoustidClient(QObject* parent) - : QObject(parent), - m_network(this), - m_timeouts(kDefaultTimeout, this) { -} - -void AcoustidClient::setTimeout(int msec) { - VERIFY_OR_DEBUG_ASSERT(msec > 0) { - kLogger.warning() << "Invalid timeout" << msec << "[ms]"; - return; - } - m_timeouts.setTimeout(msec); -} - -void AcoustidClient::start(int id, const QString& fingerprint, int duration) { - QUrlQuery urlQuery; - // The default format is JSON - //urlQuery.addQueryItem("format", "json"); - urlQuery.addQueryItem("client", CLIENT_APIKEY); - urlQuery.addQueryItem("duration", QString::number(duration)); - urlQuery.addQueryItem("meta", "recordingids"); - urlQuery.addQueryItem("fingerprint", fingerprint); - // application/x-www-form-urlencoded request bodies must be percent encoded. - QByteArray body = urlQuery.query(QUrl::FullyEncoded).toLatin1(); - - QUrl url(ACOUSTID_URL); - QNetworkRequest req(url); - req.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded"); - req.setRawHeader("Content-Encoding", "gzip"); - - kLogger.debug() - << "POST request:" << ACOUSTID_URL - << "body:" << body; - - QNetworkReply* reply = m_network.post(req, gzipCompress(body)); - m_pendingReplies[reply] = id; - connect(reply, &QNetworkReply::finished, this, &AcoustidClient::onReplyFinished); - m_timeouts.addReply(reply); -} - -void AcoustidClient::cancel(int id) { - QNetworkReply* reply = m_pendingReplies.key(id); - if (!reply) { - return; - } - cancelPendingReply(reply); -} - -void AcoustidClient::cancelPendingReply(QNetworkReply* reply) { - DEBUG_ASSERT(reply); - m_timeouts.removeReply(reply); - m_pendingReplies.remove(reply); - if (reply->isRunning()) { - reply->abort(); - } -} - -void AcoustidClient::cancelAll() { - while (!m_pendingReplies.isEmpty()) { - QNetworkReply* reply = m_pendingReplies.firstKey(); - cancelPendingReply(reply); - DEBUG_ASSERT(!m_pendingReplies.contains(reply)); - } -} - -void AcoustidClient::onReplyFinished() { - QNetworkReply* reply = qobject_cast(sender()); - VERIFY_OR_DEBUG_ASSERT(reply) { - return; - } - reply->deleteLater(); - - if (!m_pendingReplies.contains(reply)) { - // Already Cancelled - return; - } - - m_timeouts.removeReply(reply); - int id = m_pendingReplies.take(reply); - - const int statusCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); - const QByteArray body = reply->readAll(); - if (kLogger.debugEnabled()) { - kLogger.debug() - << "POST reply status:" << statusCode - << "body:" << body; - } - - const auto jsonResponse = QJsonDocument::fromJson(body); - QString statusText; - if (jsonResponse.isObject()) { - statusText = jsonResponse.object().value("status").toString(); - } - - if (statusCode != 200 || statusText != "ok") { - QString errorMessage; - int errorCode = 0; - if (jsonResponse.isObject() && statusText == "error") { - const auto error = jsonResponse.object().value("error").toObject(); - errorMessage = error.value("message").toString(); - errorCode = error.value("message").toInt(errorCode); - } - emit networkError( - statusCode, - "AcoustID", - errorMessage, - errorCode); - return; - } - - QStringList recordingIds; - DEBUG_ASSERT(jsonResponse.isObject()); - DEBUG_ASSERT(jsonResponse.object().value(QStringLiteral("results")).isArray()); - const QJsonArray results = jsonResponse.object().value(QStringLiteral("results")).toArray(); - double maxScore = -1.0; // uninitialized (< 0) - // Results are expected to be ordered by score (descending) - for (const auto result : results) { - DEBUG_ASSERT(result.isObject()); - const auto resultObject = result.toObject(); - const auto resultId = - resultObject.value(QStringLiteral("id")).toString(); - DEBUG_ASSERT(!resultId.isEmpty()); - // The default score is 1.0 if missing - const double score = - resultObject.value(QStringLiteral("score")).toDouble(1.0); - DEBUG_ASSERT(score >= 0.0); - DEBUG_ASSERT(score <= 1.0); - if (maxScore < 0.0) { - // Initialize the maximum score - maxScore = score; - } - DEBUG_ASSERT(score <= maxScore); - if (score < maxScore && !recordingIds.isEmpty()) { - // Ignore all remaining results with lower values - // than the maximum score - break; - } - const auto recordings = result.toObject().value(QStringLiteral("recordings")); - if (recordings.isUndefined()) { - if (kLogger.debugEnabled()) { - kLogger.debug() - << "No recording(s) available for result" - << resultId - << "with score" - << score; - } - continue; - } else { - DEBUG_ASSERT(recordings.isArray()); - const QJsonArray recordingsArray = recordings.toArray(); - if (kLogger.debugEnabled()) { - kLogger.debug() - << "Found" - << recordingsArray.size() - << "recording(s) for result" - << resultId - << "with score" - << score; - } - for (const auto recording : recordingsArray) { - DEBUG_ASSERT(recording.isObject()); - const auto recordingObject = recording.toObject(); - const auto recordingId = - recordingObject.value(QStringLiteral("id")).toString(); - DEBUG_ASSERT(!recordingId.isEmpty()); - recordingIds.append(recordingId); - } - } - } - emit finished(id, recordingIds); -} diff --git a/src/musicbrainz/acoustidclient.h b/src/musicbrainz/acoustidclient.h deleted file mode 100644 index 67a0b764055..00000000000 --- a/src/musicbrainz/acoustidclient.h +++ /dev/null @@ -1,64 +0,0 @@ -/***************************************************************************** - * Copyright © 2012 John Maguire * - * David Sansome * - * This work is free. You can redistribute it and/or modify it under the * - * terms of the Do What The Fuck You Want To Public License, Version 2, * - * as published by Sam Hocevar. * - * See http://www.wtfpl.net/ for more details. * - *****************************************************************************/ - -#ifndef ACOUSTIDCLIENT_H -#define ACOUSTIDCLIENT_H - -#include -#include -#include - -#include "musicbrainz/network.h" -#include "track/track.h" - -class AcoustidClient : public QObject { - Q_OBJECT - - // Gets a MBID from a Chromaprint fingerprint. - // A fingerprint identifies one particular encoding of a song and is created - // by Fingerprinter. An MBID identifies the actual song and can be passed to - // Musicbrainz to get metadata. - // You can create one AcoustidClient 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 AcoustidClient. - - public: - AcoustidClient(QObject* parent = 0); - - // Network requests will be aborted after this interval. - void setTimeout(int msec); - - // Starts a request and returns immediately. Finished() will be emitted - // later with the same ID. - void start(int id, const QString& fingerprint, int duration); - - // 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(); - - signals: - void finished(int id, QStringList mbRecordingIds); - void networkError(int httpStatus, QString app, QString message, int code); - - private slots: - void onReplyFinished(); - - private: - void cancelPendingReply(QNetworkReply* reply); - - QNetworkAccessManager m_network; - NetworkTimeouts m_timeouts; - QMap m_pendingReplies; -}; - -#endif // ACOUSTIDCLIENT_H diff --git a/src/musicbrainz/musicbrainzclient.cpp b/src/musicbrainz/musicbrainzclient.cpp deleted file mode 100644 index 7679797f08e..00000000000 --- a/src/musicbrainz/musicbrainzclient.cpp +++ /dev/null @@ -1,303 +0,0 @@ -/***************************************************************************** - * Copyright © 2012 John Maguire * - * David Sansome * - * This work is free. You can redistribute it and/or modify it under the * - * terms of the Do What The Fuck You Want To Public License, Version 2, * - * as published by Sam Hocevar. * - * See http://www.wtfpl.net/ for more details. * - *****************************************************************************/ - -#include -#include -#include -#include -#include -#include -#include -#include - -#include "musicbrainz/musicbrainzclient.h" - -#include "util/logger.h" -#include "util/version.h" -#include "defs_urls.h" - - -namespace { - -mixxx::Logger kLogger("MusicBrainzClient"); - -const QString kRecordingUrl = "https://musicbrainz.org/ws/2/recording/"; -const QString kDateRegex = "^[12]\\d{3}"; -constexpr int kDefaultTimeout = 5000; // msec -constexpr int kDefaultErrorCode = 0; - -QString decodeText(const QByteArray& data, const QStringRef codecName) { - QTextStream textStream(data); - if (!codecName.isEmpty()) { - textStream.setCodec(QTextCodec::codecForName(codecName.toUtf8())); - } - return textStream.readAll(); -} - -} // anonymous namespace - - - - -MusicBrainzClient::MusicBrainzClient(QObject* parent) - : QObject(parent), - m_network(this), - m_timeouts(kDefaultTimeout, this) { -} - -void MusicBrainzClient::start(int id, QStringList recordingIds) { - nextRequest(id, Request(std::move(recordingIds))); -} - -void MusicBrainzClient::nextRequest(int id, Request request) { - DEBUG_ASSERT(!m_requests.contains(id)); - if (request.recordingIds.isEmpty()) { - emit finished(id, uniqueResults(request.results)); - return; - } - const auto recordingId = request.recordingIds.takeFirst(); - - typedef QPair Param; - QList parameters; - parameters << Param("inc", "artists+releases+media"); - - QUrl url(kRecordingUrl + recordingId); - QUrlQuery query; - query.setQueryItems(parameters); - url.setQuery(query); - kLogger.debug() - << "GET request:" - << url.toString(); - QNetworkRequest req(url); - // http://musicbrainz.org/doc/XML_Web_Service/Rate_Limiting#Provide_meaningful_User-Agent_strings - QString mixxxMusicBrainzId(Version::applicationName() + "/" + Version::version() + " ( " + MIXXX_WEBSITE_URL + " )"); - // HTTP request headers must be latin1. - req.setRawHeader("User-Agent", mixxxMusicBrainzId.toLatin1()); - QNetworkReply* reply = m_network.get(req); - m_requests[id] = std::move(request); - m_replies[reply] = id; - connect(reply, &QNetworkReply::finished, this, &MusicBrainzClient::replyFinished); - m_timeouts.addReply(reply); -} - -void MusicBrainzClient::cancel(int id) { - m_requests.remove(id); - QNetworkReply* reply = m_replies.key(id); - if (reply) { - reply->abort(); - } -} - -void MusicBrainzClient::cancelAll() { - m_requests.clear(); - auto replies = m_replies; - m_replies.clear(); - for (auto it = replies.constBegin(); - it != replies.constEnd(); ++it) { - it.key()->abort(); - } -} - -void MusicBrainzClient::replyFinished() { - QNetworkReply* reply = qobject_cast(sender()); - if (!reply) { - return; - } - reply->deleteLater(); - - if (!m_replies.contains(reply)) { - return; - } - int id = m_replies.take(reply); - - if (!m_requests.contains(id)) { - return; - } - Request request = m_requests.take(id); - - int status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); - QTextStream textReader(reply); - const QByteArray body(reply->readAll()); - QXmlStreamReader reader(body); - - // HTTP status of successful results: - // 200: Found - // 301: Found, but UUID moved permanently in database - // 404: Not found in database, i.e. empty result - if (status != 200 && status != 301 && status != 404) { - qDebug() - << "MusicBrainzClient GET reply" - << "status:" << status - << "body:" << body; - QString errorMessage; - int errorCode = kDefaultErrorCode; - while (!reader.atEnd()) { - if (reader.readNext() == QXmlStreamReader::StartElement) { - const QStringRef name = reader.name(); - if (name == QStringLiteral("message")) { - errorMessage = reader.readElementText(); - } else if (name == QStringLiteral("code")) { - bool ok; - int code = reader.readElementText().toInt(&ok); - if (ok) { - errorCode = code; - } - } - } - } - emit networkError( - reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(), - "MusicBrainz", errorMessage, errorCode); - return; - } - - QStringRef codecName; - while (!reader.atEnd()) { - switch (reader.readNext()) { - case QXmlStreamReader::Invalid: - { - qWarning() << "MusicBrainzClient GET reply body:" - << decodeText(body, codecName); - qWarning() - << "MusicBrainzClient GET decoding error:" - << reader.errorString(); - break; - } - case QXmlStreamReader::StartDocument: - { - // The character encoding is always an ASCII string - codecName = reader.documentEncoding(); - qDebug() << "MusicBrainzClient GET reply codec:" - << codecName; - qDebug() << "MusicBrainzClient GET reply body:" - << decodeText(body, codecName); - break; - } - case QXmlStreamReader::StartElement: - { - if (reader.name() == "recording") { - ResultList tracks = parseTrack(reader); - for (const Result& track: tracks) { - if (!track.m_title.isEmpty()) { - request.results << track; - } - } - } - break; - } - default: - { - // ignore any other token type - } - } - } - nextRequest(id, request); -} - -MusicBrainzClient::ResultList MusicBrainzClient::parseTrack(QXmlStreamReader& reader) { - Result result; - QList releases; - - while (!reader.atEnd()) { - QXmlStreamReader::TokenType type = reader.readNext(); - - if (type == QXmlStreamReader::StartElement) { - QStringRef name = reader.name(); - if (name == "title") { - result.m_title = reader.readElementText(); - } else if (name == "length") { - // convert msec to sec - result.m_duration = reader.readElementText().toInt()*10000000; - } else if (name == "artist") { - parseArtist(reader, result.m_artist); - } else if (name == "release") { - releases << parseRelease(reader); - } - } - - if (type == QXmlStreamReader::EndElement && reader.name() == "recording") { - break; - } - } - - ResultList ret; - foreach (const Release& release, releases) { - ret << release.CopyAndMergeInto(result); - } - return ret; -} - -void MusicBrainzClient::parseArtist(QXmlStreamReader& reader, QString& artist) { - while (!reader.atEnd()) { - QXmlStreamReader::TokenType type = reader.readNext(); - - if (type == QXmlStreamReader::StartElement && reader.name() == "name") { - artist = reader.readElementText(); - } - - if (type == QXmlStreamReader::EndElement && reader.name() == "artist") { - return; - } - } -} - -MusicBrainzClient::Release MusicBrainzClient::parseRelease(QXmlStreamReader& reader) { - Release ret; - - while (!reader.atEnd()) { - QXmlStreamReader::TokenType type = reader.readNext(); - - if (type == QXmlStreamReader::StartElement) { - QStringRef name = reader.name(); - if (name == "title") { - ret.m_album = reader.readElementText(); - } else if (name == "date") { - QRegExp regex(kDateRegex); - if (regex.indexIn(reader.readElementText()) == 0) { - ret.m_year = regex.cap(0).toInt(); - } - } else if (name == "track-list") { - ret.m_track = reader.attributes().value("offset").toString().toInt() + 1; - consumeCurrentElement(reader); - } - } - - if (type == QXmlStreamReader::EndElement && reader.name() == "release") { - break; - } - } - - return ret; -} - -MusicBrainzClient::ResultList MusicBrainzClient::uniqueResults(const ResultList& results) { - // TODO: QSet::fromList(const QList&) is deprecated and should be - // replaced with QSet(list.begin(), list.end()). - // However, the proposed alternative has just been introduced in Qt - // 5.14. Until the minimum required Qt version of Mixx is increased, - // we need a version check here -#if QT_VERSION >= QT_VERSION_CHECK(5, 14, 0) - ResultList ret = QSet(results.begin(), results.end()).values(); -#else - ResultList ret = QSet::fromList(results).toList(); -#endif - std::sort(ret.begin(), ret.end()); - return ret; -} - -void MusicBrainzClient::consumeCurrentElement(QXmlStreamReader& reader) { - int level = 1; - while (level != 0 && !reader.atEnd()) { - switch (reader.readNext()) { - case QXmlStreamReader::StartElement: ++level; break; - case QXmlStreamReader::EndElement: --level; break; - default: break; - } - } -} diff --git a/src/musicbrainz/musicbrainzclient.h b/src/musicbrainz/musicbrainzclient.h deleted file mode 100644 index e8014c39765..00000000000 --- a/src/musicbrainz/musicbrainzclient.h +++ /dev/null @@ -1,143 +0,0 @@ -/***************************************************************************** - * Copyright © 2012 John Maguire * - * David Sansome * - * This work is free. You can redistribute it and/or modify it under the * - * terms of the Do What The Fuck You Want To Public License, Version 2, * - * as published by Sam Hocevar. * - * See http://www.wtfpl.net/ for more details. * - *****************************************************************************/ - -#ifndef MUSICBRAINZCLIENT_H -#define MUSICBRAINZCLIENT_H - -#include -#include -#include -#include -#include - -#include "musicbrainz/network.h" - -class MusicBrainzClient : public QObject { - Q_OBJECT - - // Gets metadata for a particular MBID. - // An MBID is created from a fingerprint using chromaprint library. - // 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: - MusicBrainzClient(QObject* parent = 0); - - struct Result { - Result() : m_duration(0), m_track(0), m_year(-1) {} - - bool operator <(const Result& other) const { - #define cmp(field) \ - if (field < other.field) return true; \ - if (field > other.field) return false; - - cmp(m_track); - cmp(m_year); - cmp(m_title); - cmp(m_artist); - return false; - - #undef cmp - } - - bool operator ==(const Result& other) const { - return m_title == other.m_title && - m_artist == other.m_artist && - m_album == other.m_album && - m_duration == other.m_duration && - m_track == other.m_track && - m_year == other.m_year; - } - - QString m_title; - QString m_artist; - QString m_album; - int m_duration; - int m_track; - int m_year; - }; - typedef QList ResultList; - - - // Starts a request and returns immediately. finished() will be emitted - // later with the same ID. - void start(int id, QStringList recordingIds); - static void consumeCurrentElement(QXmlStreamReader& reader); - - // 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(); - - signals: - // Finished signal emitted when fechting songs tags - void finished(int id, const MusicBrainzClient::ResultList& result); - void networkError(int httpStatus, QString app, QString message, int code); - - private slots: - void replyFinished(); - - private: - struct Release { - Release() : m_track(0), m_year(0) {} - - Result CopyAndMergeInto(const Result& orig) const { - Result ret(orig); - ret.m_album = m_album; - ret.m_track = m_track; - ret.m_year = m_year; - return ret; - } - - QString m_album; - int m_track; - int m_year; - }; - - static ResultList parseTrack(QXmlStreamReader& reader); - static void parseArtist(QXmlStreamReader& reader, QString& artist); - static Release parseRelease(QXmlStreamReader& reader); - static ResultList uniqueResults(const ResultList& results); - - private: - struct Request final { - Request() = default; - Request(const Request&) = default; - Request(Request&&) = default; - Request& operator=(const Request&) = default; - Request& operator=(Request&&) = default; - explicit Request(QStringList recordingIds) - : recordingIds(recordingIds) { - } - QStringList recordingIds; - ResultList results; - }; - - void nextRequest(int id, Request request); - - QNetworkAccessManager m_network; - NetworkTimeouts m_timeouts; - QMap m_requests; - QMap m_replies; -}; - -inline uint qHash(const MusicBrainzClient::Result& result) { - return qHash(result.m_album) ^ - qHash(result.m_artist) ^ - result.m_duration ^ - qHash(result.m_title) ^ - result.m_track ^ - result.m_year; -} - -#endif // MUSICBRAINZCLIENT_H diff --git a/src/musicbrainz/network.cpp b/src/musicbrainz/network.cpp deleted file mode 100644 index 34f19ccec0a..00000000000 --- a/src/musicbrainz/network.cpp +++ /dev/null @@ -1,84 +0,0 @@ -/***************************************************************************** - * Copyright © 2012 John Maguire * - * David Sansome * - * This work is free. You can redistribute it and/or modify it under the * - * terms of the Do What The Fuck You Want To Public License, Version 2, * - * as published by Sam Hocevar. * - * See http://www.wtfpl.net/ for more details. * - *****************************************************************************/ - -#include -#include -#include -#include -#include - -#include "musicbrainz/network.h" - -NetworkAccessManager::NetworkAccessManager(QObject* parent) - : QNetworkAccessManager(parent) { -} - -QNetworkReply* NetworkAccessManager::createRequest(Operation op, - const QNetworkRequest& request, - QIODevice* outgoingData) { - QNetworkRequest new_request(request); - new_request.setRawHeader("User-Agent", QString("%1 %2").arg( - QCoreApplication::applicationName(), - QCoreApplication::applicationVersion()).toUtf8()); - - if (op == QNetworkAccessManager::PostOperation && - !new_request.header(QNetworkRequest::ContentTypeHeader).isValid()) { - new_request.setHeader(QNetworkRequest::ContentTypeHeader, - "application/x-www-form-urlencoded"); - } - - // Prefer the cache unless the caller has changed the setting already - if (request.attribute(QNetworkRequest::CacheLoadControlAttribute).toInt() - == QNetworkRequest::PreferNetwork) { - new_request.setAttribute(QNetworkRequest::CacheLoadControlAttribute, - QNetworkRequest::PreferCache); - } - - return QNetworkAccessManager::createRequest(op, new_request, outgoingData); -} - - -NetworkTimeouts::NetworkTimeouts(int timeout_msec, QObject* parent) - : QObject(parent), - m_timeout_msec(timeout_msec) { -} - -void NetworkTimeouts::addReply(QNetworkReply* reply) { - if (!reply->isRunning() || m_timers.contains(reply)) { - return; - } - - connect(reply, &QNetworkReply::destroyed, this, &NetworkTimeouts::replyFinishedOrDestroyed); - connect(reply, &QNetworkReply::finished, this, &NetworkTimeouts::replyFinishedOrDestroyed); - m_timers[reply] = startTimer(m_timeout_msec); -} - -void NetworkTimeouts::removeReply(QNetworkReply* reply) { - if (reply && m_timers.contains(reply)) { - killTimer(m_timers.take(reply)); - } -} - -void NetworkTimeouts::replyFinishedOrDestroyed() { - QNetworkReply* reply = qobject_cast(sender()); - removeReply(reply); -} - -void NetworkTimeouts::timerEvent(QTimerEvent* e) { - const int timerId = e->timerId(); - killTimer(timerId); // oneshot - QNetworkReply* reply = m_timers.key(timerId); - if (!reply) { - return; - } - m_timers.remove(reply); - if (reply->isRunning()) { - reply->abort(); - } -} diff --git a/src/musicbrainz/network.h b/src/musicbrainz/network.h deleted file mode 100644 index 7d9f9a2bce5..00000000000 --- a/src/musicbrainz/network.h +++ /dev/null @@ -1,50 +0,0 @@ -/***************************************************************************** - * Copyright © 2012 John Maguire * - * David Sansome * - * This work is free. You can redistribute it and/or modify it under the * - * terms of the Do What The Fuck You Want To Public License, Version 2, * - * as published by Sam Hocevar. * - * See http://www.wtfpl.net/ for more details. * - *****************************************************************************/ - -#ifndef NETWORK_H -#define NETWORK_H - -#include -#include -#include - -class NetworkAccessManager : public QNetworkAccessManager { - Q_OBJECT - - public: - NetworkAccessManager(QObject* parent = 0); - - protected: - QNetworkReply* createRequest(Operation op, const QNetworkRequest& request, - QIODevice* outgoingData); -}; - - -class NetworkTimeouts : public QObject { - Q_OBJECT - - public: - NetworkTimeouts(int timeout_msec, QObject* parent = 0); - - void addReply(QNetworkReply* reply); - void removeReply(QNetworkReply* reply); - void setTimeout(int msec) { m_timeout_msec = msec; } - - protected: - void timerEvent(QTimerEvent* e); - - private slots: - void replyFinishedOrDestroyed(); - - private: - int m_timeout_msec; - QMap m_timers; -}; - -#endif // NETWORK_H diff --git a/src/musicbrainz/tagfetcher.cpp b/src/musicbrainz/tagfetcher.cpp index 4b31c19ec4c..987caf53c5f 100644 --- a/src/musicbrainz/tagfetcher.cpp +++ b/src/musicbrainz/tagfetcher.cpp @@ -1,125 +1,225 @@ -/***************************************************************************** - * Copyright © 2012 John Maguire * - * David Sansome * - * This work is free. You can redistribute it and/or modify it under the * - * terms of the Do What The Fuck You Want To Public License, Version 2, * - * as published by Sam Hocevar. * - * See http://www.wtfpl.net/ for more details. * - *****************************************************************************/ +#include "musicbrainz/tagfetcher.h" #include -#include -#include -#include +#include -#include "musicbrainz/tagfetcher.h" #include "musicbrainz/chromaprinter.h" -#include "musicbrainz/musicbrainzclient.h" -TagFetcher::TagFetcher(QObject* parent) - : QObject(parent), - m_pFingerprintWatcher(NULL), - m_AcoustidClient(this), - m_MusicbrainzClient(this) { - connect(&m_AcoustidClient, &AcoustidClient::finished, - this, &TagFetcher::mbRecordingIdsFound); - connect(&m_MusicbrainzClient, &MusicBrainzClient::finished, - this, &TagFetcher::tagsFetched); - connect(&m_AcoustidClient, &AcoustidClient::networkError, - this, &TagFetcher::networkError); - connect(&m_MusicbrainzClient, &MusicBrainzClient::networkError, - this, &TagFetcher::networkError); -} +namespace { -QString TagFetcher::getFingerprint(const TrackPointer tio) { - return ChromaPrinter(NULL).getFingerprint(tio); +constexpr int kAcoustIdTimeoutMillis = 5000; // msec + +constexpr int kMusicBrainzTimeoutMillis = 5000; // msec + +} // anonymous namespace + +TagFetcher::TagFetcher(QObject* parent) + : QObject(parent), + m_fingerprintWatcher(this) { } -void TagFetcher::startFetch(const TrackPointer track) { +void TagFetcher::startFetch( + TrackPointer pTrack) { cancel(); - // qDebug() << "start to fetch track metadata"; - QList tracks; - tracks.append(track); - m_tracks = tracks; - QFuture future = QtConcurrent::mapped(m_tracks, getFingerprint); - m_pFingerprintWatcher = new QFutureWatcher(this); - m_pFingerprintWatcher->setFuture(future); - connect(m_pFingerprintWatcher, &QFutureWatcher::resultReadyAt, - this, &TagFetcher::fingerprintFound); + m_pTrack = pTrack; emit fetchProgress(tr("Fingerprinting track")); + const auto fingerprintTask = QtConcurrent::run([this, pTrack] { + return ChromaPrinter(this).getFingerprint(pTrack); + }); + m_fingerprintWatcher.setFuture(fingerprintTask); + connect( + &m_fingerprintWatcher, + &QFutureWatcher::finished, + this, + &TagFetcher::slotFingerprintReady); } -void TagFetcher::cancel() { - // qDebug()<< "Cancel tagfetching"; - if (m_pFingerprintWatcher) { - m_pFingerprintWatcher->cancel(); +void TagFetcher::abortAcoustIdTask() { + if (m_pAcoustIdTask) { + disconnect(m_pAcoustIdTask.get()); + m_pAcoustIdTask->deleteBeforeFinished(); + m_pAcoustIdTask = nullptr; + } +} - delete m_pFingerprintWatcher; - m_pFingerprintWatcher = NULL; +void TagFetcher::abortMusicBrainzTask() { + if (m_pMusicBrainzTask) { + disconnect(m_pMusicBrainzTask.get()); + m_pMusicBrainzTask->deleteBeforeFinished(); + m_pMusicBrainzTask = nullptr; } +} - m_AcoustidClient.cancelAll(); - m_MusicbrainzClient.cancelAll(); - m_tracks.clear(); +void TagFetcher::cancel() { + // qDebug()<< "Cancel tagfetching"; + m_fingerprintWatcher.cancel(); + abortAcoustIdTask(); + abortMusicBrainzTask(); + m_pTrack.reset(); } -void TagFetcher::fingerprintFound(int index) { - QFutureWatcher* watcher = static_cast*>(sender()); - if (!watcher || index >= m_tracks.count()) { +void TagFetcher::slotFingerprintReady() { + if (!m_pTrack || + !m_fingerprintWatcher.isFinished()) { return; } - const QString fingerprint = watcher->resultAt(index); - const TrackPointer ptrack = m_tracks[index]; - + const QString fingerprint = m_fingerprintWatcher.result(); if (fingerprint.isEmpty()) { - emit resultAvailable(ptrack, QList()); + emit resultAvailable(m_pTrack, QList()); return; } - emit fetchProgress(tr("Identifying track")); - // qDebug() << "start to look up the MBID"; - m_AcoustidClient.start(index, fingerprint, ptrack->getDurationInt()); + abortAcoustIdTask(); + + emit fetchProgress(tr("Identifying track through Acoustid")); + DEBUG_ASSERT(!m_pAcoustIdTask); + m_pAcoustIdTask = make_parented( + &m_network, + fingerprint, + m_pTrack->getDurationInt(), + this); + connect(m_pAcoustIdTask.get(), + &mixxx::AcoustIdLookupTask::succeeded, + this, + &TagFetcher::slotAcoustIdTaskSucceeded); + connect(m_pAcoustIdTask.get(), + &mixxx::AcoustIdLookupTask::failed, + this, + &TagFetcher::slotAcoustIdTaskFailed); + connect(m_pAcoustIdTask.get(), + &mixxx::AcoustIdLookupTask::networkError, + this, + &TagFetcher::slotAcoustIdTaskNetworkError); + m_pAcoustIdTask->invokeStart( + kAcoustIdTimeoutMillis); } -void TagFetcher::mbRecordingIdsFound(int index, QStringList mbRecordingIds) { - if (index >= m_tracks.count()) { +void TagFetcher::slotAcoustIdTaskSucceeded( + QList recordingIds) { + abortAcoustIdTask(); + if (!m_pTrack) { return; } - const TrackPointer pTrack = m_tracks[index]; - - if (mbRecordingIds.isEmpty()) { - emit resultAvailable(pTrack, QList()); + if (recordingIds.isEmpty()) { + emit resultAvailable(m_pTrack, QList()); return; } - emit fetchProgress(tr("Downloading Metadata")); - //qDebug() << "start to fetch tags from MB"; - m_MusicbrainzClient.start(index, mbRecordingIds); + emit fetchProgress(tr("Retrieving metadata from MusicBrainz")); + DEBUG_ASSERT(!m_pMusicBrainzTask); + m_pMusicBrainzTask = make_parented( + &m_network, + std::move(recordingIds), + this); + connect(m_pMusicBrainzTask.get(), + &mixxx::MusicBrainzRecordingsTask::succeeded, + this, + &TagFetcher::slotMusicBrainzTaskSucceeded); + connect(m_pMusicBrainzTask.get(), + &mixxx::MusicBrainzRecordingsTask::failed, + this, + &TagFetcher::slotMusicBrainzTaskFailed); + connect(m_pMusicBrainzTask.get(), + &mixxx::MusicBrainzRecordingsTask::networkError, + this, + &TagFetcher::slotMusicBrainzTaskNetworkError); + m_pMusicBrainzTask->invokeStart( + kMusicBrainzTimeoutMillis); +} + +void TagFetcher::slotAcoustIdTaskFailed( + mixxx::network::JsonWebResponse response) { + cancel(); + emit networkError( + response.statusCode, + "AcoustID", + response.content.toJson(), + -1); +} + +void TagFetcher::slotAcoustIdTaskNetworkError( + QUrl requestUrl, + QNetworkReply::NetworkError errorCode, + QString errorString, + QByteArray errorContent) { + Q_UNUSED(requestUrl); + Q_UNUSED(errorContent); + cancel(); + emit networkError( + mixxx::network::kHttpStatusCodeInvalid, + "AcoustID", + errorString, + errorCode); } -void TagFetcher::tagsFetched(int index, const MusicBrainzClient::ResultList& results) { - if (index >= m_tracks.count()) { +void TagFetcher::slotMusicBrainzTaskNetworkError( + QUrl requestUrl, + QNetworkReply::NetworkError errorCode, + QString errorString, + QByteArray errorContent) { + Q_UNUSED(requestUrl); + Q_UNUSED(errorContent); + cancel(); + emit networkError( + mixxx::network::kHttpStatusCodeInvalid, + "MusicBrainz", + errorString, + errorCode); +} + +void TagFetcher::slotMusicBrainzTaskFailed( + mixxx::network::WebResponse response, + int errorCode, + QString errorMessage) { + cancel(); + emit networkError( + response.statusCode, + "MusicBrainz", + errorMessage, + errorCode); +} + +void TagFetcher::slotMusicBrainzTaskSucceeded( + QList trackReleases) { + auto pOriginalTrack = std::move(m_pTrack); + abortMusicBrainzTask(); + if (!pOriginalTrack) { return; } - // qDebug() << "Tagfetcher got musicbrainz results and now parses them"; - const TrackPointer originalTrack = m_tracks[index]; + QList tracksGuessed; - foreach (const MusicBrainzClient::Result& result, results) { - TrackPointer track( + for (const auto& trackRelease : qAsConst(trackReleases)) { + mixxx::TrackMetadata trackMetadata; + trackMetadata.refTrackInfo().setTitle(trackRelease.title); + trackMetadata.refTrackInfo().setArtist(trackRelease.artist); + trackMetadata.refTrackInfo().setTrackNumber(trackRelease.trackNumber); + trackMetadata.refTrackInfo().setTrackTotal(trackRelease.trackTotal); + trackMetadata.refTrackInfo().setYear(trackRelease.date); + trackMetadata.refAlbumInfo().setTitle(trackRelease.albumTitle); + trackMetadata.refAlbumInfo().setArtist(trackRelease.albumArtist); +#if defined(__EXTRA_METADATA__) + trackMetadata.refTrackInfo().setDiscNumber(trackRelease.discNumber); + trackMetadata.refTrackInfo().setDiscTotal(trackRelease.discTotal); + trackMetadata.refTrackInfo().setMusicBrainzArtistId(trackRelease.artistId); + trackMetadata.refTrackInfo().setMusicBrainzRecordingId(trackRelease.recordingId); + trackMetadata.refTrackInfo().setMusicBrainzReleaseId(trackRelease.trackReleaseId); + trackMetadata.refAlbumInfo().setMusicBrainzArtistId(trackRelease.albumArtistId); + trackMetadata.refAlbumInfo().setMusicBrainzReleaseId(trackRelease.albumReleaseId); + trackMetadata.refAlbumInfo().setMusicBrainzReleaseGroupId(trackRelease.releaseGroupId); +#endif // __EXTRA_METADATA__ + TrackPointer pTrack = Track::newTemporary( - originalTrack->getFileInfo(), - originalTrack->getSecurityToken())); - track->setTitle(result.m_title); - track->setArtist(result.m_artist); - track->setAlbum(result.m_album); - track->setDuration(result.m_duration); - track->setTrackNumber(QString::number(result.m_track)); - track->setYear(QString::number(result.m_year)); - tracksGuessed << track; + pOriginalTrack->getFileInfo(), + pOriginalTrack->getSecurityToken()); + pTrack->importMetadata(std::move(trackMetadata)); + pTrack->setDuration(trackRelease.duration); + tracksGuessed << pTrack; } - emit resultAvailable(originalTrack, tracksGuessed); + emit resultAvailable( + std::move(pOriginalTrack), + std::move(tracksGuessed)); } diff --git a/src/musicbrainz/tagfetcher.h b/src/musicbrainz/tagfetcher.h index 0f96f22b406..60d0ad06030 100644 --- a/src/musicbrainz/tagfetcher.h +++ b/src/musicbrainz/tagfetcher.h @@ -1,59 +1,79 @@ -/***************************************************************************** - * Copyright © 2012 John Maguire * - * David Sansome * - * This work is free. You can redistribute it and/or modify it under the * - * terms of the Do What The Fuck You Want To Public License, Version 2, * - * as published by Sam Hocevar. * - * See http://www.wtfpl.net/ for more details. * - *****************************************************************************/ - -#ifndef TAGFETCHER_H -#define TAGFETCHER_H +#pragma once #include #include -#include "musicbrainz/musicbrainzclient.h" -#include "musicbrainz/acoustidclient.h" +#include "musicbrainz/web/acoustidlookuptask.h" +#include "musicbrainz/web/musicbrainzrecordingstask.h" #include "track/track.h" - +#include "util/parented_ptr.h" class TagFetcher : public QObject { Q_OBJECT - // High level interface to Fingerprinter, AcoustidClient and - // MusicBrainzClient. + // Implements retrieval of metadata in 3 subsequent stages: + // 1. Chromaprint -> AcoustID fingerprint + // 2. AcoustID -> MusicBrainz recording UUIDs + // 3. MusicBrainz -> MusicBrainz track releases public: - TagFetcher(QObject* parent = 0); + explicit TagFetcher( + QObject* parent = nullptr); - void startFetch(const TrackPointer track); + void startFetch( + TrackPointer pTrack); public slots: void cancel(); signals: - void resultAvailable(const TrackPointer originalTrack, - const QList& tracksGuessed); - void fetchProgress(QString); - void networkError(int httpStatus, QString app, QString message, int code); + void resultAvailable( + TrackPointer originalTrack, + QList tracksGuessed); + void fetchProgress( + QString message); + void networkError( + int httpStatus, + QString app, + QString message, + int code); private slots: - void fingerprintFound(int index); - void mbRecordingIdsFound(int index, QStringList mbRecordingIds); - void tagsFetched(int index, const MusicBrainzClient::ResultList& result); + void slotFingerprintReady(); + + void slotAcoustIdTaskSucceeded( + QList recordingIds); + void slotAcoustIdTaskFailed( + mixxx::network::JsonWebResponse response); + void slotAcoustIdTaskNetworkError( + QUrl requestUrl, + QNetworkReply::NetworkError errorCode, + QString errorString, + QByteArray errorContent); + + void slotMusicBrainzTaskSucceeded( + QList trackReleases); + void slotMusicBrainzTaskFailed( + mixxx::network::WebResponse response, + int errorCode, + QString errorMessage); + void slotMusicBrainzTaskNetworkError( + QUrl requestUrl, + QNetworkReply::NetworkError errorCode, + QString errorString, + QByteArray errorContent); private: - // has to be static so we can call it with QtConcurrent and have a nice - // responsive UI while the fingerprint is calculated - static QString getFingerprint(const TrackPointer tio); + void abortAcoustIdTask(); + void abortMusicBrainzTask(); - QFutureWatcher* m_pFingerprintWatcher; - AcoustidClient m_AcoustidClient; - MusicBrainzClient m_MusicbrainzClient; + QNetworkAccessManager m_network; - // Code can already be run on an arbitrary number of input tracks - QList m_tracks; -}; + QFutureWatcher m_fingerprintWatcher; -#endif // TAGFETCHER_H + parented_ptr m_pAcoustIdTask; + + parented_ptr m_pMusicBrainzTask; + + TrackPointer m_pTrack; +}; diff --git a/src/track/track.h b/src/track/track.h index 487a7fc0b3a..515b38db918 100644 --- a/src/track/track.h +++ b/src/track/track.h @@ -284,7 +284,7 @@ class Track : public QObject { // Set/get track metadata and cover art (optional) all at once. void importMetadata( mixxx::TrackMetadata importedMetadata, - QDateTime metadataSynchronized); + QDateTime metadataSynchronized = QDateTime()); // Merge additional metadata that is not (yet) stored in the database // and only available from file tags. void mergeImportedMetadata( diff --git a/src/util/compatibility.h b/src/util/compatibility.h index 60247e06e14..43563108e45 100644 --- a/src/util/compatibility.h +++ b/src/util/compatibility.h @@ -206,3 +206,14 @@ inline void atomicStoreRelaxed(QAtomicPointer& atomicPtr, T* newValue) { atomicPtr.store(newValue); #endif } + +#if QT_VERSION < QT_VERSION_CHECK(5, 6, 0) +template +inline uint qHash(const QList& key, uint seed = 0) { + uint hash = 0; + for (const auto& elem : key) { + hash ^= qHash(elem, seed); + } + return hash; +} +#endif From 631b94616e870faef24b681160b730659cb1b509 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Tue, 3 Mar 2020 23:52:18 +0100 Subject: [PATCH 5/9] Rename function to join track number strings --- src/library/dlgtagfetcher.cpp | 4 ++-- src/test/tracknumberstest.cpp | 2 +- src/track/trackmetadatataglib.cpp | 12 ++++++------ src/track/tracknumbers.cpp | 4 ++-- src/track/tracknumbers.h | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/library/dlgtagfetcher.cpp b/src/library/dlgtagfetcher.cpp index c1c0f7420c2..1f6f8d47210 100644 --- a/src/library/dlgtagfetcher.cpp +++ b/src/library/dlgtagfetcher.cpp @@ -7,7 +7,7 @@ namespace { QStringList track2row(const Track& track) { - const auto trackNumbers = TrackNumbers::joinStrings( + const QString trackNumberAndTotal = TrackNumbers::joinAsString( track.getTrackNumber(), track.getTrackTotal()); QStringList row; @@ -16,7 +16,7 @@ QStringList track2row(const Track& track) { << track.getYear() << track.getAlbum() << track.getAlbumArtist() - << trackNumbers + << trackNumberAndTotal << track.getTitle() << track.getArtist(); return row; diff --git a/src/test/tracknumberstest.cpp b/src/test/tracknumberstest.cpp index 8a4be7f990b..b3a71f7af00 100644 --- a/src/test/tracknumberstest.cpp +++ b/src/test/tracknumberstest.cpp @@ -69,7 +69,7 @@ class TrackNumbersTest : public testing::Test { EXPECT_EQ(expectedTrackNumber, actualTrackNumber); EXPECT_EQ(expectedTrackTotal, actualTrackTotal); - EXPECT_EQ(inputValue, TrackNumbers::joinStrings(actualTrackNumber, actualTrackTotal)); + EXPECT_EQ(inputValue, TrackNumbers::joinAsString(actualTrackNumber, actualTrackTotal)); } }; diff --git a/src/track/trackmetadatataglib.cpp b/src/track/trackmetadatataglib.cpp index 1ae02cf6f2d..3ff215e80b7 100644 --- a/src/track/trackmetadatataglib.cpp +++ b/src/track/trackmetadatataglib.cpp @@ -2312,7 +2312,7 @@ bool exportTrackMetadataIntoID3v2Tag(TagLib::ID3v2::Tag* pTag, writeID3v2CommentsFrameWithoutDescription(pTag, trackMetadata.getTrackInfo().getComment()); writeID3v2TextIdentificationFrame(pTag, "TRCK", - TrackNumbers::joinStrings( + TrackNumbers::joinAsString( trackMetadata.getTrackInfo().getTrackNumber(), trackMetadata.getTrackInfo().getTrackTotal())); @@ -2403,7 +2403,7 @@ bool exportTrackMetadataIntoID3v2Tag(TagLib::ID3v2::Tag* pTag, true); #if defined(__EXTRA_METADATA__) - writeID3v2TextIdentificationFrame(pTag, "TPOS", TrackNumbers::joinStrings( + writeID3v2TextIdentificationFrame(pTag, "TPOS", TrackNumbers::joinAsString( trackMetadata.getTrackInfo().getDiscNumber(), trackMetadata.getTrackInfo().getDiscTotal())); @@ -2538,7 +2538,7 @@ bool exportTrackMetadataIntoAPETag(TagLib::APE::Tag* pTag, const TrackMetadata& // part of the tag with the custom string from the track metadata // (pass-through without any further validation) writeAPEItem(pTag, "Track", - toTString(TrackNumbers::joinStrings( + toTString(TrackNumbers::joinAsString( trackMetadata.getTrackInfo().getTrackNumber(), trackMetadata.getTrackInfo().getTrackTotal()))); @@ -2564,7 +2564,7 @@ bool exportTrackMetadataIntoAPETag(TagLib::APE::Tag* pTag, const TrackMetadata& toTString(formatTrackPeak(trackMetadata))); #if defined(__EXTRA_METADATA__) - auto discNumbers = TrackNumbers::joinStrings( + auto discNumbers = TrackNumbers::joinAsString( trackMetadata.getTrackInfo().getDiscNumber(), trackMetadata.getTrackInfo().getDiscTotal()); writeAPEItem(pTag, "Disc", toTString(discNumbers)); @@ -2781,7 +2781,7 @@ bool exportTrackMetadataIntoMP4Tag(TagLib::MP4::Tag* pTag, const TrackMetadata& break; default: kLogger.warning() << "Invalid track numbers:" - << TrackNumbers::joinStrings( + << TrackNumbers::joinAsString( trackMetadata.getTrackInfo().getTrackNumber(), trackMetadata.getTrackInfo().getTrackTotal()); } @@ -2832,7 +2832,7 @@ bool exportTrackMetadataIntoMP4Tag(TagLib::MP4::Tag* pTag, const TrackMetadata& break; default: kLogger.warning() << "Invalid disc numbers:" - << TrackNumbers::joinStrings( + << TrackNumbers::joinAsString( trackMetadata.getTrackInfo().getDiscNumber(), trackMetadata.getTrackInfo().getDiscTotal()); } diff --git a/src/track/tracknumbers.cpp b/src/track/tracknumbers.cpp index 4941e6df31a..a379b089459 100644 --- a/src/track/tracknumbers.cpp +++ b/src/track/tracknumbers.cpp @@ -98,7 +98,7 @@ void TrackNumbers::splitString( } //static -QString TrackNumbers::joinStrings( +QString TrackNumbers::joinAsString( const QString& actualText, const QString& totalText) { if (totalText.isEmpty()) { @@ -141,5 +141,5 @@ QString TrackNumbers::toString() const { QString actualText; QString totalText; toStrings(&actualText, &totalText); - return joinStrings(actualText, totalText); + return joinAsString(actualText, totalText); } diff --git a/src/track/tracknumbers.h b/src/track/tracknumbers.h index e036da12917..7e591b953df 100644 --- a/src/track/tracknumbers.h +++ b/src/track/tracknumbers.h @@ -101,7 +101,7 @@ class TrackNumbers final { QString* pActualText = nullptr, QString* pTotalText = nullptr); // Joins the actual and total strings - static QString joinStrings( + static QString joinAsString( const QString& actualText, const QString& totalText); From 064993c807488bfb15ca469a8994bad6c6b483d8 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Tue, 3 Mar 2020 23:57:12 +0100 Subject: [PATCH 6/9] Explicitly ignore XML token types --- src/musicbrainz/musicbrainzxml.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/musicbrainz/musicbrainzxml.cpp b/src/musicbrainz/musicbrainzxml.cpp index 8a0f579105e..cfaf83cf021 100644 --- a/src/musicbrainz/musicbrainzxml.cpp +++ b/src/musicbrainz/musicbrainzxml.cpp @@ -65,6 +65,10 @@ void consumeCurrentElement(QXmlStreamReader& reader) { } --level; break; + case QXmlStreamReader::Characters: + case QXmlStreamReader::Comment: + case QXmlStreamReader::ProcessingInstruction: + // ignore token types default: DEBUG_ASSERT(reader.tokenType() != QXmlStreamReader::NoToken); DEBUG_ASSERT(reader.tokenType() != QXmlStreamReader::Invalid); @@ -72,7 +76,6 @@ void consumeCurrentElement(QXmlStreamReader& reader) { DEBUG_ASSERT(reader.tokenType() != QXmlStreamReader::EndDocument); DEBUG_ASSERT(reader.tokenType() != QXmlStreamReader::DTD); DEBUG_ASSERT(reader.tokenType() != QXmlStreamReader::EntityReference); - // ignore other token types } } } From ea599457f26f455d3db5a025a46b0ddc257f21d6 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Wed, 4 Mar 2020 01:05:11 +0100 Subject: [PATCH 7/9] Pass-through MusicBrainz data type over signal/slot connection --- src/library/dlgtagfetcher.cpp | 220 +++++++++++++++------------------ src/library/dlgtagfetcher.h | 7 +- src/musicbrainz/tagfetcher.cpp | 72 ++++++----- src/musicbrainz/tagfetcher.h | 6 +- 4 files changed, 149 insertions(+), 156 deletions(-) diff --git a/src/library/dlgtagfetcher.cpp b/src/library/dlgtagfetcher.cpp index 1f6f8d47210..b618b65e225 100644 --- a/src/library/dlgtagfetcher.cpp +++ b/src/library/dlgtagfetcher.cpp @@ -1,25 +1,27 @@ +#include "library/dlgtagfetcher.h" + #include #include -#include "library/dlgtagfetcher.h" #include "track/tracknumbers.h" namespace { -QStringList track2row(const Track& track) { +QStringList trackReleaseColumnValues( + const mixxx::musicbrainz::TrackRelease& trackRelease) { const QString trackNumberAndTotal = TrackNumbers::joinAsString( - track.getTrackNumber(), - track.getTrackTotal()); - QStringList row; - row.reserve(6); - row - << track.getYear() - << track.getAlbum() - << track.getAlbumArtist() + trackRelease.trackNumber, + trackRelease.trackTotal); + QStringList columnValues; + columnValues.reserve(6); + columnValues + << trackRelease.date + << trackRelease.albumTitle + << trackRelease.albumArtist << trackNumberAndTotal - << track.getTitle() - << track.getArtist(); - return row; + << trackRelease.title + << trackRelease.artist; + return columnValues; } void addTrack( @@ -33,7 +35,7 @@ void addTrack( } // anonymous namespace -DlgTagFetcher::DlgTagFetcher(QWidget *parent) +DlgTagFetcher::DlgTagFetcher(QWidget* parent) : QDialog(parent), m_tagFetcher(parent), m_networkResult(NetworkResult::Ok) { @@ -43,23 +45,15 @@ DlgTagFetcher::DlgTagFetcher(QWidget *parent) void DlgTagFetcher::init() { setupUi(this); - connect(btnApply, &QPushButton::clicked, - this, &DlgTagFetcher::apply); - connect(btnQuit, &QPushButton::clicked, - this, &DlgTagFetcher::quit); - connect(btnPrev, &QPushButton::clicked, - this, &DlgTagFetcher::previous); - connect(btnNext, &QPushButton::clicked, - this, &DlgTagFetcher::next); - connect(results, &QTreeWidget::currentItemChanged, - this, &DlgTagFetcher::resultSelected); + connect(btnApply, &QPushButton::clicked, this, &DlgTagFetcher::apply); + connect(btnQuit, &QPushButton::clicked, this, &DlgTagFetcher::quit); + connect(btnPrev, &QPushButton::clicked, this, &DlgTagFetcher::previous); + connect(btnNext, &QPushButton::clicked, this, &DlgTagFetcher::next); + connect(results, &QTreeWidget::currentItemChanged, this, &DlgTagFetcher::resultSelected); - connect(&m_tagFetcher, &TagFetcher::resultAvailable, - this, &DlgTagFetcher::fetchTagFinished); - connect(&m_tagFetcher, &TagFetcher::fetchProgress, - this, &DlgTagFetcher::fetchTagProgress); - connect(&m_tagFetcher, &TagFetcher::networkError, - this, &DlgTagFetcher::slotNetworkResult); + connect(&m_tagFetcher, &TagFetcher::resultAvailable, this, &DlgTagFetcher::fetchTagFinished); + connect(&m_tagFetcher, &TagFetcher::fetchProgress, this, &DlgTagFetcher::fetchTagProgress); + connect(&m_tagFetcher, &TagFetcher::networkError, this, &DlgTagFetcher::slotNetworkResult); // Resize columns, this can't be set in the ui file results->setColumnWidth(0, 50); // Year column @@ -101,73 +95,68 @@ void DlgTagFetcher::slotTrackChanged(TrackId trackId) { void DlgTagFetcher::apply() { int resultIndex = m_data.m_selectedResult; - if (resultIndex > -1) { - mixxx::TrackMetadata importedMetadata; - m_data.m_results[resultIndex]->readTrackMetadata(&importedMetadata); - mixxx::TrackMetadata updatedMetadata; - m_track->readTrackMetadata(&updatedMetadata); - if (!importedMetadata.getTrackInfo().getArtist().isEmpty()) { - updatedMetadata.refTrackInfo().setArtist( - importedMetadata.getTrackInfo().getArtist()); - } - if (!importedMetadata.getTrackInfo().getTitle().isEmpty()) { - updatedMetadata.refTrackInfo().setTitle( - importedMetadata.getTrackInfo().getTitle()); - } - if (!importedMetadata.getTrackInfo().getTrackNumber().isEmpty()) { - updatedMetadata.refTrackInfo().setTrackNumber( - importedMetadata.getTrackInfo().getTrackNumber() - ); - } - if (!importedMetadata.getTrackInfo().getTrackTotal().isEmpty()) { - updatedMetadata.refTrackInfo().setTrackTotal( - importedMetadata.getTrackInfo().getTrackTotal() - ); - } - if (!importedMetadata.getTrackInfo().getYear().isEmpty()) { - updatedMetadata.refTrackInfo().setYear( - importedMetadata.getTrackInfo().getYear()); - } - if (!importedMetadata.getAlbumInfo().getArtist().isEmpty()) { - updatedMetadata.refAlbumInfo().setArtist( - importedMetadata.getAlbumInfo().getArtist()); - } - if (!importedMetadata.getAlbumInfo().getTitle().isEmpty()) { - updatedMetadata.refAlbumInfo().setTitle( - importedMetadata.getAlbumInfo().getTitle()); - } + if (resultIndex < 0) { + return; + } + DEBUG_ASSERT(resultIndex < m_data.m_results.size()); + const mixxx::musicbrainz::TrackRelease& trackRelease = + m_data.m_results[resultIndex]; + mixxx::TrackMetadata trackMetadata; + m_track->readTrackMetadata(&trackMetadata); + if (!trackRelease.artist.isEmpty()) { + trackMetadata.refTrackInfo().setArtist( + trackRelease.artist); + } + if (!trackRelease.title.isEmpty()) { + trackMetadata.refTrackInfo().setTitle( + trackRelease.title); + } + if (!trackRelease.trackNumber.isEmpty()) { + trackMetadata.refTrackInfo().setTrackNumber( + trackRelease.trackNumber); + if (!trackRelease.trackTotal.isEmpty()) { + trackMetadata.refTrackInfo().setTrackTotal( + trackRelease.trackTotal); + } + if (!trackRelease.date.isEmpty()) { + trackMetadata.refTrackInfo().setYear( + trackRelease.date); + } + if (!trackRelease.albumArtist.isEmpty()) { + trackMetadata.refAlbumInfo().setArtist( + trackRelease.albumArtist); + } + if (!trackRelease.albumTitle.isEmpty()) { + trackMetadata.refAlbumInfo().setTitle( + trackRelease.albumTitle); + } #if defined(__EXTRA_METADATA__) - if (!importedMetadata.getTrackInfo().getMusicBrainzArtistId().isNull()) { - updatedMetadata.refTrackInfo().setMusicBrainzArtistId( - importedMetadata.getTrackInfo().getMusicBrainzArtistId() - ); - } - if (!importedMetadata.getTrackInfo().getMusicBrainzRecordingId().isNull()) { - updatedMetadata.refTrackInfo().setMusicBrainzRecordingId( - importedMetadata.getTrackInfo().getMusicBrainzRecordingId() - ); - } - if (!importedMetadata.getTrackInfo().getMusicBrainzReleaseId().isNull()) { - updatedMetadata.refTrackInfo().setMusicBrainzReleaseId( - importedMetadata.getTrackInfo().getMusicBrainzReleaseId() - ); - } - if (!importedMetadata.getAlbumInfo().getMusicBrainzArtistId().isNull()) { - updatedMetadata.refAlbumInfo().setMusicBrainzArtistId( - importedMetadata.getAlbumInfo().getMusicBrainzArtistId() - ); - } - if (!importedMetadata.getAlbumInfo().getMusicBrainzReleaseId().isNull()) { - updatedMetadata.refAlbumInfo().setMusicBrainzReleaseId( - importedMetadata.getAlbumInfo().getMusicBrainzReleaseId()); - } - if (!importedMetadata.getAlbumInfo().getMusicBrainzReleaseGroupId().isNull()) { - updatedMetadata.refAlbumInfo().setMusicBrainzReleaseGroupId( - importedMetadata.getAlbumInfo().getMusicBrainzReleaseGroupId()); - } -#endif // __EXTRA_METADATA__ - m_track->importMetadata(std::move(updatedMetadata)); + if (!trackRelease.artistId.isNull()) + trackMetadata.refTrackInfo().setMusicBrainzArtistId( + trackRelease.artistId); + } + if (!trackRelease.recordingId.isNull()) { + trackMetadata.refTrackInfo().setMusicBrainzRecordingId( + trackRelease.recordingId); + } + if (!trackRelease.trackReleaseId.isNull()) { + trackMetadata.refTrackInfo().setMusicBrainzReleaseId( + trackRelease.trackReleaseId); } + if (!trackRelease.albumArtistId.isNull()) { + trackMetadata.refAlbumInfo().setMusicBrainzArtistId( + trackRelease.albumArtistId); + } + if (!trackRelease.albumReleaseId.isNull()) { + trackMetadata.refAlbumInfo().setMusicBrainzReleaseId( + trackRelease.albumReleaseId); + } + if (!trackRelease.releaseGroupId.isNull()) { + trackMetadata.refAlbumInfo().setMusicBrainzReleaseGroupId( + trackRelease.releaseGroupId); + } +#endif // __EXTRA_METADATA__ + m_track->importMetadata(std::move(trackMetadata)); } void DlgTagFetcher::quit() { @@ -180,21 +169,18 @@ void DlgTagFetcher::fetchTagProgress(QString text) { loadingStatus->setText(status.arg(text)); } -void DlgTagFetcher::fetchTagFinished(const TrackPointer track, - const QList& tracks) { - // check if the answer is for this track - if (m_track->getLocation() != track->getLocation()) { - return; - } - +void DlgTagFetcher::fetchTagFinished( + mixxx::musicbrainz::TrackRelease originalTrackRelease, + QList guessedTrackReleases) { m_data.m_pending = false; - m_data.m_results = tracks; - // qDebug() << "number of results = " << tracks.size(); + m_data.m_original = originalTrackRelease; + m_data.m_results = guessedTrackReleases; + // qDebug() << "number of results = " << guessedTrackReleases.size(); updateStack(); } void DlgTagFetcher::slotNetworkResult(int httpError, QString app, QString message, int code) { - m_networkResult = httpError == 0 ? NetworkResult::UnknownError : NetworkResult::HttpError; + m_networkResult = httpError == 0 ? NetworkResult::UnknownError : NetworkResult::HttpError; m_data.m_pending = false; QString strError = tr("HTTP Status: %1"); QString strCode = tr("Code: %1"); @@ -226,25 +212,25 @@ void DlgTagFetcher::updateStack() { results->clear(); addDivider(tr("Original tags"), results); - addTrack(track2row(*m_track), -1, results); + addTrack(trackReleaseColumnValues(m_data.m_original), -1, results); addDivider(tr("Suggested tags"), results); { int trackIndex = 0; - QSet trackRows; - foreach (const TrackPointer track, m_data.m_results) { - const auto trackRow = track2row(*track); + QSet allColumnValues; // deduplication + for (const auto& trackRelease : qAsConst(m_data.m_results)) { + const auto columnValues = trackReleaseColumnValues(trackRelease); // Ignore duplicate results - if (!trackRows.contains(trackRow)) { - trackRows.insert(trackRow); - addTrack(trackRow, trackIndex, results); + if (!allColumnValues.contains(columnValues)) { + allColumnValues.insert(columnValues); + addTrack(columnValues, trackIndex, results); } ++trackIndex; } } // Find the item that was selected last time - for (int i=0 ; imodel()->rowCount() ; ++i) { + for (int i = 0; i < results->model()->rowCount(); ++i) { const QModelIndex index = results->model()->index(i, 0); const QVariant id = index.data(Qt::UserRole); if (!id.isNull() && id.toInt() == m_data.m_selectedResult) { @@ -267,9 +253,9 @@ void DlgTagFetcher::addDivider(const QString& text, QTreeWidget* parent) const { } void DlgTagFetcher::resultSelected() { - if (!results->currentItem()) - return; + if (!results->currentItem()) + return; - const int resultIndex = results->currentItem()->data(0, Qt::UserRole).toInt(); - m_data.m_selectedResult = resultIndex; + const int resultIndex = results->currentItem()->data(0, Qt::UserRole).toInt(); + m_data.m_selectedResult = resultIndex; } diff --git a/src/library/dlgtagfetcher.h b/src/library/dlgtagfetcher.h index 58beb7f5d3d..da2a3f2bdcd 100644 --- a/src/library/dlgtagfetcher.h +++ b/src/library/dlgtagfetcher.h @@ -25,7 +25,9 @@ class DlgTagFetcher : public QDialog, public Ui::DlgTagFetcher { void previous(); private slots: - void fetchTagFinished(const TrackPointer,const QList& tracks); + void fetchTagFinished( + mixxx::musicbrainz::TrackRelease originalTrackRelease, + QList guessedTrackReleases); void resultSelected(); void fetchTagProgress(QString); void slotNetworkResult(int httpStatus, QString app, QString message, int code); @@ -46,7 +48,8 @@ class DlgTagFetcher : public QDialog, public Ui::DlgTagFetcher { bool m_pending; int m_selectedResult; - QList m_results; + mixxx::musicbrainz::TrackRelease m_original; + QList m_results; }; Data m_data; diff --git a/src/musicbrainz/tagfetcher.cpp b/src/musicbrainz/tagfetcher.cpp index 987caf53c5f..eadcab3f9ce 100644 --- a/src/musicbrainz/tagfetcher.cpp +++ b/src/musicbrainz/tagfetcher.cpp @@ -11,6 +11,32 @@ constexpr int kAcoustIdTimeoutMillis = 5000; // msec constexpr int kMusicBrainzTimeoutMillis = 5000; // msec +mixxx::musicbrainz::TrackRelease trackReleaseFromTrack( + const Track& track) { + mixxx::TrackMetadata trackMetadata; + track.readTrackMetadata(&trackMetadata); + mixxx::musicbrainz::TrackRelease trackRelease; + trackRelease.title = trackMetadata.getTrackInfo().getTitle(); + trackRelease.artist = trackMetadata.getTrackInfo().getArtist(); + trackRelease.trackNumber = trackMetadata.getTrackInfo().getTrackNumber(); + trackRelease.trackTotal = trackMetadata.getTrackInfo().getTrackTotal(); + trackRelease.date = trackMetadata.getTrackInfo().getYear(); + trackRelease.albumTitle = trackMetadata.getAlbumInfo().getTitle(); + trackRelease.albumArtist = trackMetadata.getAlbumInfo().getArtist(); +#if defined(__EXTRA_METADATA__) + trackRelease.discNumber = trackMetadata.getTrackInfo().getDiscNumber(); + trackRelease.discTotal = trackMetadata.getTrackInfo().getDiscTotal(); + trackRelease.artistId = trackMetadata.getTrackInfo().getMusicBrainzArtistId(); + trackRelease.recordingId = trackMetadata.getTrackInfo().getMusicBrainzRecordingId(); + trackRelease.trackReleaseId = trackMetadata.getTrackInfo().getMusicBrainzReleaseId(); + trackRelease.albumArtistId = trackMetadata.getAlbumInfo().getMusicBrainzArtistId(); + trackRelease.albumReleaseId = trackMetadata.getAlbumInfo().getMusicBrainzReleaseId(); + trackRelease.releaseGroupId = trackMetadata.getAlbumInfo().getMusicBrainzReleaseGroupId(); +#endif // __EXTRA_METADATA__ + trackRelease.duration = trackMetadata.getDuration(); + return trackRelease; +} + } // anonymous namespace TagFetcher::TagFetcher(QObject* parent) @@ -68,7 +94,9 @@ void TagFetcher::slotFingerprintReady() { const QString fingerprint = m_fingerprintWatcher.result(); if (fingerprint.isEmpty()) { - emit resultAvailable(m_pTrack, QList()); + emit resultAvailable( + trackReleaseFromTrack(*m_pTrack), + QList()); return; } @@ -105,7 +133,9 @@ void TagFetcher::slotAcoustIdTaskSucceeded( } if (recordingIds.isEmpty()) { - emit resultAvailable(m_pTrack, QList()); + emit resultAvailable( + trackReleaseFromTrack(*m_pTrack), + QList()); return; } @@ -184,42 +214,16 @@ void TagFetcher::slotMusicBrainzTaskFailed( } void TagFetcher::slotMusicBrainzTaskSucceeded( - QList trackReleases) { + QList guessedTrackReleases) { auto pOriginalTrack = std::move(m_pTrack); abortMusicBrainzTask(); if (!pOriginalTrack) { + // aborted return; } - - QList tracksGuessed; - for (const auto& trackRelease : qAsConst(trackReleases)) { - mixxx::TrackMetadata trackMetadata; - trackMetadata.refTrackInfo().setTitle(trackRelease.title); - trackMetadata.refTrackInfo().setArtist(trackRelease.artist); - trackMetadata.refTrackInfo().setTrackNumber(trackRelease.trackNumber); - trackMetadata.refTrackInfo().setTrackTotal(trackRelease.trackTotal); - trackMetadata.refTrackInfo().setYear(trackRelease.date); - trackMetadata.refAlbumInfo().setTitle(trackRelease.albumTitle); - trackMetadata.refAlbumInfo().setArtist(trackRelease.albumArtist); -#if defined(__EXTRA_METADATA__) - trackMetadata.refTrackInfo().setDiscNumber(trackRelease.discNumber); - trackMetadata.refTrackInfo().setDiscTotal(trackRelease.discTotal); - trackMetadata.refTrackInfo().setMusicBrainzArtistId(trackRelease.artistId); - trackMetadata.refTrackInfo().setMusicBrainzRecordingId(trackRelease.recordingId); - trackMetadata.refTrackInfo().setMusicBrainzReleaseId(trackRelease.trackReleaseId); - trackMetadata.refAlbumInfo().setMusicBrainzArtistId(trackRelease.albumArtistId); - trackMetadata.refAlbumInfo().setMusicBrainzReleaseId(trackRelease.albumReleaseId); - trackMetadata.refAlbumInfo().setMusicBrainzReleaseGroupId(trackRelease.releaseGroupId); -#endif // __EXTRA_METADATA__ - TrackPointer pTrack = - Track::newTemporary( - pOriginalTrack->getFileInfo(), - pOriginalTrack->getSecurityToken()); - pTrack->importMetadata(std::move(trackMetadata)); - pTrack->setDuration(trackRelease.duration); - tracksGuessed << pTrack; - } + const mixxx::musicbrainz::TrackRelease originalTrackRelease = + trackReleaseFromTrack(*pOriginalTrack); emit resultAvailable( - std::move(pOriginalTrack), - std::move(tracksGuessed)); + std::move(originalTrackRelease), + std::move(guessedTrackReleases)); } diff --git a/src/musicbrainz/tagfetcher.h b/src/musicbrainz/tagfetcher.h index 60d0ad06030..c185ceb6472 100644 --- a/src/musicbrainz/tagfetcher.h +++ b/src/musicbrainz/tagfetcher.h @@ -28,8 +28,8 @@ class TagFetcher : public QObject { signals: void resultAvailable( - TrackPointer originalTrack, - QList tracksGuessed); + mixxx::musicbrainz::TrackRelease originalTrackRelease, + QList guessedTrackReleases); void fetchProgress( QString message); void networkError( @@ -52,7 +52,7 @@ class TagFetcher : public QObject { QByteArray errorContent); void slotMusicBrainzTaskSucceeded( - QList trackReleases); + QList guessedTrackReleases); void slotMusicBrainzTaskFailed( mixxx::network::WebResponse response, int errorCode, From c369b634349efb483ed5afc73f58d5953e50e715 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Wed, 4 Mar 2020 09:09:06 +0100 Subject: [PATCH 8/9] Fix placement of curly braces --- src/library/dlgtagfetcher.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/library/dlgtagfetcher.cpp b/src/library/dlgtagfetcher.cpp index b618b65e225..69244643b2b 100644 --- a/src/library/dlgtagfetcher.cpp +++ b/src/library/dlgtagfetcher.cpp @@ -114,6 +114,7 @@ void DlgTagFetcher::apply() { if (!trackRelease.trackNumber.isEmpty()) { trackMetadata.refTrackInfo().setTrackNumber( trackRelease.trackNumber); + } if (!trackRelease.trackTotal.isEmpty()) { trackMetadata.refTrackInfo().setTrackTotal( trackRelease.trackTotal); @@ -131,7 +132,7 @@ void DlgTagFetcher::apply() { trackRelease.albumTitle); } #if defined(__EXTRA_METADATA__) - if (!trackRelease.artistId.isNull()) + if (!trackRelease.artistId.isNull()) { trackMetadata.refTrackInfo().setMusicBrainzArtistId( trackRelease.artistId); } From b805083f8310113b437288b6913948bd7e6d6c02 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Wed, 4 Mar 2020 11:37:32 +0100 Subject: [PATCH 9/9] Avoid unnecessary conversion --- src/library/dlgtagfetcher.cpp | 31 ++++++++++++++++++++++++++++--- src/library/dlgtagfetcher.h | 3 +-- src/musicbrainz/tagfetcher.cpp | 34 +++------------------------------- src/musicbrainz/tagfetcher.h | 2 +- 4 files changed, 33 insertions(+), 37 deletions(-) diff --git a/src/library/dlgtagfetcher.cpp b/src/library/dlgtagfetcher.cpp index 69244643b2b..0b6b682064a 100644 --- a/src/library/dlgtagfetcher.cpp +++ b/src/library/dlgtagfetcher.cpp @@ -7,6 +7,25 @@ namespace { +QStringList trackColumnValues( + const Track& track) { + mixxx::TrackMetadata trackMetadata; + track.readTrackMetadata(&trackMetadata); + const QString trackNumberAndTotal = TrackNumbers::joinAsString( + trackMetadata.getTrackInfo().getTrackNumber(), + trackMetadata.getTrackInfo().getTrackTotal()); + QStringList columnValues; + columnValues.reserve(6); + columnValues + << trackMetadata.getTrackInfo().getYear() + << trackMetadata.getAlbumInfo().getTitle() + << trackMetadata.getAlbumInfo().getArtist() + << trackNumberAndTotal + << trackMetadata.getTrackInfo().getTitle() + << trackMetadata.getTrackInfo().getArtist(); + return columnValues; +} + QStringList trackReleaseColumnValues( const mixxx::musicbrainz::TrackRelease& trackRelease) { const QString trackNumberAndTotal = TrackNumbers::joinAsString( @@ -171,10 +190,12 @@ void DlgTagFetcher::fetchTagProgress(QString text) { } void DlgTagFetcher::fetchTagFinished( - mixxx::musicbrainz::TrackRelease originalTrackRelease, + TrackPointer pTrack, QList guessedTrackReleases) { + VERIFY_OR_DEBUG_ASSERT(pTrack == m_track) { + return; + } m_data.m_pending = false; - m_data.m_original = originalTrackRelease; m_data.m_results = guessedTrackReleases; // qDebug() << "number of results = " << guessedTrackReleases.size(); updateStack(); @@ -212,8 +233,12 @@ void DlgTagFetcher::updateStack() { results->clear(); + VERIFY_OR_DEBUG_ASSERT(m_track) { + return; + } + addDivider(tr("Original tags"), results); - addTrack(trackReleaseColumnValues(m_data.m_original), -1, results); + addTrack(trackColumnValues(*m_track), -1, results); addDivider(tr("Suggested tags"), results); { diff --git a/src/library/dlgtagfetcher.h b/src/library/dlgtagfetcher.h index da2a3f2bdcd..341f7807deb 100644 --- a/src/library/dlgtagfetcher.h +++ b/src/library/dlgtagfetcher.h @@ -26,7 +26,7 @@ class DlgTagFetcher : public QDialog, public Ui::DlgTagFetcher { private slots: void fetchTagFinished( - mixxx::musicbrainz::TrackRelease originalTrackRelease, + TrackPointer pTrack, QList guessedTrackReleases); void resultSelected(); void fetchTagProgress(QString); @@ -48,7 +48,6 @@ class DlgTagFetcher : public QDialog, public Ui::DlgTagFetcher { bool m_pending; int m_selectedResult; - mixxx::musicbrainz::TrackRelease m_original; QList m_results; }; Data m_data; diff --git a/src/musicbrainz/tagfetcher.cpp b/src/musicbrainz/tagfetcher.cpp index eadcab3f9ce..093c940dfdd 100644 --- a/src/musicbrainz/tagfetcher.cpp +++ b/src/musicbrainz/tagfetcher.cpp @@ -11,32 +11,6 @@ constexpr int kAcoustIdTimeoutMillis = 5000; // msec constexpr int kMusicBrainzTimeoutMillis = 5000; // msec -mixxx::musicbrainz::TrackRelease trackReleaseFromTrack( - const Track& track) { - mixxx::TrackMetadata trackMetadata; - track.readTrackMetadata(&trackMetadata); - mixxx::musicbrainz::TrackRelease trackRelease; - trackRelease.title = trackMetadata.getTrackInfo().getTitle(); - trackRelease.artist = trackMetadata.getTrackInfo().getArtist(); - trackRelease.trackNumber = trackMetadata.getTrackInfo().getTrackNumber(); - trackRelease.trackTotal = trackMetadata.getTrackInfo().getTrackTotal(); - trackRelease.date = trackMetadata.getTrackInfo().getYear(); - trackRelease.albumTitle = trackMetadata.getAlbumInfo().getTitle(); - trackRelease.albumArtist = trackMetadata.getAlbumInfo().getArtist(); -#if defined(__EXTRA_METADATA__) - trackRelease.discNumber = trackMetadata.getTrackInfo().getDiscNumber(); - trackRelease.discTotal = trackMetadata.getTrackInfo().getDiscTotal(); - trackRelease.artistId = trackMetadata.getTrackInfo().getMusicBrainzArtistId(); - trackRelease.recordingId = trackMetadata.getTrackInfo().getMusicBrainzRecordingId(); - trackRelease.trackReleaseId = trackMetadata.getTrackInfo().getMusicBrainzReleaseId(); - trackRelease.albumArtistId = trackMetadata.getAlbumInfo().getMusicBrainzArtistId(); - trackRelease.albumReleaseId = trackMetadata.getAlbumInfo().getMusicBrainzReleaseId(); - trackRelease.releaseGroupId = trackMetadata.getAlbumInfo().getMusicBrainzReleaseGroupId(); -#endif // __EXTRA_METADATA__ - trackRelease.duration = trackMetadata.getDuration(); - return trackRelease; -} - } // anonymous namespace TagFetcher::TagFetcher(QObject* parent) @@ -95,7 +69,7 @@ void TagFetcher::slotFingerprintReady() { const QString fingerprint = m_fingerprintWatcher.result(); if (fingerprint.isEmpty()) { emit resultAvailable( - trackReleaseFromTrack(*m_pTrack), + m_pTrack, QList()); return; } @@ -134,7 +108,7 @@ void TagFetcher::slotAcoustIdTaskSucceeded( if (recordingIds.isEmpty()) { emit resultAvailable( - trackReleaseFromTrack(*m_pTrack), + m_pTrack, QList()); return; } @@ -221,9 +195,7 @@ void TagFetcher::slotMusicBrainzTaskSucceeded( // aborted return; } - const mixxx::musicbrainz::TrackRelease originalTrackRelease = - trackReleaseFromTrack(*pOriginalTrack); emit resultAvailable( - std::move(originalTrackRelease), + pOriginalTrack, std::move(guessedTrackReleases)); } diff --git a/src/musicbrainz/tagfetcher.h b/src/musicbrainz/tagfetcher.h index c185ceb6472..d0f14ad1edc 100644 --- a/src/musicbrainz/tagfetcher.h +++ b/src/musicbrainz/tagfetcher.h @@ -28,7 +28,7 @@ class TagFetcher : public QObject { signals: void resultAvailable( - mixxx::musicbrainz::TrackRelease originalTrackRelease, + TrackPointer pTrack, QList guessedTrackReleases); void fetchProgress( QString message);