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.

Sign up for GitHub

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

Jump to bottom

audio_simple_with_error.ogg gives wrong decoded output #24

Closed
est31 opened this issue Dec 8, 2017 · 3 comments
Closed

audio_simple_with_error.ogg gives wrong decoded output #24

est31 opened this issue Dec 8, 2017 · 3 comments
Labels
bug

Comments

@est31
Copy link
Member

est31 commented Dec 8, 2017 •
edited
Loading

Originally reported at ggez/ggez#207 then brought to rodio at RustAudio/rodio#147 . Our output is very bad audio. It can be heard, and comparison shows a difference as well:

Comparing output for audio_simple_with_error.ogg : 4409 differing packets of allowed 1.

A small example is: audio_simple_err.zip (actually an ogg file but I had to change the extension in order for github to accept it).
@est31 est31 added the bug label Dec 8, 2017
est31 added a commit that referenced this issue Mar 22, 2018
@est31
Add test file for issue #24
3683595
@est31
Copy link
Member Author

est31 commented Mar 22, 2018

Interestingly, the file has only a single mapping, just like the single-mapping.ogg test file from xiph. Maybe the two files are about the same bug?

@est31
Copy link
Member Author

est31 commented Sep 23, 2018

Just ran stb_vorbis on the file, and I found a mismatch in the residue. singlemap-test.ogg gives a mismatch as well. So this points even more towards the same bug.

@est31 est31 closed this as completed in 93c7744 Oct 7, 2018
@est31
Copy link
Member Author

est31 commented Oct 7, 2018 •
edited
Loading

Indeed, it was the same bug. Fixed in 1073e58 .

est31 added a commit that referenced this issue Oct 7, 2018
@est31
Fix an issue in the float32_unpack routine
1073e58 
There has been a bug that led lewton to wrongly decode
singlemap-test.ogg as well as audio_simple_with_error.ogg.

I tracked it down by printing out intermediary values
for lewton as well as for stb_vorbis and comparing them.

First, I saw that there was a mismatch in the pre imdct
values. Then, I tracked it down to a mismatch in the
residue values. After that, I found out that the codebook
values were wrong. And then, I saw that the settings for
codebook_minimum_value and codebook_delta_value have been
parsed wrongly by lewton, which caused all the other
mismatches. This wrong parsing has been caused by
a bug in the behaviour of the float conversion routine
float32_unpack.

The deployed fix was to replace the overengineered
and overoptimized bit twiddling implementation
by a more straight forward, spec following one.
Performance impact wasn't measured, but the code is
super cold: It's called only during header setup
and even there only twice per codebook.
In singlemap-test.ogg, it was called
a few dozen times or such.

Fixes #24, and also addresses parts of #26.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@est31