Skip to content

fix(audio): Whisper uses AudioReader - #809

Open
tawei-amd wants to merge 8 commits into
mainfrom
fix/audio-reader-permissive
Open

fix(audio): Whisper uses AudioReader#809
tawei-amd wants to merge 8 commits into
mainfrom
fix/audio-reader-permissive

Conversation

@tawei-amd

@tawei-amd tawei-amd commented Oct 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

What

Decoding (src/common/audio/audio_decoder.cpp, permissive libraries only):

Format Decoder License
WAV / RF64 / W64 / AIFF, FLAC (native + Ogg), MP3 dr_wav / dr_flac / dr_mp3, vendored at dfe83776, which includes post-release hardening fixes (e.g. a dr_mp3 Xing heap over-read) Public domain / MIT-0
Ogg Vorbis libvorbis + vorbisfile 1.3.7, libogg 1.3.6 BSD-3-Clause
Ogg Opus, WebM/Matroska Opus (browser MediaRecorder output) libopus 1.6.1 + a small Ogg demuxer and a bounds-checked EBML parser (unknown-size elements, all lacing modes, DiscardPadding) BSD-3-Clause
AAC (.m4a/.mp4/ADTS) rejected, with a message listing the supported formats n/a

Ogg Opus output length follows RFC 7845: final granule minus start offset minus pre-skip.

Resampling (audio_resampler.cpp): a re-implementation of swresample's default filter design, so samples stay where FFmpeg put them. Kaiser-windowed sinc (β 9, cutoff 0.97, filter_size 32, rounded up to an even tap count), exact rational phases up to 1024 (otherwise 1024 phases with linear interpolation), and the same edge mirroring as invert_initial_buffer / resample_flush.

AudioReader keeps its API and downmix modes. Whisper's two _load_audio overloads (about 500 lines of FFmpeg code) now call AudioReader at 16 kHz with MEAN downmix, then quantize to 16-bit exactly as swresample's float→s16 conversion did. Whisper's input is therefore unchanged: on all 84 ASR eval clips, 99.95% of samples are bit-identical to main's, and the rest differ by one 16-bit step.

Input hardening: source sample rates outside 4–768 kHz are rejected. Fuzzing found that a WAV header claiming 1 Hz would make the 16 kHz resample of a 0.5 MB upload about 8.5 GB.

Build:

  • src/cmake/FlmAudioCodecs.cmake builds libopus, libogg and libvorbis statically via ExternalProject, with pinned URLs, SHA-256 hashes, and short directories for Windows' path limit. It also compiles audio_decoder.cpp with -fwrapv, so the decoders' wrapping arithmetic on corrupt input is defined.
  • src/test harnesses: CMake links the module automatically; the Makefiles use the system -lvorbisfile -lvorbis -logg -lopus. No harness references FFmpeg any more, and phi4_core's FFmpeg stub headers are gone.
  • THIRD_PARTY_NOTICES.txt adds libopus, libogg, libvorbis and dr_libs.

Verification

Run locally against FFmpeg 8 (PyAV) and libsndfile 1.2.2, driving the real AudioReader:

Input Native-rate decode vs FFmpeg 16 kHz mono pipeline vs FFmpeg
WAV 16/24-bit/float/u8, 6-ch WAVEX, RF64, W64, AIFF, FLAC 16/24 bit-exact 131–141 dB SNR
Ogg Vorbis 135.7 dB, same length 133.7 dB
Ogg Opus (libsndfile and FFmpeg muxers), WebM Opus mono/stereo 138–139 dB, same length ~133 dB
MP3 (synthetic, repo test_audio.mp3, all 84 ASR eval clips) 118–124 dB; eval clips ≥108 dB vs FFmpeg, ≥127.8 dB vs mpg123 ≥108 dB
AAC in M4A, ADTS rejected cleanly n/a
  • Resampler alone vs swresample on 11 rate pairs (8k–96k ↔ 16k/24k/48k, mono/stereo, non-reducible 44099 Hz), including edges: 132–142 dB SNR, identical lengths.
  • Fuzzing: 6,000 truncated or mutated inputs (all formats) under ASan + UBSan: no crash, leak or undefined behavior.
    • An earlier version used stb_vorbis, which leaked and hit undefined behavior on corrupt Ogg; that's why this PR uses libvorbis.
    • A Matroska CodecID is sanitized before it appears in error messages.
  • Speed: a 110 s 48 kHz stereo MP3 → 16 kHz mono takes 0.26 s, about 420× real time (FFmpeg: 0.09 s).
  • Output length can differ from FFmpeg's by ±1 sample after resampling. FFmpeg's flush rounding depends on its internal buffering, so the old code wasn't consistent here either.

