Fix the last of the WAV header guesswork: file headers, 8-bit sign, EXTENSIBLE - #14
Merged
Merged
Conversation
…XTENSIBLE Four defects in `recode.audio`, all in how a WAV header is read. `extract_wav_header_from_file` still inferred the header size as `chunk_size + 8 - subchunk2_size` -- the very arithmetic #4 removed everywhere else, and the last instance left. It reads bytes 40-44 as the audio size, which is true only of a bare 44-byte header. On real ffmpeg output it returned nearly the whole file as "header" (26499 of 26562 bytes on a 6-channel sample); it disagreed with `header_size_of_wav_bytes` on the same bytes and never raised on anything. It now walks the RIFF structure like everything else, reading only as much of the file as the header occupies, and raising `ValueError` when the file is not a WAV. 8-bit WAV PCM is unsigned (0..255, silence at 128); recode read and wrote it signed, so digital silence decoded as -128 and the files it wrote were misread by every other tool. Fixed in the WAV path only, behind a keyword-only `eight_bit_unsigned=True` on `decode_wav_bytes`/`encode_wav_bytes`; passing `False` reproduces today's bytes exactly. `soundfile` now agrees with recode in both directions on 8-bit files. Closes #12. `WAVE_FORMAT_EXTENSIBLE` (fmt tag 0xFFFE) is what ffmpeg writes for anything above two channels or sixteen bits, and stdlib `wave` only learned to read it in 3.12 -- so those files raised `unknown format: 65534` on 3.10, the version CI runs. `Wave_read` stays the primary reader; a `fmt ` chunk it refuses is parsed directly, accepting the same PCM SubFormat GUID 3.12 insists on, so every other `wave.Error` still surfaces unchanged. A 6-channel ffmpeg file now decodes byte-identically on 3.10 and 3.12. Closes #13. Finally, #11 dropped `header_size_of_wav_bytes`' `meta` parameter and the `MIN_WAV_N_BYTES` constant while the new docstring promised `meta` was still accepted. Both are restored, `meta` as an ignored no-op. Also splits the chunk walk out of `_wav_data_chunk` into `_wav_chunk`, which is what the three new readers share. No behaviour change there. Suite: 151 -> 172 passing, green on 3.10, 3.11, 3.12 and 3.13. Claude-Session: https://claude.ai/code/session_01L1aQPB34n7PU7jmbztSjBe
thorwhalen
added a commit
that referenced
this pull request
Sep 22, 2026
encode_wav_bytes(np.int8 array, width_bytes=1) worked before #14 and raised OverflowError after it (numpy 2) because the +128 bias was applied to np.int8 scalars. Samples are now widened with operator.index before the shift, which also keeps non-integer samples failing loudly instead of truncating. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Four defects in
recode.audio, all in how a WAV header is read:extract_wav_header_from_filestill inferred header size aschunk_size + 8 - subchunk2_size— the arithmetic every other reader in themodule had already moved away from. On real ffmpeg output it returned nearly
the whole file as "header" and never raised on non-WAV input. Now walks the
RIFF structure like the rest of the module and raises
ValueErroron anon-WAV file.
signed, so digital silence decoded as -128 and files it wrote were misread by
every other tool. Fixed in the WAV path behind a new keyword-only
eight_bit_unsigned=Trueondecode_wav_bytes/encode_wav_bytes; passingFalsereproduces today's bytes exactly. Closes 8-bit WAV is decoded as signed, but WAV 8-bit PCM is unsigned (silence reads as -128) #12.WAVE_FORMAT_EXTENSIBLE(fmt tag0xFFFE, what ffmpeg writes for >2channels or >16 bits) made stdlib
waveraiseunknown format: 65534onPython 3.10 (the CI version; stdlib only learned to read it in 3.12). A
fmtchunkWave_readrefuses is now parsed directly, accepting the samePCM SubFormat GUID 3.12 accepts; every other
wave.Errorstill surfacesunchanged. Closes WAVE_FORMAT_EXTENSIBLE decodes on Python 3.12 but raises on 3.10 (the CI version) #13.
header_size_of_wav_bytes'smetaparameter andMIN_WAV_N_BYTES,both dropped in Fix #4: locate the WAV
datachunk instead of inferring where it must be #11 while the docstring still promised them (metaas anignored no-op).
No public export or existing default changed except
eight_bit_unsigned(new keyword-only, defaults to
True— the only behavior-changing default inthis PR, and it's a bug fix: 8-bit WAV silence was previously mis-decoded).
Dependents check (fleet_dependents.json:
recode-> [hum,know], bothpresent on this box):
humonly callsrecode.decode_wav_bytes(never passeseight_bit_unsigned, so gets the new default). Installed this branch'srecodeoverhum's env and ran its non-hardware-dependent test suite:22 passed, 1 skipped (a
pyo-audio-engine test, skipped identically withoutthis change — no audio backend on this box), 1 deselected. No 8-bit WAV
fixtures in
hum's suite, so this is an import/wiring check rather than abehavioral one for the 8-bit path specifically.
knowcallsrecode.mk_codec(inrecode/base.py, untouched by thisdiff — confirmed via
git diffshowing zero changes tobase.py/__init__.py)and
recode.decode_wav_bytes(inknow/boxes/dols.py). Could not installknowon this box —audiostream2py->pyaudioneedsportaudio.h, asystem audio library not present here.
know's own test suite(
test_main_exports.py,test_slabsIter.py) doesn't exercisedecode_wav_bytesat all (the latter reads WAV viasoundfile, notrecode), so even a successful install would not have tested the changedpath. Recorded as a source-level read in DECISIONS.md: the risk is confined
to 8-bit WAV input, which is uncommon for keyboard/stream audio capture, and
is a documented, opt-outable bug fix rather than an arbitrary default change.
Branch sat pushed with green CI and no PR for two weeks (thorwhalen/fleet_stuff
cleanup). Verified: master has not moved since the branch was cut (no rebase
needed).
recodehas nopyproject.toml/wads config (setup.py/setup.cfgonly) -> gated with plain venv+pytest (py3.10, matching CI): 172/172 passed
(145 in
test_recode.py+ 27 doctests), matching the commit's own claimed"151 -> 172 passing". Hosted CI on the branch was already green from the
original 2026-09-08 push (3.10, 3.11, 3.12, 3.13).
Closes #12
Closes #13
🤖 Generated with Claude Code