Fix for ogg trailing null bytes #674
Conversation
|
Seems useful. Is there some example file to test this? Would also be good to have tests covering this case. |
|
Found an encoding of a CC-BY-SA song that has the issue, Bad Sign by Brad Sucks import mutagen
audio = mutagen.File("test.ogg")
audio['artist'] = "Brad Sucks"
audio.save()Will cause
In this case, the file has one null byte at the end, if I remove it with a hex editor and run that script, its happy. If I just append null bytes to a good ogg file, then test with that script, it doesn't cause the error. So maybe it's todo with the length of the file per the ogg page headers. |
There was a problem hiding this comment.
Thanks for the example file. The fix mostly seems fine, with some minor comments.
But we really should have a test for this. Have a look at test_ogg.py, specifically the test_renumber* tests there. There is already a test_renumber_extradata, which tests failing for some garbage data at the end.
There could be a similar test_renumber_trailing_null_bytes that adds some null bytes (26, to test for the edge case) and checks that the call to renumber succeeded, but the trailing bytes got preserved.
|
Yep, good suggestions. I'll work on a test when I have some time. |
|
Added test It fails without my change, passes with my change. Narrowing down what the issue is, it happens specifically when I do not have enough knowledge to know what that actually means, my test file needed to be renubered, and the test files I tried to generate to trigger the issue didn't. |
Fixes #591
So I have encountered some ogg files from some encoder that seems to append one or two null bytes to the file.
All existing tests pass.