Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
simonltwick opened this issue Jul 19, 2019 · 4 comments
Open

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

simonltwick opened this issue Jul 19, 2019 · 4 comments
Labels

Comments

@simonltwick
Copy link

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
Copy link
Author

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

@phw
Copy link
Collaborator

phw commented Sep 16, 2019

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
Copy link
Collaborator

phw commented Sep 16, 2019

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
Copy link
Author

simonltwick commented Sep 16, 2019

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?

@phw phw added the docs label Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants