From 465369d79e8a0fda0a9e33b75df92161af0711c7 Mon Sep 17 00:00:00 2001 From: Jonas Kvinge Date: Sun, 9 Aug 2020 20:10:53 +0200 Subject: [PATCH] Base initial score on album cover sizes retrieved from API --- src/covermanager/albumcoverfetcher.h | 19 +++++- src/covermanager/albumcoverfetchersearch.cpp | 46 +++++++------ src/covermanager/albumcoverfetchersearch.h | 4 +- src/covermanager/deezercoverprovider.cpp | 70 ++++++++++++-------- src/covermanager/lastfmcoverprovider.cpp | 1 + src/covermanager/lastfmcoverprovider.h | 8 +-- src/covermanager/musixmatchcoverprovider.cpp | 43 +++++------- src/covermanager/qobuzcoverprovider.cpp | 5 +- src/covermanager/spotifycoverprovider.cpp | 1 + src/covermanager/tidalcoverprovider.cpp | 29 +++++--- 10 files changed, 132 insertions(+), 94 deletions(-) diff --git a/src/covermanager/albumcoverfetcher.h b/src/covermanager/albumcoverfetcher.h index fe2b86fa6..7d4a0ad11 100644 --- a/src/covermanager/albumcoverfetcher.h +++ b/src/covermanager/albumcoverfetcher.h @@ -61,7 +61,7 @@ struct CoverSearchRequest { // This structure represents a single result of some album's cover search request. struct CoverSearchResult { - explicit CoverSearchResult() : score(0.0) {} + explicit CoverSearchResult() : score_provider(0.0), score_match(0.0), score_quality(0.0), number(0) {} // Used for grouping in the user interface. QString provider; @@ -73,8 +73,23 @@ struct CoverSearchResult { // An URL of a cover image QUrl image_url; + // Image size + QSize image_size; + + // Score for this provider + float score_provider; + + // Score for match + float score_match; + + // Score for image quality + float score_quality; + + // The result number + int number; + // Total score for this result - float score; + float score() const { return score_provider + score_match + score_quality; } }; Q_DECLARE_METATYPE(CoverSearchResult) diff --git a/src/covermanager/albumcoverfetchersearch.cpp b/src/covermanager/albumcoverfetchersearch.cpp index ece150c7c..1c1f68770 100644 --- a/src/covermanager/albumcoverfetchersearch.cpp +++ b/src/covermanager/albumcoverfetchersearch.cpp @@ -135,7 +135,7 @@ void AlbumCoverFetcherSearch::ProviderSearchResults(CoverProvider *provider, con for (int i = 0 ; i < results_copy.count() ; ++i) { results_copy[i].provider = provider->name(); - results_copy[i].score = provider->quality(); + results_copy[i].score_provider = provider->quality(); QString request_artist = request_.artist.toLower(); QString request_album = request_.album.toLower(); @@ -143,17 +143,17 @@ void AlbumCoverFetcherSearch::ProviderSearchResults(CoverProvider *provider, con QString result_album = results_copy[i].album.toLower(); if (result_artist == request_artist) { - results_copy[i].score += 0.5; + results_copy[i].score_match += 0.5; } if (result_album == request_album) { - results_copy[i].score += 0.5; + results_copy[i].score_match += 0.5; } if (result_artist != request_artist && result_album != request_album) { - results_copy[i].score -= 1.5; + results_copy[i].score_match -= 1.5; } if (request_album.isEmpty() && result_artist != request_artist) { - results_copy[i].score -= 1; + results_copy[i].score_match -= 1; } // Decrease score if the search was based on artist and song title, and the resulting album is a compilation or live album. @@ -168,9 +168,12 @@ void AlbumCoverFetcherSearch::ProviderSearchResults(CoverProvider *provider, con result_album.contains("live") || result_album.contains("concert") )) { - results_copy[i].score -= 1; + results_copy[i].score_match -= 1; } + // Set the initial image quality score besed on the size returned by the API, this is recalculated when the image is received. + results_copy[i].score_quality += ScoreImage(results_copy[i].image_size); + } // Add results from the current provider to our pool @@ -231,7 +234,7 @@ void AlbumCoverFetcherSearch::FetchMoreImages() { ++i; CoverSearchResult result = results_.takeFirst(); - qLog(Debug) << "Loading" << result.image_url << "from" << result.provider << "with current score" << result.score; + qLog(Debug) << "Loading" << result.image_url << "from" << result.provider << "with current score" << result.score(); QNetworkRequest req(result.image_url); req.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true); @@ -278,9 +281,13 @@ void AlbumCoverFetcherSearch::ProviderCoverFetchFinished(QNetworkReply *reply) { if (QImageReader::supportedMimeTypes().contains(mimetype.toUtf8())) { QImage image; if (image.loadFromData(reply->readAll())) { - result.score += ScoreImage(image); - candidate_images_.insert(result.score, CandidateImage(result, image)); - qLog(Debug) << reply->url() << "from" << result.provider << "scored" << result.score; + if (result.image_size != QSize(0,0) && result.image_size != image.size()) { + qLog(Debug) << "API size for image" << result.image_size << "for" << reply->url() << "from" << result.provider << "did not match retrieved size" << image.size(); + } + result.image_size = image.size(); + result.score_quality = ScoreImage(image.size()); + candidate_images_.insert(result.score(), CandidateImage(result, image)); + qLog(Debug) << reply->url() << "from" << result.provider << "scored" << result.score(); } else { qLog(Error) << "Error decoding image data from" << reply->url(); @@ -310,18 +317,15 @@ void AlbumCoverFetcherSearch::ProviderCoverFetchFinished(QNetworkReply *reply) { } -float AlbumCoverFetcherSearch::ScoreImage(const QImage &image) const { +float AlbumCoverFetcherSearch::ScoreImage(const QSize size) const { - // Invalid images score nothing - if (image.isNull()) { - return 0.0; - } + if (size.width() == 0 || size.height() == 0) return 0.0; // A 500x500px image scores 1.0, bigger scores higher - const float size_score = std::sqrt(float(image.width() * image.height())) / kTargetSize; + const float size_score = std::sqrt(float(size.width() * size.height())) / kTargetSize; // A 1:1 image scores 1.0, anything else scores less - const float aspect_score = 1.0 - float(std::max(image.width(), image.height()) - std::min(image.width(), image.height())) / std::max(image.height(), image.width()); + const float aspect_score = 1.0 - float(std::max(size.width(), size.height()) - std::min(size.width(), size.height())) / std::max(size.height(), size.width()); return size_score + aspect_score; @@ -337,7 +341,7 @@ void AlbumCoverFetcherSearch::SendBestImage() { cover_url = best_image.first.image_url; image = best_image.second; - qLog(Info) << "Using" << best_image.first.image_url << "from" << best_image.first.provider << "with score" << best_image.first.score; + qLog(Info) << "Using" << best_image.first.image_url << "from" << best_image.first.provider << "with score" << best_image.first.score(); statistics_.chosen_images_by_provider_[best_image.first.provider]++; statistics_.chosen_images_++; @@ -375,5 +379,9 @@ bool AlbumCoverFetcherSearch::ProviderCompareOrder(CoverProvider *a, CoverProvid } bool AlbumCoverFetcherSearch::CoverSearchResultCompareScore(const CoverSearchResult &a, const CoverSearchResult &b) { - return a.score > b.score; + return a.score() > b.score(); +} + +bool AlbumCoverFetcherSearch::CoverSearchResultCompareNumber(const CoverSearchResult &a, const CoverSearchResult &b) { + return a.number < b.number; } diff --git a/src/covermanager/albumcoverfetchersearch.h b/src/covermanager/albumcoverfetchersearch.h index 560e47974..af7aed358 100644 --- a/src/covermanager/albumcoverfetchersearch.h +++ b/src/covermanager/albumcoverfetchersearch.h @@ -59,6 +59,8 @@ class AlbumCoverFetcherSearch : public QObject { CoverSearchStatistics statistics() const { return statistics_; } + static bool CoverSearchResultCompareNumber(const CoverSearchResult &a, const CoverSearchResult &b); + signals: // It's the end of search (when there was no fetch-me-a-cover request). void SearchFinished(const quint64, const CoverSearchResults &results); @@ -77,7 +79,7 @@ class AlbumCoverFetcherSearch : public QObject { void AllProvidersFinished(); void FetchMoreImages(); - float ScoreImage(const QImage &image) const; + float ScoreImage(const QSize size) const; void SendBestImage(); static bool ProviderCompareOrder(CoverProvider *a, CoverProvider *b); diff --git a/src/covermanager/deezercoverprovider.cpp b/src/covermanager/deezercoverprovider.cpp index 663a24bd5..c05749e0f 100644 --- a/src/covermanager/deezercoverprovider.cpp +++ b/src/covermanager/deezercoverprovider.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -45,6 +46,7 @@ #include "core/logging.h" #include "core/song.h" #include "albumcoverfetcher.h" +#include "albumcoverfetchersearch.h" #include "jsoncoverprovider.h" #include "deezercoverprovider.h" @@ -197,26 +199,26 @@ void DeezerCoverProvider::HandleSearchReply(QNetworkReply *reply, const int id) disconnect(reply, nullptr, this, nullptr); reply->deleteLater(); - CoverSearchResults results; - QByteArray data = GetReplyData(reply); if (data.isEmpty()) { - emit SearchFinished(id, results); + emit SearchFinished(id, CoverSearchResults()); return; } QJsonValue value_data = ExtractData(data); if (!value_data.isArray()) { - emit SearchFinished(id, results); + emit SearchFinished(id, CoverSearchResults()); return; } QJsonArray array_data = value_data.toArray(); if (array_data.isEmpty()) { - emit SearchFinished(id, results); + emit SearchFinished(id, CoverSearchResults()); return; } + QMap results; + int i = 0; for (const QJsonValue &json_value : array_data) { if (!json_value.isObject()) { @@ -270,36 +272,46 @@ void DeezerCoverProvider::HandleSearchReply(QNetworkReply *reply, const int id) } QString album = obj_album["title"].toString(); - QString cover; - if (obj_album.contains("cover_xl")) { - cover = obj_album["cover_xl"].toString(); - } - else if (obj_album.contains("cover_big")) { - cover = obj_album["cover_big"].toString(); - } - else if (obj_album.contains("cover_medium")) { - cover = obj_album["cover_medium"].toString(); - } - else if (obj_album.contains("cover_small")) { - cover = obj_album["cover_small"].toString(); - } - else { - Error("Invalid Json reply, data array value album object is missing cover.", obj_album); - continue; - } - QUrl url(cover); - - album.remove(Song::kAlbumRemoveDisc); - album.remove(Song::kAlbumRemoveMisc); + album = album.remove(Song::kAlbumRemoveDisc); + album = album.remove(Song::kAlbumRemoveMisc); CoverSearchResult cover_result; cover_result.artist = artist; cover_result.album = album; - cover_result.image_url = url; - results << cover_result; + + bool have_cover = false; + QList> cover_sizes = QList>() << qMakePair(QString("cover_xl"), QSize(1000, 1000)) + << qMakePair(QString("cover_big"), QSize(500, 500)); + for (const QPair &cover_size : cover_sizes) { + if (!obj_album.contains(cover_size.first)) continue; + QString cover = obj_album[cover_size.first].toString(); + if (!have_cover) { + have_cover = true; + ++i; + } + QUrl url(cover); + if (!results.contains(url)) { + cover_result.image_url = url; + cover_result.image_size = cover_size.second; + cover_result.number = i; + results.insert(url, cover_result); + } + } + + if (!have_cover) { + Error("Invalid Json reply, data array value album object is missing cover.", obj_album); + } } - emit SearchFinished(id, results); + + if (results.isEmpty()) { + emit SearchFinished(id, CoverSearchResults()); + } + else { + CoverSearchResults cover_results = results.values(); + std::stable_sort(cover_results.begin(), cover_results.end(), AlbumCoverFetcherSearch::CoverSearchResultCompareNumber); + emit SearchFinished(id, cover_results); + } } diff --git a/src/covermanager/lastfmcoverprovider.cpp b/src/covermanager/lastfmcoverprovider.cpp index d55926766..573292a2f 100644 --- a/src/covermanager/lastfmcoverprovider.cpp +++ b/src/covermanager/lastfmcoverprovider.cpp @@ -275,6 +275,7 @@ void LastFmCoverProvider::QueryFinished(QNetworkReply *reply, const int id, cons cover_result.artist = artist; cover_result.album = album; cover_result.image_url = url; + cover_result.image_size = QSize(size, size); results << cover_result; } emit SearchFinished(id, results); diff --git a/src/covermanager/lastfmcoverprovider.h b/src/covermanager/lastfmcoverprovider.h index 482cf6a75..a9a8a9721 100644 --- a/src/covermanager/lastfmcoverprovider.h +++ b/src/covermanager/lastfmcoverprovider.h @@ -50,10 +50,10 @@ class LastFmCoverProvider : public JsonCoverProvider { private: enum LastFmImageSize { Unknown, - Small, - Medium, - Large, - ExtraLarge + Small = 34, + Medium = 64, + Large = 174, + ExtraLarge = 300 }; QByteArray GetReplyData(QNetworkReply *reply); diff --git a/src/covermanager/musixmatchcoverprovider.cpp b/src/covermanager/musixmatchcoverprovider.cpp index e8155a8fb..4f56dd46a 100644 --- a/src/covermanager/musixmatchcoverprovider.cpp +++ b/src/covermanager/musixmatchcoverprovider.cpp @@ -196,36 +196,27 @@ void MusixmatchCoverProvider::HandleSearchReply(QNetworkReply *reply, const int return; } - QString cover; - - if (obj_album.contains("coverart800x800")) { - cover = obj_album["coverart800x800"].toString(); - } - else if (obj_album.contains("coverart500x500")) { - cover = obj_album["coverart500x500"].toString(); - } - else if (obj_album.contains("coverart350x350")) { - cover = obj_album["coverart350x350"].toString(); - } - - if (cover.isEmpty()) { - emit SearchFinished(id, results); - return; - } - QUrl cover_url(cover); - if (!cover_url.isValid()) { - Error("Received cover url is not valid.", cover); - emit SearchFinished(id, results); - return; - } - CoverSearchResult result; result.artist = obj_album["artistName"].toString(); result.album = obj_album["name"].toString(); - result.image_url = cover_url; - if (artist.toLower() == result.artist.toLower() || album.toLower() == result.album.toLower()) { - results.append(result); + if (artist.toLower() != result.artist.toLower() && album.toLower() != result.album.toLower()) { + emit SearchFinished(id, results); + return; + } + + QList> cover_sizes = QList>() << qMakePair(QString("coverart800x800"), QSize(800, 800)) + << qMakePair(QString("coverart500x500"), QSize(500, 500)) + << qMakePair(QString("coverart350x350"), QSize(300, 300)); + + for (const QPair &cover_size : cover_sizes) { + if (!obj_album.contains(cover_size.first)) continue; + QUrl cover_url(obj_album[cover_size.first].toString()); + if (cover_url.isValid()) { + result.image_url = cover_url; + result.image_size = cover_size.second; + results << result; + } } emit SearchFinished(id, results); diff --git a/src/covermanager/qobuzcoverprovider.cpp b/src/covermanager/qobuzcoverprovider.cpp index c50f39634..958e97f57 100644 --- a/src/covermanager/qobuzcoverprovider.cpp +++ b/src/covermanager/qobuzcoverprovider.cpp @@ -493,13 +493,14 @@ void QobuzCoverProvider::HandleSearchReply(QNetworkReply *reply, const int id) { } QUrl cover_url(obj_image["large"].toString()); - album.remove(Song::kAlbumRemoveDisc); - album.remove(Song::kAlbumRemoveMisc); + album = album.remove(Song::kAlbumRemoveDisc); + album = album.remove(Song::kAlbumRemoveMisc); CoverSearchResult cover_result; cover_result.artist = artist; cover_result.album = album; cover_result.image_url = cover_url; + cover_result.image_size = QSize(600, 600); results << cover_result; } diff --git a/src/covermanager/spotifycoverprovider.cpp b/src/covermanager/spotifycoverprovider.cpp index 16d974d86..9502f2578 100644 --- a/src/covermanager/spotifycoverprovider.cpp +++ b/src/covermanager/spotifycoverprovider.cpp @@ -526,6 +526,7 @@ void SpotifyCoverProvider::HandleSearchReply(QNetworkReply *reply, const int id, CoverSearchResult result; result.album = album; result.image_url = url; + result.image_size = QSize(width, height); if (!artists.isEmpty()) result.artist = artists.first(); results << result; } diff --git a/src/covermanager/tidalcoverprovider.cpp b/src/covermanager/tidalcoverprovider.cpp index 9821de01a..f85fa11ae 100644 --- a/src/covermanager/tidalcoverprovider.cpp +++ b/src/covermanager/tidalcoverprovider.cpp @@ -177,37 +177,37 @@ void TidalCoverProvider::HandleSearchReply(QNetworkReply *reply, const int id) { disconnect(reply, nullptr, this, nullptr); reply->deleteLater(); - CoverSearchResults results; - QByteArray data = GetReplyData(reply); if (data.isEmpty()) { - emit SearchFinished(id, results); + emit SearchFinished(id, CoverSearchResults()); return; } QJsonObject json_obj = ExtractJsonObj(data); if (json_obj.isEmpty()) { - emit SearchFinished(id, results); + emit SearchFinished(id, CoverSearchResults()); return; } if (!json_obj.contains("items")) { Error("Json object is missing items.", json_obj); - emit SearchFinished(id, results); + emit SearchFinished(id, CoverSearchResults()); return; } QJsonValue value_items = json_obj["items"]; if (!value_items.isArray()) { - emit SearchFinished(id, results); + emit SearchFinished(id, CoverSearchResults()); return; } QJsonArray array_items = value_items.toArray(); if (array_items.isEmpty()) { - emit SearchFinished(id, results); + emit SearchFinished(id, CoverSearchResults()); return; } + CoverSearchResults results; + int i = 0; for (const QJsonValue &value_item : array_items) { if (!value_item.isObject()) { @@ -256,15 +256,22 @@ void TidalCoverProvider::HandleSearchReply(QNetworkReply *reply, const int id) { album = album.remove(Song::kAlbumRemoveDisc); album = album.remove(Song::kAlbumRemoveMisc); - cover = cover.replace("-", "/"); - QUrl cover_url (QString("%1/images/%2/%3.jpg").arg(kResourcesUrl).arg(cover).arg("1280x1280")); CoverSearchResult cover_result; cover_result.artist = artist; cover_result.album = album; - cover_result.image_url = cover_url; - results << cover_result; + cover_result.number = ++i; + + QList> cover_sizes = QList>() << qMakePair(QString("1280x1280"), QSize(1280, 1280)) + << qMakePair(QString("750x750"), QSize(750, 750)) + << qMakePair(QString("640x640"), QSize(640, 640)); + for (const QPair &cover_size : cover_sizes) { + QUrl cover_url(QString("%1/images/%2/%3.jpg").arg(kResourcesUrl).arg(cover).arg(cover_size.first)); + cover_result.image_url = cover_url; + cover_result.image_size = cover_size.second; + results << cover_result; + } } emit SearchFinished(id, results);