Fix audio pan broken with NR active (#1460) and AF mute not persisted (#1560)#1676
Fix audio pan broken with NR active (#1460) and AF mute not persisted (#1560)#1676M7HNF-Ian wants to merge 3 commits into
Conversation
MNR uses a decision-directed MMSE-Wiener filter with minimum-statistics noise floor tracking, accelerated by Apple Accelerate (vDSP / AMX). 512-point FFT at 24 kHz, 50% overlap-add, per-bin gain with temporal smoothing to suppress musical noise. Hardware-accelerated on Apple Silicon. Right-click the MNR button to open strength controls in DSP Settings. Quick-wins: - aethersdr#1503: Diversity button missing on Slice A at startup - aethersdr#1581: TNF/FDX indicators invisible in dark theme - aethersdr#1583: Date/Time status bar labels too dim - aethersdr#1576: Record/Playback buttons invisible in dark theme - aethersdr#506: Secondary text colour consistency (#506070 → #8aa8c0) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
before enabling MNR; every other NR setter (NR2/RN2/NR4/BNR/DFNR) now
also calls setMnrEnabled(false) in its mutual-exclusion block.
matching the pattern used by all other NR setters.
forward declaration (#ifdef __APPLE__) and unique_ptr member; AudioEngine.cpp
already had the full include; new/delete replaced with make_unique/reset.
std::atomic<float> m_mnrStrength replaces plain float.
"stereo int16 → … → stereo int16" to "stereo float32 → … → stereo float32";
also fixes the process() doc-comment.
strength() use store/load; processFrame() reads m_strength.load().
Allows AudioEngine to write strength from the audio thread while the
DSP dialog reads it from the UI thread without a data race.
AppSettings("MnrStrength"); setMnrStrength() now saves to AppSettings
on every change, so the value survives restarts.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ed (aethersdr#1560) aethersdr#1460 — NR2/RN2/NR4/DFNR/MNR all mono-mix (L+R)/2 for noise analysis, discarding the radio-side stereo balance. The pan slider sends audio_pan to the radio which embeds the balance in the VITA-49 PCM, but NR collapses it. Fix: - AudioEngine gains std::atomic<int> m_rxPan (0-100, centre=50) and void setRxPan(int). - Static helper applyRxPanInPlace() re-applies a linear-pan law to stereo float32 in-place; is a true no-op when pan==50. - Called at the end of processNr2() and after every other NR process() call in feedAudioData (RN2/NR4/DFNR/MNR paths). The non-NR path is untouched — the radio still owns pan there. - VfoWidget emits rxPanChanged(int) when the user moves the pan slider; wireVfoWidget connects it to m_audio->setRxPan(). aethersdr#1560 — The per-slice VFO mute (AF 🔊 button) sends audio_mute=1 to the radio, but the radio does not persist this between client sessions. On reconnect the slice starts unmuted. Fix: - VfoWidget emits audioMuteToggled(bool) when the user clicks the mute button (guarded by m_updatingFromModel so model-driven syncs don't trigger a save). - wireVfoWidget saves SliceAudioMuted_{id} to AppSettings on every user toggle, then immediately restores the saved state by calling s->setAudioMute(true) if needed (radio is already connected at this point, so the command goes through cleanly). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Review — PR #1676
Thanks for the contribution, @Chaosuk97! The pan-with-NR fix is a real user-facing annoyance and the mute persistence is a nice quality-of-life improvement. Overall this looks solid — clean wiring, correct mutual exclusion, and the MNR DSP implementation is well-structured. A few items to address:
Issues
1. libdeepfilter.a binary is outside scope and very large (~33 MB)
third_party/deepfilter/lib/darwin-arm64/libdeepfilter.a is a new 35 MB binary added to the repo. This isn't mentioned in the PR description and doesn't relate to either #1460 or #1560. Was this accidentally included? If it's needed for DFNR on macOS, it should be in a separate PR with context on where it was built and how to reproduce it. Either way, adding a 35 MB binary to git history permanently is worth discussing first.
2. SliceAudioMuted_{id} key uses runtime slice ID — may not be stable across sessions
In wireVfoWidget, the mute state is saved as SliceAudioMuted_0, SliceAudioMuted_1, etc. Slice IDs are assigned by the radio and may differ between sessions (e.g., if the user creates slices in a different order, or the radio reassigns IDs). This means the persisted mute could apply to the wrong slice on reconnect. Consider using the slice letter (A/B/C/D) instead of the numeric ID, since the letter is the user-visible identifier and maps 1:1 to the first 4 slices most users create. Worth checking how BandSettings handles this.
3. applyRxPanInPlace frame count calculation could be negative for tiny buffers
processed.size() / (2 * static_cast<int>(sizeof(float)))If processed.size() is somehow less than 8 (e.g., an empty or partial buffer from a filter error path), this integer division would yield 0 which is fine, but worth a quick note that it's safe. The current code is correct for all realistic buffer sizes.
4. DSP overlay panel width bump: setFixedWidth(200) → setFixedWidth(280)
This is a 40% wider overlay panel. It may look odd on smaller screens or when the panadapter is narrow. Was this needed to fit the MNR button, or just aesthetic? If it's just for MNR (which is hidden on non-Apple), consider making this conditional.
Scope concerns
This PR bundles three distinct features:
- Pan fix (#1460) — core bug fix
- Mute persistence (#1560) — feature addition
- MNR — entire new NR mode (MacNRFilter, DSP dialog tab, VFO button, overlay wiring, CMake changes, Accelerate framework linkage)
The MNR work is substantial and arguably deserves its own PR — it touches CMakeLists.txt, adds new source files, new signals, new UI elements, and a new DSP algorithm. The pan fix and mute persistence are straightforward bug fixes that could land independently. Splitting would make each easier to review and bisect.
The dark-theme color fixes (#607080 → #8aa8c0, record/play button colors, TNF/FDX indicators, GPS labels) and the diversity button startup fix are also bundled here without being mentioned in the PR title or description. These overlap with PR #1672 from the same author — check whether they conflict.
Looks good
- Pan law implementation is correct: linear, symmetric, true no-op at centre
std::atomic<int> m_rxPanis the right choice for cross-thread pan value sharing- NR mutual exclusion updated consistently across all 6 NR modes
m_updatingFromModelguard onaudioMuteToggledprevents save loops- MacNRFilter DSP: sqrt-Hann OLA, min-stats noise tracking, temporal gain smoothing, vDSP acceleration — all textbook correct
- MNR hidden at compile time on non-Apple with
#ifndef __APPLE__ isValid()check aftervDSP_create_fftsetupwith proper fallback
Nice work on the DSP implementation especially — the MMSE-Wiener with decision-directed a-priori SNR estimation is a good choice for this use case.
…rsdr#1676 review) - Use QChar('A' + sliceId) as the SliceAudioMuted settings key so the persisted mute state survives sessions where the radio assigns different numeric slice IDs (matches DaxChannel_SliceA naming convention) (aethersdr#1560) - Add nFrames <= 0 early-exit and explanatory comment to applyRxPanInPlace so zero-length / partial buffers on error paths never enter the loop (aethersdr#1460) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Closing in favour of a clean cherry-pick from main — the original branch was cut from feat/mnr-and-quickfixes so the PR diff included MNR work. New PR opened from fix/1460-pan-1560-mute-clean. |
…rsdr#1676 review) - Use QChar('A' + sliceId) as the SliceAudioMuted settings key so the persisted mute state survives sessions where the radio assigns different numeric slice IDs (matches DaxChannel_SliceA naming convention) (aethersdr#1560) - Add nFrames <= 0 early-exit and explanatory comment to applyRxPanInPlace so zero-length / partial buffers on error paths never enter the loop (aethersdr#1460) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…#1560) (#1685) * About: add Ian M7HNF (Chaosuk97) to Contributors Credits NR2 gainMax cap + output clamp fix (#1696, issue #1507). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix audio pan broken with NR (#1460) and AF mute not persisted (#1560) discarding the radio-side stereo balance. The pan slider sends audio_pan to the radio which embeds the balance in the VITA-49 PCM, but NR collapses it. Fix: - AudioEngine gains std::atomic<int> m_rxPan (0-100, centre=50) and void setRxPan(int). - Static helper applyRxPanInPlace() re-applies a linear-pan law to stereo float32 in-place; is a true no-op when pan==50. - Called at the end of processNr2() and after every other NR process() call in feedAudioData (RN2/NR4/DFNR/MNR paths). The non-NR path is untouched — the radio still owns pan there. - VfoWidget emits rxPanChanged(int) when the user moves the pan slider; wireVfoWidget connects it to m_audio->setRxPan(). radio, but the radio does not persist this between client sessions. On reconnect the slice starts unmuted. Fix: - VfoWidget emits audioMuteToggled(bool) when the user clicks the mute button (guarded by m_updatingFromModel so model-driven syncs don't trigger a save). - wireVfoWidget saves SliceAudioMuted_{id} to AppSettings on every user toggle, then immediately restores the saved state by calling s->setAudioMute(true) if needed (radio is already connected at this point, so the command goes through cleanly). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Review fixes: stable slice letter key and nFrames safety guard (#1676 review) - Use QChar('A' + sliceId) as the SliceAudioMuted settings key so the persisted mute state survives sessions where the radio assigns different numeric slice IDs (matches DaxChannel_SliceA naming convention) (#1560) - Add nFrames <= 0 early-exit and explanatory comment to applyRxPanInPlace so zero-length / partial buffers on error paths never enter the loop (#1460) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Jeremy Fielder <kk7gwy@aethersdr.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Audio panning stops working when NR2 or NR4 noise reduction is active #1460 — Moving the pan slider has no effect when NR2/NR4/RN2/DFNR/MNR is active. All client-side NR filters mono-mix
(L+R)/2for spectral analysis, discarding the radio-side stereo balance embedded in the VITA-49 PCM. Pan appears broken because the NR output is always centred.Upon start of aethersdr the previous audio mute state is not remembered. #1560 — The per-slice AF mute button (🔊 in the VFO Audio tab) sends
audio_mute=1to the radio, but the radio does not persist this between client sessions. On reconnect the slice always starts unmuted.Fix
#1460:
AudioEnginegainsstd::atomic<int> m_rxPan(0–100, 50 = centre) updated viasetRxPan(int).applyRxPanInPlace()re-applies a linear pan law to stereofloat32in-place; a no-op when pan == 50 (centre), so there is zero overhead for the common case.processNr2()and after every other NRprocess()call infeedAudioData(RN2 / NR4 / DFNR / MNR). The non-NR path is unchanged — the radio still owns panning there.VfoWidgetemitsrxPanChanged(int)alongside the existingm_slice->setAudioPan()call;wireVfoWidgetconnects it tom_audio->setRxPan().#1560:
VfoWidgetemitsaudioMuteToggled(bool)when the user clicks the mute button (guarded bym_updatingFromModelso model-driven syncs don't trigger spurious saves).wireVfoWidgetsavesSliceAudioMuted_{id}toAppSettingson every user toggle, then immediately restores the saved state by callings->setAudioMute(true)when the slice is wired up. The radio is already connected at this point so the command goes through cleanly.Test plan
🤖 Generated with Claude Code