From 43d558d6f618683ed7eb0a51b6672004a61cd0f8 Mon Sep 17 00:00:00 2001 From: Rouzax Date: Wed, 15 Apr 2026 09:40:28 +0000 Subject: [PATCH 1/2] #1555 - Stop-gap: drop MusicBrainz IDs when artist name and MBID counts differ Slim::Schema::Contributor::add splits the artist name tag and the MusicBrainz ID tag independently and zips them positionally. When the lists differ in length (e.g. a joined-display "Artist A & Artist B" with 2 MBIDs, or "Artist A ft. Artist B" with 1 name and 2 MBIDs), the first MBID was bound to a joined-display name, poisoning the contributors row. Subsequent tracks whose MBID matched that row then inherited the wrong display name (see also the earlier report in #1052). Guard against the mismatch and drop the ambiguous MBIDs so the row is created without an MBID binding. Aligned Picard-convention tags are unaffected. Already-poisoned libraries need a wipecache. This is a defensive stop-gap, not a structural fix. Issue #1555 remains open: the proper resolution is to read the plural ARTISTS / ALBUMARTISTS Picard tags as the indexing source of truth, which will be a separate follow-up. Signed-off-by: Rouzax --- Changelog9.html | 1 + Slim/Schema/Contributor.pm | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/Changelog9.html b/Changelog9.html index 819add419c..5ce7262408 100644 --- a/Changelog9.html +++ b/Changelog9.html @@ -30,6 +30,7 @@

Version 9.2.0


