From 3b4f6597be7760d56846f81a4194f773e129a658 Mon Sep 17 00:00:00 2001 From: OzGav Date: Sat, 7 Feb 2026 23:32:34 +1100 Subject: [PATCH] Support multiple artist and other tags in FLAC/OGG files (#3076) --- music_assistant/helpers/tags.py | 147 +++++++++++++++++- .../providers/filesystem_local/constants.py | 1 + tests/core/test_tags.py | 137 +++++++++++++++- tests/fixtures/MultipleArtists.flac | Bin 0 -> 1631 bytes 4 files changed, 283 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/MultipleArtists.flac diff --git a/music_assistant/helpers/tags.py b/music_assistant/helpers/tags.py index 57a1a508..4d6de102 100644 --- a/music_assistant/helpers/tags.py +++ b/music_assistant/helpers/tags.py @@ -16,6 +16,7 @@ from typing import Any import mutagen from music_assistant_models.enums import AlbumType from music_assistant_models.errors import InvalidDataError +from mutagen._vorbis import VCommentDict from mutagen.mp4 import MP4Tags from music_assistant.constants import MASS_LOGGER_NAME, UNKNOWN_ARTIST @@ -925,10 +926,149 @@ def _parse_id3_tags(tags: dict[str, Any]) -> dict[str, Any]: return result +def _vorbis_get_single(tags: VCommentDict, key: str) -> str | None: + """Get single value from Vorbis comments (first item if multiple exist). + + :param tags: VCommentDict from mutagen. + :param key: Tag name (case insensitive). + """ + values = tags.get(key) # type: ignore[no-untyped-call] + return values[0] if values else None + + +def _vorbis_get_multi(tags: VCommentDict, key: str) -> list[str] | None: + """Get all values from Vorbis comments as a list. + + :param tags: VCommentDict from mutagen. + :param key: Tag name (case insensitive). + """ + values = tags.get(key) # type: ignore[no-untyped-call] + return list(values) if values else None + + +def _parse_vorbis_artist_tags(tags: VCommentDict, result: dict[str, Any]) -> None: + """Parse artist-related tags from Vorbis comments into result dict. + + Handles multiple ARTIST/ALBUMARTIST fields per Vorbis spec, as well as + explicit ARTISTS tag which take precedence. + + :param tags: VCommentDict from mutagen. + :param result: Dictionary to store parsed tags. + """ + # Artist tags - check for multiple values (per Vorbis spec recommendation) + # Multiple ARTIST fields are treated the same as an ARTISTS tag + artist_values = _vorbis_get_multi(tags, "ARTIST") + if artist_values: + if len(artist_values) > 1: + # Multiple ARTIST fields - treat as authoritative list (like ARTISTS tag) + result["artists"] = artist_values + else: + # Single ARTIST field - use normal parsing logic + result["artist"] = artist_values[0] + + # Album artist tags - same logic for multiple values + albumartist_values = _vorbis_get_multi(tags, "ALBUMARTIST") + if albumartist_values: + if len(albumartist_values) > 1: + # Multiple ALBUMARTIST fields - treat as authoritative list + result["albumartists"] = albumartist_values + else: + result["albumartist"] = albumartist_values[0] + + # Explicit ARTISTS tag takes precedence if present + if artists := _vorbis_get_multi(tags, "ARTISTS"): + result["artists"] = artists + + +def _parse_vorbis_tags(tags: VCommentDict) -> dict[str, Any]: + """Parse Vorbis comment tags (FLAC, OGG Vorbis, OGG Opus, etc.). + + Vorbis comments support multiple values for the same field name per the spec. + For example, multiple ARTIST fields can be used instead of a single ARTISTS field. + See: https://xiph.org/vorbis/doc/v-comment.html + + :param tags: VCommentDict from mutagen (FLAC, OGG, etc.). + """ + result: dict[str, Any] = {} + + # Basic tags + if title := _vorbis_get_single(tags, "TITLE"): + result["title"] = title + if album := _vorbis_get_single(tags, "ALBUM"): + result["album"] = album + + # Artist tags (handles multiple ARTIST/ALBUMARTIST fields per Vorbis spec) + _parse_vorbis_artist_tags(tags, result) + + # Genre (multi-value) + if genre := _vorbis_get_multi(tags, "GENRE"): + result["genre"] = genre + + # MusicBrainz tags (single value) + if mb_album := _vorbis_get_single(tags, "MUSICBRAINZ_ALBUMID"): + result["musicbrainzalbumid"] = mb_album + if mb_rg := _vorbis_get_single(tags, "MUSICBRAINZ_RELEASEGROUPID"): + result["musicbrainzreleasegroupid"] = mb_rg + if mb_track := _vorbis_get_single(tags, "MUSICBRAINZ_TRACKID"): + result["musicbrainzrecordingid"] = mb_track + if mb_reltrack := _vorbis_get_single(tags, "MUSICBRAINZ_RELEASETRACKID"): + result["musicbrainztrackid"] = mb_reltrack + + # MusicBrainz tags (multi-value) + if mb_aa_ids := _vorbis_get_multi(tags, "MUSICBRAINZ_ALBUMARTISTID"): + result["musicbrainzalbumartistid"] = mb_aa_ids + if mb_a_ids := _vorbis_get_multi(tags, "MUSICBRAINZ_ARTISTID"): + result["musicbrainzartistid"] = mb_a_ids + + # Additional tags + if barcode := _vorbis_get_multi(tags, "BARCODE"): + result["barcode"] = barcode + if isrc := _vorbis_get_multi(tags, "ISRC"): + result["isrc"] = isrc + + # Date + if date := _vorbis_get_single(tags, "DATE"): + result["date"] = date + + # Lyrics + if lyrics := _vorbis_get_single(tags, "LYRICS"): + result["lyrics"] = lyrics + + # Compilation flag + if compilation := _vorbis_get_single(tags, "COMPILATION"): + result["compilation"] = compilation + + # Album type (MusicBrainz) + if albumtype := _vorbis_get_single(tags, "MUSICBRAINZ_ALBUMTYPE"): + result["musicbrainzalbumtype"] = albumtype + # Also check RELEASETYPE which is an alternative tag name + if not result.get("musicbrainzalbumtype"): + if releasetype := _vorbis_get_single(tags, "RELEASETYPE"): + result["musicbrainzalbumtype"] = releasetype + + # ReplayGain tags + if rg_track := _vorbis_get_single(tags, "REPLAYGAIN_TRACK_GAIN"): + result["replaygaintrackgain"] = rg_track + if rg_album := _vorbis_get_single(tags, "REPLAYGAIN_ALBUM_GAIN"): + result["replaygainalbumgain"] = rg_album + + # Sort tags + if artistsort := _vorbis_get_multi(tags, "ARTISTSORT"): + result["artistsort"] = artistsort + if albumartistsort := _vorbis_get_multi(tags, "ALBUMARTISTSORT"): + result["albumartistsort"] = albumartistsort + if titlesort := _vorbis_get_single(tags, "TITLESORT"): + result["titlesort"] = titlesort + if albumsort := _vorbis_get_single(tags, "ALBUMSORT"): + result["albumsort"] = albumsort + + return result + + def parse_tags_mutagen(input_file: str) -> dict[str, Any]: """Parse tags from an audio file using Mutagen. - Supports ID3 tags (MP3) and MP4 tags (AAC/M4A/ALAC). + Supports Vorbis comments (FLAC, OGG), ID3 tags (MP3), and MP4 tags (AAC/M4A/ALAC). :param input_file: Path to the audio file. """ @@ -941,8 +1081,13 @@ def parse_tags_mutagen(input_file: str) -> dict[str, Any]: # Check if MP4/M4A/AAC file (uses MP4Tags) if isinstance(audio.tags, MP4Tags): result = _parse_mp4_tags(audio.tags) + # Check if Vorbis comments (FLAC, OGG Vorbis, OGG Opus, etc.) + elif isinstance(audio.tags, VCommentDict): + result = _parse_vorbis_tags(audio.tags) else: # ID3 tags (MP3) and other formats + # TODO: Add _parse_apev2_tags() for WavPack/Musepack/Monkey's Audio to extract + # MusicBrainz IDs and multi-artist tags (APEv2 uses different tag names than ID3). tags_dict = dict(audio.tags) result = _parse_id3_tags(tags_dict) diff --git a/music_assistant/providers/filesystem_local/constants.py b/music_assistant/providers/filesystem_local/constants.py index 181cf461..92f0dc11 100644 --- a/music_assistant/providers/filesystem_local/constants.py +++ b/music_assistant/providers/filesystem_local/constants.py @@ -137,6 +137,7 @@ TRACK_EXTENSIONS = { "ape", "mpc", "mp2", + "m2a", "mp1", "dra", "mpeg", diff --git a/tests/core/test_tags.py b/tests/core/test_tags.py index 172d1fe4..2fbc766e 100644 --- a/tests/core/test_tags.py +++ b/tests/core/test_tags.py @@ -1,15 +1,17 @@ """Tests for parsing audio file tags (ID3, MP4/AAC, etc.).""" import pathlib +from unittest.mock import MagicMock from music_assistant.constants import UNKNOWN_ARTIST from music_assistant.helpers import tags -from music_assistant.helpers.tags import split_artists +from music_assistant.helpers.tags import _parse_vorbis_tags, split_artists RESOURCES_DIR = pathlib.Path(__file__).parent.parent.resolve().joinpath("fixtures") FILE_MP3 = str(RESOURCES_DIR.joinpath("MyArtist - MyTitle.mp3")) FILE_M4A = str(RESOURCES_DIR.joinpath("MyArtist - MyTitle.m4a")) +FILE_FLAC = str(RESOURCES_DIR.joinpath("MultipleArtists.flac")) async def test_parse_metadata_from_id3tags() -> None: @@ -74,6 +76,26 @@ async def test_parse_metadata_from_mp4tags() -> None: assert _tags.tags.get("albumartistsort") == ["MyAlbumArtist Sort"] # type: ignore[comparison-overlap] +async def test_parse_metadata_from_flac_with_multiple_artist_fields() -> None: + """Test parsing of FLAC file with multiple ARTIST fields (per Vorbis spec).""" + _tags = await tags.async_parse_tags(FILE_FLAC) + assert _tags.album == "Test Album" + assert _tags.title == "Test Track" + # Multiple ARTIST fields should be treated as authoritative list + assert _tags.artists == ("Artist One", "Artist Two", "Artist Three") + # Multiple ALBUMARTIST fields should be treated as authoritative list + assert _tags.album_artists == ("Album Artist 1", "Album Artist 2") + assert _tags.genres == ("Rock", "Pop") + assert _tags.year == 2024 + # MusicBrainz IDs + assert _tags.musicbrainz_artistids == ("mb-artist-id-1", "mb-artist-id-2", "mb-artist-id-3") + assert _tags.musicbrainz_albumartistids == ("mb-albumartist-id-1", "mb-albumartist-id-2") + assert _tags.musicbrainz_recordingid == "mb-track-id" + # Track/disc from Vorbis comments + assert _tags.track == 5 + assert _tags.disc == 1 + + async def test_parse_metadata_from_filename() -> None: """Test parsing of parsing metadata from filename.""" filename = str(RESOURCES_DIR.joinpath("MyArtist - MyTitle without Tags.mp3")) @@ -186,3 +208,116 @@ def test_split_artists_with_not_split() -> None: # "with" SHOULD split when expected_count=2 indicates multiple artists result = split_artists("Artist A with Artist B", expected_count=2) assert result == ("Artist A", "Artist B") + + +def _create_mock_vorbis_tags(tag_dict: dict[str, list[str]]) -> MagicMock: + """Create a mock VCommentDict with the given tags. + + :param tag_dict: Dictionary mapping tag names to lists of values. + """ + mock = MagicMock() + mock.get = lambda key: tag_dict.get(key.upper()) + return mock + + +def test_parse_vorbis_tags_multiple_artist_fields() -> None: + """Test that multiple ARTIST fields are treated as authoritative artist list.""" + # Per Vorbis spec: multiple ARTIST fields should list all artists + mock_tags = _create_mock_vorbis_tags( + { + "TITLE": ["My Song"], + "ALBUM": ["My Album"], + "ARTIST": ["Artist 1", "Artist 2", "Artist 3"], + } + ) + + result = _parse_vorbis_tags(mock_tags) + + # Multiple ARTIST fields should be stored as "artists" (plural) + assert result.get("artists") == ["Artist 1", "Artist 2", "Artist 3"] + # Single "artist" key should NOT be set when multiple artists are present + assert "artist" not in result + assert result.get("title") == "My Song" + assert result.get("album") == "My Album" + + +def test_parse_vorbis_tags_single_artist_field() -> None: + """Test that a single ARTIST field is stored as singular artist.""" + mock_tags = _create_mock_vorbis_tags( + { + "TITLE": ["My Song"], + "ARTIST": ["Single Artist"], + } + ) + + result = _parse_vorbis_tags(mock_tags) + + # Single ARTIST should use singular key for normal parsing logic + assert result.get("artist") == "Single Artist" + assert "artists" not in result + + +def test_parse_vorbis_tags_multiple_albumartist_fields() -> None: + """Test that multiple ALBUMARTIST fields are treated as authoritative list.""" + mock_tags = _create_mock_vorbis_tags( + { + "ALBUMARTIST": ["Album Artist 1", "Album Artist 2"], + } + ) + + result = _parse_vorbis_tags(mock_tags) + + # Multiple ALBUMARTIST fields should be stored as "albumartists" (plural) + assert result.get("albumartists") == ["Album Artist 1", "Album Artist 2"] + assert "albumartist" not in result + + +def test_parse_vorbis_tags_single_albumartist_field() -> None: + """Test that a single ALBUMARTIST field is stored as singular.""" + mock_tags = _create_mock_vorbis_tags( + { + "ALBUMARTIST": ["Single Album Artist"], + } + ) + + result = _parse_vorbis_tags(mock_tags) + + assert result.get("albumartist") == "Single Album Artist" + assert "albumartists" not in result + + +def test_parse_vorbis_tags_explicit_artists_tag_takes_precedence() -> None: + """Test that explicit ARTISTS tag takes precedence over multiple ARTIST fields.""" + mock_tags = _create_mock_vorbis_tags( + { + "ARTIST": ["Artist A", "Artist B"], # Multiple ARTIST fields + "ARTISTS": [ + "Explicit Artist 1", + "Explicit Artist 2", + "Explicit Artist 3", + ], # Explicit tag + } + ) + + result = _parse_vorbis_tags(mock_tags) + + # ARTISTS tag should take precedence + assert result.get("artists") == ["Explicit Artist 1", "Explicit Artist 2", "Explicit Artist 3"] + + +def test_parse_vorbis_tags_musicbrainz_ids() -> None: + """Test that MusicBrainz IDs are parsed correctly from Vorbis tags.""" + mock_tags = _create_mock_vorbis_tags( + { + "ARTIST": ["Artist 1", "Artist 2"], + "MUSICBRAINZ_ARTISTID": ["mb-id-1", "mb-id-2"], + "MUSICBRAINZ_ALBUMID": ["mb-album-id"], + "MUSICBRAINZ_TRACKID": ["mb-track-id"], + } + ) + + result = _parse_vorbis_tags(mock_tags) + + assert result.get("musicbrainzartistid") == ["mb-id-1", "mb-id-2"] + assert result.get("musicbrainzalbumid") == "mb-album-id" + assert result.get("musicbrainzrecordingid") == "mb-track-id" diff --git a/tests/fixtures/MultipleArtists.flac b/tests/fixtures/MultipleArtists.flac new file mode 100644 index 0000000000000000000000000000000000000000..54b3b1f343b3994d45bb8bc98dde455109b8ea0a GIT binary patch literal 1631 zcmYfENpxmlU{Dfb5CT%XK*V*#;R6GMpw+AL$Yn1CpWWAqs14dab)gvx1Cs(!p1~)v zEX~Y7&(KWI(7-?#C?EjDA)X;VuC^hm#U%-X|5nhEGWb? zIK$p)fg+sR~fD4DreuaRK$ZyZQyW+6Lt( zXLG`N0r>?Wj*DZ6tF4iNk%=;p;~N_6>FgBb=;;>~53$PA#WpudHxcY=-OLnSL*kSf z5vR;p7u`}&$f4K_@+p$_q-rpd$7Tc6S0J;2o