Repository navigation
fix(sonora): flush denormals during stream processing, like upstream - #38
Open
dignifiedquire wants to merge 7 commits into
Open
dignifiedquire wants to merge 7 commits into
dignifiedquire wants to merge 7 commits into
Conversation
Add an `aec3_render` group with two cases, `48k_mono_active` and `48k_mono_silent`. Each runs AEC3 only at 48 kHz mono: 1 s of active render and echo, then 6 s (600 frames) of the measured condition, then times one render plus one capture call per iteration. The silent case feeds an exact-zero render reference and low-level capture noise (about -60 dBFS peak), which is the case reported in issue #34. The 6 s settle matters. Without flush-to-zero, the fast render states (QMF, decimators, buffers) turn subnormal 0.1-0.4 s into silence, the two reverb models about 2 s in, and the low-render detector's average power about 4 s in. Timing starts about 1.9 s after the latest onset, so a before-fix baseline measures the full slowdown. The benchmark lands before the fix so that x86 hardware can record a baseline and then compare the silent/active ratio after it. It is a manual tool, not a CI gate. Refs #34 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H29DamSLugosGXJSz1e5Yx
Root cause: several recursive states never decay to zero on silent input under IEEE gradual underflow. The two-band QMF all-pass states, the AEC3 render and capture decimator biquads, the capture high-pass filter, the reverb models and some smoothers settle a few ulps above zero, in the subnormal range, and stay there. On x86 every operation on a subnormal operand takes a microcode assist, which is why AEC3 ran 10-50x slower while the render reference was silent. Upstream WebRTC hides this with DenormalDisabler at every AudioProcessingImpl stream entry point; the port had no equivalent. Approach: port DenormalDisabler as a private scoped guard in crates/sonora/src/denormal_disabler.rs, and create it first in process_stream, process_reverse_stream, process_stream_i16 and process_reverse_stream_i16. The four functions are #[inline(never)] so caller code cannot be inlined into the flush-to-zero window. - x86 and x86_64 with SSE (i686 included): MXCSR FTZ and DAZ (0x8040), upstream's mask. - aarch64 with NEON: FPCR.FZ (bit 24). - 32-bit ARM, arm64ec, wasm32, i586, soft-float aarch64 and Miri: no-op. AudioProcessing logs "Denormal disabler unsupported" at construction, as upstream does. As upstream, the guard sets the bits only if they are not all set, and restores the caller's exact control word only if it changed it, also during unwinding. A caller that already runs with FTZ keeps it. What runs under the guard: all work a process call does itself, including re-initialization after a stream-format change and queued runtime settings. Upstream does the same, except that its i16 ProcessStream re-initializes before it creates the guard (audio_processing_impl.cc:1232-1235). Direct calls to initialize() and apply_config(), and the public DSP building blocks, run unguarded, as upstream. Documentation: a "Floating-point environment" section on AudioProcessing states which targets flush, that the caller's setting is restored (also on panic), that subnormal input samples read as zero, that code the call reaches (tracing subscribers, the allocator, the panic hook) runs in flush-to-zero mode, that a comparison with a subnormal operand can change its result, and that on Linux a thread started during the call inherits flush-to-zero for its whole life. The CHANGELOG records the change under Unreleased. Tests (in the new module; none are skipped; the target-dependent ones assert against is_supported(), so no-op targets check the no-op): - FTZ and DAZ take effect, Inf and NaN are kept, nested guards keep the outer state, and the caller's state is restored on drop and on unwind. - All four public process calls leave the caller's environment unchanged, both from a default caller and from one that set FTZ. - A deterministic regression test runs the issue scenario (speech, then silent render with low-level capture noise of about -75 dBFS RMS, then all-zero) through the f32 and i16 APIs and requires zero subnormal values in the processor state. A control run with the guard bypassed (test builds only) must still find subnormals in 12 named sources, so the scenario cannot silently stop exercising the bug. It uses no timing, so it runs on real x86_64 in CI. Measured on GitHub's Intel runners (Xeon Platinum 8573C and 8370C), the silent-render benchmark drops from about 735 us to 71-92 us per call (8-10x), back to the active-render cost. AMD EPYC runners, which penalize subnormals far less, gain about 1.1x. Caveat: changing the FP environment is formally undefined behavior in Rust (RFC 3514), as it is for nih-plug and no_denormals. The guard's # Safety contract also names the hazard that LLVM may move register-only float operations across the guard boundary; disassembly of the release builds shows no such motion today. Refs #34 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H29DamSLugosGXJSz1e5Yx
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## sync/post-m145-fixes #38 +/- ##
========================================================
+ Coverage 93.69% 93.84% +0.14%
========================================================
Files 146 147 +1
Lines 31484 31752 +268
Branches 31484 31752 +268
========================================================
+ Hits 29499 29797 +298
+ Misses 1762 1731 -31
- Partials 223 224 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H29DamSLugosGXJSz1e5Yx
…actly The guard promises to restore the exact control word it read. Every restore test started from the default word, where that word, the default and the word with the flush bits cleared are all equal, so a Drop that wrote `word & !MASK` or a fixed default word still passed. guard_restores_non_default_word_exactly starts from a word the guard must change and that such a restore loses: MXCSR with FTZ set and DAZ clear on x86/x86_64, FPCR.DN set on aarch64. It fails when Drop writes a fixed default word (both targets) or `word & !MASK` (x86_64). On aarch64 the second is the same as a correct restore, because `new` saves only words with FZ clear. The API-level test now also checks that DAZ does not leak after each process call, and the subnormal scan documents that it also counts f64 values, which can only cause a false failure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H29DamSLugosGXJSz1e5Yx
…pstream The public "Floating-point environment" section and the CHANGELOG now say that changing the floating-point environment is formally undefined behavior in Rust, that upstream's DenormalDisabler, nih-plug and no_denormals also change it, and how sonora limits the change. Before, only the private guard docs said so. The CHANGELOG also notes that upstream sets the x86 bits only in clang builds (rtc_base/denormal_disabler.cc checks __clang__), so a caller that replaces a GCC-built C++ library gets FTZ/DAZ on x86 where it had IEEE gradual underflow. The guard's safety docs no longer say that only register-only float operations can cross the guard boundary. Like the asm comments, they now cover values on the stack and behind & or &mut. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H29DamSLugosGXJSz1e5Yx
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H29DamSLugosGXJSz1e5Yx
…o-zero 48d497b added "sonora limits the change to the calling thread for the length of each call" to the floating-point environment section on AudioProcessing (audio_processing.rs:432-433) and to the CHANGELOG. In the docs this contradicts the sentence just before it: a thread started during the call, on platforms where new threads inherit the floating-point environment, keeps flush-to-zero for its whole life (denormal_disabler.rs:47-50 says the same). The CHANGELOG bullet left out that caveat, so there the claim read as absolute. Both now say that sonora sets flush-to-zero only on the calling thread and restores that thread's setting after each call. The docs point to the inheritance caveat above; the CHANGELOG states it. The rest of the doc paragraph is rewrapped, with no other wording changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H29DamSLugosGXJSz1e5Yx
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
2 participants
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.
Summary
This PR fixes the AEC3 slowdown on x86 while the render reference is silent (Refs #34).
Cause: on silent input, several recursive filter states never decay to exact zero. Instead they settle in the subnormal float range. That includes:
On x86 every operation on a subnormal takes a slow microcode assist. Upstream WebRTC avoids this with
DenormalDisablerat everyAudioProcessingImplstream entry point. The port had no equivalent.Fix: port that guard as a private scoped guard (
crates/sonora/src/denormal_disabler.rs). Each of the four stream entry points creates it first, and every publicprocess_*call and the C API reach those four.0x8040, upstream's mask)As upstream does, the guard sets the bits only if they aren't already set. It restores the caller's exact control word on drop, including during unwinding, so shared threads (for example tokio workers) are unaffected.
Results on real x86_64
The
aec3_renderbenchmark (added in the first commit) measures AEC3 only at 48 kHz mono. The job builds and runs it twice on the same GitHub runner, first with the four guard lines removed ("before") and then as committed ("after"). Each variant builds in its own target dir, and a check confirms that only the "after" binary containsldmxcsr. Run 36598783119, attempt 2.Verification
silent_render_leaves_no_subnormal_stateThe guard is effectively free. Measured locally it cost 36–61 ns per call, and the active-frame benchmarks didn't move.
Caveats (documented in code and in the new "Floating-point environment" section on
AudioProcessing)no_denormals. The guard's# Safetycontract names the residual hazards. The disassembly shows no FP operation moved across the boundary.tracingsubscribers, the global allocator and the panic hook. A comparison with a subnormal operand can change its result there. On Linux, a thread spawned inside the window inherits flush-to-zero (verified on arm64 Linux).+crt-static) links a softwarefmafthat ignores FTZ/DAZ, so subnormals can survive in the biquads. perf: use C++-order arithmetic except on AArch64 and x86 with fma #37 fixes this by removing thefmafcalls; with both applied, the regression test passes under+crt-static.Commits
1fc169ebench(sonora): add silent-render AEC3 benchmark7428065fix(sonora): flush denormals during stream processing, like upstream00d55ectest(sonora): check the denormal guard restores a non-default word exactly48d497bdocs(sonora): note the undefined-behavior status and clang-only x86 upstream20280abdocs(sonora): say that threads started during a call can keep flush-to-zeroThe branch also merges #36's branch twice (
e59dc1f,cc961e6), so the CHANGELOG stays conflict-free as #36 changes.Updates after the review pass
guard_restores_non_default_word_exactly: it starts from a control word the guard must change (FTZ only on x86; FPCR.DN on aarch64) and checks that the word is restored bit for bit. It catches aDropthat clears the mask (x86) or writes a fixed default word (both architectures). The API-level test now also checks that DAZ doesn't leak. There are 9 guard tests now.DenormalDisabler, nih-plug andno_denormalsalso do. They also say that upstream sets the x86 bits only in clang builds, so callers replacing a GCC-built C++ library will see a mode change.🤖 Generated with Claude Code
https://claude.ai/code/session_01H29DamSLugosGXJSz1e5Yx