feat(audio): AudioOutputRouter registry for output-following sinks (#3306)

[jensenpat]

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

[jensenpat]

Defensive review (self) — completeness + correctness

Ran a two-front review before this is reviewed: a sweep of the last two weeks of audio PRs/issues, and an exhaustive codebase sweep of every output sink. Summary so others don't have to redo it:

Follower completeness — confirmed exhaustive. Every QAudioSink/PortAudio output owner in the tree, bucketed:

• Router followers (correct): ClientPuduMonitor, QsoRecorder — the only two sinks whose contract is "play to the user's main selected output."
• AudioEngine-internal (follow via RX restart, correctly not routed): RX speaker, CW sidetone (QAudio + PortAudio backends), Quindar local sink.
• Own-device by design (correctly NOT routed): WFM WaveOutWriter (Feature/wfm demod: add WideFM software demodulator via DAX IQ #3407) — plays to its own WfmDeviceDialog device; registering it would be a regression. Now called out explicitly in the header + docs so it isn't "fixed" later by mistake.
• No sink reads QMediaDevices::defaultAudioOutput() at playback time except as the documented null-fallback in the two followers.

Behaviour equivalence — confirmed. All device-change paths (user selection via applyAudioDeviceSelection, removal/OS-default via resetMissingAudioDevicesToDefault) route through setOutputDevice() → outputDeviceChanged, which the router consumes. Seed-at-registration + re-seed-on-change matches the prior #3378 lambda exactly. The init block is race-free (synchronous on the GUI thread in buildUI).

Hardening added after review:

• Fan-out now iterates a snapshot so a follower mutating the list mid-callback can't invalidate the loop (+ a reentrancy unit test; suite now 11/11).
• WFM/WaveOutWriter documented as intentionally-not-routed.
• Corrected the two sink-header comments to name the router.

Heads-up (not addressed here, by design):

• Client Side recording produces a silent file — TX audio is never captured (QsoRecorder::feedTxAudio has no signal connected) #3556 (silent Client-Side TX recording — QsoRecorder::feedTxAudio has no connect()) is a missing-connection bug, not device-following; this PR neither fixes nor worsens it, but its fix touches the adjacent MainWindow region — rebase coordination.
• This edits a hot MainWindow region (the Refactor: break up the 15k-line MainWindow.cpp monolith (phased, behavior-preserving) #3351 decomposition; Refactor Phase 2: extract SwrSweepController + FreeDvSessionController from MainWindow (header member reduction) #3557 is the next open phase). Will rebase before un-drafting.

CI: build / macOS / accessibility green; Windows + analyze pending.

[aethersdr-agent]

Reviewed the full diff against AetherSDR conventions, correctness, scope, and error handling. This is a clean, well-targeted refactor — the behaviour-identical claim holds up and the structural hardening is real. Nice work, @jensenpat.

Strengths

• Conventions — RAII throughout: router is parent-owned (new AudioOutputRouter(this)), followers are QPointer-guarded against early destruction, and the template/std::function overload split keeps the common case to one self-documenting line. No QSettings/AppSettings surface touched. Namespacing is consistent (AetherSDR for router, sinks, and MainWindow).
• Reentrancy — the snapshot copy in setCurrentDevice() before the fan-out is the correct guard against a follower mutating m_followers mid-loop, and the comment correctly reasons that an added-during-fan-out follower is already seeded by addFollower() so skipping it isn't a miss. The test (add-during-fan-out) exercises exactly this.
• Threading — bridging outputDeviceChanged (audio worker thread) → setCurrentDevice via QueuedConnection keeps all m_followers/m_device access on the GUI thread, matching the prior hand-wired behaviour; no data race introduced.
• Behaviour parity — seeding setCurrentDevice() before the two addFollower() calls reproduces the old seed-then-resubscribe ordering exactly.
• Scope & docs — every changed file is in scope; docs/audio-sink-factory.md is updated to mark Layer 3 landed and, helpfully, documents the intentionally-not-routed sinks (AudioEngine-internal + WFM WaveOutWriter) so a future reader doesn't "fix" them by registering.

Minor (non-blocking)

• The std::function addFollower overload has no lifetime guard, unlike the template — correct by design (the template is the guarded convenience path), and the header comment makes that explicit. Worth keeping in mind if a future caller registers a lambda capturing a raw sink pointer; the guarded addFollower(T*) form should stay the default.

No bugs, null-pointer risks, or resource leaks found; all 6 CI checks (build, macOS, Windows, CodeQL, accessibility) are green, and the headless 9/9 unit test covers seeding, fan-out, the QPointer-guard path, and reentrancy. Thanks for the thorough PR description and the test coverage.

---

_{🤖 aethersdr-agent · cost: $3.0307 · model: claude-opus-4-8}

[aethersdr-agent]

Reviewed the full diff against the four review axes (conventions, bugs/null/leak risks, scope, boundary error handling). This is a tidy, well-targeted refactor and I have no blocking concerns — all six checks are green (build, check-macos, check-windows, CodeQL, Qt a11y, cpp analysis).

What I verified holds up:

• Seeding / ordering in MainWindow. The sequence — register the QueuedConnection forwarder, then setCurrentDevice(...), then addFollower(...) for each sink — is correct. The queued forwarder can't fire synchronously during construction, setCurrentDevice updates m_device against an empty follower list, and each addFollower then seeds exactly once. No double-seed, no missed seed.
• Reentrancy. setCurrentDevice fanning out over a copy of m_followers is the right call, and the reasoning that an add-during-fan-out follower is already seeded by addFollower (since m_device is updated first) is sound. The test covers this path explicitly.
• Lifetime. The QPointer<T> guard in the template overload is correct, and the forwarder lambda binds this as context so it auto-disconnects with MainWindow. No leak (router is parented to this), no dangling-follower crash.
• Threading. QueuedConnection keeps the fan-out on the GUI thread, matching the prior hand-wired behaviour — no regression.

Scope: every file earns its place — the two sink headers are comment-only doc updates, and the docs/audio-sink-factory.md changes accurately reflect what landed. Nothing out of scope.

One small note (non-blocking): the forwarder re-reads m_audio->outputDevice() at delivery time rather than capturing the value at emit time. This is identical to the previous behaviour and harmless (last-write-wins on coalesced changes), so no change needed — just flagging that the router's setCurrentDevice(const QAudioDevice&) slot signature invites connecting outputDeviceChanged directly if it ever carries the device as an argument, which would remove the read-back hop.

The headless unit test (9/9, including the QPointer-guard and reentrancy paths) is exactly the right level for this logic, and turning "remember to join the lambda" into "register once" genuinely closes the uncoupling class behind #2899/#3361/#3378. Nice, careful work — thanks @jensenpat.

---

_{🤖 aethersdr-agent · cost: $2.1699 · model: claude-opus-4-8}

[ten9876]

Approving — clean, behavior-identical structural hardening with a headless unit test.

Verified:

• Router core: setCurrentDevice fans out over a snapshot of the follower list — correct for re-entrant registration (a follower could register another mid-fan-out); the skip-the-mid-add case is already seeded by addFollower, so it's not a miss.
• Lifetime: the template addFollower(T*) QPointer-guards the sink — a follower destroyed before the router is a harmless no-op.
• Threading preserved: the outputDeviceChanged → setCurrentDevice forwarder stays a QueuedConnection, so followers are touched on the GUI thread, identical to the hand-wired version.
• MainWindow rewiring is behavior-identical with correct ordering (setCurrentDevice sets m_device before addFollower seeds each).
• Scoping documented and right — AudioEngine-internal sinks and the WFM WaveOutWriter are intentionally excluded, with "do not register" comments to stop a future reader from re-opening the regression.
• New headless audio_output_router_test + CI green across all six.

Exactly the right "turn a recurring footgun into a registration" hardening (#2899/#3361/#3378). One non-blocking nit (dead QPointer-null followers are never pruned from m_followers) — negligible for a register-once list of two; I'll file a follow-up rather than gate on it. Nice work @jensenpat.