Commit 623a97a
Fixes #5200.
## What
Stock Windows/MSVC builds have never compiled
`CwSidetonePortAudioSink.cpp`: PortAudio detection ran exclusively
through pkg-config, which does not resolve on Windows, so
`HAVE_PORTAUDIO` was never defined and every Windows build silently took
the push-model `CwSidetoneQAudioSink` sidetone (2 ms timer). The WASAPI
host-API preference added in #3193 *specifically for Windows CW jitter*
has consequently never been active in any shipped build. Windows is the
build-configuration route to the same push-path landing spot as #4978's
Linux runtime route (per the split documented in #4890).
**Build plumbing** mirrors the established FFTW3/hidapi
Windows-dependency pattern:
- **`scripts/setup/setup-portaudio.ps1`** — downloads PortAudio v19.7.0
(pinned, SHA256-verified, same `_verify_sha256.ps1` helper as the other
setup scripts), builds the static lib with MSVC (WASAPI requested
explicitly; WDM-KS / DirectSound / MME from upstream's Windows
defaults), installs into `third_party/portaudio/`.
- **`CMakeLists.txt`** — a `WIN32` detection branch that soft-detects
`third_party/portaudio/` (absent → build proceeds, with a
`message(WARNING)` naming the script and the consequence), checking both
the header and the static lib it links, and naming that lib's system
dependencies from PortAudio's own CMake export.
- **CI wiring** — the Windows CI job runs the script (after
`msvc-dev-cmd`, same as hidapi), plus an assert step that fails the job
if CMake did not actually find PortAudio, so this job cannot go green
having compiled nothing new.
**Sink fixes.** Merely compiling the sink was NOT enough. Live A/B
testing by ear (FLEX-6300 into a dummy load, keyboard iambic paddles)
found it audibly broken on Windows, three defects deep — all in code
that had only ever run against CoreAudio:
1. **Windows friendly names are not unique.** The test box has **three
active endpoints all named "TOSHIBA-TV (NVIDIA High Definition Audio)"**
(one per HDMI connector; five including unplugged ones). Name matching
selected a live-but-unwired port that accepted the stream and played it
into nothing — instrumentation showed 50k callbacks rendering a clean
0.566-peak tone into an inaudible endpoint. The fix matches the Qt
device to the PortAudio WASAPI device **by endpoint ID**
(`PaWasapi_GetIMMDevice` → `IMMDevice::GetId` compared against
`QAudioDevice::id()`), demoting name matching to fallback. This is the
Windows analog of the name-match fragility skerker is fixing on Linux in
#5123/#5135 — same disease, platform-appropriate cure.
2. **The exact-match branch defeated #3193.**
`findPortAudioOutputDevice()` returned the *first* exact name match in
enumeration order — DirectSound on Windows — so the WASAPI preference
(written only into the partial-match branch) never ran. Exact matches
are now collected and the same WASAPI preference applied; a multi-exact
match with no WASAPI candidate names the losing candidates rather than
silently taking the first.
3. **`suggestedLatency = 0.0` is a CoreAudio-ism.** DirectSound built an
unservable buffer ring: the stream ran, the callback rendered a clean
tone, and the speaker output was garbled crackle. Windows now requests
the device's `defaultLowOutputLatency` (22 ms reported on WASAPI shared
for this endpoint); other platforms unchanged.
**Observability**, because the diagnosis above needed instrumentation
the start line lacked — `hostApi=` on the started line, a stopping line
with `callbacks=` / `peak=` / `underflows=` / `overflows=`, and the
endpoint-ID match. All on `lcAudioSummary` so they reach a **default**
support bundle; `lcAudio` sits at `QtWarningMsg`, so a `qCInfo` on it
would not. A started stream that renders silence or garbage is now
distinguishable from a working one without asking the operator to re-run
with debug logging. Underflow counting exempts the first couple of
stream-prime callbacks, which report `paOutputUnderflow` on essentially
every host and are not deadline misses.
**`src/core/CwSidetoneEdgeProbe.h`** (new) is the bench instrument
behind the timing tables in the thread: `AETHER_CW_EDGE_PROBE=1` makes
**both** sidetone sinks capture envelope transitions at the sink
boundary with a running sample counter, dumped at `stop()` as
`EDGEPROBE` lines. Positions are stream-sample-exact, so element
durations and onset spacing are measured on the stream's own clock — no
loopback recording, no wall-clock jitter in the instrument. Disabled it
costs one bool test per rendered buffer and allocates nothing.
`CwSidetoneQAudioSink` is wired up too, which is what makes the
push-vs-callback A/B a like-for-like comparison; that is the only change
here to a file that runs on every platform.
`tests/cw_sidetone_edge_probe_test.cpp` covers the probe: the
empty-stream reset (reverting it fails the test with `got 480001, want
0`), the zero-crossing rule the falling-edge detection rests on,
short-element detection, and the absolute-threshold limitation — pinned
so it stays a deliberate property.
## Shipped Windows builds switch to the callback sink
This is the maintainer decision #5200 asked to be made explicitly, and
it has been made: `windows-installer.yml` runs the setup script, so
**shipped Windows installers now build with `HAVE_PORTAUDIO` and ship
the PortAudio callback sidetone** — the same transport macOS and Linux
already ship, and what #3193 intended but never delivered.
Note there is no per-platform default being changed: `CwSidetoneBackend`
has always defaulted to `"PortAudio"` on every platform. On Windows that
default was simply unsatisfiable, because the sink was never compiled.
This makes it reachable.
**The trade being accepted**, from the element timing further down this
thread:
- **Latency (the win):** ~22 ms buffered depth to the DAC against the
push sink's ~50 ms — roughly **28 ms less key-to-ear delay**. That is
what a CW operator feels at the key and what #3193 was chasing.
- **Rhythm (the cost):** a small jitter tail at 30 WPM that two
independent Windows boxes both show and neither explains — displacements
quantized around 10.3–10.8 ms, with `underflows= 0` across 169,523
callbacks, so it is *not* a device deadline miss. At 20 WPM the tail is
absent (SD 0.262 ms, 0 of 176 elements beyond 3SD). The in-stream rhythm
of the push path remains tighter (SD 0.26 ms vs 0.96 ms on the
originating box).
Latency wins; the tail is tracked rather than dismissed, and the
instrumentation in this PR is what will explain it.
The installer job also gains an assert mirroring the GPU-spectrum guard
— the `WIN32` detection is soft-optional, and this is the workflow that
produces what users install, so a silent skip would ship the push
sidetone while the release notes claimed otherwise. It fails the build
instead.
`CwSidetoneBackend=QAudioSink` remains the escape hatch, and the
PortAudio-start-failure → QAudioSink fallback (with the
consequence-naming log from the #4978 fixes) is unchanged.
## Verified on real hardware (Windows 11, MSVC, Qt 6.10.3, FLEX-6300 on
a dummy load)
Full build from clean configure; app run, connected, keyboard iambic
paddles keyed by both an operator and synthesized key events; every
claim below is **by ear**, A/B on the same physical endpoint:
- **QAudioSink control:** clean tones (establishes the endpoint + gate
path).
- **Sink as first compiled** (DirectSound selected by defect 2, garbled
by defect 3): audibly broken — first reported as "sounds crap", then
near-silent on a different endpoint pick (defect 1).
- **Sink with all three fixes:** `matched WASAPI endpoint by ID
"{0.0.0.00000000}.{35f3f303-…}"` → `hostApi= Windows WASAPI …
outputLatency= 22 ms` → **clean tones**, operator-confirmed, radio
keying verified via `cw key` wire traces and clean unkey after every
run.
Element-timing measurement per @williamscody's #4890 methodology is in
the thread below, as is @skerker's independent replication on macOS,
Windows and Linux — the macOS arm is the regression check that mattered,
since that platform already ships this sink.
A process note worth stating plainly: the PR as first opened claimed the
sink "works" from a clean start line and low reported latency. That
claim was wrong — a started stream was rendering garbage. The by-ear A/B
is what caught it, and the `callbacks=` / `peak=` instrumentation is
there so the next person can catch it from a log instead.
## Not exercised
- MME-truncation partial-match path (all live selections here resolved
exact or by ID).
- PortAudio start-failure → QAudioSink fallback on Windows.
- The Windows-only `#ifdef Q_OS_WIN` paths have no automated coverage —
`findPortAudioOutputDevice()` needs live device enumeration, and a
synthetic peer would be worse than nothing. They rest on the hardware
A/B above and @skerker's Windows bench.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Jeremy [KK7GWY] <kk7gwy@aethersdr.com>
1 parent 2bed375 commit 623a97a
13 files changed
Lines changed: 825 additions & 17 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
522 | 522 | | |
523 | 523 | | |
524 | 524 | | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
525 | 534 | | |
526 | 535 | | |
527 | 536 | | |
| |||
612 | 621 | | |
613 | 622 | | |
614 | 623 | | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
615 | 649 | | |
616 | 650 | | |
617 | 651 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
131 | 131 | | |
132 | 132 | | |
133 | 133 | | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
134 | 152 | | |
135 | 153 | | |
136 | 154 | | |
| |||
239 | 257 | | |
240 | 258 | | |
241 | 259 | | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
242 | 282 | | |
243 | 283 | | |
244 | 284 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
41 | 41 | | |
42 | 42 | | |
43 | 43 | | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
44 | 49 | | |
45 | 50 | | |
46 | 51 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
296 | 296 | | |
297 | 297 | | |
298 | 298 | | |
299 | | - | |
300 | | - | |
301 | | - | |
302 | | - | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
303 | 336 | | |
304 | 337 | | |
305 | 338 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4113 | 4113 | | |
4114 | 4114 | | |
4115 | 4115 | | |
4116 | | - | |
4117 | | - | |
| 4116 | + | |
| 4117 | + | |
| 4118 | + | |
| 4119 | + | |
4118 | 4120 | | |
4119 | 4121 | | |
4120 | 4122 | | |
| |||
0 commit comments