diff --git a/api/funkwhale_api/music/metadata.py b/api/funkwhale_api/music/metadata.py index 9a61e67e1..ad8a6b6a5 100644 --- a/api/funkwhale_api/music/metadata.py +++ b/api/funkwhale_api/music/metadata.py @@ -1,8 +1,13 @@ +import base64 import datetime import logging -import mutagen import pendulum +import mutagen._util +import mutagen.oggtheora +import mutagen.oggvorbis +import mutagen.flac + from django import forms logger = logging.getLogger(__name__) @@ -83,6 +88,31 @@ def clean_flac_pictures(apic): return pictures +def clean_ogg_pictures(metadata_block_picture): + pictures = [] + for b64_data in [metadata_block_picture]: + + try: + data = base64.b64decode(b64_data) + except (TypeError, ValueError): + continue + + try: + picture = mutagen.flac.Picture(data) + except mutagen.flac.FLACError: + continue + + pictures.append( + { + "mimetype": picture.mime, + "content": picture.data, + "description": "", + "type": picture.type.real, + } + ) + return pictures + + def get_mp3_recording_id(f, k): try: return [t for t in f.tags.getall("UFID") if "musicbrainz.org" in t.owner][ @@ -197,6 +227,10 @@ CONF = { "musicbrainz_recordingid": {"field": "musicbrainz_trackid"}, "license": {}, "copyright": {}, + "pictures": { + "field": "metadata_block_picture", + "to_application": clean_ogg_pictures, + }, }, }, "OggTheora": { @@ -216,9 +250,8 @@ CONF = { "musicbrainz_artistid": {"field": "MusicBrainz Artist Id"}, "musicbrainz_albumartistid": {"field": "MusicBrainz Album Artist Id"}, "musicbrainz_recordingid": {"field": "MusicBrainz Track Id"}, - # somehow, I cannot successfully create an ogg theora file - # with the proper license field - # "license": {"field": "license"}, + "license": {}, + "copyright": {}, }, }, "MP3": { @@ -288,10 +321,11 @@ ALL_FIELDS = [ class Metadata(object): - def __init__(self, path): - self._file = mutagen.File(path) + def __init__(self, filething, kind=mutagen.File): + self._file = kind(filething) if self._file is None: - raise ValueError("Cannot parse metadata from {}".format(path)) + raise ValueError("Cannot parse metadata from {}".format(filething)) + self.fallback = self.load_fallback(filething, self._file) ft = self.get_file_type(self._file) try: self._conf = CONF[ft] @@ -301,7 +335,40 @@ class Metadata(object): def get_file_type(self, f): return f.__class__.__name__ + def load_fallback(self, filething, parent): + """ + In some situations, such as Ogg Theora files tagged with MusicBrainz Picard, + part of the tags are only available in the ogg vorbis comments + """ + try: + filething.seek(0) + except AttributeError: + pass + if isinstance(parent, mutagen.oggtheora.OggTheora): + try: + return Metadata(filething, kind=mutagen.oggvorbis.OggVorbis) + except (ValueError, mutagen._util.MutagenError): + raise + pass + def get(self, key, default=NODEFAULT): + try: + return self._get_from_self(key) + except TagNotFound: + if not self.fallback: + if default != NODEFAULT: + return default + else: + raise + else: + return self.fallback.get(key, default=default) + except UnsupportedTag: + if not self.fallback: + raise + else: + return self.fallback.get(key, default=default) + + def _get_from_self(self, key, default=NODEFAULT): try: field_conf = self._conf["fields"][key] except KeyError: diff --git a/api/tests/music/test_metadata.py b/api/tests/music/test_metadata.py index 4413f3a3b..5e5590d7d 100644 --- a/api/tests/music/test_metadata.py +++ b/api/tests/music/test_metadata.py @@ -1,9 +1,11 @@ import datetime import os import uuid - import pytest +import mutagen.oggtheora +import mutagen.oggvorbis + from funkwhale_api.music import metadata DATA_DIR = os.path.dirname(os.path.abspath(__file__)) @@ -145,7 +147,7 @@ def test_can_get_metadata_from_id3_mp3_file(field, value): assert data.get(field) == value -@pytest.mark.parametrize("name", ["test.mp3", "sample.flac"]) +@pytest.mark.parametrize("name", ["test.mp3", "sample.flac", "with_cover.ogg"]) def test_can_get_pictures(name): path = os.path.join(DATA_DIR, name) data = metadata.Metadata(path) @@ -243,3 +245,20 @@ def test_metadata_all_ignore_parse_errors_false(mocker): mocker.patch.object(data, "get", side_effect=metadata.ParseError("Failure")) with pytest.raises(metadata.ParseError): data.all(ignore_parse_errors=False) + + +def test_metadata_fallback_ogg_theora(mocker): + path = os.path.join(DATA_DIR, "with_cover.ogg") + data = metadata.Metadata(path) + + assert isinstance(data._file, mutagen.oggtheora.OggTheora) + assert isinstance(data.fallback, metadata.Metadata) + assert isinstance(data.fallback._file, mutagen.oggvorbis.OggVorbis) + + expected_result = data.fallback.get("pictures") + fallback_get = mocker.spy(data.fallback, "get") + + assert expected_result is not None + assert data.get("pictures", "default") == expected_result + + fallback_get.assert_called_once_with("pictures", "default") diff --git a/api/tests/music/with_cover.ogg b/api/tests/music/with_cover.ogg new file mode 100644 index 000000000..ddd3ab700 Binary files /dev/null and b/api/tests/music/with_cover.ogg differ diff --git a/changes/changelog.d/469.bugfix b/changes/changelog.d/469.bugfix new file mode 100644 index 000000000..0754dac6d --- /dev/null +++ b/changes/changelog.d/469.bugfix @@ -0,0 +1 @@ +Fixed parsing of embedded file cover for ogg files tagged with MusicBrainz (#469)