refactor(audio): finish Phase 6 — Pudu/QSO/CW sinks onto the factory (#3306 6b/6c)#3655
Conversation
There was a problem hiding this comment.
Reviewed the three new commits (Int16-first enabler, 6b Pudu/QSO, 6c CW) on top of the stacked #3630/#3631 parents. This is clean, well-documented work — the factory migration is mechanical and faithful, the FormatPreference enabler is a minimal threaded-through addition with the Auto default keeping RX/mic byte-identical, and the new audio_output_router_test (reentrancy + QPointer-guard coverage) and +3 golden negotiation rows are exactly the right tests. All six CI checks (build/macOS/Windows/CodeQL/a11y) are green. A couple of observations, none blocking:
1. CW sidetone on Linux: rate moves 48k → 24k, and desiredRateHz no longer drives selection. The old ladder led with desiredRateHz (always passed as 48000 from AudioEngine), so a Linux device that supports 48k opened at 48k. The factory ladder is now walked with preferredRateOverride=0, so Linux follows primaryRateOrder → native 24k first. desiredRateHz survives only in the fallback-reason logging. This is sonically equivalent (RegenerateAtRate retunes the generator to the device rate) and actually brings the sidetone in line with the RX sink's Linux native-24k policy — so I read it as intended consistency rather than a regression. Worth a one-line behaviour note though: the PR description calls out the Win/Mac 48k change but not the Linux CW 48k→24k shift. If preserving the caller's requested rate ever matters, passing preferredRateOverride=desiredRateHz would restore the old bias.
2. (Parent #3631, visible in this diff) QuindarLocalSink walks the ladder candidate without forcing channelCount(2). Pudu/QSO/CW all do c.setChannelCount(kChannels) before probing; Quindar uses cand as-is. Ladder candidates carry caps.channels = preferredFormat().channelCount(), which is normally 2 but isn't guaranteed (a mono-preferred or multichannel endpoint would yield a non-stereo candidate, and the Quindar tone is generated stereo). Since this lives in the stacked Quindar commit rather than the 6b/6c commits under review here, just flagging it for that PR's pass — not something to fix in this one.
Nice incremental win that QSO playback now survives Float-only WASAPI devices via the shared Int16→Float guard instead of bailing. Thanks @jensenpat — the docs update and the "intentionally not routed" notes in AudioOutputRouter.h are especially helpful for the next person.
🤖 aethersdr-agent · cost: $6.0452 · model: claude-opus-4-8
…ersdr#3306) Phase 6b enabler. The Output ladder is Float-first by default (right for the float-native RX speaker / sidetone / Quindar). The Int16-native playback sinks (QSO playback, Pudu monitor — they hold recorded/captured int16) want the opposite: Int16 on a normal device so they need no conversion, with Float only as the fallback for Float-only endpoints (aethersdr#3231). Add an optional `FormatPreference { Auto, Int16First, Float32First }` threaded through buildLadder()/negotiate() and the AudioDeviceNegotiator wrapper. Default Auto preserves today's behaviour exactly (RX/mic untouched). Three golden rows document the Int16-first output: normal device -> Int16 at the per-OS rate (Win/Mac 48k, Linux native 24k), Float-only device -> Float fallback. 28/28. Refs aethersdr#3306 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aethersdr#3306 6b) Both playback sinks held their own bespoke Int16 rate ladders. They now negotiate via AudioDeviceNegotiator with the new Int16First preference: on a normal device they get Int16 (no conversion, as before) at the per-OS rate (Win/Mac 48k — dodging the WASAPI 24k resampler artifacts aethersdr#2120, same policy as the RX sink — Linux native 24k), with the universal 44.1k and preferredFormat fallbacks supplied in one place. They walk the ladder with isFormatSupported (trusted — that is exactly how aethersdr#3231's Float-only WASAPI devices are detected). ClientPuduMonitor keeps its Int16->Float conversion (aethersdr#3231). QsoRecorder playback *gains* the same guard: it previously bailed outright on a Float-only device; now it falls back to Float and converts. Net behaviour on normal devices is unchanged except the 24k->48k preferred-rate shift on Win/Mac (consistent with RX; the recording is resampled up-front, one-shot). Builds & links clean; negotiation 28/28, wrapper 8/8, router 9/9. Refs aethersdr#3306 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ersdr#3306 6c) CwSidetoneQAudioSink replaced its bespoke {desired,48,44.1,24}kHz × {Float,Int16} probe loop with a walk of the shared AudioDeviceNegotiator ladder (RegenerateAtRate — the generator retunes to the negotiated rate). The tick handles both Float and Int16, so it walks the default Float-first ladder and takes the first supported rung; the VB-Audio / Int16-only-WASAPI Int16 fallback (aethersdr#2629) is preserved by the ladder's per-rate Float-then-Int16 ordering, and the universal 44.1k + preferredFormat fallbacks come for free. The PortAudio backend (primary on Linux/macOS) is untouched — it enumerates via PortAudio, which the Qt negotiator can't drive; this migrates only the QAudio fallback backend. CW is always started at a constant 48k, so Win/Mac negotiate 48k as before; Linux now prefers native 24k (benign for a tone — the generator retunes). Builds & links clean into the full app. Refs aethersdr#3306 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
4935037 to
23cff2a
Compare
ten9876
left a comment
There was a problem hiding this comment.
Approving — completes Phase 6; rebased clean, CI green across all six, no conflict vs current main (the #3632 QsoRecorder overlap is in disjoint regions).
Verified:
- Int16-first FormatPreference enabler defaults to Auto → RX/mic byte-identical (backward-compatible), +3 golden rows.
- 6b: Pudu/QSO playback via Int16First with the universal 44.1k/preferredFormat fallbacks; QSO playback gains the Int16→Float guard so it no longer bails on a Float-only WASAPI device (real fix).
- 6c: CW sidetone QAudio walks the Float-first ladder (VB-Audio #2629 Int16 fallback preserved); PortAudio backend untouched.
Soak note: Pudu/QSO/CW now prefer 48k on Win/Mac (deliberate #2120 policy; Linux stays 24k), strictly-additive otherwise — mirrors the RX sink. Deployed to the macOS test box. Every device-facing sink now negotiates through one path. Nice work @jensenpat.
Closes out Phase 6 of the audio sink factory (#3306) — every auxiliary output sink now negotiates format through the shared factory instead of a bespoke per-sink ladder.
Commits
Int16-first
FormatPreferenceenabler — the Output ladder is Float-first (right for the float-native RX/sidetone/Quindar). The Int16-native playback sinks want the opposite, so this adds an optionalFormatPreference { Auto, Int16First, Float32First }threaded throughbuildLadder/negotiate+ the wrapper. DefaultAutoleaves RX/mic byte-identical. +3 golden rows (28/28).6b — Pudu monitor + QSO playback negotiate via
AudioDeviceNegotiator(Int16First, PreservePan). On a normal device they get Int16 at the per-OS rate (no conversion, as before) with the universal 44.1k + preferredFormat fallbacks in one place.ClientPuduMonitorkeeps its Int16→Float guard (fix(audio): add preferredFormat() fallback for WASAPI Float32 output devices #3231);QsoRecorderplayback gains the same guard — it previously bailed on a Float-only WASAPI device, now it works.6c — CW sidetone (QAudio backend) walks the Float-first ladder (Int16 fallback preserved for VB-Audio CWX keying with PC Audio: No CW sidetone monitor heard #2629). The PortAudio backend is untouched (Qt negotiator can't drive it).
Behaviour notes (for soak)
Normal devices are unchanged except Pudu/QSO/CW now prefer 48k on Win/Mac (was 24k for Pudu/QSO) — this is the factory's deliberate #2120 policy (dodges WASAPI 24k-resampler artifacts, same as the RX sink); playback is resampled up-front, one-shot. Linux stays native 24k. Strictly-additive wins: QSO playback now survives Float-only devices; all three gain the 44.1k rung.
Test / build
audio_format_negotiation_test28/28,audio_device_negotiator_test8/8,audio_output_router_test9/9 — all green under CTest.After this, the factory covers every device-facing sink. Remaining #3306 items (TCI-TX client-rate, PipeWire DAX resampler, Windows mic) are separate.
Refs #3306
💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat