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

Dealing with junk at the end of a file? #7

Open
est31 opened this issue Sep 13, 2018 · 1 comment
Open

Dealing with junk at the end of a file? #7

est31 opened this issue Sep 13, 2018 · 1 comment

Comments

@est31
Copy link
Member

est31 commented Sep 13, 2018

The square-with-junk.ogg file from the libnogg test suite has junk at the end of the file. This doesn't seem to be a problem for libvorbis or libnogg. But the ogg returns an error instead of end of stream when it encounters that junk (it reads past the other junk in the file just fine).

If we want to tolerate this trailing junk, we need to be able to distinguish between a reader where we had already read valid ogg stuff, and one where we hadn't, so that we don't return "no pages" when encountering a non-ogg file. A non ogg file is different from an empty file!

This ability to distinguish requires us to store more state, and I'm not sure whether it's worth it.

@est31
Copy link
Member Author

est31 commented Sep 13, 2018

See also RustAudio/lewton@eb231f7

est31 pushed a commit that referenced this issue May 29, 2022
* Do not return error on garbage data after last Ogg page

Some admittely badly-encoded Ogg Vorbis files in the wild contain
trailing garbage data after the last Ogg page, such as this one:
https://freesound.org/people/stomachache/sounds/157587/.

The page reading code explicitly handled this case by returning an
error, which makes sense to do given that the Ogg specification is not
clear on how this condition should be dealt with. Moreover, programs
like `ogginfo` also show warnings for these kind of files. However,
being this strict causes codec crates that depend on this one to not be
able to play back such files properly, as discussed in
#7.

Fix the situation by being more lenient with this nonsense. Some
additional status information is kept to distinguish this case from
empty files.

* Add unit test for proper trailing data after last page handling

* Ensure that trailing data is not returned in a packet in unit test

I don't expect this additional check to be useful in practice, but it is
an invariant that should be held in the test case after all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant