Fix DFNR stereo pumping - #4788
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request addresses DFNR RX “stereo pumping” by changing MonoDspStereoAdapter to keep the mono DSP output waveform authoritative and use delayed dry stereo only to estimate/apply a slow left/right balance, preventing dry program material from leaking back into the processed output.
Changes:
- Reworked
MonoDspStereoAdapterto apply a balance estimate (from delayed dry stereo) to the processed mono waveform, removing the prior processed↔dry envelope/mix behavior. - Added/updated regression tests to prove panned/noisy dry input cannot replace/modulate processed output, processed silence stays silent, and mono-cancelling input holds/recover balance.
- Updated adapter documentation and internal state to reflect the new balance-estimation approach.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/core/MonoDspStereoAdapter.cpp |
Replaces dry/processed mixing with slow balance estimation and applies that balance to the processed mono waveform. |
src/core/MonoDspStereoAdapter.h |
Updates class description and member state to match the new balance-only behavior. |
tests/mono_dsp_stereo_adapter_test.cpp |
Adds targeted regressions for DFNR-style pumping/dry-leak scenarios and updates older tests to assert waveform authority. |
There was a problem hiding this comment.
Good diagnosis and the right shape of fix — making the DFNR waveform authoritative and demoting the dry stereo to a balance estimate is clearly better than the old processed-to-dry envelope, and better than the dry-stereo fallback it replaces (that fallback discarded the denoised signal entirely, so DFNR was doing nothing whenever L/R weren't bit-identical). The 2·level/(sum) normalization is also exactly right for level preservation: a hard-panned dry gives the mono DSP a 6 dB-down input, and a balance of 2.0 puts it back. Thanks for the hardware A/B and the falsified regression.
Three things I'd like looked at before merge, all in the same "the balance is now persistent state" family. The largest is that the two fallback paths in takeProcessedMono() still emit centered processed, so a dry underrun snaps an established image to center and back — the same artifact class this PR is fixing. The second is a confirmed one: kPowerFloor is now gating an envelope with a 150× smaller coefficient, and I reproduced a channel latching at exactly zero (full mute) where the true balance is 0.095.
Would like fixed before merge
MonoDspStereoAdapter.cpp:179— latency-preroll and dry-underrun fallbacks bypass the persisted balance, causing an abrupt L/R image snapMonoDspStereoAdapter.cpp:165—kPowerFloorvskBalanceEnvelopeCoeffmismatch latches the quieter channel to a hard mute (reproduced)
Polish
MonoDspStereoAdapter.cpp:173— thekMaxBalanceGainclamp is unreachable by constructiontests/mono_dsp_stereo_adapter_test.cpp:277— the new regression can't distinguish the headline constant it's meant to pin
Non-blocking notes
MonoDspStereoAdapter.h:7— blast radius: BNR and Specbleach share this adapter, and only DFNR was verified on hardware
🤖 aethersdr-agent · cost: $5.1564 · model: claude-opus-5
|
Thanks for your expertise on this Robbie. I really liked the 26.7.1 DFNR for high noise environments with little SSB degradation. Looking forward to your DSP design change. |
jensenpat
left a comment
There was a problem hiding this comment.
Review feedback addressed: fallback paths now carry the persisted balance, kBalancePowerFloor scales with the balance coefficient, and the unreachable clamp is gone. Checks green, commits signed.
Summary
Fixes #4766.
DFNR still routed its mono denoised waveform through
MonoDspStereoAdapter, which switched to delayed dry stereo whenever the input channels were not effectively identical. It then drove that dry signal with a fast processed-to-dry power envelope, producing the same speech-dependent pumping family previously fixed in RN2.Keep the processed mono waveform authoritative and use the delayed dry stereo only to estimate a slow normalized left/right balance. Mono-cancelling input holds the last trustworthy balance rather than leaking dry audio or destabilizing the estimator. Latency preroll and dry-buffer underruns also retain that held balance instead of briefly snapping to center.
The balance estimator now scales its numerical floor with its slower coefficient so a quiet but valid channel does not latch at zero. The normalization is intrinsically bounded to
[0, 2], so the unreachable output clamps were removed.Regression coverage proves that panned/noisy dry input cannot replace or modulate an unrelated processed waveform, processed silence remains silent, quiet channels remain represented, the balance transition stays intentionally slow, dry underruns retain the established balance, and mono cancellation preserves and cleanly recovers it.
Shared-adapter scope
MonoDspStereoAdapteris shared byDeepFilterFilter(DFNR),NvidiaAfxFilter(BNR), andSpecbleachFilter. The class-level contract and regression tests therefore cover the common adapter behavior for all three call sites. Because each processor supplies a mono waveform, genuinely independent stereo content cannot survive this path; the adapter preserves only the dry input's slow left/right level balance while keeping the processed mono waveform authoritative.Hardware A/B listening for this PR was performed with DFNR. NVIDIA BNR is unavailable in this macOS/arm64 build, so no local BNR listening claim is made.
Constitution principle honored
Principle XI — Fixes Are Demonstrated. The targeted regression fails against the pre-fix adapter (
panned dry replaced or modulated processed waveform at frame 6000) and passes with this change; the fix was also built, bridge-probed, and listened to on a real radio before publication.Test plan
/opt/homebrew/bin/cmake --build build-codex-arm64 -j8)Additional local proof:
ctest --test-dir build-codex-arm64 -R '^(mono_dsp_stereo_adapter_test|rnnoise_filter_test|spectral_nr_test)$' --output-on-failure— 3/3 passedpython3 tools/check_engine_boundary.py --strict— 0 blocking findingsbuild.ninjazero-match guard fail; that pre-existing CMake issue is unchanged by this PR.audioCapture probeDspStereo DFNR strict— full coverage, audible output, intended 4:1 stereo balance preserved with zero ratio errortxAllowed=falseChecklist
docs/COMMIT-SIGNING.md)AppSettingscalls — no settings changesMeterSmoother— no meter UI changesCHANGELOG.mdchange is needed for this focused bug fix