fix(audio): route monitor + QSO playback to selected output device (#3361). Principle I.#3378
Conversation
…3361). Principle I. ClientPuduMonitor::startPlayback() and QsoRecorder::startPlayback() both opened QMediaDevices::defaultAudioOutput() unconditionally, ignoring the user's choice in Radio Settings > Audio. On Windows where the system default rarely matches the AetherSDR output selection, the post-DSP TX capture (AetherVoice TX record/playback) was playing out of the wrong physical device — buffer drains, play button goes green, but no audio arrives at the user's selected sink. Adopt the same defensive selection pattern AudioEngine::startSidetoneStream() already uses: prefer m_outputDevice when it's still present in the live QMediaDevices::audioOutputs() list, fall back to defaultAudioOutput() when the saved selection has vanished (hotplug / unplug between selection and playback). MainWindow seeds both objects from AudioEngine::outputDevice() at construction and re-seeds them on AudioEngine::outputDeviceChanged so the user's later selections are honoured without restarting the app. Also extend ClientPuduMonitor's format ladder from 24/48 → preferredFormat to 24/48/44.1 → preferredFormat, mirroring CwSidetoneQAudioSink, so devices that reject 48 kHz Int16 but accept 44.1 kHz don't fall straight through to the preferred-format leg of the ladder. Blast radius: risk_score=0.197, 6 high-risk callers affected (top: MainWindow::MainWindow, MainWindow::wirePanadapter, MainWindow::registerMidiParams). All affected callers reach the touched classes via existing APIs; the change is additive (new setOutputDevice setter + new m_outputDevice member), no existing signatures change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Clean, well-scoped fix — thanks for this. The setter pattern (header-inline setOutputDevice, null device = system-default fallback) is minimal, and validating the saved id against the live audioOutputs() list before opening the sink correctly handles the hotplug-between-selection-and-now case. Routing from MainWindow at construction plus an outputDeviceChanged re-seed covers both initial state and live changes. The m_audio->outputDevice() read across threads in the lambda is consistent with the existing pattern at MainWindow.cpp:8184 and RadioSetupDialog.cpp:2195.
Two small observations:
-
44.1 kHz Int16 fallback is added to
ClientPuduMonitoronly. It mirrorsCwSidetoneQAudioSink's 48/44.1 ladder, which is reasonable defense — butQsoRecorder::startPlayback(src/core/QsoRecorder.cpp:377-381) still has only a 24→48 ladder, then bails. If the 44.1 step is worth adding to monitor playback on devices that reject 48 kHz Int16, the same devices will reject QSO playback too. Worth either applying the same ladder toQsoRecorderfor parity, or splitting the format-ladder change into its own PR — the routing fix and the codec-ladder hardening are independent concerns, and bundling them obscures the scope stated in the title. -
Minor: the
outputDeviceChangedsignal carries noQAudioDevicepayload, so the lambda has to round-trip back intom_audioto fetch the new device. Not a blocker (matches the existing convention), but if you ever revisit the signal, threading it through as a parameter would letMainWindowskip the cross-thread member read.
Otherwise no concerns — no null/leak risks, conventions look right, and the comment lines call out the "still-present in live list" rationale clearly for future readers.
🤖 aethersdr-agent · cost: $18.2500 · model: claude-opus-4-7
NF0T
left a comment
There was a problem hiding this comment.
Clean, correct fix for the audio routing miss. The device selection pattern — default fallback first, replace only if the stored device is still present in the live audioOutputs() list — correctly handles null/unset, device present, and hotplug-between-selection-and-playback. Both ClientPuduMonitor and QsoRecorder get the same idiom. Qt::QueuedConnection on the outputDeviceChanged wiring is correct — signal fires from the audio thread, lambda runs on main thread before reading m_audio->outputDevice(), consistent with the existing pattern at MainWindow.cpp:8184 and RadioSetupDialog.cpp:2195.
The 44.1 kHz Int16 fallback step added to ClientPuduMonitor's format ladder is correct in isolation — mirrors CwSidetoneQAudioSink and the log string is correctly updated to reference 44.1 as the last-tried Int16 format. The asymmetry with QsoRecorder (which still has a shorter 24→48 ladder) is noted; a follow-up issue has been filed to track parity. No regression — QsoRecorder's behavior is unchanged from before this PR.
QAudioDevice stored by value with isNull() fallback is correct. Comments explain the hotplug rationale clearly. All conventions respected — no QSettings, no raw ownership issues. CI green on all five checks.
This is a bot-generated PR — reviewed as required by CODEOWNERS since @aethersdr-agent is not a team member.
✅ Approved.
…3385). Principle XIII. PR #3378 extended ClientPuduMonitor::startPlayback()'s format negotiation to 24/48/44.1 kHz Int16, matching CwSidetoneQAudioSink and handling Windows HFP/SCO routes and USB DACs that reject 48 kHz Int16 but accept 44.1 kHz. The same ladder extension was missed in QsoRecorder::startPlayback(), so on such devices monitor playback now succeeds at 44.1 kHz while QSO recording playback silently bails. Insert the same 44.1 kHz fallback step between the existing 48 kHz attempt and the bail. preparePlaybackPcm(sinkRate) already constructs a Resampler at the requested rate, so no resampler changes are needed. Per the issue body, scope is "format negotiation only" — the structured AudioSummaryLogger reporting and Float32 preferredFormat fallback that CPM also carries are out of scope for this fix. Blast radius: risk_score=0.158, 3 high-risk affected (top: MainWindow::MainWindow, MainWindow::onSliceAdded, MainWindow::registerMidiParams). All are signal-wiring/construction sites that depend on the public method and signal contract, both unchanged by this internal ladder extension.
…ethersdr#3306) Replace the hand-wired "connect each external playback sink to outputDeviceChanged" lambda in MainWindow with a single registry. External sinks that own their own QAudioSink (the post-DSP Pudu monitor, QSO playback) already followed the user-selected output device correctly — every device-change path (user selection, device removal, OS-default change) flows through AudioEngine::setOutputDevice() -> outputDeviceChanged. The risk was purely structural: each NEW external sink had to remember to join that lambda or it would "uncouple" and keep playing on the old/default endpoint (the recurring class behind aethersdr#2899 / aethersdr#3361 / aethersdr#3378). AudioOutputRouter (src/core/AudioOutputRouter.{h,cpp}) makes following a registration instead of hand-wiring: - addFollower(sink) takes any object with setOutputDevice(QAudioDevice) (or a std::function); the follower is seeded immediately and re-seeded on every change, QPointer-guarded against early destruction. - One QueuedConnection forwards outputDeviceChanged to setCurrentDevice(), which fans out to all followers on the GUI thread (matching prior behaviour) over a snapshot so a follower mutating the list mid-fan-out can't invalidate it. - Depends only on Qt Core/Multimedia, so the registry/fan-out is unit-tested headless (tests/audio_output_router_test, 11/11) without a live AudioEngine, including the QPointer-guard and reentrancy paths. MainWindow registers m_finalMonitor + m_qsoRecorder; a future external sink follows correctly just by registering — no new connect to forget. Behaviour- identical hardening. Intentionally NOT routed (documented): the AudioEngine- internal sinks (RX speaker, CW sidetone, Quindar — follow via the RX restart), and the WFM demodulator's WaveOutWriter (plays to its own WfmDeviceDialog device by design, aethersdr#3407). Builds & links clean into the full app; all three audio tests pass under CTest. Refs aethersdr#3306 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…3306) (#3630) ## What Layer 3 of the consolidated audio sink factory (#3306): a single **registry** for playback sinks that must follow the user-selected output device, replacing the hand-wired per-sink `connect`-to-`outputDeviceChanged` lambda in `MainWindow`. ## Why The external playback sinks that own their own `QAudioSink` — the post-DSP Pudu monitor (`ClientPuduMonitor`) and QSO playback (`QsoRecorder`) — already followed the selected device correctly: every device-change path (user selection, device removal, OS-default change) flows through `AudioEngine::setOutputDevice()` → `outputDeviceChanged`, and a `MainWindow` lambda re-seeded both. The risk was purely **structural** — every *new* external sink had to remember to join that lambda, or it would "uncouple" and keep playing on the old/default endpoint. That's the recurring class behind #2899 (CW sidetone) and #3361/#3378 (monitor + QSO). ## How `AudioOutputRouter` (`src/core/AudioOutputRouter.{h,cpp}`) turns following into registration: - `addFollower(sink)` accepts any object exposing `setOutputDevice(const QAudioDevice&)` (or a `std::function`). The follower is **seeded immediately** and **re-seeded on every change**, `QPointer`-guarded against early destruction. - One `QueuedConnection` forwards `AudioEngine::outputDeviceChanged` → `setCurrentDevice()`, which fans out to all followers on the GUI thread (matching the previous behaviour). - Depends only on Qt Core/Multimedia, so the registry/fan-out is **unit-tested headless** (`tests/audio_output_router_test`, 9/9) without a live `AudioEngine` — including the QPointer-guard path. `MainWindow` now registers `m_finalMonitor` + `m_qsoRecorder` with the router instead of hand-wiring; a future external sink follows correctly just by registering. The AudioEngine-internal sinks (RX speaker, CW sidetone, Quindar) already follow via the RX restart and are intentionally **not** routed here. ## Behaviour **Identical** to the hand-wired version — this is the *hardening* that stops the uncoupling class from silently reappearing, not a behaviour change. ## Test / build - `audio_output_router_test` **9/9**; `audio_format_negotiation_test` 25/25; `audio_device_negotiator_test` 8/8 — all green under CTest. - Full macOS app builds & links clean (RelWithDebInfo). Refs #3306 💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ethersdr#3361). Principle I. (aethersdr#3378) ## Summary Fixes aethersdr#3361 ### What was changed fix(audio): route monitor + QSO playback to selected output device (aethersdr#3361). Principle I. ### Files modified - `src/core/ClientPuduMonitor.cpp` - `src/core/ClientPuduMonitor.h` - `src/core/QsoRecorder.cpp` - `src/core/QsoRecorder.h` - `src/gui/MainWindow.cpp` ``` src/core/ClientPuduMonitor.h | 10 ++++++++++ src/core/QsoRecorder.cpp | 12 ++++++++++++ src/core/QsoRecorder.h | 10 ++++++++++ src/gui/MainWindow.cpp | 12 ++++++++++++ 5 files changed, 69 insertions(+), 2 deletions(-) ``` --- Generated by AetherClaude (automated agent for AetherSDR) --- <sub>🤖 aethersdr-agent · cost: $14.3082 · model: claude-opus-4-7</sub> Co-authored-by: aethersdr-agent[bot] <273844287+aethersdr-agent[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Fixes #3361
What was changed
fix(audio): route monitor + QSO playback to selected output device (#3361). Principle I.
Files modified
src/core/ClientPuduMonitor.cppsrc/core/ClientPuduMonitor.hsrc/core/QsoRecorder.cppsrc/core/QsoRecorder.hsrc/gui/MainWindow.cppGenerated by AetherClaude (automated agent for AetherSDR)
🤖 aethersdr-agent · cost: $14.3082 · model: claude-opus-4-7