Skip to content

Commit ea92633

Browse files
committed
Improve testing and incorrect tag warning
1 parent e3af889 commit ea92633

File tree

3 files changed

+47
-15
lines changed

3 files changed

+47
-15
lines changed

music_assistant/helpers/tags.py

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -291,13 +291,8 @@ def artists(self) -> tuple[str, ...]:
291291
if tag := self.tags.get("artists"):
292292
# Runtime check: mutagen returns list[str] for Vorbis multi-field
293293
if isinstance(tag, list) and len(tag) > 1: # type: ignore[unreachable]
294-
# Multiple ARTISTS fields in Vorbis - non-standard tagging.
295-
# Should use multiple ARTIST (singular) fields instead.
296-
LOGGER.warning( # type: ignore[unreachable]
297-
"Multiple ARTISTS fields found. Use multiple ARTIST fields instead: %s",
298-
tag,
299-
)
300-
artists = clean_tuple(tag)
294+
# Multiple ARTIST fields from Vorbis - already separated, no splitting needed
295+
artists = clean_tuple(tag) # type: ignore[unreachable]
301296
else:
302297
# Single field (ID3 TXXX:ARTISTS) - split on semicolons
303298
artists = split_items(tag)
@@ -358,13 +353,8 @@ def album_artists(self) -> tuple[str, ...]:
358353
if tag := self.tags.get("albumartists"):
359354
# Runtime check: mutagen returns list[str] for Vorbis multi-field
360355
if isinstance(tag, list) and len(tag) > 1: # type: ignore[unreachable]
361-
# Multiple ALBUMARTISTS fields in Vorbis - non-standard tagging.
362-
LOGGER.warning( # type: ignore[unreachable]
363-
"Multiple ALBUMARTISTS fields found. Use multiple ALBUMARTIST fields "
364-
"instead: %s",
365-
tag,
366-
)
367-
artists = clean_tuple(tag)
356+
# Multiple ALBUMARTIST fields from Vorbis - already separated, no splitting needed
357+
artists = clean_tuple(tag) # type: ignore[unreachable]
368358
else:
369359
# Single field (ID3 TXXX:ALBUMARTISTS) - split on semicolons
370360
artists = split_items(tag)
@@ -1012,10 +1002,23 @@ def _parse_vorbis_artist_tags(tags: VCommentDict, result: dict[str, Any]) -> Non
10121002
else:
10131003
result["albumartist"] = albumartist_values[0]
10141004

1015-
# Explicit ARTISTS tag takes precedence if present
1005+
# ARTISTS (plural) is non-standard in Vorbis - it's a MusicBrainz/Picard ID3 convention.
1006+
# Vorbis spec recommends multiple ARTIST (singular) fields instead.
1007+
# Accept it but warn. See: https://xiph.org/vorbis/doc/v-comment.html
10161008
if artists := _vorbis_get_multi(tags, "ARTISTS"):
1009+
LOGGER.warning(
1010+
"ARTISTS tag found in Vorbis file. Use multiple ARTIST fields instead: %s",
1011+
artists,
1012+
)
10171013
result["artists"] = artists
10181014

1015+
if albumartists := _vorbis_get_multi(tags, "ALBUMARTISTS"):
1016+
LOGGER.warning(
1017+
"ALBUMARTISTS tag found in Vorbis file. Use multiple ALBUMARTIST fields instead: %s",
1018+
albumartists,
1019+
)
1020+
result["albumartists"] = albumartists
1021+
10191022

10201023
def _parse_vorbis_tags(tags: VCommentDict) -> dict[str, Any]:
10211024
"""Parse Vorbis comment tags (FLAC, OGG Vorbis, OGG Opus, etc.).

tests/core/test_tags.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
FILE_MP3 = str(RESOURCES_DIR.joinpath("MyArtist - MyTitle.mp3"))
1818
FILE_M4A = str(RESOURCES_DIR.joinpath("MyArtist - MyTitle.m4a"))
1919
FILE_FLAC = str(RESOURCES_DIR.joinpath("MultipleArtists.flac"))
20+
FILE_FLAC_SEMICOLON = str(RESOURCES_DIR.joinpath("ArtistWithSemicolon.flac"))
2021
FILE_WV = str(RESOURCES_DIR.joinpath("MyArtist - MyTitle.wv"))
2122

2223

@@ -488,6 +489,34 @@ def test_vorbis_multiple_artist_fields_semicolon_in_name() -> None:
488489
assert len(audio_tags.artists) == len(audio_tags.musicbrainz_artistids)
489490

490491

492+
async def test_flac_multiple_artist_fields_semicolon_e2e() -> None:
493+
"""End-to-end test: FLAC with multiple ARTIST fields, one containing semicolon.
494+
495+
Tests real file parsing to ensure the full pipeline correctly handles
496+
artist names with semicolons when using multiple ARTIST fields per Vorbis spec.
497+
498+
See: https://xiph.org/vorbis/doc/v-comment.html
499+
See: https://musicbrainz.org/artist/2ade7b3c-a6f1-4d00-b7f7-fc60abf25dba
500+
"""
501+
audio_tags = await tags.async_parse_tags(FILE_FLAC_SEMICOLON)
502+
503+
# Verify the artists are correctly parsed without splitting on semicolons
504+
assert audio_tags.artists == ("ave;new", "佐倉紗織")
505+
assert "ave" not in audio_tags.artists
506+
assert "new" not in audio_tags.artists
507+
508+
# Verify MB Artist IDs match
509+
assert audio_tags.musicbrainz_artistids == (
510+
"2ade7b3c-a6f1-4d00-b7f7-fc60abf25dba",
511+
"822c07bd-1f8a-4fef-acdb-8acfe82fbef5",
512+
)
513+
assert len(audio_tags.artists) == len(audio_tags.musicbrainz_artistids)
514+
515+
# Verify other tags
516+
assert audio_tags.title == "Call My Dears"
517+
assert audio_tags.album == "Lovable"
518+
519+
491520
def test_id3_artist_tag_semicolon_single_mbid() -> None:
492521
"""Test that single ARTIST tag with semicolon is not split when 1 MB ID exists.
493522
1.32 KB
Binary file not shown.

0 commit comments

Comments
 (0)