Found and fixed during CI

Issue Fix
libvorbis 1.3.7 declares cmake_minimum_required(2.8.12), which CMake 4 rejects pass CMAKE_POLICY_VERSION_MINIMUM=3.5
Windows runner's connection reset while downloading from downloads.xiph.org mirror list (GitHub / xiph / OSUOSL), same SHA-256
M_PI undefined on MSVC local constant
ASR CI: gemma4 chat WER 5% → 8–9% on every platform. The eval clips are quiet 16 kHz MP3s, and dr_mp3 synthesizes 16-bit PCM unless DR_MP3_FLOAT_OUTPUT is defined (only ~35–45 dB vs FFmpeg/mpg123) DR_MP3_FLOAT_OUTPUT; gemma back to main's 5.05%
Whisper pooled WER moved by about ±2 points depending on whether its input was 16-bit-quantized keep main's s16 quantization for Whisper (see above)

Still to check:

  • CI builds on Linux (.deb, portable, HRX) and Windows (portable, MSI, HRX); first build of the audio ExternalProjects there.
  • asr-test vs main on all four platforms (re-running on a22aecf6).
  • Audio model sweeps (gemma4e / gemma4_12b / qwen3.5-omni, Whisper with test1.ogg).

🤖 Generated with Claude Code

tawei-amd and others added 6 commits October 16, 2026 16:14
tawei-amd @claude
fix(image): decode with libjpeg-turbo + lodepng instead of FFmpeg
69bb411 
ImageReader used FFmpeg (avcodec + swscale) to decode JPEG/PNG, convert to
RGB24, resize, and encode PNG. Replace it with permissive libraries and
match Pillow, which the HF image processors use, exactly:

- JPEG: libjpeg-turbo 3.2.0 (IJG/BSD-3-Clause/zlib), libjpeg API with its
  defaults (accurate integer IDCT, smooth chroma upsampling), i.e. what PIL
  decodes with. CMYK/YCCK images use Pillow's CMYK->RGB formula, including
  its inversion of Adobe CMYK. Fatal libjpeg errors longjmp back instead of
  calling exit().
- PNG: LodePNG (zlib license), vendored in src/include/lodepng; it also
  replaces FFmpeg's PNG encoder in save_png.
- resize_image: a port of Pillow's bicubic resampler (Resample.c) instead
  of swscale bicubic, so resized pixels equal PIL.Image.resize(BICUBIC).
