quodlibet / mutagen

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

ID3: Consider size of extended header when reading ID3 data #631

Closed phw closed 1 year ago

phw commented 1 year ago

Reading of ID3 tags with extended headers could fail, as the data being read did not consider that the extended header had already been read. This meant it would read more data then actually part of the ID3 tags, and if not enough data was available (e.g. the ID3 is located at the end of the file) it would fail.

Fixes #630

phw commented 1 year ago

@lazka maybe it should be discussed if there is any value in mutagen supporting the extended header, maybe at least in a way that it tries to preserve it, at least those flags that make sense. Currently upon saving the extended header will be gone as it is not supported.

But this PR is purely for fixing the loading of such files, as at least this part is implemented, but was not fully working.

lazka commented 1 year ago

@lazka maybe it should be discussed if there is any value in mutagen supporting the extended header, maybe at least in a way that it tries to preserve it, at least those flags that make sense. Currently upon saving the extended header will be gone as it is not supported.

Hm, I wouldn't try to preserve them, since they have to be kept in sync with the data and we can't just start failing on save when writing a new tag, but we could expose them on load and allow them as extra save options.

But, I'm not sure that this is all worth it. We should at least document somewhere which parts of the spec we implement and which we don't, like footers for example.