Skip to content

fix(audio): QsoRecorder playback stuck-mute on sink open failure (#3306 audit)#3891

Merged
ten9876 merged 1 commit into
aethersdr:mainfrom
jensenpat:aether/fix-qso-recorder-stuck-mute
Jun 30, 2026
Merged

fix(audio): QsoRecorder playback stuck-mute on sink open failure (#3306 audit)#3891
ten9876 merged 1 commit into
aethersdr:mainfrom
jensenpat:aether/fix-qso-recorder-stuck-mute

Conversation

@jensenpat

Copy link
Copy Markdown
Collaborator

Bug (P0 — dead RX audio)

QsoRecorder::startPlayback() mutes live RX (muteRxRequested(true)) but never checked whether the playback QAudioSink actually opened. If start() fails synchronously — e.g. a WASAPI/CoreAudio device that false-positives isFormatSupported() then refuses at open — the stateChanged handler runs stopPlayback() while m_playing is still false, so its if (!m_playing) return guard skips the muteRxRequested(false). Execution then falls through and mutes RX anyway. Live RX stays dead until the app is restarted.

This violates the #3230 "unmute-on-bail" invariant (any sink that mutes RX must unmute on every failure path).

Fix

Add the immediate post-start() error check that ClientPuduMonitor::startPlayback() already has (lines 319-332): on StoppedState + error, tear the sink down and return before emitting muteRxRequested(true). Because QSO playback mutes only after a successful open (and QSO does not pre-mute during recording), the failure path simply returns with RX left live — no stuck mute, no spurious unmute.

Provenance

Found by a multi-agent audio-sink mute/negotiation audit (adversarially verified, 3/3 skeptics confirmed). This is the slice-flag QSO recorder hotspot.

Test / build

Full macOS app builds & links clean; the headless audio tests (negotiation 28/28, device wrapper 8/8, router 14/14) pass. The fix mirrors the already-merged ClientPuduMonitor pattern. A live repro needs a device that fails open mid-playback (WASAPI false-positive); recommend a Windows soak.

Refs #3306

💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat

…hersdr#3306 audit)

QsoRecorder::startPlayback() mutes live RX (muteRxRequested(true)) but never
checked whether the playback QAudioSink actually opened. If start() failed
synchronously — e.g. a WASAPI/CoreAudio device that false-positives
isFormatSupported() then refuses at open — the stateChanged handler ran
stopPlayback() while m_playing was still false, so its `if (!m_playing) return`
guard skipped the muteRxRequested(false); execution then fell through and muted
RX anyway. Live RX stayed dead until the app was restarted.

Add the immediate post-start() error check that ClientPuduMonitor::startPlayback()
already has (lines 319-332): on StoppedState+error, tear the sink down and
return BEFORE emitting muteRxRequested(true), so a failed playback can never
strand RX in a muted state (aethersdr#3230 invariant). Found by the audio-sink mute/
negotiation audit; this is the slice-flag QSO recorder hotspot.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jensenpat jensenpat marked this pull request as ready for review June 29, 2026 00:31
@jensenpat jensenpat requested a review from a team as a code owner June 29, 2026 00:31

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the bug and the fix against the codebase — this is correct and well-scoped.

Bug confirmed. In startPlayback(), m_playSink->start() is followed unconditionally by m_playing = true; emit muteRxRequested(true). On a synchronous open failure, the direct-connected stateChanged(StoppedState) re-enters onPlaybackSinkStatestopPlayback(), which no-ops on its if (!m_playing) return guard (line 459) since m_playing is still false. Control then falls through and mutes RX with a dead sink — and because m_playing ends up true over a stopped sink, nothing unmutes it. The stuck-mute is real.

Fix is right. The post-start() StoppedState && error() != NoError check tears the sink down and returns before muteRxRequested(true), so the failure path leaves RX live. It faithfully mirrors ClientPuduMonitor::startPlayback() (src/core/ClientPuduMonitor.cpp:319), and satisfies the #3230 unmute-on-bail invariant — here by never muting in the first place, which is the correct shape for QSO playback since it (unlike the monitor) doesn't pre-mute during recording.

Conventions all check out:

  • qCWarning(lcAudio) matches the file's logging category.
  • disconnect(this) before teardown prevents any further re-entrancy — good.
  • Nice touch using deleteLater() (this file's idiom, matching stopPlayback) rather than copying PuduMonitor's bare delete — consistent with local style.
  • Cleanup (m_playBuffer.close(), null-out) mirrors stopPlayback's teardown.

Scope is a single file, single failure path, no behavior change on the success path. As the PR notes, the synchronous-open-failure path can't be exercised on Linux CI and needs a Windows/WASAPI soak for a live repro — worth flagging to a maintainer with hardware, but not a blocker given it mirrors an already-merged, proven pattern.

Thanks for the careful audit and the clear write-up, @jensenpat — this is a clean P0 fix.


🤖 aethersdr-agent · cost: $1.5568 · model: claude-opus-4-8

@ten9876 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High-effort local review done (8 finder angles + adversarial verify) — no surviving findings. The guard correctly returns before muteRxRequested(true) on synchronous open failure, the same-thread Direct connection makes the re-entrant stopPlayback() no-op as documented, and the async-failure-after-mute path is still covered by the existing stopPlayback(). The one candidate I chased (a synchronous IdleState stranding the mute) is foreclosed by preparePlaybackPcm()'s raw.isEmpty() guard before the sink is created. Clean P0 fix mirroring the merged ClientPuduMonitor pattern. Thanks @jensenpat.

@ten9876 ten9876 merged commit 9c1eec4 into aethersdr:main Jun 30, 2026
6 checks passed
ten9876 added a commit that referenced this pull request Jul 2, 2026
#3961)

## Release documentation prep for **v26.7.1** (2026-07-02)

29 commits since v26.6.5. Docs-only + version bump — no code behavior
change.

### Version bump 26.6.5 → 26.7.1
The three canonical locations (per `AGENTS.md`):
- `CMakeLists.txt:2` — `project(AetherSDR VERSION 26.7.1)` →
`AETHERSDR_VERSION` (the app's version string)
- `README.md` — Current version line
- `AGENTS.md` — Current version line

### `CHANGELOG.md`
Promoted `[Unreleased]` → **`## [v26.7.1] — 2026-07-02`** in house style
(v-prefix, em-dash, "N commits since…" opener + headline), authored from
all merged PRs and categorized:
- **Added:** 3D stacked-trace spectrum (#3899), 3D dBm scale (#3937),
in-process NVIDIA BNR + AFX download-on-demand (#3902), TX meter
mouse-over readouts (#3936), SWR manual sweep range (#3885), CW sidetone
capture (#3895), KiwiSDR metadata scaffolding (#3898), bridge verbs
(#3920)
- **Changed:** BNR container/NIM backend removed, 60 fps + per-pixel GPU
FFT trace (#3958), FlexLib-sourced model capabilities (#3954/#2177), RX
speaker-latency cut (#3897), Display pane sections (#3935)
- **Fixed:** #3922, #3926, #3941, #3940, #3942, #3921, #3903, #3892,
#3924, #3891, #3890, #3900, #3947
- Fresh empty `[Unreleased]` restored above the release block.
Internal/CI/docs/self-fix PRs (#3931/#3916/#3934/#3929) omitted per
house style.

### `ROADMAP.md`
- Cycle header → "post-v26.7.1"
- NVIDIA BNR removed from **In flight** (shipped this cycle)
- Five v26.7.1 highlights prepended to **Recently shipped**

### `README.md`
- Highlights: GPU spectrum bullet now notes the **per-pixel FFT trace at
up to 60 fps** and the **3D stacked-trace** spectrum mode

### Reviewer notes
- Touches protected paths (`*.md` + `AGENTS.md` Tier-1 +
`CMakeLists.txt`) — needs `@aethersdr/infrastructure` sign-off.
- Tagging `v26.7.1` and cutting the release build are **not** part of
this PR (docs prep only).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

2 participants