- Reject images over 178,956,970 pixels (Pillow's decompression-bomb limit)
  instead of attempting multi-GB allocations from tiny headers.

libjpeg-turbo refuses add_subdirectory(), so src/cmake/FlmImageCodecs.cmake
builds it statically via ExternalProject (pinned URL + SHA-256, libjpeg API
only, which also keeps out the libspng/zlib copy TurboJPEG bundles) and
defines flm_image_codecs for flm and the src/test harnesses. The harness
Makefiles link the system libjpeg; image-only harnesses drop FFmpeg
entirely, the audio ones keep it minus swscale. Also drop the unused FFmpeg
includes from modeling_gemma3_text.hpp and add both libraries to
THIRD_PARTY_NOTICES.txt.

FFmpeg itself stays for audio (AudioReader, Whisper) and is removed in a
follow-up together with swscale.

Verified against Pillow 12.3 (libjpeg-turbo 3.1.4.1): bit-exact decode on
24 images (4:4:4/4:2:2/4:2:0, progressive, optimized, grayscale, Adobe
CMYK, 1x1, 2049x1531, every PNG color type, and the repo's real photos and
PNGs including the vision sweep test images), bit-exact resize on 27
size pairs (55x down to 1x1, upscales, real and synthetic), exact
save_png round trip; truncated/corrupt/unsupported inputs rejected.

Co-Authored-By: Claude <[email protected]>
tawei-amd @claude
build(image): keep libjpeg-turbo ExternalProject paths short for Windows
d732a0b 
The default <name>-prefix/src/<name>-build layout put libjpeg-turbo's
try-compile tlog files at 262 characters on the self-hosted Windows
runners, past MAX_PATH, so MSBuild failed its first compiler check
(GetOutOfDateItems: item metadata %(FullPath) cannot be applied).
Use short working directories under build/ljt instead (-47 chars).

Co-Authored-By: Claude <[email protected]>
tawei-amd @claude
fix(audio): decode without FFmpeg; Whisper uses AudioReader
0b0007f 
AudioReader and Whisper's own decoder used FFmpeg (avformat/avcodec +
swresample). Replace both with permissively licensed decoders and a
resampler that reproduces swresample's default filter, so audio reaching
the models stays numerically the same:

- audio_decoder.cpp: format sniffing plus
  - WAV/RF64/W64/AIFF, FLAC (native and Ogg) and MP3 via dr_libs
    (public domain / MIT-0), vendored at commit dfe83776, which carries the
    post-release hardening fixes (e.g. dr_mp3 Xing heap over-read);
  - Ogg Vorbis via libvorbis/vorbisfile + libogg (BSD-3-Clause);
  - Ogg Opus and WebM/Matroska Opus via libopus (BSD-3-Clause) with a
    small Ogg demuxer and a bounds-checked EBML/Matroska parser (unknown-
    size elements, all lacing modes, DiscardPadding). Ogg Opus length
    follows RFC 7845 (start offset + pre-skip + final granule).
  - AAC (.m4a/.mp4/ADTS) is rejected with a clear message listing the
    supported formats.
- audio_resampler.cpp: Kaiser-windowed sinc with swresample's defaults
  (beta 9, cutoff 0.97, filter_size 32, even tap count, exact rational
  phases up to 1024 else interpolated, mirrored edges as in
  invert_initial_buffer/resample_flush).
- AudioReader keeps its API and downmix modes; Whisper's two _load_audio
  overloads (~500 lines of FFmpeg code) now call AudioReader at 16 kHz
  with MEAN downmix, matching the previous swr result (s16 output made swr
  average stereo), but without the 16-bit quantization.

Build: src/cmake/FlmAudioCodecs.cmake builds libopus 1.6.1, libogg 1.3.6
and libvorbis 1.3.7 statically via ExternalProject (pinned URLs and
SHA-256, short dirs for Windows MAX_PATH) and compiles audio_decoder.cpp
with -fwrapv. The src/test harnesses link it (CMake) or the system
libraries (Makefiles) and no longer reference FFmpeg at all; phi4_core's
FFmpeg stub headers are gone since no production header includes FFmpeg.
THIRD_PARTY_NOTICES.txt lists the new libraries. FFmpeg itself is now
unused by the code; dropping it from the build and packages follows
separately.

Verified against FFmpeg 8 (PyAV) and libsndfile on 22 files:
- lossless (WAV 8/16/24-bit/float, 6ch, RF64, W64, AIFF, FLAC): bit-exact;
- Ogg Vorbis, Ogg Opus (two muxers), WebM Opus mono/stereo: 133-139 dB SNR
  and identical lengths; MP3 ~90 dB (different decoder), real
  test_audio.mp3 max abs diff 4.6e-5;
