diff --git a/src/libs/metadata/impl/Parser.cpp b/src/libs/metadata/impl/Parser.cpp index 564ce5d6..9cd9b38f 100644 --- a/src/libs/metadata/impl/Parser.cpp +++ b/src/libs/metadata/impl/Parser.cpp @@ -266,6 +266,54 @@ 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 createParser(ParserBackend parserBackend, ParserReadStyle parserReadStyle) @@ -416,6 +464,8 @@ namespace lms::metadata track.remixerArtists = getArtists(tagReader, { TagType::Remixers, TagType::Remixer }, { TagType::RemixersSortOrder, TagType::RemixerSortOrder }, {}, _artistTagDelimiters, _defaultTagDelimiters); 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/Parser.cpp b/src/libs/metadata/test/Parser.cpp index 7d0a5e14..912d53a5 100644 --- a/src/libs/metadata/test/Parser.cpp +++ b/src/libs/metadata/test/Parser.cpp @@ -584,6 +584,131 @@ namespace lms::metadata EXPECT_EQ(track->artistDisplayName, "Artist1, Artist2"); // reconstruct the artist display name } + TEST(Parser, 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{ Parser{}.parse(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(Parser, 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{ Parser{}.parse(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(Parser, advisory) { auto doTest = [](std::string_view value, std::optional expectedValue) { @@ -657,5 +782,4 @@ namespace lms::metadata doTest("2020/01", core::PartialDateTime{ 2020, 1 }); doTest("2020", core::PartialDateTime{ 2020 }); } - } // namespace lms::metadata