Decode MPEG-2 and MPEG-2.5 MP3 short blocks - #823
Merged
Merged
Conversation
A 24 kHz or 22.05 kHz MP3 aborted the inference worker the moment it hit a frame with short blocks: scalefactorBandShort covered the three MPEG-1 rates plus 16000 and panicked on everything else, which is six of the nine rates a frame header can carry. Those rates are what most text-to-speech and podcast exports use. The process died with SIGABRT, the request came back 503, and any other inference in flight on that worker died with it, even though the transcribe handler already turns a decode failure into a 400 before it looks up a model. The short tables for 22050, 24000, 12000, 11025 and 8000 are added from ISO 13818-3, with 12000 and 11025 sharing the 16000 table. 8000 also had no long table, in requantize.zig and in the second copy in huffman.zig, so it fell through to the MPEG-1 48 kHz layout and decoded to noise rather than crashing: a 440 Hz tone kept a tenth of its energy at 440 Hz instead of four fifths. Decoded against ffmpeg, every rate now correlates 0.97 to 0.99, matching the 0.99 the already-supported 44.1 kHz path scores on the same clips. A sample rate outside the nine is now refused when the frame header is read, so a gap like this one fails its own request instead of reaching a table lookup with no answer. The corpus gains six generated fixtures, one per newly supported rate plus an 8 kHz tone that pins the long bands, all of which abort the decoder without this change. They needed wiring as much as writing: the codec files behind the MP3 facade are only analyzed when something references them, so a filtered run of the curated audio suite collected none of their tests, and the whole MP3 decoder suite was unreachable from it.
Contributor
Author
|
/ci run f5f1a73 |
Two of the checked-in conformance vectors decoded short, which nothing noticed because no MP3 test was reachable from the audio suite. Both losses moved audio in time, which for transcription means every later timestamp is wrong. A frame whose granules are empty is a silent frame, and silence occupies its place in the clip. The decoder skipped those frames outright, so l3-si lost the five silent frames in its middle and pulled everything after them 5760 samples earlier. Against ffmpeg it scored 0.69 correlation at an offset of 2996 samples; it is now sample-exact at offset zero. The skip existed to drop the leading Xing/Info/VBRI frame, which really is metadata rather than audio, so that frame is now recognised by its signature instead of by being empty. A stream that stops mid-frame still carries that frame's header and side info, and Layer III keeps most of a frame's main data in earlier frames, so the audio is usually all in the reservoir. The iterator dropped the frame anyway: l3-compl ends 23 bytes into a 192 byte frame and lost its last 1152 samples. The tail is now decoded when it continues the stream being read and the reservoir satisfies it, and is still refused when a stray sync word near the end has no stream behind it. l3-compl is now bit-exact against ffmpeg over all 217 frames. Conformance cases gain `expected_samples` where a reference decoder agrees on the exact length, since a lower bound cannot see a frame quietly dropped or duplicated, and l3-si_huff's expectation moves to the 86400 samples ffmpeg produces from the 85248 it had been relaxed to. The corpus tests now run in the curated audio suite, which is what would have caught all of this.
Contributor
Author
|
/ci run e5c98ff |
Wiring the MP3 tests into the curated audio gate showed the gate could only ever reach tests a filter names, and a filtered run analyzes none of the files behind the codec facades. The module test root now runs unfiltered as well, which turns 55 failures loose at once: this change fixes all of them, and the suite is green at 511 tests. Three were decoder bugs. Short blocks were three times too loud. The IMDCT derived its normalization from the transform length, so the 12-point transform scaled by 2/6 where the 36-point one scaled by 2/18. Long blocks were bit-exact against ffmpeg and short blocks were not, which is why a 16 kHz tone drifted to 0.94 correlation and every window switch left an artifact. Both sizes now share one constant, and all six low sampling frequency fixtures decode sample-exact. An AAC access unit that followed a long enhancement fill with a shorter one dropped the subfields the longer one had set. The merge gated each field on the aggregate's latest payload length rather than on the length its subfields came from, so noise, stereo, harmonic, detail and phase hints reverted to defaults and two streams that differ only in those bytes decoded identically. The aggregate now records which payload its subfields came from. Parametric stereo restarted its carry counter on access units that had inherited their payload rather than carried one, so a stream of no-fill units never decayed. Only a unit with its own payload resets it now. The rest were tests. Two synthetic element builders wrote two pair codewords for a scalefactor band eight coefficients wide, which no decoder can read; the stereo entry point was handed a core rate where it takes the output rate; an SBR config named a 48 kHz extension over a 44.1 kHz core, which SBR cannot produce; hand-computed Huffman and LSF part2 expectations disagreed with the tables they were derived from; and several assertions measured the first decoded block, where the filterbank is still ramping, or an absolute epsilon tied to how loud a synthetic fixture happened to be. Those now compare against a second decode that differs only in the property under test. The corpus directory tests resolve their path from the runtime root or the package, and stop leaking the names they remove from the set.
Contributor
Author
|
/ci run df7ab01 |
Contributor
Author
|
/ci run 2a5ad2d |
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.
Fixes #822.
The crash
scalefactorBandShortcovered the three MPEG-1 rates plus 16000 and panicked on everything else, so six of the nine sample rates a Layer III header can carry aborted the process the moment a frame used short blocks: 22050 and 24000 (MPEG-2), and 11025, 12000 and 8000 (MPEG-2.5). 24 kHz and 22.05 kHz are what most text-to-speech and podcast exports use. The inference runtime is a separate worker process, so the@panictook it down with SIGABRT: the request returned 503 and anything else in flight on that worker failed too, which is theEmbedTransientFailurethe report saw in index enrichment. The handler already had the right behaviour for a file it cannot read - a 400UNSUPPORTED, decided before it even resolves the model - and a panic is what skipped it.The missing tables are added from ISO 13818-3, 8 kHz gains the long table it had been falling back to the MPEG-1 48 kHz layout for (in both copies), and a sample rate outside the nine is now refused when the frame header is read, so a future gap fails its own request instead of reaching a table lookup with no answer.
Three more decoder bugs found on the way
Short blocks were three times too loud. The IMDCT derived its normalization from the transform length, so the 12-point transform scaled by 2/6 where the 36-point one scaled by 2/18. Long blocks were bit-exact against ffmpeg and short blocks were not, which is why a 16 kHz tone sat at 0.94 correlation and every window switch left an artifact. Both sizes now share one constant.
Frames were leaving the timeline. A frame whose granules are empty is a silent frame, and the decoder skipped it outright:
l3-silost the five silent frames in its middle and pulled everything after them 5760 samples earlier. The skip existed to drop the leading Xing/Info/VBRI frame, which really is metadata, so that frame is now recognised by its signature instead of by being empty. A stream that stops mid-frame also lost its last frame even though Layer III keeps most of a frame's main data in earlier frames; the tail now decodes when the reservoir satisfies it, and is still refused when a stray sync word near the end has no stream behind it.AAC dropped enhancement subfields, and parametric stereo never decayed. An access unit that followed a long SBR fill with a shorter one reverted its noise, stereo, harmonic, detail and phase hints to defaults, because the merge gated each field on the aggregate's latest payload length rather than the length its subfields came from. Separately, PS restarted its carry counter on units that had inherited a payload, so a run of no-fill units never attenuated.
What the vectors say now
Correlation against ffmpeg's decode of the same file, best-offset aligned:
l3-he_modestill differs on 2 of its 128 frames, both in joint-stereo mode runs. That is the one measurement in this table that has not reached the reference, and it deserves its own investigation.The suite that was not running
audio_module_test_root.zigonly collects tests from files that analyzed code references, and under--test-filternothing referenced the codec files behind the MP3 facade - so no MP3 decoder test was reachable from the curated audio gate at all. The root now names those files, and the gate runs the module root unfiltered as well as through its 213 filters.That turned 55 pre-existing failures loose at once. All 55 are fixed here: three were the decoder bugs above, one was a Huffman table index that could run past the end of its table, and the rest were tests - synthetic AAC elements that wrote two pair codewords for a band eight coefficients wide, a stereo entry point handed a core rate where it takes the output rate, an SBR config naming a 48 kHz extension over a 44.1 kHz core, hand-computed Huffman and LSF part2 expectations that disagreed with the tables they came from, and assertions that measured the first decoded block where the filterbank is still ramping or an absolute epsilon tied to fixture loudness. Those now compare against a second decode differing only in the property under test.
Testing
zig build test-audio test-audio-internals- 951/951, including the 511-test unfiltered run that did not exist before