From 8464e8c8e5de71b87483459174328b301ca0a589 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 10 Apr 2025 22:58:57 -0400 Subject: [PATCH] feat(scanner): enhance album artist mapping logic for better display name handling Signed-off-by: Deluan --- model/metadata/map_participants.go | 35 +++++++++++++++++--- model/metadata/map_participants_test.go | 43 +++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/model/metadata/map_participants.go b/model/metadata/map_participants.go index e8be6aaab..8072dfc8c 100644 --- a/model/metadata/map_participants.go +++ b/model/metadata/map_participants.go @@ -224,13 +224,38 @@ func (md Metadata) mapDisplayArtist() string { } func (md Metadata) mapDisplayAlbumArtist(mf model.MediaFile) string { + // 1. Check for explicit album artist tags first + albumArtistDisplay := md.mapDisplayName(model.TagAlbumArtist, model.TagAlbumArtists) + if albumArtistDisplay != "" { + return albumArtistDisplay + } + + // 2. No explicit album artist, check for track artist tags + artistTag := model.TagTrackArtist + artistsTag := model.TagTrackArtists + hasMultiValuedArtistTag := len(md.tags[artistTag]) > 1 || len(md.tags[artistsTag]) > 1 + hasSingleValuedArtistTag := len(md.tags[artistTag]) == 1 || len(md.tags[artistsTag]) == 1 + + if hasMultiValuedArtistTag { + // Special case from tests: Use only the first artist if ARTIST/ARTISTS tag is multi-valued + if len(md.tags[artistTag]) > 1 { + return md.tags[artistTag][0] + } + // We know artistsTag must have > 1 here + return md.tags[artistsTag][0] + } + // If execution reaches here, hasMultiValuedArtistTag is false + if hasSingleValuedArtistTag { + // For single artist values (including those with separators like ';'), use the artist display name + return md.mapDisplayArtist() + } + + // 3. Neither album artist nor track artist tags found. Fallback logic: fallbackName := consts.UnknownArtist if md.Bool(model.TagCompilation) { fallbackName = consts.VariousArtists } - return cmp.Or( - md.mapDisplayName(model.TagAlbumArtist, model.TagAlbumArtists), - mf.Participants.First(model.RoleAlbumArtist).Name, - fallbackName, - ) + + // Use the name from the first participant found for RoleAlbumArtist, or the default fallback + return cmp.Or(mf.Participants.First(model.RoleAlbumArtist).Name, fallbackName) } diff --git a/model/metadata/map_participants_test.go b/model/metadata/map_participants_test.go index 5317a4bcf..48136594c 100644 --- a/model/metadata/map_participants_test.go +++ b/model/metadata/map_participants_test.go @@ -370,6 +370,49 @@ var _ = Describe("Participants", func() { Expect(artist1.MbzArtistID).To(Equal(mbid2)) }) }) + + Context("Single-valued ARTIST tag with semicolon, no ARTISTS tags", func() { + BeforeEach(func() { + mf = toMediaFile(model.RawTags{ + "ARTIST": {"hey; ho"}, + }) + }) + + It("should use the full string as display name", func() { + Expect(mf.Artist).To(Equal("hey; ho")) + }) + + It("should split the artist by semicolon", func() { + participants := mf.Participants + Expect(participants).To(SatisfyAll( + HaveKeyWithValue(model.RoleArtist, HaveLen(2)), + )) + + By("adding the first artist to the participants") + artist0 := participants[model.RoleArtist][0] + Expect(artist0.ID).ToNot(BeEmpty()) + Expect(artist0.Name).To(Equal("hey")) + Expect(artist0.OrderArtistName).To(Equal("hey")) + + By("adding the second artist to the participants") + artist1 := participants[model.RoleArtist][1] + Expect(artist1.ID).ToNot(BeEmpty()) + Expect(artist1.Name).To(Equal("ho")) + Expect(artist1.OrderArtistName).To(Equal("ho")) + }) + + It("should use the full artist name as albumArtist when no ALBUMARTIST is provided", func() { + Expect(mf.AlbumArtist).To(Equal("hey; ho")) + + participants := mf.Participants + Expect(participants).To(SatisfyAll( + HaveKeyWithValue(model.RoleAlbumArtist, HaveLen(2)), + )) + + albumArtist := participants[model.RoleAlbumArtist][0] + Expect(albumArtist.Name).To(Equal("hey")) + }) + }) }) Describe("ALBUMARTIST(S) tags", func() {