From ab898b697c7b1438794c31cb4b458730b2b2b293 Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 16 Nov 2025 11:14:20 +0100 Subject: [PATCH] First consider album artists when multiple names are used for the same artist mbid and no artist info is present, ref #731 --- src/libs/database/impl/Types.cpp | 32 ++++++++++ .../database/impl/objects/TrackArtistLink.cpp | 3 + src/libs/database/include/database/Types.hpp | 3 + .../database/objects/TrackArtistLink.hpp | 6 ++ src/libs/database/test/TrackArtistLink.cpp | 53 +++++++++++++++++ .../steps/ScanStepArtistReconciliation.cpp | 58 ++++++++++++------- src/libs/subsonic/impl/responses/Artist.cpp | 29 +--------- 7 files changed, 137 insertions(+), 47 deletions(-) diff --git a/src/libs/database/impl/Types.cpp b/src/libs/database/impl/Types.cpp index 6100b86a..f07a30fd 100644 --- a/src/libs/database/impl/Types.cpp +++ b/src/libs/database/impl/Types.cpp @@ -23,6 +23,37 @@ namespace lms::db { + core::LiteralString trackArtistLinkTypeToString(TrackArtistLinkType type) + { + switch (type) + { + case TrackArtistLinkType::Arranger: + return "arranger"; + case TrackArtistLinkType::Artist: + return "artist"; + case TrackArtistLinkType::Composer: + return "composer"; + case TrackArtistLinkType::Conductor: + return "conductor"; + case TrackArtistLinkType::Lyricist: + return "lyricist"; + case TrackArtistLinkType::Mixer: + return "mixer"; + case TrackArtistLinkType::Performer: + return "performer"; + case TrackArtistLinkType::Producer: + return "producer"; + case TrackArtistLinkType::ReleaseArtist: + return "albumartist"; + case TrackArtistLinkType::Remixer: + return "remixer"; + case TrackArtistLinkType::Writer: + return "writer"; + } + + return "unknown"; + } + static const std::set allowedAudioBitrates{ 64000, 96000, @@ -41,4 +72,5 @@ namespace lms::db { return allowedAudioBitrates.find(bitrate) != std::cend(allowedAudioBitrates); } + } // namespace lms::db diff --git a/src/libs/database/impl/objects/TrackArtistLink.cpp b/src/libs/database/impl/objects/TrackArtistLink.cpp index f0bfd591..462f60f7 100644 --- a/src/libs/database/impl/objects/TrackArtistLink.cpp +++ b/src/libs/database/impl/objects/TrackArtistLink.cpp @@ -59,6 +59,9 @@ namespace lms::db query.where("t.release_id = ?").bind(params.release); } + if (params.mbidMatched) + query.where("t_a_l.artist_mbid_matched = ?").bind(*params.mbidMatched); + switch (params.sortMethod) { case TrackArtistLinkSortMethod::None: diff --git a/src/libs/database/include/database/Types.hpp b/src/libs/database/include/database/Types.hpp index ac0350f0..37bfcfa4 100644 --- a/src/libs/database/include/database/Types.hpp +++ b/src/libs/database/include/database/Types.hpp @@ -27,6 +27,7 @@ #include #include "core/Exception.hpp" +#include "core/LiteralString.hpp" #include "core/TaggedType.hpp" namespace lms::db @@ -280,6 +281,8 @@ namespace lms::db Writer = 10, }; + core::LiteralString trackArtistLinkTypeToString(TrackArtistLinkType type); + // User selectable transcoding output formats enum class TranscodingOutputFormat { diff --git a/src/libs/database/include/database/objects/TrackArtistLink.hpp b/src/libs/database/include/database/objects/TrackArtistLink.hpp index 03527f64..f4e8406d 100644 --- a/src/libs/database/include/database/objects/TrackArtistLink.hpp +++ b/src/libs/database/include/database/objects/TrackArtistLink.hpp @@ -51,6 +51,7 @@ namespace lms::db ArtistId artist; // if set, links involved with this artist ReleaseId release; // if set, artists involved in this release TrackId track; // if set, artists involved in this track + std::optional mbidMatched; TrackArtistLinkSortMethod sortMethod{ TrackArtistLinkSortMethod::None }; FindParameters& setRange(std::optional _range) @@ -78,6 +79,11 @@ namespace lms::db track = _track; return *this; } + FindParameters& setMBIDMatched(std::optional _mbidMatched) + { + mbidMatched = _mbidMatched; + return *this; + } FindParameters& setSortMethod(TrackArtistLinkSortMethod _method) { sortMethod = _method; diff --git a/src/libs/database/test/TrackArtistLink.cpp b/src/libs/database/test/TrackArtistLink.cpp index e827cc31..27d1a487 100644 --- a/src/libs/database/test/TrackArtistLink.cpp +++ b/src/libs/database/test/TrackArtistLink.cpp @@ -222,4 +222,57 @@ namespace lms::db::tests EXPECT_EQ(links[1]->getArtistName(), "MyArtistOldName"); } } + + TEST_F(DatabaseFixture, TrackArtistLink_findWithMBIDMatched) + { + ScopedArtist artist{ session, "MyArtist", core::UUID::fromString("97d1fb6f-db09-4760-b0b3-816559bcb632") }; + ScopedTrack track1{ session }; + ScopedTrack track2{ session }; + + { + auto transaction{ session.createWriteTransaction() }; + auto link1{ session.create(track1.get(), artist.get(), TrackArtistLinkType::Artist, false) }; + auto link2{ session.create(track2.get(), artist.get(), TrackArtistLinkType::Artist, true) }; + } + + { + auto transaction{ session.createReadTransaction() }; + + TrackArtistLink::FindParameters params; + + std::vector links; + TrackArtistLink::find(session, params, [&](const TrackArtistLink::pointer& link) { + links.push_back(link); + }); + ASSERT_EQ(links.size(), 2); + } + + { + auto transaction{ session.createReadTransaction() }; + + TrackArtistLink::FindParameters params; + params.setMBIDMatched(false); + + std::vector links; + TrackArtistLink::find(session, params, [&](const TrackArtistLink::pointer& link) { + links.push_back(link); + }); + ASSERT_EQ(links.size(), 1); + EXPECT_EQ(links[0]->getTrack()->getId(), track1.getId()); + } + + { + auto transaction{ session.createReadTransaction() }; + + TrackArtistLink::FindParameters params; + params.setMBIDMatched(true); + + std::vector links; + TrackArtistLink::find(session, params, [&](const TrackArtistLink::pointer& link) { + links.push_back(link); + }); + ASSERT_EQ(links.size(), 1); + EXPECT_EQ(links[0]->getTrack()->getId(), track2.getId()); + } + } } // namespace lms::db::tests \ No newline at end of file diff --git a/src/libs/services/scanner/impl/steps/ScanStepArtistReconciliation.cpp b/src/libs/services/scanner/impl/steps/ScanStepArtistReconciliation.cpp index f7df576c..3b095f18 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepArtistReconciliation.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepArtistReconciliation.cpp @@ -23,6 +23,7 @@ #include #include "core/ILogger.hpp" + #include "database/IDb.hpp" #include "database/Session.hpp" #include "database/objects/Artist.hpp" @@ -73,6 +74,24 @@ namespace lms::scanner assert(newArtist != artistInfo->getArtist()); artistInfo.modify()->setArtist(newArtist); } + + db::TrackArtistLink::pointer getMostRecentMBIDArtistLink(db::Session& session, db::ArtistId artistId, std::optional linkType = std::nullopt) + { + db::TrackArtistLink::FindParameters params; + params.setArtist(artistId); + params.setLinkType(linkType); + params.setSortMethod(db::TrackArtistLinkSortMethod::OriginalDateDesc); + params.setMBIDMatched(true); + params.setRange(db::Range{ .offset = 0, .size = 1 }); + + db::TrackArtistLink::pointer foundLink; + db::TrackArtistLink::find(session, params, [&](const db::TrackArtistLink::pointer& link) { + foundLink = link; + }); + + return foundLink; + } + } // namespace bool ScanStepArtistReconciliation::needProcess([[maybe_unused]] const ScanContext& context) const @@ -112,8 +131,9 @@ namespace lms::scanner // - artist name changed over time (ex: Rhapsody then Rhapsody of Fire), legit use case // - user renamed the artist // Name to pick in order of priority: - // - name specified in artist info (if present) - // - name as referenced in the latest release + // - name specified in artist info + // - name as referenced in the latest release of the artist + // - name as referenced in the latest link (any type) struct ArtistToUpdate { @@ -153,26 +173,24 @@ namespace lms::scanner if (hasArtistInfo) continue; - std::optional artistToUpdate; + db::TrackArtistLink::pointer artistMostRecentLink{ getMostRecentMBIDArtistLink(session, artist->getId(), db::TrackArtistLinkType::ReleaseArtist) }; + if (!artistMostRecentLink) + artistMostRecentLink = getMostRecentMBIDArtistLink(session, artist->getId()); - db::TrackArtistLink::FindParameters params; - params.setArtist(artist->getId()); - params.setSortMethod(db::TrackArtistLinkSortMethod::OriginalDateDesc); - params.setRange(db::Range{ .offset = 0, .size = 1 }); - db::TrackArtistLink::find(session, params, [&](const db::TrackArtistLink::pointer& link) { - if (link->getArtistName() != artist->getName()) - { - artistToUpdate.emplace(); - artistToUpdate->artist = artist; - artistToUpdate->newName = link->getArtistName(); - artistToUpdate->newSortName = link->getArtistSortName(); - } - }); - - if (artistToUpdate) + if (!artistMostRecentLink) { - LMS_LOG(DBUPDATER, DEBUG, "Updating artist " << artist << " name to '" << artistToUpdate->newName << "' using most recent release reference"); - artistsToUpdate.emplace_back(std::move(*artistToUpdate)); + LMS_LOG(DBUPDATER, DEBUG, "Unable to fix name discrepancy for artist " << artist << ": no link found!"); + continue; + } + + if (artistMostRecentLink->getArtistName() != artist->getName()) + { + ArtistToUpdate& artistToUpdate{ artistsToUpdate.emplace_back() }; + artistToUpdate.artist = artist; + artistToUpdate.newName = artistMostRecentLink->getArtistName(); + artistToUpdate.newSortName = artistMostRecentLink->getArtistSortName(); + + LMS_LOG(DBUPDATER, DEBUG, "Updating artist " << artist << " name to '" << artistToUpdate.newName << "' using most recent '" << db::trackArtistLinkTypeToString(artistMostRecentLink->getType()) << "' link reference"); } } diff --git a/src/libs/subsonic/impl/responses/Artist.cpp b/src/libs/subsonic/impl/responses/Artist.cpp index a54da524..af4e88b4 100644 --- a/src/libs/subsonic/impl/responses/Artist.cpp +++ b/src/libs/subsonic/impl/responses/Artist.cpp @@ -22,6 +22,7 @@ #include "core/ITraceLogger.hpp" #include "core/Service.hpp" #include "core/String.hpp" + #include "database/objects/Artist.hpp" #include "database/objects/Artwork.hpp" #include "database/objects/Release.hpp" @@ -57,33 +58,7 @@ namespace lms::api::subsonic std::string_view toString(TrackArtistLinkType type) { - switch (type) - { - case TrackArtistLinkType::Arranger: - return "arranger"; - case TrackArtistLinkType::Artist: - return "artist"; - case TrackArtistLinkType::Composer: - return "composer"; - case TrackArtistLinkType::Conductor: - return "conductor"; - case TrackArtistLinkType::Lyricist: - return "lyricist"; - case TrackArtistLinkType::Mixer: - return "mixer"; - case TrackArtistLinkType::Performer: - return "performer"; - case TrackArtistLinkType::Producer: - return "producer"; - case TrackArtistLinkType::ReleaseArtist: - return "albumartist"; - case TrackArtistLinkType::Remixer: - return "remixer"; - case TrackArtistLinkType::Writer: - return "writer"; - } - - return "unknown"; + return db::trackArtistLinkTypeToString(type).str(); } } // namespace utils