diff --git a/api/funkwhale_api/music/metadata.py b/api/funkwhale_api/music/metadata.py
index 9a61e67e14ee8dd213e2f88d0d986d2e84a7c0c1..ad8a6b6a5d027160bd79464b88d5d7d6dce7bbbc 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 4413f3a3b077453ae516875c28b88ec50cf015ba..5e5590d7dba3d8696c35d8605ef3f2f818f9c8d6 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 0000000000000000000000000000000000000000..ddd3ab7009e108474d6d15d08640672775b463a1
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 0000000000000000000000000000000000000000..0754dac6dc2e2ffc60b8f2113a70361d3c112291
--- /dev/null
+++ b/changes/changelog.d/469.bugfix
@@ -0,0 +1 @@
+Fixed parsing of embedded file cover for ogg files tagged with MusicBrainz (#469)