Emtpy Vorbis Comment

[har0ke]

I got a few files containing tailing empty vorbis comment. I am aware that no "specification" explicitly allows this, but all applications (vlc, rythmbox, totem, my old mp3-player, etc.) that i tested, are able to correctly process these files. So my suggestion would be to just ignore empty comments.

[ruuda]

Are you running into a decoding error specifically? If so, are you able to share one of these files? Just the header blocks without the audio data would be fine.

[har0ke]

This check fails, since an empty comment has also no "=" sign in it: https://github.com/ruuda/claxon/blob/e0a45dca1e3a2c4543e199313482ae68ab3d9696/src/metadata.rs#L465 I've created a fix you might use as a reference to understand and/or fix the issue (https://github.com/har0ke/claxon/commit/1b38b67e6e13b1d7a3fe9f974413b32b3bc9c13d).

Edit: I can also send you a file if necessary.

[ruuda]

I can also send you a file if necessary.

That would be great, you can send it to me privately if you wish. Then I can reduce it to a minimal example (without audio data, and irrelevant metadata removed) and add it to the test suite, to ensure we don't regress in the future.

[har0ke]

Sounds good. I sent you the file privately.

[ruuda]

Thanks, I’ve received it all right. From what I can tell, official metaflac 1.3.3 does not allow creating such files, but it does read them.

• The changelog for libflac 1.3.2 mentions “Only write vorbis-comments if they are non-empty”, and the vendor string of your file is reference libFLAC 1.2.1 20070917, so I suspect the empty comment sneaked in due to a bug in an application that used libflac 1.2.1.
• metaflac --list --block-type=VORBIS_COMMENT prints the last comment as empty, whereas ffmpeg does not list the empty comment at all. metaflac --export-tags-to=tags.txt writes a file with a blank line at the end.

• As expected from the changelog, when I try to metaflac --import-tags-from=tags.txt the tags onto a different file (to create a test case), or when I try to metaflac --set-tag='', metaflac says:

  ERROR: malformed vorbis comment field "",
         field contains no '=' character

I suppose there is no harm in ignoring empty Vorbis comments rather than reporting an error, and given that this happens in the wild, we probably have to. I’ll try to build an old version of metaflac so I can transplant the Vorbis comment block onto the small test sample.

---

It seems that apart from the empty Vorbis comment, your file is missing a frame sync code. If I strip the Vorbis comment block with metaflac --remove --block-type=VORBIS_COMMENT, then Claxon reports frame sync code missing. flac --test reports Got error code 0:FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC, and ffmpeg prints

[flac @ 0x55c24107d600] invalid sync code
[flac @ 0x55c24107d600] invalid frame header
[flac @ 0x55c24107d600] decode_frame() failed
Error while decoding stream #0:0: Invalid data found when processing input

but it does recover and decodes file. Most likely this is why players can still play the file. Claxon currently does not support seeking, which is required to recover, so it would not be able to decode the file even without the Vorbis comment block.

[ruuda]

I ended up adapting one of the existing test cases in a hex editor to reproduce the issue, and I added a regression test. Your fix makes it pass, and I think it is appropriate, so I cherry-picked it.

Can you confirm that the issue has been resolved on master now?

[har0ke]

Yes, the faulty comment of that file is now correctly processed. As for the invalid sync code, i would say that this is a different issue. I might look into that if i require that feature in the future.