audio: find libsoxr on Apple Silicon, and stop the unfiltered decimation - #359
Closed
CryptVenture wants to merge 2 commits into
Closed
audio: find libsoxr on Apple Silicon, and stop the unfiltered decimation#359CryptVenture wants to merge 2 commits into
CryptVenture wants to merge 2 commits into
Conversation
write_pcm16_wav was the only writer the framework had, so every generated file was capped at 16-bit PCM regardless of sample rate. wav_reader already handles 8/16/24/32-bit, float32/64, A-law and mu-law, so the asymmetry was the writer's alone. That is a hard quality ceiling for 44.1/48 kHz music work and for anything that will be processed further. Adds WavWriteOptions to write_wav(): WavSampleFormat Pcm16 (default) | Pcm24 | Float32 WavDitherMode None (default) | TriangularPdf WavPeakPolicy HardClip (default) | LookaheadLimit Every default reproduces the previous behaviour, and write_pcm16_wav is now a thin wrapper whose output is byte-identical -- verified in the test against a fixture built from the previous implementation, so the ~70 existing call sites are unaffected. Measured on the new paths: float32 round trip exact, 0.000e+00 error pcm24 round trip 130.57 dB SNR, 1.19e-7 max error pcm16 round trip 82.41 dB SNR (unchanged) Dither is opt-in because it is a deliberate choice, not a universal improvement, and must not be applied twice in a chain. At -60 dBFS it moves undithered quantisation harmonics from -54.62 dBc to -67.33 dBc. The peak policy exists because the writer's only prior behaviour was a hard clamp at +/-1.0: a +1 dBFS overshoot rails 29.97% of samples at -26.69 dB THD+N. LookaheadLimit is a real limiter -- per-frame peak, a sliding-window minimum over +/-5 ms, two cascaded box filters for a smooth envelope, channel-linked -- and measures 0.00% railed at -98.01 dB THD+N for 1.04 dB of level. A buffer that never crosses the ceiling comes back bit-identical, which the test asserts. A memoryless soft-clip waveshaper was implemented first and rejected: it measured -26.7 dB THD+N, indistinguishable from the clamp it was meant to replace. Overshoot is a gain problem, not a curve problem. Also adds a 4 GiB RIFF size guard, and a WavSink alongside WavPcm16Sink so a sink can carry a format. Tests: tests/unittests/test_wav_writer_formats.cpp -- round trips and error bounds per depth, full RIFF header verification per format (including the fact chunk for float32), byte-identity of write_pcm16_wav, dither reproducibility for a fixed seed, and the limiter assertions above. Build: cmake -S . -B build -DENGINE_BUILD_TESTS=ON && cmake --build build Test: ctest -R wav_writer_formats_test (no model weights required) Backend tested: CPU (pure host code); full suite 39/39 on macOS/Metal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATa5YkLUPMDPRL7w1gCo9p
Two resampling defects that compound. Both cost audio quality silently.
1. SoxrApi loads libsoxr with dlopen("libsoxr.dylib") -- a bare leaf name.
On Apple Silicon dyld's default search path covers /usr/local/lib and
/usr/lib but not /opt/homebrew/lib, so a Homebrew libsoxr is never
found and resample_mono_soxr_or_linear falls back to linear
interpolation. The fallback warning goes through the debug logger,
which defaults to disabled, so the degradation is invisible.
Adds absolute-path candidates for the common install prefixes plus an
AUDIOCPP_SOXR_LIBRARY override, and prints a one-time stderr notice on
fallback so it can no longer happen unnoticed.
2. read_mono_resampled -- the input path for every denoise and
super-resolution entry point -- called resample_mono_linear
unconditionally, with no anti-alias filter at all.
Measured on the real sources, a 12 kHz tone decimated 48 -> 16 kHz for
a 16 kHz model:
resample_mono_linear 0.00 dBc <- full amplitude
sinc width 6 -55.06 dBc
sinc width 64 -117.08 dBc
soxr -288.17 dBc
0.00 dBc is not a typo. 48 -> 16 kHz is an exact 3:1 ratio, so the
interpolation fraction is identically zero and linear interpolation
degenerates into plain sample-dropping: the alias arrives unattenuated.
FlashSR is then asked to re-synthesise the band that was just
destroyed.
Adds resample_mono_soxr_or_sinc (soxr when available, else the in-tree
windowed sinc, with output_length_policy honoured on the fallback -- the
linear path ignored it) and torchaudio_sinc_hann_playback_options() at
lowpass_filter_width 64.
The shared TorchaudioSincHannResampleOptions default stays at width 6.
That is torchaudio.transforms.Resample's own default, and the ~36 call
sites taking it are feature-extraction paths whose contract is bit-parity
with a Python reference; widening it would change model inputs
everywhere. Only paths that produce audio a listener hears were switched
over -- the audio-utility input path and the mix bus.
Width 64 was chosen from the measured knee on a 44.1 -> 48 -> 44.1 round
trip: 60.54 dB at width 6, 122.96 dB at 64, 141.42 dB at 128 for 2.4x the
CPU. Width 64 costs 6.51 ms for 4 s of mono, i.e. 615x realtime.
Tests: tests/unittests/test_audio_resample_quality.cpp asserts the folded
4 kHz alias is at or below -60 dBc through the fixed path, by coherent
single-bin DFT. That threshold fails the old linear path by 60 dB and
fails a regression to the width-6 default, while passing both the sinc
fallback and soxr -- so it does not depend on whether libsoxr is
installed on the build machine. Also asserts the output-length policy is
honoured on the fallback, and pins the shared defaults so a future change
to them is deliberate.
Build: cmake -S . -B build -DENGINE_BUILD_TESTS=ON && cmake --build build
Test: ctest -R audio_resample_quality_test (no model weights required)
Backend tested: CPU (host DSP); full suite 40/40 on macOS/Metal.
Note: the test writes its probe as float32, so it depends on the WAV
output formats change in the preceding commit -- a 16-bit container would
put a -96 dBc floor under a -117 dBc measurement.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ATa5YkLUPMDPRL7w1gCo9p
This was referenced Aug 30, 2026
0xShug0
marked this pull request as draft
August 31, 2026 00:37
0xShug0
marked this pull request as ready for review
September 1, 2026 16:50
Contributor
Author
|
Closing for now to stay inside the 3-concurrent-PR policy (see the discussion on #422). This one has also gone stale against main; I will rebase it and resubmit once a review slot is free. |
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.
What this changes
Two resampling defects that compound, and both cost quality silently.
1. libsoxr is never found on Apple Silicon
SoxrApiloads libsoxr withdlopen("libsoxr.dylib")— a bare leaf name. On Apple Silicon dyld's default search path covers/usr/local/liband/usr/libbut not/opt/homebrew/lib, so a Homebrew libsoxr is never found andresample_mono_soxr_or_linearfalls back to linear interpolation. The fallback warning goes through the debug logger, which defaults to disabled, so the degradation is invisible.Adds absolute-path candidates for the common install prefixes plus an
AUDIOCPP_SOXR_LIBRARYoverride, and a one-time stderr notice on fallback so it can no longer happen unnoticed.2. The enhancement-model input path has no anti-alias filter at all
read_mono_resampled— the input path for every denoise and super-resolution entry point — calledresample_mono_linearunconditionally.Measured against the real sources, a 12 kHz tone decimated 48 → 16 kHz for a 16 kHz model:
resample_mono_linear(previous)0.00 dBc is not a typo. 48 → 16 kHz is an exact 3:1 ratio, so the interpolation fraction is identically zero and linear interpolation degenerates into plain sample-dropping — the alias arrives unattenuated. FlashSR is then asked to re-synthesise the band that was just destroyed.
The default that was deliberately not changed
TorchaudioSincHannResampleOptions::lowpass_filter_widthstays at 6. That istorchaudio.transforms.Resample's own default, and the ~36 call sites taking it are feature-extraction paths whose contract is bit-parity with a Python reference — widening it would change model inputs everywhere. Per CONTRIBUTING's framework-module rule, the capability was added astorchaudio_sinc_hann_playback_options()(width 64) and only paths producing audio a listener hears were switched over: the audio-utility input path and the mix bus.Width 64 chosen from the measured knee on a 44.1 → 48 → 44.1 round trip:
Also adds
resample_mono_soxr_or_sinc— soxr when available, else the in-tree windowed sinc, withoutput_length_policyhonoured on the fallback (the linear path ignored it).Validation
Build
Test
tests/unittests/test_audio_resample_quality.cppasserts the folded 4 kHz alias is at or below −60 dBc by coherent single-bin DFT. That threshold fails the old linear path by 60 dB and fails a regression to the width-6 default, while passing both the sinc fallback and soxr — so it does not depend on whether libsoxr is installed on the build machine. It also pins the shared defaults so a future change to them is deliberate.Backend tested: CPU (host DSP). Full suite 40/40 on macOS/Metal, Apple M4 Max.
Affects
Output changes for
deepfilternet2,rnnoise,zipenhancerandflashsrwhenever the source rate differs from the model rate — that is the fix. Same-rate input is a no-op fast path in both old and new code, so 16 kHz → 16 kHz and 48 kHz → 48 kHz are untouched. Output length may differ by ±1 sample (sinc sizes withceil, the old linear path withllround).Any process that hits a soxr fallback now prints one line to stderr, once per process.
🤖 Generated with Claude Code
https://claude.ai/code/session_01ATa5YkLUPMDPRL7w1gCo9p