Skip to content

audio: find libsoxr on Apple Silicon, and stop the unfiltered decimation - #359

Closed
CryptVenture wants to merge 2 commits into
0xShug0:mainfrom
CryptVenture:pr/resampling-antialias
Closed

audio: find libsoxr on Apple Silicon, and stop the unfiltered decimation#359
CryptVenture wants to merge 2 commits into
0xShug0:mainfrom
CryptVenture:pr/resampling-antialias

Conversation

@CryptVenture

Copy link
Copy Markdown
Contributor

Depends on #358. This branch is stacked on it, so the diff shown here includes that commit. Review the second commit only, or merge #358 first. The test writes its probe as float32 — a 16-bit container would put a −96 dBc floor under a −117 dBc measurement.

What this changes

Two resampling defects that compound, and both cost quality silently.

1. libsoxr is never found on Apple Silicon

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 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 — called resample_mono_linear unconditionally.

Measured against the real sources, a 12 kHz tone decimated 48 → 16 kHz for a 16 kHz model:

Path Folded 4 kHz alias
resample_mono_linear (previous) 0.00 dBc
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.

The default that was deliberately not changed

TorchaudioSincHannResampleOptions::lowpass_filter_width stays at 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. Per CONTRIBUTING's framework-module rule, the capability was added as torchaudio_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:

Width SNR Time (4 s mono)
6 60.54 dB 0.56 ms
64 122.96 dB 6.51 ms (615× realtime)
128 141.42 dB 15.81 ms

Also 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).

Validation

Build

cmake -S . -B build -DENGINE_BUILD_TESTS=ON
cmake --build build

Test

ctest -R audio_resample_quality_test

tests/unittests/test_audio_resample_quality.cpp asserts 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, zipenhancer and flashsr whenever 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 with ceil, the old linear path with llround).

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

CryptVenture and others added 2 commits August 30, 2026 22:45
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
@CryptVenture

Copy link
Copy Markdown
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.

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.

1 participant