From 1bca63b3f2f1dcc07adee7f7b589c44681de7777 Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 29 Sep 2024 23:05:21 +0200 Subject: [PATCH] Relaxed constraint to build the artist display name, ref #491 --- src/libs/metadata/impl/Parser.cpp | 37 ++++++++++++++++++++--------- src/libs/metadata/test/Parser.cpp | 39 +++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/src/libs/metadata/impl/Parser.cpp b/src/libs/metadata/impl/Parser.cpp index a6657c57..2a0ab2ab 100644 --- a/src/libs/metadata/impl/Parser.cpp +++ b/src/libs/metadata/impl/Parser.cpp @@ -198,26 +198,41 @@ namespace lms::metadata return std::any_of(std::cbegin(subStrs), std::cend(subStrs), [&str](const std::string& subStr) { return str.find(subStr) != std::string_view::npos; }); } - std::string computeArtistDisplayName(std::span artists, const std::optional artistTag, std::span artistTagDelimiters) + std::string computeArtistDisplayName(std::span artists, const std::optional artistTag, std::span artistsTag, std::span artistTagDelimiters) { + std::string artistDisplayName; + if (artists.size() == 1) - return artists.front().name; + artistDisplayName = artists.front().name; else if (artists.size() > 1) { std::vector artistNames; std::transform(std::cbegin(artists), std::cend(artists), std::back_inserter(artistNames), [](const Artist& artist) -> std::string_view { return artist.name; }); - // Picard use case: if we manage to match all artists in the "artist" tag (considered single-valued), and if it does not contain any custom artist delimiter, we use it as the display name + // Picard use case: if we manage to match all artists in the "artist" tag (considered single-valued), and if no custom delimiter was used, we use it as the display name // Otherwise, we reconstruct the string using a standard, hardcoded, join - if (artistTag && !strIsContainingAny(*artistTag, artistTagDelimiters) && strIsMatchingArtistNames(*artistTag, artistNames)) - return *artistTag; - else - return core::stringUtils::joinStrings(artistNames, ", "); + if (artistTag && strIsMatchingArtistNames(*artistTag, artistNames)) + { + if (artistsTag.size() == artists.size()) + { + // artists was used + if (std::none_of(std::begin(artistsTag), std::cend(artistsTag), [&](std::string_view tag) { return strIsContainingAny(tag, artistTagDelimiters); })) + artistDisplayName = *artistTag; + } + else + { + // artist was used + if (!strIsContainingAny(*artistTag, artistTagDelimiters)) + artistDisplayName = *artistTag; + } + } + + if (artistDisplayName.empty()) + artistDisplayName = core::stringUtils::joinStrings(artistNames, ", "); } - return ""; + return artistDisplayName; } - } // namespace std::unique_ptr createParser(ParserBackend parserBackend, ParserReadStyle parserReadStyle) @@ -339,7 +354,7 @@ namespace lms::metadata track.medium = getMedium(tagReader); track.artists = getArtists(tagReader, { TagType::Artists, TagType::Artist }, { TagType::ArtistSortOrder }, { TagType::MusicBrainzArtistID }, _artistTagDelimiters, _defaultTagDelimiters); - track.artistDisplayName = computeArtistDisplayName(track.artists, getTagValueAs(tagReader, TagType::Artist), _artistTagDelimiters); + track.artistDisplayName = computeArtistDisplayName(track.artists, getTagValueAs(tagReader, TagType::Artist), getTagValuesAs(tagReader, TagType::Artists, {}), _artistTagDelimiters); track.conductorArtists = getArtists(tagReader, { TagType::Conductors, TagType::Conductor }, { TagType::ConductorsSortOrder, TagType::ConductorSortOrder }, {}, _artistTagDelimiters, _defaultTagDelimiters); track.composerArtists = getArtists(tagReader, { TagType::Composers, TagType::Composer }, { TagType::ComposersSortOrder, TagType::ComposerSortOrder }, {}, _artistTagDelimiters, _defaultTagDelimiters); @@ -400,7 +415,7 @@ namespace lms::metadata release->name = std::move(*releaseName); release->sortName = getTagValueAs(tagReader, TagType::AlbumSortOrder).value_or(""); release->artists = getArtists(tagReader, { TagType::AlbumArtists, TagType::AlbumArtist }, { TagType::AlbumArtistsSortOrder, TagType::AlbumArtistSortOrder }, { TagType::MusicBrainzReleaseArtistID }, _artistTagDelimiters, _defaultTagDelimiters); - release->artistDisplayName = computeArtistDisplayName(release->artists, getTagValueAs(tagReader, TagType::AlbumArtist), _artistTagDelimiters); + release->artistDisplayName = computeArtistDisplayName(release->artists, getTagValueAs(tagReader, TagType::AlbumArtist), getTagValuesAs(tagReader, TagType::AlbumArtists, {}), _artistTagDelimiters); release->mbid = getTagValueAs(tagReader, TagType::MusicBrainzReleaseID); release->groupMBID = getTagValueAs(tagReader, TagType::MusicBrainzReleaseGroupID); release->mediumCount = getTagValueAs(tagReader, TagType::TotalDiscs); diff --git a/src/libs/metadata/test/Parser.cpp b/src/libs/metadata/test/Parser.cpp index f683c80f..06e101fa 100644 --- a/src/libs/metadata/test/Parser.cpp +++ b/src/libs/metadata/test/Parser.cpp @@ -273,6 +273,45 @@ namespace lms::metadata EXPECT_EQ(track->medium->release->artistDisplayName, "AlbumArtist1, AlbumArtist2"); } + TEST(Parser, customDelimitersNotForDisplayString) + { + const TestTagReader testTags{ + { + { TagType::Artist, { "Artist1 & Artist2" } }, + { TagType::Artists, { "Artist1", "Artist2" } }, + } + }; + + Parser parser; + static_cast(parser).setArtistTagDelimiters(std::vector{ " & " }); + + std::unique_ptr track{ parser.parse(testTags) }; + + ASSERT_EQ(track->artists.size(), 2); + EXPECT_EQ(track->artists[0].name, "Artist1"); + EXPECT_EQ(track->artists[1].name, "Artist2"); + EXPECT_EQ(track->artistDisplayName, "Artist1 & Artist2"); + } + + TEST(Parser, customDelimitersUsedForArtist) + { + const TestTagReader testTags{ + { + { TagType::Artist, { "Artist1 & Artist2" } }, + } + }; + + Parser parser; + static_cast(parser).setArtistTagDelimiters(std::vector{ " & " }); + + std::unique_ptr track{ parser.parse(testTags) }; + + ASSERT_EQ(track->artists.size(), 2); + EXPECT_EQ(track->artists[0].name, "Artist1"); + EXPECT_EQ(track->artists[1].name, "Artist2"); + EXPECT_EQ(track->artistDisplayName, "Artist1, Artist2"); // reconstructed since a custom delimiter was hit for parsing + } + TEST(Parser, noArtistInArtist) { const TestTagReader testTags{