search

quodlibet / mutagen

Python module for handling audio metadata
https://mutagen.readthedocs.io
GNU General Public License v2.0
1.55k stars 158 forks source link

MP4 fails with string argument for freeform text ('----') tags #391

Open simonltwick opened 5 years ago

simonltwick commented 5 years ago

mutagen.mp4.MP4 container does not accept a string value for freeform text tags, but expects (and returns) a bytes argument. This is inconsistent with python3 default of string values.

code:

import mutagen.mp4.MP4 as MP4 m = MP4() m['----:test:item'] = 'test value' Traceback (most recent call last): File "", line 1, in File "/usr/lib/python3/dist-packages/mutagen/_file.py", line 75, in setitem self.tags[key] = value File "/usr/lib/python3/dist-packages/mutagen/mp4/init.py", line 375, in setitem self._render(key, value) File "/usr/lib/python3/dist-packages/mutagen/mp4/init.py", line 391, in _render return render_func(self, key, value, *render_args) File "/usr/lib/python3/dist-packages/mutagen/mp4/init.py", line 632, in __render_freeform data += v TypeError: can't concat str to bytes m['----:test:item'] = b'test value' m['----:test:item'] b'test value'

Expected behaviour: accept (and return) string values, with encoding to utf-8 if required (this is implied in some of the comments in the code).

Already submitted on Ubuntu Launchpad as https://bugs.launchpad.net/bugs/1835771, where I was advised to report it here.

simonltwick commented 5 years ago

PS. The use case for this is in storing and retrieving musicbrainz ID tags, which are stored as freeform tags.

phw commented 5 years ago

Why not do the conversion to and from bytes in your code? This is just the interface, and I think it makes sense for the interface to handle bytes here since the freeform tags are AFAIK not limited to stroing UTF-8 encoded text data.

phw commented 5 years ago

Also wanted to note that this could not be changed without breaking the interface. So if there would be a way to directly write and get tags as strings this would need a separate interface for it. But I honestly don't see the reason for this, since bytes to string conversion with all possible issues is best handled in the caller's code I think.

simonltwick commented 5 years ago

Of course it's quite possible to do the conversion in user code, and this is what I have done. But as far as I can tell, it's not documented that you need a bytes value, it's inconsistent with most/all other MP4 tag values, it's inconsistent with other container types (Flac / EasyMP3 at least) and it's inconsistent with the Python3 default. If it is different to all other tags, surely it should be documented as such?