fix: correct WAV decoder normalization for 16-bit and 24-bit PCM#177
Open
Yanhu007 wants to merge 1 commit into
Open
fix: correct WAV decoder normalization for 16-bit and 24-bit PCM#177Yanhu007 wants to merge 1 commit into
Yanhu007 wants to merge 1 commit into
Conversation
16-bit PCM samples (range [-32768, 32767]) were divided by 65535 (unsigned max) instead of 32768 (signed max), resulting in output range [-0.5, 0.5] instead of [-1.0, 1.0]. 24-bit PCM samples had the same issue: after packing into int32 and dividing by 256, the result was divided by 16777215 instead of 8388608, again producing half amplitude. Fix: divide 16-bit by (1<<15) and 24-bit by (1<<31). Fixes faiface#176
|
Concept ACK However, at minimum edge cases must be checked e.g. whether +32767 maps to +1.0 and whether -32767 or -32768 maps to -1.0 Although we want this fix in neurlang/gomel ASAP, this fix may break other consumers that already wrongly expect 50% volume and compensate for this bug using 2x volume boost. It is highly recommended to release this only as a breaking change version. Please also let us know ahead before tagging it so we can coordinate rollout on our side (if merged). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #176
Problem
The WAV decoder normalizes signed PCM samples using unsigned maximum values, producing audio at ~50% amplitude:
/ 65535(1<<16-1)/ 256 / 16777215Fix
Use signed maximum values:
/ 32768(1<<15)/ 2147483648(1<<31)This matches the standard PCM normalization convention used by other audio libraries.