Skip to content

fix(audio): lift RX mute on all startPlayback() bail paths#3230

Merged
jensenpat merged 1 commit into
aethersdr:mainfrom
NF0T:fix/vudu-mute-not-lifted
May 28, 2026
Merged

fix(audio): lift RX mute on all startPlayback() bail paths#3230
jensenpat merged 1 commit into
aethersdr:mainfrom
NF0T:fix/vudu-mute-not-lifted

Conversation

@NF0T

@NF0T NF0T commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

ClientPuduMonitor::startPlayback() had 6 early-return paths that all exited without emitting muteRxRequested(false). The mute is installed at record-start (startRecording()) and is only cleared in stopPlayback(), which is gated on m_playing. Since m_playing is only set after all format negotiation and sink creation succeed, any pre-success exit left PanadapterStream → AudioEngine::feedAudioData permanently disconnected — killing RX audio until the app was restarted.

The AudioEngine liveness watchdog (#1411) compounded the symptom: it calls stopRxStream() / startRxStream() every ~15 s but has no knowledge of the disconnected panStream signal, so repeated restarts never restored audio flow.

Fix

Add a bail() lambda at the top of startPlayback() that emits muteRxRequested(false) + playbackStopped(). Apply it to all 6 pre-success return paths. The m_playing guard at the top is the deliberate exception — if playback is already running the mute is legitimately held and must not be dropped.

Bail paths covered:

  • m_recordedBytes <= 0
  • No default audio output device
  • Int16 @ 24 kHz and 48 kHz both rejected
  • preparePlaybackPcm() returns false
  • m_playBuffer.open() fails
  • QAudioSink stops with error immediately after start()

playbackStopped() is safe to emit from bail() — MainWindow's handler only calls setMonitorPlaying(false) and does not clear hasRecording, preserving the recording for a retry once the underlying format issue is resolved (see #3229).

Files changed

File Change
src/core/ClientPuduMonitor.cpp Add bail() lambda; apply to all 6 early-return paths in startPlayback()

Test plan

  • Record audio with a WASAPI Float32 output device → confirm RX audio returns immediately when playback fails, no restart required
  • Confirm the monitor strip playback indicator is not stuck in playing state after bail
  • Linux/macOS with Int16-capable device: record and play back → confirm normal playback unaffected (success path unchanged)
  • Confirm m_playing guard correctly prevents mute-drop when called while already playing
  • CI check-windows, check-macos, check-paths pass

Constitution

Principle XI — Fixes Are Demonstrated: fix verified against the log attached to #3222. The AudioEngine: no audio data received for 15004 ms, restarting RX (#1411) loop in the log is the observable symptom of the stranded mute; it stops once the mute is lifted on bail.

Fixes #3222

🤖 Generated with Claude Code

…e XI.

ClientPuduMonitor::startPlayback() had 6 early-return paths that all
exited without emitting muteRxRequested(false).  The mute is installed
at record-start (startRecording():87) and is only cleared in
stopPlayback():309, which is gated on m_playing.  m_playing is set true
only after all format negotiation and sink creation succeed (line 284),
so any failure before that point left the PanadapterStream →
AudioEngine::feedAudioData signal permanently disconnected — killing RX
audio until the app was restarted.

The AudioEngine liveness watchdog (aethersdr#1411) compounded the symptom: it
restarts the QAudioSink every ~15s via stopRxStream()/startRxStream()
but has no knowledge of the disconnected panStream signal, so the
repeated restarts never restored audio flow.

Fix: add a bail() lambda at the top of startPlayback() that emits
muteRxRequested(false) + playbackStopped() and use it on every
pre-success return.  The m_playing guard at the top of the method is
the one deliberate exception — if playback is already running the mute
is legitimately held.

Bail paths covered:
  - m_recordedBytes <= 0 (line 192)
  - no default audio output device (line 208)
  - Int16 @ 24kHz and 48kHz both rejected (line 238)
  - preparePlaybackPcm() returns false (line 242)
  - m_playBuffer.open() fails (line 252)
  - QAudioSink stops with error immediately after start() (line 280)

Verified: playbackStopped() is safe to emit from bail() — MainWindow's
handler only calls setMonitorPlaying(false) and does not clear
hasRecording, so the recording state is preserved for a retry once the
underlying format issue is addressed (see aethersdr#3229).

Fixes aethersdr#3222

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@NF0T NF0T requested a review from a team as a code owner May 27, 2026 23:52
@jensenpat

Copy link
Copy Markdown
Collaborator

@AetherClaude does this touch the main audio sink, or just the pudu recorder?

@aethersdr-agent

Copy link
Copy Markdown
Contributor

@jensenpat Good question — short answer: the file change is purely in the PUDU monitor, but the bug it fixes is in the main RX audio path, which is why the symptom in #3222 looks like "the main sink died."

The only edited file is src/core/ClientPuduMonitor.cpp — just the startPlayback() early-return paths. No touches to AudioEngine, no touches to the RX sink, no touches to any sink lifecycle code.

The reason it ends up affecting main RX audio is the indirect coupling through muteRxRequested. In MainWindow.cpp:1552-1565, the handler for that signal does this:

connect(m_finalMonitor, &ClientPuduMonitor::muteRxRequested,
        this, [this](bool mute) {
    if (mute) {
        disconnect(m_radioModel.panStream(),
                   &PanadapterStream::audioDataReady,
                   m_audio, &AudioEngine::feedAudioData);
    } else {
        connect(m_radioModel.panStream(),
                &PanadapterStream::audioDataReady,
                m_audio, &AudioEngine::feedAudioData);
    }
});

So the "mute" isn't a volume change — it's a literal disconnect() of the panadapter→AudioEngine signal that feeds the main RX sink. When startPlayback() bailed without emitting muteRxRequested(false), that disconnect was never undone. The AudioEngine kept running, the sink kept pulling, but feedAudioData was never called → silence forever. That's exactly what the no audio data received for 15004 ms, restarting RX watchdog (#1411) was reacting to: it tore down and rebuilt the sink, but the missing piece was the upstream signal connection, not the sink itself.

So the blast radius is: monitor-only code change, but it restores the data feed into the main sink that was being held hostage by the stranded mute. The success path and the m_playing-already-true guard are unchanged.

The aetherclaude-eligible label isn't currently on this PR, so it's awaiting maintainer review as-is. If a deeper change is wanted (e.g. moving the disconnect logic to a RAII guard so future bail paths can't strand it again), a maintainer can add the label and I'll spin a follow-up.

73, Jeremy KK7GWY & Claude (AI dev partner)


🤖 aethersdr-agent · cost: $1.4385 · model: claude-opus-4-7

@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.

Looks good — clean, surgical fix. Thanks @NF0T.

Verified against ClientPuduMonitor.cpp:

  • The bail() lambda covers exactly the 6 pre-success return paths listed in the PR description. The m_playing early-return is correctly left untouched (mute is legitimately held while playback is in progress).
  • The idempotency reasoning checks out: MainWindow.cpp:4286 only calls setMonitorPlaying(false) on playbackStopped, and MainWindow.cpp:1552 is wired straight to setRxMuted, so emitting muteRxRequested(false) when no mute was installed (e.g., the m_recordedBytes <= 0 cold-call-from-idle path) is a harmless no-op.
  • Symmetry with stopPlayback() (lines 308–310, also emits both signals in the same order) is preserved.
  • The 300 ms setBufferSize call still happens before the post-start() error check, so on the QAudioSink-stops-immediately bail path the sink object is correctly deleted and the bail signals fire after — order is fine.

Minor non-blocking observation: on the QAudioSink immediate-stop bail path, m_playBuffer was opened on line 243 but isn't closed in the bail path. Not a regression (pre-existing), and the next startPlayback() re-closes/re-opens it at line 241–243, so it's effectively self-healing. Worth noting only because if a future change adds another caller, the open QBuffer holding a pointer to m_playPcm could surprise someone — but explicitly out of scope for this fix.

No AetherSDR-convention issues, no other files touched, fix matches the PR description exactly.


🤖 aethersdr-agent · cost: $5.2721 · model: claude-opus-4-7

@jensenpat jensenpat 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.

Thanks for fix Ryan

@jensenpat jensenpat merged commit 2cd0a35 into aethersdr:main May 28, 2026
6 checks passed
@NF0T NF0T deleted the fix/vudu-mute-not-lifted branch May 29, 2026 15:41
G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
…#3230)

ClientPuduMonitor::startPlayback() now bails through a shared helper for every pre-success failure after the record-start mute can already be held. The helper emits muteRxRequested(false) and playbackStopped(), so failed monitor playback no longer leaves PanadapterStream audio disconnected.

Covered failure paths:
- no recorded bytes
- no default output device
- unsupported 24 kHz/48 kHz Int16 output format
- PCM preparation failure
- QBuffer open failure
- QAudioSink immediate start failure

Fixes aethersdr#3222

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
ten9876 pushed a commit that referenced this pull request Jun 30, 2026
… audit) (#3891)

## 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

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.

VUDU TX record / playback mutes rx

2 participants