fix(celt): correct haar1 AVX deinterleave, VBR nominal cap, and VBR-fragile tests - #17
Merged
Merged
Conversation
…ragile tests haar1_avx loaded its second vector at +4 instead of +8 (overlapping the first) and deinterleaved into quartet-swapped order, corrupting every other 8-sample block of stride-1 Haar transforms with n0>=16. This poisoned tf_analysis best-level selection and recombine/time-divide paths, decorrelating everything above ~1kHz (2-4kHz shape corr 0.87, 4-8kHz 0.80 vs 0.99+ from libopus at the same bitrate). Replace with an AVX2 kernel (permutevar8x32 lane fixup); pre-AVX2 CPUs fall back to the scalar path. Verified bit-near-identical to libopus float haar1 (max abs diff 5.3e-8) and healed shapes (mono 96k: 0.99/0.98/0.99/0.98/0.97, stereo 192k: ~0.999 across bands). VBR packets were hard-capped at nominal size (libopus caps CBR only), neutering the reservoir: C emits 265B where we emitted 240B. Gate the nominal cap on use_cbr. Tests: canary impulse bound tracks C parity (libopus emits 282B for the vector; bound 200 was tighter than C). Multi-frame interop tests pinned CBR: RFC 6716 code 1 requires equal-sized frames and zero-padding payloads is only neutral at equal sizes, neither of which VBR guarantees.
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
3 participants
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.
Includes #16 as base commit; happy to rebase if #16 lands first.
haar1 AVX deinterleave (the audibility fix)
haar1_avxloaded its second vector at +4 instead of +8 (overlapping the first) and deinterleaved into quartet-swapped order, corrupting every other 8-sample block of stride-1 Haar transforms with n0>=16. This poisoned tf_analysis best-level selection and recombine/time-divide paths, decorrelating everything above ~1kHz at the same bitrate (mono 96k shape corr vs source: 2-4kHz 0.87, 4-8kHz 0.80, 8-14kHz 0.81; libopus scores 0.99+).Replaced with an AVX2 kernel (permutevar8x32 lane fixup); pre-AVX2 CPUs fall back to the scalar path, which was already correct (NEON uses vld2/vst2 and is unaffected). Verified bit-near-identical to libopus float haar1 (max abs diff 5.3e-8, 0/64 lanes differ). After the fix, mono 96k shapes read 0.99/0.98/0.99/0.98/0.97 and stereo 192k reads ~0.999 across bands.
Note vs #16: that PR only gates the imports of
haar1_avx; this PR replaces the kernel body itself (and carries the same cfg-gated import pattern on the new function so 32-bit keeps compiling).VBR nominal cap
Packets were hard-capped at nominal size in VBR mode (libopus caps CBR only, opus_encoder.c), neutering the reservoir: C emits 265B where we emitted 240B on the same transient frame. The nominal cap now applies in CBR mode only. Average bitrate is unchanged; frames may now borrow above nominal like C does.
Tests
Verification (re-run after stacking onto #16)
oracle_celt_*pass, so existing vectors are unaffected).cargo fmt --checkreports 22 diffs, all inside fix: correct x86 32-bit AVX/AVX2 intrinsic imports #16's added lines under newer rustfmt (import ordering); none from this PR's hunks. Upstream has no fmt CI; left untouched to avoid noise against fix: correct x86 32-bit AVX/AVX2 intrinsic imports #16.