Fix the mixed block boundary at 8 kHz and a truncated frame leak - #829
Merged
Merged
Conversation
Two findings from the review of #823. A frame the stream cuts short is abandoned once a granule needs main data the reservoir cannot supply, and that exit freed the granule array while the main data already copied for earlier granules in the same frame stayed allocated. Truncating l3-si.bit to about 1.5 kB reaches it with one granule's copy outstanding. Mixed blocks transform their lowest bands with long windows, and the decoder placed that boundary at a fixed 36 samples. That is right at every rate except 8 kHz, where the wider scalefactor bands put it at 72: the six long bands a low sampling frequency granule declares span 72 coefficients there, so coefficients 36 through 71 were being requantized with short window gains and everything after them shifted. ffmpeg says the same thing in compute_band_indexes: "if switched mode, we handle the 36 first samples as long blocks. For 8000Hz, we handle the 72 first exponents as long blocks." The boundary now comes from the band tables, through one helper the requantizer, the reorder step and the stereo band map share, so it follows whatever the tables say rather than a constant that happened to hold for eight of the nine rates.
Contributor
Author
|
/ci run 1b3dadf |
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
1 participant
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.
Follow-up to #823, fixing both review findings.
A truncated frame leaked what it had already copied
collectFramePayloadsabandons a frame once a granule needs main data the reservoir cannot supply. That exit freed the granule array while the main data already copied for earlier granules in the same frame stayed allocated. Truncatingl3-si.bitto around 1.5 kB reaches it with one granule's copy outstanding — the new test sweeps that range, and the testing allocator reports the leak without the fix.The mixed block boundary is wider at 8 kHz
Mixed blocks transform their lowest bands with long windows, and the decoder placed that boundary at a fixed 36 samples. That is right at every rate except 8 kHz: the six long bands a low sampling frequency granule declares span 72 coefficients there, so coefficients 36 through 71 were requantized with short-window gains and everything after them shifted. ffmpeg puts it the same way in
compute_band_indexes:The boundary now comes from the band tables through one helper —
3 * scalefactorBandShort(rate)[3], which equalsscalefactorBandLong(rate)[long_end]for all nine rates — shared by the requantizer, the count1 pass, the reorder step and the stereo band map. Those four sites each carried their own36.Tests
a frame cut between its granules frees what it already copied— sweeps the truncation range that reaches the partial-granule exit; fails with the allocator's leak report on the pre-fix code.the mixed block boundary follows the band tables, and widens at 8 kHz— 36 at eight rates, 72 at 8 kHz, cross-checked against the long table's eight-band (MPEG-1) and six-band (LSF) ends.an 8 kHz mixed block keeps 72 coefficients on long gains— long bands at gain 1, short bands at gain 4; on the pre-fix code coefficient 36 comes back at 4.zig build test-audio test-audio-internals— 954/954.