- resampler vs swresample on 11 rate pairs incl. edges: 132-142 dB SNR;
- AAC (M4A, ADTS) rejected.
6,000 mutated/truncated inputs under ASan+UBSan: no crash, leak or UB
(an earlier stb_vorbis-based version leaked and hit UB, hence libvorbis).

Co-Authored-By: Claude <[email protected]>
tawei-amd @claude
build(audio): let libvorbis 1.3.7 configure under CMake 4
28d86b9 
libvorbis 1.3.7 declares cmake_minimum_required(VERSION 2.8.12), which
CMake 4 (CI runners and build containers) rejects: 'Compatibility with
CMake < 3.5 has been removed'. Pass CMAKE_POLICY_VERSION_MINIMUM=3.5 to
that ExternalProject; older CMake ignores it. Verified with CMake 3.28
and 4.4.

Co-Authored-By: Claude <[email protected]>
tawei-amd @claude
build(audio): add download mirrors for the Xiph tarballs
212e6df 
A self-hosted Windows runner had its connection to downloads.xiph.org
reset while fetching opus-1.6.1.tar.gz. Give each ExternalProject a
mirror list (GitHub release assets where they exist, downloads.xiph.org,
and the OSUOSL mirror); CMake tries them in order and the pinned SHA-256
still guarantees the same tarball. All mirrors verified hash-identical.

Co-Authored-By: Claude <[email protected]>
tawei-amd @claude
fix(audio): build with MSVC (no M_PI)
88881c8 
M_PI is not standard C++; MSVC only defines it with _USE_MATH_DEFINES, so
audio_resampler.cpp failed to compile on Windows. Use a local constant.
Also include <cstdio> explicitly for SEEK_* in audio_decoder.cpp instead
of relying on transitive includes.

Co-Authored-By: Claude <[email protected]>
@tawei-amd tawei-amd changed the title fix(audio): decode without FFmpeg; Whisper uses AudioReader fix(audio): Whisper uses AudioReader Oct 17, 2026
tawei-amd and others added 2 commits October 16, 2026 23:39
tawei-amd @claude
fix(audio): decode MP3 in float; reject implausible sample rates
d3704e3 
The ASR CI on this PR showed gemma4 chat WER worse on every platform
(5.05% -> 8-9%). All eval clips are 16 kHz mono MP3, and dr_mp3 by
default synthesizes 16-bit PCM and converts it to float, which on these
quiet clips is only ~35-45 dB from FFmpeg/mpg123 (they agree with each
other at 108-121 dB). Define DR_MP3_FLOAT_OUTPUT: our decode now matches
mpg123 at >=127.8 dB and main's FFmpeg input at >=108 dB on all 84 eval
clips (identical lengths), and synthetic/real MP3s at 118-124 dB.

Fuzzing also found that a WAV header claiming a 1 Hz sample rate makes the
16 kHz resample of a 0.5 MB upload ~8.5 GB. Accept only 4-768 kHz source
rates, which caps resampling expansion at 4x. 6,000 mutated inputs under
ASan+UBSan are clean, with no slow inputs.

Co-Authored-By: Claude <[email protected]>
tawei-amd @claude
fix(whisper): keep the 16-bit input quantization of the FFmpeg path
a22aecf 
The previous Whisper decoder asked swresample for s16 output and divided
by 32768, so Whisper has always seen 16-bit-quantized audio. Feeding it
unquantized floats moved the ASR CI's pooled WER by about +2 points on 3
of 4 platforms (and the opposite way when the input happened to be
quantized), i.e. the eval is sensitive to that rounding and this PR is
not the place to change it. Quantize exactly as swresample did
(lrint(x * 32768), clip to int16, / 32768).

On all 84 ASR eval clips the result is identical to main's Whisper input
for 99.95% of samples; the rest differ by 1 LSB (MP3 decoder rounding),
with identical lengths.

Co-Authored-By: Claude <[email protected]>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tawei-amd