diff --git a/Slim/Schema/Contributor.pm b/Slim/Schema/Contributor.pm index eba75dfeec..da58ce641c 100644 --- a/Slim/Schema/Contributor.pm +++ b/Slim/Schema/Contributor.pm @@ -33,6 +33,7 @@ my @allAlbumLinkRoles; my @inArtistsRoles; my $prefs = preferences('server'); +my $log = logger('scan.scanner'); initializeRoles(); @@ -248,6 +249,20 @@ sub add { my @brainzIDList; if ($brainzID) { @brainzIDList = Slim::Music::Info::splitTag($brainzID); + + # Issue #1555 - if the MBID count differs from the name count, the + # positional pairing below would bind an MBID to a joined-display name + # (e.g. "Artist A & Artist B" with 2 MBIDs, or "Artist A ft. Artist B" + # with 1 name after splitTag and 2 MBIDs), poisoning the contributor + # row. Drop the ambiguous MBIDs rather than mis-bind them. + if (@brainzIDList && scalar(@brainzIDList) != scalar(@artistList)) { + main::INFOLOG && $log->is_info && $log->info( + "Ignoring MBID list for '$artist': name count (" + . scalar(@artistList) . ") differs from MBID count (" + . scalar(@brainzIDList) . ")" + ); + @brainzIDList = (); + } } # Using native DBI here to improve performance during scanning @@ -356,8 +371,6 @@ sub isInLibrary { sub rescan { my ( $class, $ids, $albumId ) = @_; - my $log = logger('scan.scanner'); - my $dbh = Slim::Schema->dbh; my $contributorSth = $dbh->prepare_cached( qq{ From 0bfb15470b8cbbdb23805f05c1c29902db0fd2a8 Mon Sep 17 00:00:00 2001 From: Rouzax Date: Wed, 15 Apr 2026 11:42:31 +0000 Subject: [PATCH 2/2] #1555 - Use plural ALBUMARTISTS / ARTISTS as authoritative source of individual contributors When Picard-style plural tags (ALBUMARTISTS, ARTISTS) are present as arrayrefs on the scanned $attributes hash, use them as the authoritative list of individual contributors rather than parsing the joined singular ALBUMARTIST / ARTIST display string with splitList. The singular tag remains the display string on the track / album row. This is the structural follow-up to the stop-gap in 0dbbb98ae. With aligned plural tags now feeding Slim::Schema::Contributor->add as arrayrefs, the positional MBID zip is no longer a guess: it is an index provided by the tagger. The length-mismatch guard from the stop-gap stays as the safety net for the legacy single-string path. - Slim/Schema.pm _preCheckAttributes: defer ARTISTS, ALBUMARTISTS, TRACKARTISTS so they reach _mergeAndCreateContributors via the newTrack path (previously they were left in the regular attributes hash and lost before contributor processing). - Slim/Schema.pm _mergeAndCreateContributors: prefer $attributes->{$tag.'S'} as the contributor source when it is a non-empty arrayref, for the ARTIST / ALBUMARTIST / TRACKARTIST roles only. Other roles (COMPOSER, BAND, CONDUCTOR, ...) are unchanged. Trim plural names, drop empty-name slots and their aligned MBID slot, tolerate arrayref $contributor in the whitespace-strip step, and loosen the early skip so a track with plural but no singular still produces rows. Also accept an aligned plural sort arrayref (e.g. user-scripted multi-value ALBUMARTISTSORT/ARTISTSORT from %_albumartists_sort% / %_artists_sort%) and pass it positionally to Contributor::add; otherwise fall back to sort-by-name rather than polluting the first contributor with the joined singular sort string. - Slim/Schema.pm TRACKARTIST compilation transform (bug 2317 / 2638): also migrate the plural ARTISTS tag to TRACKARTISTS, symmetric with the existing ARTIST and MUSICBRAINZ_ARTIST_ID migration. Picard tag mapping reference: https://picard-docs.musicbrainz.org/en/latest/appendices/tag_mapping.html Backwards compatibility: files without plural tags, or with plural tags as scalars, fall through to the existing splitList path. No schema change. No format-parser change. Already-poisoned libraries from pre-stop-gap scans need a wipecache. Users who tuned splitList to work around the absence of plural-tag reading may find it is no longer needed, but it is not removed. Known limitation for MP3: Audio::Scan returns only the first value of multi-value ID3v2.4 TXXX text frames (which is how Picard writes plural tags into MP3 files). Until that is addressed, MP3 files see the poisoning gone but do not get the full multi-contributor benefit. FLAC, Ogg, Opus and MP4 are unaffected and gain the full benefit. Signed-off-by: Rouzax --- Changelog9.html | 1 + Slim/Schema.pm | 92 ++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 85 insertions(+), 8 deletions(-) diff --git a/Changelog9.html b/Changelog9.html index 5ce7262408..0fab1b763f 100644 --- a/Changelog9.html +++ b/Changelog9.html @@ -31,6 +31,7 @@

Version 9.2.0

  • #1517 - Fix work images and artwork precaching (@darrell-k)
  • #1553 - Allow plugins to shut down before closing the database (@SamInPgh)
  • #1555 - Scanner: stop-gap to prevent contributor row poisoning when the MusicBrainz ID count does not match the artist name count. (@Rouzax)
    • +
  • #1555 - Scanner: read the Picard-convention plural tags ALBUMARTISTS and ARTISTS as the authoritative list of individual contributors for the ALBUMARTIST and ARTIST roles, zipped positionally with MUSICBRAINZ_ALBUMARTIST_ID / MUSICBRAINZ_ARTIST_ID. The singular tag still provides the album/track display string. Behaviour for libraries without plural tags is unchanged. Run wipecache to benefit on already-scanned libraries. (@Rouzax)

  • diff --git a/Slim/Schema.pm b/Slim/Schema.pm index a04164313d..16e5af9023 100644 --- a/Slim/Schema.pm +++ b/Slim/Schema.pm @@ -2842,6 +2842,7 @@ sub _preCheckAttributes { MUSICBRAINZ_ARTIST_ID MUSICBRAINZ_ALBUMARTIST_ID MUSICBRAINZ_ALBUM_ID MUSICBRAINZ_ALBUM_TYPE MUSICBRAINZ_ALBUM_STATUS RELEASETYPE ALBUM_EXTID ARTIST_EXTID WORK WORKSORT + ARTISTS ALBUMARTISTS TRACKARTISTS )) { @@ -3126,34 +3127,109 @@ sub _mergeAndCreateContributors { $attributes->{'TRACKARTISTSORT'} = delete $attributes->{'ARTISTSORT'}; $attributes->{'MUSICBRAINZ_TRACKARTIST_ID'} = delete $attributes->{'MUSICBRAINZ_ARTIST_ID'} if $attributes->{'MUSICBRAINZ_ARTIST_ID'}; + # Issue #1555 - carry the plural ARTISTS tag across too, so the + # per-role plural lookup below finds it under TRACKARTISTS. + $attributes->{'TRACKARTISTS'} = delete $attributes->{'ARTISTS'} if $attributes->{'ARTISTS'}; + main::DEBUGLOG && $isDebug && $log->debug(sprintf("-- Contributor '%s' of role 'ARTIST' transformed to role 'TRACKARTIST'", - $attributes->{'TRACKARTIST'}, + ref($attributes->{'TRACKARTIST'}) eq 'ARRAY' + ? join(' / ', @{$attributes->{'TRACKARTIST'}}) + : $attributes->{'TRACKARTIST'}, )); } } my %contributors = (); + # Issue #1555 - roles for which a plural Picard tag (ARTISTS, + # ALBUMARTISTS, TRACKARTISTS) is the authoritative individual-contributor + # list. The singular tag remains the display string; the plural tag, + # when present as a non-empty arrayref after trimming, supersedes + # splitList parsing of the singular for contributor-row creation. + my %pluralRoles = map { $_ => 1 } qw(ARTIST ALBUMARTIST TRACKARTIST); + for my $tag (Slim::Schema::Contributor->contributorRoles) { - my $contributor = $attributes->{$tag} || next; + my $contributor = $attributes->{$tag}; + my $brainzID = $attributes->{"MUSICBRAINZ_${tag}_ID"}; + my $sortBy = $attributes->{$tag . 'SORT'}; + my $usedPlural = 0; + + # Issue #1555 - prefer the plural Picard tag as the individual + # contributor list when it is a non-empty arrayref with at least + # one non-blank name. Trim, drop empty-name slots, and drop the + # corresponding MBID slot so positional alignment is preserved. + # Empty-string MBID slots (for contributors without an MBID, + # Picard convention) are kept; Contributor->add's `if ($mbid)` + # guard handles them. A scalar plural value is treated as "not + # plural" and falls through to the legacy singular path. + # + # Sort alignment: Picard has no standard plural sort tag, but a + # user script can overwrite the singular ALBUMARTISTSORT / + # ARTISTSORT tag with a multi-value list aligned to the plural + # names (using %_albumartists_sort% / %_artists_sort%). Accept + # that if the sort tag is an arrayref of matching length after + # empty-slot filtering. Otherwise pass undef so Contributor->add + # falls back to sort-by-name rather than polluting the first + # contributor with the joined singular sort string. + if ($pluralRoles{$tag}) { + my $plural = $attributes->{$tag . 'S'}; + if (ref($plural) eq 'ARRAY' && @$plural) { + my $pluralMbid = $attributes->{"MUSICBRAINZ_${tag}_ID"}; + my $pluralSort = $attributes->{$tag . 'SORT'}; + my @mbids = ref($pluralMbid) eq 'ARRAY' ? @$pluralMbid : (); + my @sorts = ref($pluralSort) eq 'ARRAY' ? @$pluralSort : (); + my (@names, @alignedMbids, @alignedSorts); + for (my $i = 0; $i < @$plural; $i++) { + my $name = $plural->[$i]; + next unless defined $name; + $name =~ s/^\s+//; $name =~ s/\s+$//; + next if $name eq ''; + push @names, $name; + push @alignedMbids, $mbids[$i] if @mbids; + push @alignedSorts, $sorts[$i] if @sorts; + } + if (@names) { + $contributor = \@names; + $brainzID = @mbids ? \@alignedMbids : $brainzID; + $sortBy = (@sorts && @alignedSorts == @names) ? \@alignedSorts : undef; + $usedPlural = 1; + main::DEBUGLOG && $isDebug && $log->debug(sprintf( + "-- Using plural %sS (%d entries) as contributor source for role '%s'%s", + $tag, scalar @names, $tag, + $sortBy ? ' with aligned sort' : '', + )); + } + } + } + + # Skip unless we have a source of contributor names for this role. + # Empty string, undef, or an empty plural array all skip. + next unless $usedPlural || (defined $contributor && (ref $contributor || $contributor ne '')); - # Bug 17322, strip leading/trailing spaces from name - $contributor =~ s/^ +//; - $contributor =~ s/ +$//; + # Bug 17322, strip leading/trailing spaces from name. splitTag + # already trims arrayref elements, so the regex strip is scalar + # only. + unless (ref $contributor) { + $contributor =~ s/^ +//; + $contributor =~ s/ +$//; + } # Is ARTISTSORT/TSOP always right for non-artist # contributors? I think so. ID3 doesn't have # "BANDSORT" or similar at any rate. push @{ $contributors{$tag} }, Slim::Schema::Contributor->add({ 'artist' => $contributor, - 'brainzID' => $attributes->{"MUSICBRAINZ_${tag}_ID"}, - 'sortBy' => $attributes->{$tag.'SORT'}, + 'brainzID' => $brainzID, + 'sortBy' => $sortBy, # only store EXTID for track artist, as we don't have it for other roles 'extid' => $tag eq 'ARTIST' && $attributes->{'ARTIST_EXTID'}, }); - main::DEBUGLOG && $isDebug && $log->is_debug && $log->debug(sprintf("-- Track has contributor '$contributor' of role '$tag'")); + main::DEBUGLOG && $isDebug && $log->is_debug && $log->debug(sprintf( + "-- Track has contributor '%s' of role '$tag'", + ref($contributor) eq 'ARRAY' ? join(' / ', @$contributor) : $contributor, + )); } # Bug 15553, Primary contributor can only be Album Artist or Artist,