First consider album artists when multiple names are used for the same artist mbid and no artist info is present, ref #731

This commit is contained in:
emeric
2025-11-16 11:14:20 +01:00
parent 51bd4c75ac
commit ab898b697c
7 changed files with 137 additions and 47 deletions
+32
View File
@@ -23,6 +23,37 @@
namespace lms::db 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<Bitrate> allowedAudioBitrates{ static const std::set<Bitrate> allowedAudioBitrates{
64000, 64000,
96000, 96000,
@@ -41,4 +72,5 @@ namespace lms::db
{ {
return allowedAudioBitrates.find(bitrate) != std::cend(allowedAudioBitrates); return allowedAudioBitrates.find(bitrate) != std::cend(allowedAudioBitrates);
} }
} // namespace lms::db } // namespace lms::db
@@ -59,6 +59,9 @@ namespace lms::db
query.where("t.release_id = ?").bind(params.release); 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) switch (params.sortMethod)
{ {
case TrackArtistLinkSortMethod::None: case TrackArtistLinkSortMethod::None:
@@ -27,6 +27,7 @@
#include <Wt/WDate.h> #include <Wt/WDate.h>
#include "core/Exception.hpp" #include "core/Exception.hpp"
#include "core/LiteralString.hpp"
#include "core/TaggedType.hpp" #include "core/TaggedType.hpp"
namespace lms::db namespace lms::db
@@ -280,6 +281,8 @@ namespace lms::db
Writer = 10, Writer = 10,
}; };
core::LiteralString trackArtistLinkTypeToString(TrackArtistLinkType type);
// User selectable transcoding output formats // User selectable transcoding output formats
enum class TranscodingOutputFormat enum class TranscodingOutputFormat
{ {
@@ -51,6 +51,7 @@ namespace lms::db
ArtistId artist; // if set, links involved with this artist ArtistId artist; // if set, links involved with this artist
ReleaseId release; // if set, artists involved in this release ReleaseId release; // if set, artists involved in this release
TrackId track; // if set, artists involved in this track TrackId track; // if set, artists involved in this track
std::optional<bool> mbidMatched;
TrackArtistLinkSortMethod sortMethod{ TrackArtistLinkSortMethod::None }; TrackArtistLinkSortMethod sortMethod{ TrackArtistLinkSortMethod::None };
FindParameters& setRange(std::optional<Range> _range) FindParameters& setRange(std::optional<Range> _range)
@@ -78,6 +79,11 @@ namespace lms::db
track = _track; track = _track;
return *this; return *this;
} }
FindParameters& setMBIDMatched(std::optional<bool> _mbidMatched)
{
mbidMatched = _mbidMatched;
return *this;
}
FindParameters& setSortMethod(TrackArtistLinkSortMethod _method) FindParameters& setSortMethod(TrackArtistLinkSortMethod _method)
{ {
sortMethod = _method; sortMethod = _method;
@@ -222,4 +222,57 @@ namespace lms::db::tests
EXPECT_EQ(links[1]->getArtistName(), "MyArtistOldName"); 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<TrackArtistLink>(track1.get(), artist.get(), TrackArtistLinkType::Artist, false) };
auto link2{ session.create<TrackArtistLink>(track2.get(), artist.get(), TrackArtistLinkType::Artist, true) };
}
{
auto transaction{ session.createReadTransaction() };
TrackArtistLink::FindParameters params;
std::vector<TrackArtistLink::pointer> 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<TrackArtistLink::pointer> 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<TrackArtistLink::pointer> 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 } // namespace lms::db::tests
@@ -23,6 +23,7 @@
#include <ostream> #include <ostream>
#include "core/ILogger.hpp" #include "core/ILogger.hpp"
#include "database/IDb.hpp" #include "database/IDb.hpp"
#include "database/Session.hpp" #include "database/Session.hpp"
#include "database/objects/Artist.hpp" #include "database/objects/Artist.hpp"
@@ -73,6 +74,24 @@ namespace lms::scanner
assert(newArtist != artistInfo->getArtist()); assert(newArtist != artistInfo->getArtist());
artistInfo.modify()->setArtist(newArtist); artistInfo.modify()->setArtist(newArtist);
} }
db::TrackArtistLink::pointer getMostRecentMBIDArtistLink(db::Session& session, db::ArtistId artistId, std::optional<db::TrackArtistLinkType> 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 } // namespace
bool ScanStepArtistReconciliation::needProcess([[maybe_unused]] const ScanContext& context) const 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 // - artist name changed over time (ex: Rhapsody then Rhapsody of Fire), legit use case
// - user renamed the artist // - user renamed the artist
// Name to pick in order of priority: // Name to pick in order of priority:
// - name specified in artist info (if present) // - name specified in artist info
// - name as referenced in the latest release // - name as referenced in the latest release of the artist
// - name as referenced in the latest link (any type)
struct ArtistToUpdate struct ArtistToUpdate
{ {
@@ -153,26 +173,24 @@ namespace lms::scanner
if (hasArtistInfo) if (hasArtistInfo)
continue; continue;
std::optional<ArtistToUpdate> artistToUpdate; db::TrackArtistLink::pointer artistMostRecentLink{ getMostRecentMBIDArtistLink(session, artist->getId(), db::TrackArtistLinkType::ReleaseArtist) };
if (!artistMostRecentLink)
artistMostRecentLink = getMostRecentMBIDArtistLink(session, artist->getId());
db::TrackArtistLink::FindParameters params; if (!artistMostRecentLink)
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)
{ {
LMS_LOG(DBUPDATER, DEBUG, "Updating artist " << artist << " name to '" << artistToUpdate->newName << "' using most recent release reference"); LMS_LOG(DBUPDATER, DEBUG, "Unable to fix name discrepancy for artist " << artist << ": no link found!");
artistsToUpdate.emplace_back(std::move(*artistToUpdate)); 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");
} }
} }
+2 -27
View File
@@ -22,6 +22,7 @@
#include "core/ITraceLogger.hpp" #include "core/ITraceLogger.hpp"
#include "core/Service.hpp" #include "core/Service.hpp"
#include "core/String.hpp" #include "core/String.hpp"
#include "database/objects/Artist.hpp" #include "database/objects/Artist.hpp"
#include "database/objects/Artwork.hpp" #include "database/objects/Artwork.hpp"
#include "database/objects/Release.hpp" #include "database/objects/Release.hpp"
@@ -57,33 +58,7 @@ namespace lms::api::subsonic
std::string_view toString(TrackArtistLinkType type) std::string_view toString(TrackArtistLinkType type)
{ {
switch (type) return db::trackArtistLinkTypeToString(type).str();
{
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";
} }
} // namespace utils } // namespace utils