From 6681f8b7535b3d67ee58aa4222bed41209ef8c1f Mon Sep 17 00:00:00 2001 From: emeric Date: Thu, 27 Mar 2025 16:49:52 +0100 Subject: [PATCH] Moved fallback code from metadata lib to scanner --- src/libs/metadata/impl/AudioFileParser.cpp | 50 ------- src/libs/metadata/test/AudioFileParser.cpp | 125 ------------------ .../impl/scanners/AudioFileScanner.cpp | 51 +++++++ 3 files changed, 51 insertions(+), 175 deletions(-) diff --git a/src/libs/metadata/impl/AudioFileParser.cpp b/src/libs/metadata/impl/AudioFileParser.cpp index b0549595..34f74a8c 100644 --- a/src/libs/metadata/impl/AudioFileParser.cpp +++ b/src/libs/metadata/impl/AudioFileParser.cpp @@ -267,54 +267,6 @@ namespace lms::metadata return std::nullopt; } - - void fillInArtistsWithMbid(std::span artists, std::unordered_map& artistsWithMbid) - { - for (const Artist& artist : artists) - { - if (artist.mbid.has_value()) - { - // there may collisions, we don't want to replace - artistsWithMbid.emplace(artist.name, *artist.mbid); - } - } - } - - void fillInMbids(std::span artists, const std::unordered_map& artistsWithMbid) - { - for (Artist& artist : artists) - { - if (!artist.mbid) - { - const auto it{ artistsWithMbid.find(artist.name) }; - if (it != std::cend(artistsWithMbid)) - artist.mbid = it->second; - } - } - } - - void fillMissingMbids(Track& track) - { - // first pass: collect all artists that have mbids - std::unordered_map artistsWithMbid; - - // For now, mbids can only set in artist and album artist tags - // filling order is important: we estimate track-level artists are more likely - // to be set in other fields than album artists - fillInArtistsWithMbid(track.artists, artistsWithMbid); - if (track.medium && track.medium->release) - fillInArtistsWithMbid(track.medium->release->artists, artistsWithMbid); - - // second pass: fill in all artists that have no mbid set with the same name - fillInMbids(track.conductorArtists, artistsWithMbid); - fillInMbids(track.composerArtists, artistsWithMbid); - fillInMbids(track.lyricistArtists, artistsWithMbid); - fillInMbids(track.mixerArtists, artistsWithMbid); - fillInMbids(track.producerArtists, artistsWithMbid); - fillInMbids(track.remixerArtists, artistsWithMbid); - for (auto& [role, artists] : track.performerArtists) - fillInMbids(artists, artistsWithMbid); - } } // namespace std::unique_ptr createAudioFileParser(const AudioFileParserParameters& params) @@ -489,8 +441,6 @@ namespace lms::metadata track.remixerArtists = getArtists(tagReader, { TagType::Remixers, TagType::Remixer }, { TagType::RemixersSortOrder, TagType::RemixerSortOrder }, {}, _params); track.performerArtists = getPerformerArtists(tagReader); // artistDelimiters not supported - fillMissingMbids(track); - // If a file has originalDate but no originalYear, set it if (!track.originalYear) track.originalYear = track.originalDate.getYear(); diff --git a/src/libs/metadata/test/AudioFileParser.cpp b/src/libs/metadata/test/AudioFileParser.cpp index 154e68cb..20fe34cb 100644 --- a/src/libs/metadata/test/AudioFileParser.cpp +++ b/src/libs/metadata/test/AudioFileParser.cpp @@ -605,131 +605,6 @@ namespace lms::metadata EXPECT_EQ(track->artistDisplayName, "Artist1, Artist2"); // reconstruct the artist display name } - TEST(AudioFileParser, MBIDs_fallback) - { - TestTagReader testTags{ - { - { TagType::Artist, { "Artist1", "Artist2" } }, - { TagType::Album, { "MyAlbum" } }, - { TagType::AlbumArtists, { "Artist3", "Artist4" } }, - { TagType::MusicBrainzArtistID, { "6643f584-5edc-45ce-927d-0a4ab25c2673", "481c5912-bf1a-47f7-b03c-d34e49711706" } }, - { TagType::MusicBrainzReleaseArtistID, { "ed42bcaf-e147-4f34-8f26-d74acc97670a", "6fc64a4b-26f5-441f-993c-fd511290233b" } }, - { TagType::Composer, { "Artist1", "Artist3" } }, - { TagType::Conductor, { "Artist1", "Artist3" } }, - { TagType::Lyricist, { "Artist1", "Artist3" } }, - { TagType::Mixer, { "Artist1", "Artist3" } }, - { TagType::Producer, { "Artist1", "Artist3" } }, - { TagType::Remixers, { "Artist1", "Artist3" } }, - } - }; - - testTags.setPerformersTags({ { "RoleA", { "Artist1", "Artist3" } }, - { "RoleB", { "Artist2", "Artist4" } } }); - std::unique_ptr track{ TestAudioFileParser{}.parseMetaData(testTags) }; - - ASSERT_EQ(track->artists.size(), 2); - EXPECT_EQ(track->artists[0].name, "Artist1"); - ASSERT_TRUE(track->artists[0].mbid.has_value()); - EXPECT_EQ(track->artists[0].mbid.value(), core::UUID::fromString("6643f584-5edc-45ce-927d-0a4ab25c2673")); - EXPECT_EQ(track->artists[1].name, "Artist2"); - ASSERT_TRUE(track->artists[1].mbid.has_value()); - EXPECT_EQ(track->artists[1].mbid.value(), core::UUID::fromString("481c5912-bf1a-47f7-b03c-d34e49711706")); - - ASSERT_TRUE(track->medium.has_value()); - ASSERT_TRUE(track->medium->release.has_value()); - ASSERT_EQ(track->medium->release->artists.size(), 2); - EXPECT_EQ(track->medium->release->artists[0].name, "Artist3"); - ASSERT_TRUE(track->medium->release->artists[0].mbid.has_value()); - EXPECT_EQ(track->medium->release->artists[0].mbid.value(), core::UUID::fromString("ed42bcaf-e147-4f34-8f26-d74acc97670a")); - EXPECT_EQ(track->medium->release->artists[1].name, "Artist4"); - ASSERT_TRUE(track->medium->release->artists[1].mbid.has_value()); - EXPECT_EQ(track->medium->release->artists[1].mbid.value(), core::UUID::fromString("6fc64a4b-26f5-441f-993c-fd511290233b")); - - ASSERT_EQ(track->composerArtists.size(), 2); - EXPECT_EQ(track->composerArtists[0].name, "Artist1"); - ASSERT_TRUE(track->composerArtists[0].mbid.has_value()); - EXPECT_EQ(track->composerArtists[0].mbid.value(), core::UUID::fromString("6643f584-5edc-45ce-927d-0a4ab25c2673")); - EXPECT_EQ(track->composerArtists[1].name, "Artist3"); - ASSERT_TRUE(track->composerArtists[1].mbid.has_value()); - EXPECT_EQ(track->composerArtists[1].mbid.value(), core::UUID::fromString("ed42bcaf-e147-4f34-8f26-d74acc97670a")); - - ASSERT_EQ(track->conductorArtists.size(), 2); - EXPECT_EQ(track->conductorArtists[0].name, "Artist1"); - ASSERT_TRUE(track->conductorArtists[0].mbid.has_value()); - EXPECT_EQ(track->conductorArtists[0].mbid.value(), core::UUID::fromString("6643f584-5edc-45ce-927d-0a4ab25c2673")); - EXPECT_EQ(track->conductorArtists[1].name, "Artist3"); - ASSERT_TRUE(track->conductorArtists[1].mbid.has_value()); - EXPECT_EQ(track->conductorArtists[1].mbid.value(), core::UUID::fromString("ed42bcaf-e147-4f34-8f26-d74acc97670a")); - - ASSERT_EQ(track->lyricistArtists.size(), 2); - EXPECT_EQ(track->lyricistArtists[0].name, "Artist1"); - ASSERT_TRUE(track->lyricistArtists[0].mbid.has_value()); - EXPECT_EQ(track->lyricistArtists[0].mbid.value(), core::UUID::fromString("6643f584-5edc-45ce-927d-0a4ab25c2673")); - EXPECT_EQ(track->lyricistArtists[1].name, "Artist3"); - ASSERT_TRUE(track->lyricistArtists[1].mbid.has_value()); - EXPECT_EQ(track->lyricistArtists[1].mbid.value(), core::UUID::fromString("ed42bcaf-e147-4f34-8f26-d74acc97670a")); - - ASSERT_EQ(track->mixerArtists.size(), 2); - EXPECT_EQ(track->mixerArtists[0].name, "Artist1"); - ASSERT_TRUE(track->mixerArtists[0].mbid.has_value()); - EXPECT_EQ(track->mixerArtists[0].mbid.value(), core::UUID::fromString("6643f584-5edc-45ce-927d-0a4ab25c2673")); - EXPECT_EQ(track->mixerArtists[1].name, "Artist3"); - ASSERT_TRUE(track->mixerArtists[1].mbid.has_value()); - EXPECT_EQ(track->mixerArtists[1].mbid.value(), core::UUID::fromString("ed42bcaf-e147-4f34-8f26-d74acc97670a")); - - ASSERT_EQ(track->producerArtists.size(), 2); - EXPECT_EQ(track->producerArtists[0].name, "Artist1"); - ASSERT_TRUE(track->producerArtists[0].mbid.has_value()); - EXPECT_EQ(track->producerArtists[0].mbid.value(), core::UUID::fromString("6643f584-5edc-45ce-927d-0a4ab25c2673")); - EXPECT_EQ(track->producerArtists[1].name, "Artist3"); - ASSERT_TRUE(track->producerArtists[1].mbid.has_value()); - EXPECT_EQ(track->producerArtists[1].mbid.value(), core::UUID::fromString("ed42bcaf-e147-4f34-8f26-d74acc97670a")); - - ASSERT_EQ(track->remixerArtists.size(), 2); - EXPECT_EQ(track->remixerArtists[0].name, "Artist1"); - ASSERT_TRUE(track->remixerArtists[0].mbid.has_value()); - EXPECT_EQ(track->remixerArtists[0].mbid.value(), core::UUID::fromString("6643f584-5edc-45ce-927d-0a4ab25c2673")); - EXPECT_EQ(track->remixerArtists[1].name, "Artist3"); - ASSERT_TRUE(track->remixerArtists[1].mbid.has_value()); - EXPECT_EQ(track->remixerArtists[1].mbid.value(), core::UUID::fromString("ed42bcaf-e147-4f34-8f26-d74acc97670a")); - - ASSERT_TRUE(track->performerArtists.contains("Rolea")); - ASSERT_EQ(track->performerArtists["Rolea"].size(), 2); - EXPECT_EQ(track->performerArtists["Rolea"][0].name, "Artist1"); - ASSERT_TRUE(track->performerArtists["Rolea"][0].mbid.has_value()); - EXPECT_EQ(track->performerArtists["Rolea"][0].mbid.value(), core::UUID::fromString("6643f584-5edc-45ce-927d-0a4ab25c2673")); - EXPECT_EQ(track->performerArtists["Rolea"][1].name, "Artist3"); - ASSERT_TRUE(track->performerArtists["Rolea"][1].mbid.has_value()); - EXPECT_EQ(track->performerArtists["Rolea"][1].mbid.value(), core::UUID::fromString("ed42bcaf-e147-4f34-8f26-d74acc97670a")); - ASSERT_EQ(track->performerArtists["Roleb"].size(), 2); - EXPECT_EQ(track->performerArtists["Roleb"][0].name, "Artist2"); - ASSERT_TRUE(track->performerArtists["Roleb"][0].mbid.has_value()); - EXPECT_EQ(track->performerArtists["Roleb"][0].mbid.value(), core::UUID::fromString("481c5912-bf1a-47f7-b03c-d34e49711706")); - EXPECT_EQ(track->performerArtists["Roleb"][1].name, "Artist4"); - ASSERT_TRUE(track->performerArtists["Roleb"][1].mbid.has_value()); - EXPECT_EQ(track->performerArtists["Roleb"][1].mbid.value(), core::UUID::fromString("6fc64a4b-26f5-441f-993c-fd511290233b")); - } - - TEST(AudioFileParser, MBIDs_fallback_priority) - { - const TestTagReader testTags{ - { - { TagType::Artist, { "Artist1" } }, - { TagType::Album, { "MyAlbum" } }, - { TagType::AlbumArtists, { "Artist1" } }, - { TagType::MusicBrainzArtistID, { "6643f584-5edc-45ce-927d-0a4ab25c2673" } }, - { TagType::MusicBrainzReleaseArtistID, { "ed42bcaf-e147-4f34-8f26-d74acc97670a" } }, - { TagType::Composer, { "Artist1" } }, - } - }; - std::unique_ptr track{ TestAudioFileParser{}.parseMetaData(testTags) }; - - ASSERT_EQ(track->composerArtists.size(), 1); - EXPECT_EQ(track->composerArtists[0].name, "Artist1"); - ASSERT_TRUE(track->composerArtists[0].mbid.has_value()); - EXPECT_EQ(track->composerArtists[0].mbid.value(), core::UUID::fromString("6643f584-5edc-45ce-927d-0a4ab25c2673")); - } - TEST(AudioFileParser, release_sortNameFallback) { const TestTagReader testTags{ diff --git a/src/libs/services/scanner/impl/scanners/AudioFileScanner.cpp b/src/libs/services/scanner/impl/scanners/AudioFileScanner.cpp index 962f424e..1a0c4459 100644 --- a/src/libs/services/scanner/impl/scanners/AudioFileScanner.cpp +++ b/src/libs/services/scanner/impl/scanners/AudioFileScanner.cpp @@ -502,6 +502,54 @@ namespace lms::scanner return res; } + void fillInArtistsWithMbid(std::span artists, std::unordered_map& artistsWithMbid) + { + for (const metadata::Artist& artist : artists) + { + if (artist.mbid.has_value()) + { + // there may collisions, we don't want to replace + artistsWithMbid.emplace(artist.name, *artist.mbid); + } + } + } + + void fillInMbids(std::span artists, const std::unordered_map& artistsWithMbid) + { + for (metadata::Artist& artist : artists) + { + if (!artist.mbid) + { + const auto it{ artistsWithMbid.find(artist.name) }; + if (it != std::cend(artistsWithMbid)) + artist.mbid = it->second; + } + } + } + + void fillMissingMbids(metadata::Track& track) + { + // first pass: collect all artists that have mbids + std::unordered_map artistsWithMbid; + + // For now, mbids can only set in artist and album artist tags + // filling order is important: we estimate track-level artists are more likely + // to be set in other fields than album artists + fillInArtistsWithMbid(track.artists, artistsWithMbid); + if (track.medium && track.medium->release) + fillInArtistsWithMbid(track.medium->release->artists, artistsWithMbid); + + // second pass: fill in all artists that have no mbid set with the same name + fillInMbids(track.conductorArtists, artistsWithMbid); + fillInMbids(track.composerArtists, artistsWithMbid); + fillInMbids(track.lyricistArtists, artistsWithMbid); + fillInMbids(track.mixerArtists, artistsWithMbid); + fillInMbids(track.producerArtists, artistsWithMbid); + fillInMbids(track.remixerArtists, artistsWithMbid); + for (auto& [role, artists] : track.performerArtists) + fillInMbids(artists, artistsWithMbid); + } + class AudioFileScanOperation : public IFileScanOperation { public: @@ -541,6 +589,9 @@ namespace lms::scanner { _parsedTrack = _parser.parseMetaData(_file); + // We fill missing artist mbids with mbids found on other artist roles + fillMissingMbids(*_parsedTrack); + std::size_t index{}; _parser.parseImages(_file, [&](const metadata::Image& image) { try