Skip to content

io: Bound WAV lengths to physically available audio - #525

Merged
cjacoby-sptfy merged 4 commits into
masterfrom
cjacoby/fix-wav-data-length
Nov 3, 2026
Merged

cjacoby-sptfy merged 4 commits into
masterfrom
cjacoby/fix-wav-data-length

Conversation

@cjacoby-sptfy

@cjacoby-sptfy cjacoby-sptfy commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

A finite, seekable WAV can declare more audio data than physically exists.
For example, a 100 ms stereo PCM16 file at 48 kHz with a data size of
0xFFFFFFFF reports over six hours of audio. This can cause oversized buffer
allocations and resampling or writing of silence past physical EOF.

We discovered this while ingesting a new dataset which had a couple of files
with malformed wav headers.

Solution

Locate the audio JUCE selects and calculate how many complete frames fit
between its start and physical EOF. Use JUCE's decoding rules for frame
size, then lower its reported frame count only when it exceeds that limit.

Keep existing format acceptance and codec selection. Add regression tests
for supported PCM/float32 WAV and RF64 layouts, plus compatibility checks
against 0.9.26. A version guard flags future JUCE upgrades for review.

Result

For supported inputs, overstated lengths no longer make reads or resampling
continue through nonexistent audio. Shorter declarations and unrelated
decoder behavior stay as before.

Recovery needs a known stream length and an identifiable audio layout. It
cannot distinguish audio from trailing metadata swallowed by an oversized
data chunk, so those bytes can still be decoded as audio.

Local validation on macOS arm64:

  • 286 WAV cases pass. The full suite reports 27,215 passes, 4 skips and one
    previously observed threading-performance failure.
  • CI-pinned type checking, formatting and diff checks pass. The docs build
    has the same 17 existing diagnostics. Cross-platform checks run in CI.

Problem

The new WAV tests produce 11 errors in CI's Python type-check job because
an extensible-header GUID can be None and callback streams do not declare
the binary-I/O interface expected by AudioFile's overloads.

Solution

Narrow the optional GUID before constructing an extensible header and use
the existing BinaryIO adapter convention for the bounded callback stream.

Result

CI's pinned pyright reports zero errors across the package and test files.
All 140 WAV regression cases pass with callback behavior preserved.
Problem

The initial length fix imposed additional malformed-header validation and
rejected layouts that the existing decoder accepted. That exceeds the
scope of correcting audio declarations that extend past physical EOF.

Solution

Restore the original codec dispatch and delegate format acceptance to
JUCE. Isolate a best-effort lookup of the selected payload and decoder
stride, following repeated-chunk and RF64 traversal without new rejection
paths. Bound the native reader by physical availability only after JUCE
constructs it, and retain shorter declarations and legacy stream behavior.

Add baseline compatibility controls and regressions for understated RIFF
boundaries, normalized float32 headers, repeated selected payloads and
failed seeks. Document the narrower recovery contract.

Result

All 286 WAV tests pass and CI's pinned type checker reports zero errors.
The full suite reports 27,215 passes, four skips and only the previously
observed threaded-versus-serial I/O timing failure.
@cjacoby-sptfy
cjacoby-sptfy marked this pull request as ready for review October 23, 2026 23:18

@bushaev bushaev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cjacoby-sptfy
cjacoby-sptfy merged commit 2bcc05f into master Nov 3, 2026
40 of 41 checks passed
@cjacoby-sptfy
cjacoby-sptfy deleted the cjacoby/fix-wav-data-length branch November 3, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants