feat(audio): compile the PortAudio CW sidetone sink on Windows (#5200)

[nigelfenton]

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

Commit 1 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, WDM-KS, DirectSound, MME host APIs), installs into third_party/portaudio/.
• CMakeLists.txt — a WIN32 detection branch that soft-detects third_party/portaudio/ (absent → build proceeds without it), naming the static 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); the installer workflow gets a cached setup step.

Commit 2 exists because merely compiling the sink was NOT enough. Live A/B testing by ear (FLEX-6300 into a dummy load, keyboard iambic paddles) found the sink 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 CW sidetone: a single substring match can route an explicitly selected HDMI output to a different card (Linux/ALSA) #5123/fix(audio): stop a short PortAudio name claiming an explicitly selected sidetone output (#5123) #5135 — same disease, platform-appropriate cure.
2. The exact-match branch defeated External USB Audio Interface and Windows 11, latency is a huge issue #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.
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.

Commit 2 also adds the observability the diagnosis needed and the start line lacked: hostApi= on the started line, and a stopping line with callbacks=/peak= — a started stream that renders silence or garbage is now distinguishable from a working one in any support bundle.

The one decision to make consciously

The installer-workflow step is the user-facing behaviour change: shipped Windows builds switch their default CW sidetone from the push-model QAudioSink to the PortAudio callback sink — what #3193 intended. CwSidetoneBackend=QAudioSink remains the escape hatch, and the start-failure → QAudioSink fallback (with the consequence-naming log from the #4978 fixes) is unchanged. If you'd rather stage this, drop the windows-installer.yml hunk and the capability stays builder-opt-in.

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 of commit 1 (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 as of commit 2: 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.

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 rendered garbage. The by-ear A/B is what caught it, and the new callbacks=/peak= instrumentation is there so the next person can catch it from a log instead.

Not exercised

• Element-timing measurement (per @williamscody's CW sidetone has uneven element timing within hand-keyed characters after #4809 fix #4890 methodology) on the WASAPI callback path — the natural follow-up now that it is audible and correct; happy to run it on request.
• MME-truncation partial-match path (all live selections here resolved exact or by ID).
• PortAudio start-failure → QAudioSink fallback on Windows.
• The GitHub-runner execution of setup-portaudio.ps1 — validated locally with the identical command sequence; CI on this PR is its first runner execution.

🤖 Generated with Claude Code

[aethersdr-agent]

Issue fit

Yes, for the build-configuration half. #5200's root cause is that pkg_check_modules(portaudio-2.0) is the only detection path, so HAVE_PORTAUDIO is never defined under MSVC and src/core/CwSidetonePortAudioSink.cpp is not in the build at all. The WIN32 branch at CMakeLists.txt:215-231 plus setup-portaudio.ps1 closes that exactly, and it mirrors the FFTW3 (:238-247), Opus (:352-366) and hidapi (:2228-2250) third_party/ + scripts/setup/*.ps1 pattern the issue names as precedent. check-windows is green on 81875ab, which is real evidence the script builds and links on the runner — the parts I could not verify offline (the PA_BUILD_STATIC option name, the portaudio_static_x64.lib output name, the winmm dsound ole32 uuid setupapi list being sufficient) are all settled by that green run, since each would have been a hard failure of the setup step or the link.

Two things the issue asks for that the diff does not deliver: the issue's own "Maintainer decision worth making explicitly" (the installer wiring flips the shipped default) is included rather than deferred, and there is no CI assertion that PortAudio was actually found — see Blockers 1 and 2.

Scope

File	What it changes	Claimed by title/issue?	Verdict
scripts/setup/setup-portaudio.ps1	New: pinned v19.7.0, SHA256-verified, MSVC static build into third_party/portaudio/	Yes — issue "Fix" section names it	In scope
CMakeLists.txt:206-231	if(WIN32) third_party detection branch; pkg-config moved into else()	Yes	In scope
.github/workflows/ci.yml:549-557	Runs the setup script in check-windows	Yes ("CI wiring")	In scope
.github/workflows/windows-installer.yml:81-91	Cache + setup in the release build	Yes ("installer wiring") — but the issue flags it as a maintainer decision, not a settled requirement	Needs maintainer decision

No unrelated files, no deleted guards, no formatting churn, no CHANGELOG.md entry (correct). The pkg-config restructure is a pure move: I checked every other if(PkgConfig_FOUND) site (:247 FFTW3, :366 Opus, :2242 hidapi, :2366 ORT) and all of them already sit inside the else() of a if(WIN32), so they still see the find_package(PkgConfig QUIET) at :228 on the platforms that reach them. Nothing on Windows loses a detection path it previously had.

Blockers

1. The installer wiring silently changes the shipped Windows default, and the path it switches to has never been keyed. (windows-installer.yml:88 — inline)

AudioEngine.cpp:4120 reads AppSettings::value("CwSidetoneBackend", "PortAudio"). Today that default is unsatisfiable on Windows and every install lands on CwSidetoneQAudioSink. The moment the installer job produces a build with HAVE_PORTAUDIO, every existing Windows user with default settings moves to the callback sink on the next update — no setting changed, no prompt. #5200's own "Not yet exercised" list says actual CW keying through this sink on Windows has not been tested (blocked on #5137), and neither has the PortAudio-start-failure → QAudioSink fallback. The issue explicitly offers the alternative: "If preferred, the installer wiring can be dropped from the PR to make this opt-in-for-builders first."

This is a maintainer's call, not a code defect — the ci.yml half alone fully satisfies the PR title. Flagging it because it is the one user-visible consequence in an otherwise pure build-config change, and it should be decided rather than inherited.

2. Nothing asserts PortAudio was actually found, so the coverage this PR buys can evaporate silently. (ci.yml:549, CMakeLists.txt:217 — inline)

The stated purpose is "CI would miss real compile errors in the PortAudio sink." But if(PORTAUDIO_FOUND) at :2429 is soft-optional and the WIN32 branch keys on EXISTS .../include/portaudio.h only. If that header is ever absent — upstream tag moved, a partially-populated third_party/portaudio restored from the installer job's cache, a rename in a future PortAudio bump — CMake skips target_sources(... CwSidetonePortAudioSink.cpp) without a word and the build goes green having compiled nothing new. check-windows passing on this head does not, by itself, prove the sink compiled.

Unlike its three siblings, the branch also has no not-found message(WARNING ...): FFTW3 gets a unified FATAL_ERROR, Opus warns "Run scripts/setup/setup-opus.ps1" (:361), hidapi warns the same (:2238). Add a message() on both legs and the failure becomes visible — and then assertable, using the pattern that already lives 90 lines below the new step in the same job ("Assert GPU spectrum rendering actually enabled", ci.yml:638-647).

Nits

• setup-portaudio.ps1:72-76 — the header claims the build enables "WASAPI, WDM-KS, DirectSound, and MME host APIs", but no -DPA_USE_* flag is passed; that is upstream's Windows default, not a request. If a future bump flips PA_USE_WASAPI off, #3193's whole reason for existing silently no-ops again and nothing in this repo notices. -DPA_USE_WASAPI=ON costs one line and makes the doc-comment true by construction.
• setup-portaudio.ps1:41-44 — Confirm-Sha256 runs only inside the if (-not (Test-Path $TarFile)), so a tarball left over from an interrupted run is used unverified. Inherited verbatim from setup-hidapi.ps1, so not this PR's bug — worth fixing in both someday.
• ci.yml:549 — no actions/cache step, so every check-windows run downloads and rebuilds PortAudio from source, on a required check. FFTW3 and DeepFilterNet3 in the same job are cached; hidapi is not, so this is consistent with the closest sibling. Mentioning it only because a CMake+Ninja PortAudio build is not free.

What I tried to break

• The pkg-config move. My main worry was that find_package(PkgConfig QUIET) moving into the else() would strand a later if(PkgConfig_FOUND) block that relied on it. Grepped all 20 pkg_check_modules sites: the four unguarded-by-their-own-find_package ones (:247, :366, :2242, :2366) are each inside a if(WIN32)…else(), and :150/:578/:1340/:2175/:2192/:2219 do their own find_package(PkgConfig REQUIRED). :839 is inside the elseif(UNIX) limb. No regression.
• Link-library completeness. The list winmm dsound ole32 uuid setupapi matches what PortAudio's own CMake attaches per host API (winmm/MME, dsound/DS, setupapi/WDM-KS, ole32+uuid/WASAPI). I could not clone the v19.7.0 tree to confirm line-by-line (network denied in this environment), but a missing entry would be an unresolved-symbol link failure, and check-windows linked AetherSDR.exe successfully on 81875ab.
• macOS/Linux regression. Both stay on the pkg-config leg untouched, including the third_party/macos-deps PORTAUDIO_LIBRARY_DIRS case the :2433 comment describes. check-macos and build are green.
• Script failure paths. Missing extracted dir → Write-Error + exit 1; missing built .lib → same; $ErrorActionPreference = "Stop". The only silent path is the one in Blocker 2, and it lives in CMake, not the script.
• What I could not check: whether the installer job's actions/cache post-step saves third_party/portaudio when the setup step fails mid-build — the script New-Items include\ and lib\ before downloading, so a cached empty tree would key-hit forever and skip the setup step. Whether that can actually happen depends on cache-save-on-failure semantics I can't test here; a message() on the not-found leg (Blocker 2) makes it self-announcing either way. Every finding above is reasoned from the code and the workflow files, not reproduced at runtime — I have no build here.

Recommendation

Needs maintainer decision, then approve with nits. The engineering is clean and correctly patterned after the three existing Windows third-party deps, and CI backs it. What needs a ruling is whether the release build ships with the CW sidetone default flipped to a callback path that has not yet been keyed on Windows — #5200 offered to split that off and I think that offer is worth taking: land ci.yml now (which buys the compile coverage the title promises), open the installer wiring as its own PR once #5137 lands and keying is measured. Independent of that call, please add the not-found message() and a configure-log assertion so this coverage cannot quietly disappear later.

---

_{🤖 aethersdr-agent · cost: $6.4746 · model: claude-opus-5}

[aethersdr-agent]

Detection keys on the header, but :2439 links ${PORTAUDIO_ROOT}/lib/portaudio_static_x64.lib. A third_party/portaudio/ containing include/ but not lib/ (interrupted setup run, restored-empty cache) sets PORTAUDIO_FOUND and then fails at link; one containing neither reverts to exactly the #5200 bug with a green build.

The three sibling branches check the header too, but they all say something when it is missing — Opus at :361 and hidapi at :2238 both message(WARNING "Run scripts/setup/setup-X.ps1 …"), FFTW3 falls into a unified FATAL_ERROR. This branch is the only silent one.

[aethersdr-agent]

"same soft-optional behaviour as the pkg-config path below" is accurate, but soft-optional is what made #5200 invisible for as long as it was. Suggest making both legs announce themselves so a broken setup shows up in configure.log — which also gives ci.yml something to assert on, the way the GPU-spectrum guard at ci.yml:638-647 does:

Suggested change

	endif() # not-found leaves PORTAUDIO_FOUND unset — same soft-optional
	else()
	message(WARNING "PortAudio not found. Run scripts/setup/setup-portaudio.ps1 to build it. "
	"The CW sidetone will fall back to the push-model QAudioSink (#5200).")
	endif()

(and a matching message(STATUS "PortAudio: using third_party static build") inside the found leg).

[aethersdr-agent]

The comment's premise — "CI would miss real compile errors in the PortAudio sink" — is right, but running the setup script does not by itself guarantee the sink is in the build. CMakeLists.txt:2429 is soft-optional, so if third_party/portaudio/include/portaudio.h is ever absent, configure skips CwSidetonePortAudioSink.cpp silently and this job stays green having added zero coverage. The green run on 81875ab is consistent with the sink compiling and with it being skipped.

This job already has the fix pattern 90 lines down ("Assert GPU spectrum rendering actually enabled"): pair the message(STATUS) suggested on CMakeLists.txt with a Select-String -Path configure.log -Pattern 'PortAudio: using third_party' -Quiet step, and the coverage becomes load-bearing instead of assumed.

[aethersdr-agent]

This is the maintainer decision #5200 asked to be made explicitly, and I'd rather it be decided than inherited.

AudioEngine.cpp:4120 is AppSettings::value("CwSidetoneBackend", "PortAudio"). That default is currently unsatisfiable on Windows, so every shipped install runs CwSidetoneQAudioSink. Once this job produces a build with HAVE_PORTAUDIO, every default-config Windows user switches to the callback sink on their next update — while #5200's own "Not yet exercised" list still has actual CW keying through the Windows PortAudio sink (blocked on #5137) and PortAudio start-failure → QAudioSink fallback on Windows open.

The ci.yml step alone delivers everything the PR title claims. Dropping these two steps to a follow-up PR — as #5200 offers — gets the compile coverage now and lets the default flip land behind a measurement.

[aethersdr-agent]

The .DESCRIPTION above promises "WASAPI, WDM-KS, DirectSound, and MME host APIs", but nothing here requests them — that is upstream's Windows default, silently depended on. Since the entire point of this PR is to make #3193's WASAPI preference exist in a shipped build, a default flip in a future PortAudio bump would un-fix it with no signal. Worth stating explicitly:

Suggested change

	-DPA_BUILD_STATIC=ON `
	-DPA_BUILD_SHARED=OFF `
	-DPA_BUILD_STATIC=ON `
	-DPA_USE_WASAPI=ON `
	-DPA_BUILD_EXAMPLES=OFF `
	-DPA_BUILD_TESTS=OFF `

[aethersdr-agent]

Minor, and inherited verbatim from setup-hidapi.ps1: Confirm-Sha256 sits inside the if (-not (Test-Path $TarFile)), so a tarball left behind by an interrupted earlier run is consumed without verification. Not this PR's defect — noting it since both scripts would want the same one-line move of the check outside the guard.

[nigelfenton]

Element-timing measurement (the promised #4890 follow-up)

Ran on the same rig as the by-ear verification (Windows 11, FLEX-6300 on a dummy load, keyboard iambic paddle held ~12 s per run, 30 wpm, 600 Hz). Instrument: the new env-gated edge probe (AETHER_CW_EDGE_PROBE=1, commit d18c64f) — envelope transitions captured at the sink boundary with a running sample counter, so durations and onsets are measured on the stream's own clock, not re-recorded audio.

	PortAudio WASAPI callback (this PR)	QAudioSink push, 2 ms timer (stock Windows path)
dit duration mean	43.51 ms	43.45 ms
dit duration SD	0.96 ms	0.26 ms
dit range	32.9 – 51.3 ms	43.10 – 43.90 ms
cycle (R→R) mean	80.002 ms	80.016 ms
outlier elements	2 of 196 (−10.7 ms / +7.7 ms, isolated, clean neighbors)	0 of 211
buffered stream depth to DAC	~22 ms reported	19200-byte buffer ≈ 50 ms @ 48 kHz float stereo

(Both means sit ~3.5 ms over the nominal 40 ms dit because the 0.02 amplitude threshold catches the attack/decay ramps — systematic, cancels in the comparison. Neither sink shows a quantization grid: durations are sample-continuous on both paths.)

Reading the numbers honestly

• fix(cw): shift the sidetone anchor forward for late key edges instead of clamping (#4890) #4934 did its job on the push path. In-stream rhythm on the stock Windows sink is essentially perfect (SD 0.26 ms, zero outliers). The pre-fix(cw): shift the sidetone anchor forward for late key edges instead of clamping (#4890) #4934 5.3/10.7 ms grids reported in CW sidetone falls back to the QAudioSink push path on Linux when the PortAudio device match fails #4978 are gone.
• The callback path's win is latency, not rhythm. The probe measures the stream's own timeline and cannot see wall-clock key-to-ear delay — which is precisely where the paths differ: the push sink's perfect rhythm sits inside ~50 ms of buffer; the WASAPI callback plays within ~22 ms. The latency is what a CW operator feels at the key, and it is what External USB Audio Interface and Windows 11, latency is a huge issue #3193 was chasing.
• The callback path has a small jitter tail: 194/196 elements as tight as the push sink, but two elements landed ±8–11 ms off, most likely WASAPI wake jitter or an output underflow. The callback currently ignores statusFlags; counting paOutputUnderflow there is a cheap follow-up that would turn this from a guess into a number.

Net: rhythm equal-or-slightly-noisier (2/196 tail), delivery ~28 ms sooner. For QSK CW the latency matters more than two displaced elements per ~16 seconds, but that trade — and the underflow-counter follow-up — is the maintainer's call to weigh.

🤖 Generated with Claude Code

[nigelfenton]

All three review items addressed in d48d67d:

Blocker 1 (installer wiring = silent default flip): agreed it should be decided, not inherited — the windows-installer.yml hunk is dropped from this PR. Shipped Windows builds keep the push-model QAudioSink until the maintainer flips the default deliberately; I'll offer that as a one-hunk follow-up PR whenever wanted. This PR is now CI coverage + builder opt-in only.

One factual update to the blocker's premise, though: "the path it switches to has never been keyed" was true of the issue text but is stale for the PR — since the verification section was rewritten, the callback sink has been keyed extensively on real hardware (operator hand-keying + synthesized key events, FLEX-6300 on a dummy load, clean by ear), and the follow-up comment below adds sample-exact element timing for both sinks (194/196 elements as tight as the push path, ~28 ms less delivery latency). What remains unexercised is the PortAudio-start-failure → QAudioSink fallback — which alone still justifies deferring the shipped default.

Blocker 2 (coverage can evaporate silently): the WIN32 branch now message()s on both legs — a grep-stable PortAudio (third_party) found — CW sidetone callback sink enabled STATUS, and a not-found WARNING naming the setup script and the consequence — and check-windows gains an "Assert PortAudio sidetone sink actually enabled" step mirroring the GPU-spectrum guard exactly: missing detection now fails the job instead of going green having compiled nothing.

Nit (WASAPI by upstream default): setup-portaudio.ps1 now passes -DPA_USE_WASAPI=ON explicitly, and its doc-comment states which host APIs are requested versus inherited — #3193's reason to exist is no longer droppable by an upstream default change.

🤖 Generated with Claude Code

[ten9876]

Issue fit

#5200 is a good catch: PortAudio detection ran only through pkg-config, so HAVE_PORTAUDIO was never defined on MSVC, CwSidetonePortAudioSink.cpp has never been compiled in a shipped Windows build, and #3193's WASAPI host-API preference — added specifically for Windows CW jitter — has therefore never been active anywhere. The diff fixes the build-configuration half and then, in commit 2, fixes the three defects that only surfaced once the sink actually ran on something other than CoreAudio. Finding those by live A/B against a FLEX-6300 and iambic paddles is the part of this that could not have been done any other way.

Verified empirically, with a large caveat: the full AetherSDR target configures and builds clean at this head on macOS/arm64, so the else() branch is intact and the non-Windows path is not disturbed. That is the only thing my platform can tell you. This machine has no PortAudio on its pkg-config path, so CwSidetonePortAudioSink.cpp was not compiled here either — every statement below about the sink, the probe and the setup script is read, not run, and none of the Windows behaviour is reachable from here at all.

No blockers. One finding and two nits.

The CI coverage assert is the right pattern

- name: Assert PortAudio sidetone sink actually enabled
  ...  if (-not (Select-String -Path configure.log -Pattern 'PortAudio \(third_party\) found' -Quiet)) { ... exit 1 }

with the CMake side deliberately emitting that exact line and a comment saying CI greps for it. This is precisely the failure mode I flagged on #4862 today — a soft-optional dependency that goes missing leaves the job green having compiled nothing new. Naming the line, asserting it, and saying in both files that the other end depends on it is better than either half alone.

Scope

Everything is explained by #5200. No CHANGELOG.md entry — correct. CwSidetoneQAudioSink.{h,cpp} (+5) is touched only to route its buffers through the same probe, which is the point of a shared diagnostic. scripts/setup/setup-portaudio.ps1 follows the established pinned-version + _verify_sha256.ps1 shape of the other setup scripts.

Finding — not blocking

m_edgeProbe.dump() runs while the callback is still live. (inline: CwSidetonePortAudioSink.cpp:435) stop() dumps the probe three lines before Pa_StopStream(m_stream), so dump() reads m_edges/m_count and then resets m_count, m_samplePos, m_tone and m_quietRun while paCallback may still be calling scan() and writing those same members. Only reachable with AETHER_CW_EDGE_PROBE=1, so it is a bench-only race — but it is a race, and the very next comment in the function shows the barrier is already understood: "Halt the callback before clearing the generator pointer so we don't race with paCallback dereferencing a torn-down generator." The probe wants to be on the same side of Pa_StopStream that the generator pointer is.

Nits

• The probe costs 128 KB whether or not it is enabled. std::array<Edge, 8192> is a by-value member, so both sinks carry it in every build and every run; m_enabled only gates the work, not the storage. A std::unique_ptr<std::array<...>> allocated in the constructor when enabled would make the disabled case free, and the constructor is not real-time.
• The callback-count and peak instrumentation is unconditional while the edge probe beside it is env-gated. m_cbCount.fetch_add plus a full frameCount * 2 peak scan and a CAS loop run in every callback of every build. It is cheap and real-time safe — no allocation, no lock — so this is a consistency note rather than a performance one: two diagnostics added by the same commit, one gated and one not.

Verified vs. read

Built: the full app target on macOS at this head — clean, confirming the WIN32 branch does not disturb the else() pkg-config path. Read: the probe's real-time safety (constructor-time qEnvironmentVariable, single bool test per buffer, bounded record() with no allocation or locking, qint64 sample positions so no overflow, dump() correctly outside the callback), the CMake gating, and the CI assert. Not verified at all: anything Windows — the sink is not compiled on this machine, so the three defects commit 2 fixes, the endpoint-by-ID selection, and the sidetone's actual behaviour rest entirely on your bench session. For a PR whose whole subject is "this code has never been compiled on the platform it matters on", that is the right shape of evidence and I have no way to add to it.

[ten9876]

Finding, non-blocking — the probe dump is on the wrong side of the stream barrier.

dump() does more than read. Its tail resets the probe:

        m_count = 0;
        m_samplePos = 0;
        m_tone = false;
        m_quietRun = 0;

and at this point Pa_StopStream(m_stream) has not been called yet, so paCallback can still be running m_edgeProbe.scan(dst, frameCount) on another thread — writing m_tone, m_quietRun, m_samplePos and possibly m_edges[m_count++] while dump() iterates and then zeroes them. Worst case is a torn dump or an edge recorded past the reset; nothing that corrupts audio, but the numbers this exists to produce are exactly the ones that would be wrong.

The comment immediately below already states the rule for the neighbouring member:

Halt the callback before clearing the generator pointer so we don't race with paCallback dereferencing a torn-down generator.

The probe wants the same treatment — it is the same thread, the same barrier, and the same reasoning:

Suggested change

	m_edgeProbe.dump("PortAudio", m_actualRate);
	// Halt the callback before clearing the generator pointer so we
	// don't race with paCallback dereferencing a torn-down generator.
	Pa_StopStream(m_stream);
	void CwSidetonePortAudioSink::stop()
	{
	if (m_stream) {
	qCInfo(lcAudio) << "CwSidetonePortAudioSink: stopping —"
	<< "callbacks=" << m_cbCount.load(std::memory_order_relaxed)
	<< "peak=" << (m_cbPeakMicro.load(std::memory_order_relaxed) / 1e6);
	// Halt the callback before clearing the generator pointer so we
	// don't race with paCallback dereferencing a torn-down generator.
	// Same barrier for the edge probe: dump() resets the counters that
	// scan() is still writing until the stream is actually stopped.
	Pa_StopStream(m_stream);
	m_edgeProbe.dump("PortAudio", m_actualRate);
	m_generator.store(nullptr, std::memory_order_release);
	Pa_CloseStream(m_stream);

Only reachable under AETHER_CW_EDGE_PROBE=1, so this is bench-only and not a shipping defect — worth fixing anyway, since a diagnostic that races the thing it measures is the one class of bug that wastes the most time later.

[ten9876]

Nit, non-blocking — the storage is unconditional even though the work is gated.

std::array<Edge, 8192> with Edge{qint64, bool} is 128 KB after padding, held by value, in both sidetone sinks and in every build regardless of the environment variable. m_enabled short-circuits scan() and dump(), so the CPU cost is the single bool test the header comment promises — but the memory is resident either way.

The constructor is not on the audio path, so this can be free when disabled:

    CwSidetoneEdgeProbe()
        : m_enabled(qEnvironmentVariable("AETHER_CW_EDGE_PROBE") == QLatin1String("1"))
    {
        if (m_enabled)
            m_edges = std::make_unique<std::array<Edge, kMaxEdges>>();
    }

record() then guards on the pointer it already has to check m_count against.

Entirely optional — 128 KB is not a lot, and a fixed array is easier to reason about in code that neighbours a real-time callback. Raising it mainly because the header is otherwise scrupulous about the disabled case costing nothing, and this is the one place where it does cost something.

Related, from the review body: m_cbCount / m_cbPeakMicro in paCallback are not env-gated, so a full frameCount * 2 peak scan plus a CAS loop runs in every callback of every build. Both are real-time safe and cheap, so no objection — just noting that the same commit added two diagnostics to the same callback and gated only one of them.

[nigelfenton]

Thanks — the race is real and it is fixed in 6e0d8c47.

dump() before the barrier. You are right, and the annoying part is that the comment two lines below already states the principle it violated. dump() now runs after Pa_StopStream(), and the comment explains that the probe belongs on that side of the barrier for the same reason the generator pointer does.

The 128 KB nit. Fixed and measured — std::array by value becomes a std::unique_ptr allocated in the constructor only when the probe is armed:

sizeof(CwSidetoneEdgeProbe): ~131,120 -> 40 bytes

That is ~128 KB per sink, two sinks, every build. scan()/record() still never allocate; the one allocation happens at construction, which is not real-time.

Since that changed the probe's storage I did not trust the green build. A standalone harness drove three 600 Hz bursts through scan() both ways:

armed:    EDGES RECORDED = 6   (3 rising + 3 falling)   PASS
disabled: EDGES RECORDED = 0                            PASS

Both survive 28,800 frames with no crash, and reuse-after-dump works. I added a small edgeCount() accessor so that assertion does not have to reach into private state.

The unconditional callback-count/peak instrumentation I have left as-is, and I am flagging that so you can push back rather than letting it pass silently. You called it a consistency note rather than a performance one, and it is real-time safe — no allocation, no lock. Gating it would remove the callbacks= and peak= numbers from the stop() log line, which is what made the Windows sidetone behaviour diagnosable in the first place. Happy to gate it behind the same env var if you would rather the two diagnostics matched.

[nigelfenton]

Rebased onto main (56edc21b) — this branch now sits on top of @skerker's #5123 fix (623cd691).

One file conflicted: src/core/CwSidetonePortAudioSink.cpp, two hunks. ci.yml did not, despite four upstream commits touching it.

The first hunk was mechanical — 623cd691 moved normalizedDeviceName() into CwSidetoneDeviceMatch.h and changed the signature; this branch had inserted wasapiEndpointId() at that spot.

The second was semantic. 623cd691 returns on the first exact match; a2378cf8 had deliberately removed that early return, because on Windows the same endpoint enumerates under several host APIs with an identical name, DirectSound sorts before WASAPI, and returning early hands the sidetone to DirectSound (#5200). Taking either side wholesale would have silently reverted the other. The resolution keeps both, since they answer different questions: classifyDeviceNameMatch() decides whether a row matches (your rule, unchanged), the exact/partial collection decides which host API wins.

Verified on Windows with HAVE_PORTAUDIO defined, so the file genuinely compiled rather than being skipped: cw_sidetone_device_match_test gives 13 checks, 0 failures, including both #5123 HDMI rows.

Still open, and I'll fix it here: your PR body names #5201 as the follow-up for the multi-partial WASAPI branch that returns a substituted device without setting partialMatchName. The rebase absorbed your work but did not close that — the wasapiCandidates.size() == 1 return is still a partial match reported as clean. I have the Windows box, so I'll take it in this PR.

[nigelfenton]

Reporting hole closed in 197183bf.

The multi-partial WASAPI branch returned a device picked by host-API preference among several partial name matches but never set partialMatchName — and since the caller keys m_fallbackOccurred / m_fallbackReason off that value, the substitution was invisible in the CW sidetone summary and the support bundle. Host-API preference decides which partial row to open, not whether the name matched, so it is still a substitution.

Reaching the branch. It is unreachable through a real QAudioDevice on this box: the endpoint-ID fast path (a2378cf8) matches first, so a hardware run passes without ever entering it. I exercised it by compiling the selection function with only that fast path excised, then requesting "Realtek Digital Output (Realte" — a strict prefix that equals no row and matches three (MME, DirectSound, WASAPI), exactly one of them WASAPI, which is the branch's size() == 1 guard.

A/B over that harness, same input, differing only in this commit's two lines:

	selected	partialMatchName	caller reports
before	idx 24, WASAPI	(empty)	clean match
after	idx 24, WASAPI	"Realtek Digital Output (Realtek(R) Audio)"	fallbackOccurred + reason

Same device either way — only the reporting changes. The control run is what confirms the hole was real rather than theoretical.

A TOSHIBA-TV prefix cannot reach the branch here, incidentally: this box exposes three identically-named TOSHIBA-TV WASAPI rows, so wasapiCandidates.size() == 1 fails and it falls through to the multi-match refusal. Same non-unique-name shape that motivated the endpoint-ID match in the first place.

The exact-match returns are deliberately untouched — an exact match is the operator's device, so flagging it would be a false substitution warning.

Local suites after the change: cw_sidetone_device_match_test 13/13, cw_sidetone_test pass, cw_sidetone_start_policy_test 9/9. Full app build clean.

Not verified: the harness proves the selection function's return contract, not the end-to-end summary text — I have not observed the support bundle rendering this substitution on a live sidetone session with a genuinely non-unique partial match, because the ID path pre-empts it on this hardware.

[nigelfenton]

Interaction with #5128 / #5129, and a caveat this puts on my timing table

Checked these against each other after @skerker raised it. No merge conflict — zero file overlap between this PR and either of those, so they can land in any order. But there is a one-directional semantic interaction worth recording, because it changes what my element-timing numbers above mean.

This PR is the transport: which sink carries the sidetone to the DAC. #5128 and #5129 are the source: when edges reach CwSidetoneGenerator in the first place. Both sinks call CwSidetoneGenerator::process(), so anything that changes edge timing upstream changes what the sink is handed.

#5128 removes the GUI echo on the local iambic path — the emit cwKeyDownChanged(down) in RadioModel, whose removal comment states the echo was "re-timing the following element to the GUI thread's rhythm — or, when the queued hop lands after the element ended, re-keying the gate for a spurious blip (#4976)."

Verified against origin/main: the emit sites are live (RadioModel.cpp:4378, :4425) and MainWindow.cpp:1434 connects cwKeyDownChanged to m_audio->setCwKeyDown. So the echo was present in the base my measurement ran on — and the local iambic path is exactly the one I keyed from.

The caveat

The callback path has a small jitter tail: 194/196 elements as tight as the push sink, but two elements landed ±8–11 ms off, most likely WASAPI wake jitter or an output underflow.

There was a third candidate upstream that I could not have seen: the echo raising the generator's monotonic floor to wake time. Both arms of my A/B carried it equally, so the comparison stands — same source path either side. But the absolute figures (SD 0.96 ms, 2 outliers in 196) were measured with that echo present, and may move once #5128 lands. Anyone reading the table as a fixed characterisation of the callback path should know that.

Consequences

1. It strengthens the case for counting paOutputUnderflow. With three plausible causes for the tail rather than two, the flag the callback currently ignores is the only thing that separates them. Doing it now, before fix(cw): stop echoing iambic keyer edges back into the sidetone gate (#4976) #5128 changes the baseline, means the next measurement can attribute the tail rather than guess at it. I am adding that to this PR.

2. @skerker — worth deciding before you run the HaliKey comparison. If those four runs are on a branch without fix(cw): stop echoing iambic keyer edges back into the sidetone gate (#4976) #5128, they inherit the same echo and are directly comparable to my numbers; if they include it, they are cleaner but not comparable. Either is useful, but it should be a choice rather than an accident. Your call as the author of both.

3. Once fix(cw): stop echoing iambic keyer edges back into the sidetone gate (#4976) #5128 and feat(cw): carry the CWX keyer's scheduled edge instants to the sidetone (#4977) #5129 land, the element-timing measurement is worth re-running on this branch — the interesting question then is whether the tail survives at all.

[skerker]

macOS regression bench — 34480859 vs 56edc21b

The sink changes here are shared code, so I ran this on macOS — Intel i9-9980HK, Radeon Pro 560X, macOS 15.7.9, Qt 6.8.3 — where the PortAudio callback sink already ships. Control arm is 56edc21b — origin/main's head and this PR's merge-base — so the two builds differ by exactly this PR's commits.

No regression measurable on this rig. @nigelfenton

Twelve keyed runs, six per arm, identical protocol: separate dit / dah / K runs of ~20 s at 30 and 20 WPM. FLEX-8400 into a dummy load, HaliKey MIDI interface with iambic paddle, stock builds, built-in logging only. Element boundaries come from CW iambic key-edge (ms-granular), which exists at both SHAs — that instrument carries the comparison.

Class	Control 56edc21b	PR 34480859
30 WPM dit	40.03 ms, SD 2.37, n=261	40.03 ms, SD 2.05, n=255
30 WPM dah	120.14 ms, SD 2.25, n=132	120.17 ms, SD 2.62, n=126
20 WPM dit	60.20 ms, SD 2.25, n=153	60.17 ms, SD 2.23, n=167
20 WPM dah	179.86 ms, SD 2.23, n=83	180.51 ms, SD 2.07, n=87

Largest cross-arm difference in any class: 0.65 ms. Effective speed 29.98 / 19.93 (control) vs 29.98 / 19.94 (PR). schedMs — the keyer's absolute grid — is flat at SD 0.00 in all twelve runs on both arms, taking only its two theoretical values per speed. The ~2 ms wall-clock SD is the instrument's own floor: it does not scale with element length, and the grid it measures has zero spread.

Selection resolves identically on both arms:

CwSidetonePortAudioSink: matched selected Qt output "MacBook Pro Speakers" to PortAudio output MacBook Pro Speakers
started device= MacBook Pro Speakers hostApi= Core Audio rate= 48000 Hz outputLatency= 10.3958 ms

An explicit AudioOutputDeviceId was set deliberately — left empty, the start policy takes the BackendDefault path and findPortAudioOutputDevice() is never called at all.

ctest: control 315/318, PR 316/318. Both real failures (hl2_state_restore_test, phone_tx_filter_numeric_entry_test) reproduce identically on 56edc21b. vkamp_connection_test is flaky on both arms and unreachable from this diff. Sidetone suites, read off the binaries directly: cw_sidetone_start_policy_test 9/9, cw_sidetone_device_match_test 13 checks / 0 failures, cw_sidetone_test pass — identical on both arms.

From the new instrumentation

underflows= 0 overflows= 0 across 357,891 callbacks in six runs.

The edge probe reproduces your documented artifact: element means run long by a constant +3.42 to +3.62 ms across both speeds and both element classes. It also drags the probe's dah:dit ratio to ~2.83 — (120+3.5)/(40+3.5) — where the keyer's grid ratio is 3.000.

Sink-side, CoreAudio callback path:

Run	Class	n	mean (ms)	SD
30 WPM (dits)	dit	255	43.457	1.923
30 WPM (dahs)	dah	126	123.536	0.426
30 WPM (Ks)	dit / dah	35 / 70	43.621 / 123.497	0.465 / 0.269
20 WPM (dits)	dit	167	63.535	0.532
20 WPM (dahs)	dah	87	183.573	0.461
20 WPM (Ks)	dit / dah	31 / 62	63.420 / 183.540	0.257 / 0.532

Your Windows dit run was 43.51 ms mean / cycle 80.002; ours is 43.46 / 80.030. One run each on different OS, machine and operator — the SD difference between them is not settled by this data.

Source and sink agree element-for-element. In every run the CW iambic key-edge count equals the probe edge count exactly — 510/510, 252/252, 210/210, 334/334, 174/174, 186/186. Across 833 elements nothing was dropped, duplicated or invented between the keyer and the rendered buffer. That took both instruments; neither shows it alone.

Three displaced edges in 833 elements (0.36%). Two were single edges that self-corrected inside one cycle (space 66.104 → element 13.771, sum 79.875 ms against an 80.030 ms cycle). underflows=0 rules out a device deadline miss, and schedMs was flat for those elements, so the displacement sits between schedule and render. Cause not established, and not attributable to this PR — the probe does not exist at 56edc21b, so there is no baseline to compare against.

Scope

Covers macOS only: device selection, element timing, stream health, full suites. Does not cover the Windows half of this PR (none of it compiles here), the PortAudio start-failure → QAudioSink fallback, or the QAudioSink push path on macOS.

Both arms sit on a base carrying the cwKeyDownChanged GUI echo, as your Windows numbers do — absolute figures here are comparable to yours and provisional against #5128.

Full evidence attached: both whole session logs, per-run extracts and the analysis scripts, with a README and per-arm run index. Sanitized — callsign, radio handles, chassis serial, MAC, addresses and local username redacted; radio model, software version, device names and SHAs retained.

aethersdr-pr5201-mac-sidetone-regression-2026-08-27-public.zip

— authored by agent (Claude Code) on behalf of @skerker

[nigelfenton]

Thank you — this is the arm I could not run, on the platform where the sink already ships, and it is the regression risk that actually mattered for this PR. Twelve keyed runs into a dummy load is a lot of paddle work.

What your data settles that mine could not

The +3.5 ms element offset is the probe, not either of our rigs. You measured a constant +3.42 to +3.62 ms across both speeds and both element classes; I saw the same artifact on Windows. Two operating systems, two machines, two operators, same constant — that moves it from "an oddity on nigelfenton's box" to a property of the instrument.

A lead on where it comes from, offered as a lead rather than a finding: CwSidetoneGenerator shapes each element with a raised-cosine envelope, m_shapingMs defaulting to 5.0 ms (m_rampLength{240} at 48 kHz). An element that ramps up and down necessarily occupies more wall clock than its keyed length, and the probe sees rendered samples. The arithmetic does not land cleanly — one full ramp is 5.00 ms against your 3.5 — so this is not the answer yet, but it is where I would look first, and it predicts the offset should track shapingMs if anyone varies it. Worth noting the ratio distortion you flagged (~2.83 against a grid 3.000) falls straight out of a constant addend, which is consistent with a fixed envelope cost rather than anything speed-dependent.

Source and sink agree element-for-element. 833 elements, key-edge count equal to probe edge count in every run — nothing dropped, duplicated or invented between keyer and rendered buffer. You are right that neither instrument shows this alone, and it is the most useful single result here: it means the two halves of this PR's instrumentation corroborate rather than merely coexist.

underflows=0 overflows=0 across 357,891 callbacks. Those counters landed in 34480859 after your question about what the timing tail was hiding, so this is the first real data they have produced — and a clean zero on CoreAudio is the useful control against the Windows WASAPI numbers.

On the three displaced edges

Your handling of these is the part I would have got wrong. 0.36%, two self-correcting inside one cycle, underflows=0 ruling out a device deadline miss, schedMs flat — and then not attributing them to this PR because the probe does not exist at 56edc21b and there is no baseline. That is the correct call and I would not want it softened: an instrument that only exists on one arm cannot indict that arm.

If it is worth chasing later, the displacement sitting between schedule and render points at the same seam #5129 is about.

Scope, acknowledged as you stated it

Nothing here covers the Windows half, the PortAudio-failure → QAudioSink fallback, or the QAudioSink push path on macOS — and both arms carry the cwKeyDownChanged echo, so the absolute figures stay provisional against #5128 (still open, as is #5129). I will not claim this PR is verified beyond what you actually measured.

What it does establish, and what I could not: the shared sink changes cause no measurable timing regression on macOS, on the platform that had the most to lose from them.

[skerker]

Windows bench — 34480859 vs 56edc21b on a third machine

@nigelfenton — run on Windows with the HaliKey MIDI interface with paddle attached.

Both arms are plain: #5128 is out, so these numbers carry the cwKeyDownChanged echo exactly as yours and the macOS ones do, and are directly comparable to both. That was the choice you asked me to make deliberately rather than by accident.

What this measures: QAudioSink push — what Windows ships today — against PortAudio pull, what this PR compiles. That makes it an independent replication of your measurement on different hardware, and not a regression A/B like the macOS run, where both arms are the same PortAudio sink.

Control is 56edc21b, this PR's merge-base (upstream/main has since moved to 3096621e, which does not touch this path). The two builds differ by exactly this PR's commits.

Machine: Intel Core i9-14900HX (24C/32T), 31.7 GB, Windows 11 Pro 10.0.26200 x64, RTX 5060 Laptop, MSVC 14.44.35207, CMake 3.31.6, Qt 6.8.3. Radio FLEX-8400 into a dummy load; HaliKey MIDI interface with paddle attached; local iambic keyer (localIambic=true); 600 Hz sidetone; stock builds, built-in logging only.

Which sink each arm actually started

Control:

CwSidetoneQAudioSink: started rate= 48000 Hz format= Float buffer= 19200 bytes (push, 2ms timer)
backend="QAudioSink" device="Speakers (Realtek(R) Audio)" rate=48000Hz path=push  fallback=no

PR:

CwSidetonePortAudioSink: selected Qt output "Speakers (Realtek(R) Audio)" matched WASAPI endpoint by ID "{0.0.0.00000000}.{...}"
CwSidetonePortAudioSink: matched selected Qt output "Speakers (Realtek(R) Audio)" to PortAudio output Speakers (Realtek(R) Audio)
CwSidetonePortAudioSink: started device= Speakers (Realtek(R) Audio) hostApi= Windows WASAPI rate= 48000 Hz outputLatency= 22 ms
backend="PortAudio" device="Speakers (Realtek(R) Audio)" rate=48000Hz path=pull  fallback=no

hostApi= Windows WASAPI, outputLatency= 22 ms. An explicit AudioOutputDeviceId was set deliberately — left empty, the start policy takes the BackendDefault path and findPortAudioOutputDevice() is never called, so the ID match above would not occur.

Evidence that each arm actually ran the transport it claims

Three independent layers, so the claim does not rest on a single startup line.

1 — Binary level. The control build cannot run the PortAudio pull sink at all. Scanning each AetherSDR.exe for the strings its two sinks emit:

	control 56edc21b	PR 34480859
CwSidetonePortAudioSink	absent	present
Windows WASAPI	absent	present
CwSidetoneQAudioSink	present	present
push, 2ms timer	present	present

Consistent with the build logs: CwSidetonePortAudioSink.cpp.obj is compiled 0 times on control and 1 time on the PR arm. The PR binary carries both sinks because the QAudioSink path remains the fallback.

2 — Session-log level, zero cross-contamination. In the control arm's whole 31,026-line session log, occurrences of portaudio, case-insensitive: 0. In the PR arm's session log, occurrences of CwSidetoneQAudioSink: started: 0.

3 — Per-run level. One uninterrupted PortAudio stream spanned all six PR-arm runs:

21:51:52.639  CwSidetonePortAudioSink: started device= Speakers (Realtek(R) Audio) hostApi= Windows WASAPI rate= 48000 Hz outputLatency= 22 ms
      ... the six keyed runs, 21:53:44 -> 21:59:09 ...
21:59:24.681  CwSidetonePortAudioSink: stopping — callbacks= 169523 peak= 0.565685 underflows= 0 overflows= 0

The control arm's sidetone sink over its own runs — every start is the push path:

21:02:44.568  CwSidetoneQAudioSink: started rate= 48000 Hz format= Float buffer= 19200 bytes (push, 2ms timer)
21:03:15.436  CwSidetoneQAudioSink: started rate= 48000 Hz format= Float buffer= 19200 bytes (push, 2ms timer)
21:32:39.394  CwSidetoneQAudioSink: started rate= 48000 Hz format= Float buffer= 19200 bytes (push, 2ms timer)

Element timing — source side

Twelve runs, six per arm, identical protocol: separate dit / dah / K runs of ~20 s at 30 and 20 WPM. Element boundaries from CW iambic key-edge (ms-granular), which exists at both SHAs — that instrument carries the comparison. Run boundaries were derived from the data (inter-edge gaps > 1.5 s), not from operator-called marks.

Run	Class	Control 56edc21b (push)	PR 34480859 (pull)
30 WPM (dits)	dit	40.018 ms, SD 0.853, n=339	39.962 ms, SD 0.809, n=266
30 WPM (dahs)	dah	119.971 ms, SD 0.886, n=136	120.000 ms, SD 0.757, n=130
30 WPM (Ks)	dit	39.825 ms, SD 0.874, n=40	40.122 ms, SD 0.714, n=41
30 WPM (Ks)	dah	120.440 ms, SD 0.962, n=75	120.355 ms, SD 0.706, n=76
20 WPM (dits)	dit	60.037 ms, SD 0.735, n=219	59.994 ms, SD 0.804, n=176
20 WPM (dahs)	dah	180.087 ms, SD 0.674, n=92	180.128 ms, SD 0.610, n=86
20 WPM (Ks)	dit	60.032 ms, SD 0.836, n=31	60.000 ms, SD 0.835, n=44
20 WPM (Ks)	dah	180.431 ms, SD 0.840, n=58	180.400 ms, SD 0.739, n=80

Largest cross-arm difference in any class: 0.297 ms; every other class under 0.1 ms. Effective speed from the dit runs: 30.00 / 20.00 WPM on both arms. schedMs — the keyer's absolute grid — is flat at SD 0.000 in all twelve runs on both arms, taking only its theoretical values. dah:dit mean ratio on the PR arm's K runs: 3.000 and 3.007.

Element timing — sink side (PR arm only)

The probe does not exist at 56edc21b, so there is no control-arm sink measurement from this box.

Run	Class	n	mean (ms)	SD
30 WPM (dits)	dit	266	43.721	2.062
30 WPM (dahs)	dah	130	123.513	0.207
30 WPM (Ks)	dit / dah	41 / 76	43.935 / 123.605	1.487 / 0.567
20 WPM (dits)	dit	176	63.413	0.262
20 WPM (dahs)	dah	86	183.478	1.722
20 WPM (Ks)	dit / dah	44 / 80	63.684 / 184.026	1.097 / 1.754

Means run long by a constant +3.35 to +3.76 ms, reproducing the threshold artifact you documented (macOS saw +3.42 to +3.62).

Your 30 WPM dit run next to this one, both on the pull path:

	yours (FLEX-6300)	this box (i9-14900HX)
dit mean	43.51 ms	43.721 ms
dit SD	0.96 ms	2.062 ms
dit SD excluding elements beyond 3SD	—	0.509 ms
dit range	32.9 – 51.3 ms	32.79 – 54.40 ms
cycle R→R mean	80.002 ms	80.040 ms
elements beyond 3SD	2 of 196	10 of 266

At 20 WPM the same run type gives SD 0.262 ms, range 63.10–63.65 ms, 0 of 176 beyond 3SD, and cycle R→R SD 0.030 ms.

Stream health, and the underflow counter

CwSidetonePortAudioSink: stopping — callbacks= 169523 peak= 0.565685 underflows= 0 overflows= 0

Zero underflows and zero overflows across 169,523 callbacks covering all six PR-arm runs, with the displaced elements above present in the same stream. PortAudio was opened with framesPerBuffer = 128; measured delivery was 128.0 frames per buffer (2.667 ms), derived from 169,523 callbacks over 452.04 s of stream time. The displacements cluster at ±10.3–10.8 ms, and four of them are the identical value 53.90 ms.

Source and sink agree element-for-element

Probe edges segmented independently by sample position; log edges segmented by wall-clock gap. Per run: 266/266, 130/130, 117/117, 176/176, 86/86, 124/124 — 1,818 key edges, 1,818 rendered edges, nothing dropped, duplicated or invented between keyer and rendered buffer.

Build-side items from the "not exercised" list

• scripts/setup/setup-portaudio.ps1 ran on a real Windows box — exit 0 in 5.6 s, its own SHA256 check verified the v19.7.0 archive, 23 objects, host APIs compiled = WASAPI, WDM-KS, DirectSound, MME (ASIOSDK not found — no SDK on this box).
• The CI grep string appears verbatim: PortAudio (third_party) found — CW sidetone callback sink enabled.
• The sink TU compiles only on the PR arm — CwSidetonePortAudioSink.cpp.obj: 0 occurrences on control, 1 on PR (2890 vs 2891 build steps). At the merge-base Windows does not compile the sidetone sink at all.
• Both arms clean-built from scratch, separate worktrees and build dirs: 0 errors, 221 warnings each — no new MSVC warning from this PR.

Suites

ctest: control 311/317, PR 310/317. Six failures are identical on both arms and therefore pre-existing on this base. The seventh, vkamp_connection_test, was run in isolation 5×: 5/5 fail on both arms — the control arm's pass inside the full run was the fluke. Sidetone suites read off the binaries directly: cw_sidetone_start_policy_test 9/9, cw_sidetone_device_match_test 13 checks / 0 failures, cw_sidetone_test pass — identical on both arms.

Scope

Covers Windows on this one machine: which sink starts, device selection, element timing on both sides, stream health, full suites. Does not cover the PortAudio-start-failure → QAudioSink fallback (the third arm was not built), ASIO (no SDK here), or any other Windows audio stack. Both arms carry the cwKeyDownChanged echo, so absolute figures are provisional against #5128. Sink-side push-vs-pull cannot be compared from this box, since the probe does not compile at the merge-base.

Build SHAs verified in Help→About on both arms; the PR arm's About screenshot is in the bundle. Full evidence attached: both whole session logs, per-run extracts, raw probe edges, stream health, and the analysis scripts, with a README and per-arm run index. Sanitized — callsign, radio handles, client GUID, chassis serial, MAC, addresses and local username redacted; radio model, software version, device names and SHAs retained.

aethersdr-pr5201-windows-sidetone-2026-08-27-public.zip

— authored by agent (Claude Code) on behalf of @skerker

[skerker]

Linux leg — device resolution, 34480859 vs 56edc21b

@nigelfenton

Completing the third platform: Linux is where the 81 lines this PR compiles outside #ifdef Q_OS_WIN (the findPortAudioOutputDevice() restructure) could regress, because Qt's device names come from PulseAudio/PipeWire while PortAudio enumerates ALSA (#4978). Both arms compile the PortAudio sink here, so unlike the Windows leg this is a same-transport regression check, like macOS. No keying — this leg only measures which device each arm resolves at startup. Ubuntu 24.04.4 (x86_64, i9-14900HX), PipeWire 1.0.5, system PortAudio 19, Qt 6.8.3; both arms clean-built, configure-log SHA-verified.

Four launches — each arm with no saved output device, and each with an explicit saved selection of the machine's only real output:

Saved device	control 56edc21b	PR 34480859
unset	PortAudio pull, device= default, 48 kHz, fallback=no	PortAudio pull, device= default, 48 kHz, fallback=no
explicit	no name match → QAudioSink push 24 kHz, fallback=yes	no name match → QAudioSink push 24 kHz, fallback=yes

No regression: the arms agree in both configurations. The explicit-case fallback is the pre-existing #4978 behavior, byte-for-byte at the merge-base — the selected Qt/Pulse name ("Built-in Audio Analog Stereo") can never appear in PortAudio's all-ALSA candidate list ("HDA Intel PCH…", "pulse", "default"), so the sink falls back on both arms. Not introduced by this PR. The new hostApi= ALSA in the started line and the stream-health counters showed up as expected on the PR arm.

ctest: 315/317 on both arms with identical failure sets (hl2_state_restore_test, phone_tx_filter_numeric_entry_test), each failing deterministically when re-run alone on each arm — pre-existing at the merge-base, unrelated to this PR.

Evidence: full bundle attached — README with the matrix and per-launch criterion→evidence map, whole unedited session logs for all four launches (plus the live device-switch capture), configure logs proving each arm's baked SHA, and both full ctest logs. Home paths masked; sha256 063c412b19e373a7857fe886ece7de7e70e2285f15e31fdddd744e0788afdb8e.

aethersdr-pr5201-linux-device-resolution-2026-08-28.zip

— authored by agent (Claude Code) on behalf of @skerker

[nigelfenton]

@skerker — thank you. That is three platforms, twelve-plus keyed runs into a dummy load, and two of them on hardware I do not have. Between them these two comments close three of the four items on this PR's "Not exercised" list, which I had expected to have to argue about rather than have answered.

What the Windows leg settles

It is an independent replication, not a re-run of mine. Different machine, different operator, different radio (FLEX-8400 against my 6300), and the dit means land at 43.721 ms against my 43.51 ms with cycle R→R at 80.040 ms against my 80.002 ms. That is the measurement I reported, reproduced by someone else.

The three-layer transport proof is the part I would not have thought to build. A startup line saying hostApi= Windows WASAPI is the sort of evidence I have been burned by before — this PR exists because a sink that compiled and started was still inaudible, so "it said it started" is exactly the claim I distrust most. Scanning both binaries for the strings each sink emits (CwSidetonePortAudioSink and Windows WASAPI absent from the control), then zero portaudio matches across 31,026 control log lines and zero CwSidetoneQAudioSink: started on the PR arm, settles it at a level a log line cannot. The control build cannot run the pull sink; the object-count difference (0 vs 1 compilations of CwSidetonePortAudioSink.cpp.obj) says why.

underflows=0 overflows=0 across 169,523 callbacks, with the displaced elements present in the same stream, is the counter I added at your prompting doing the job it was added for — ruling out a device deadline miss as the explanation for the tail. And framesPerBuffer measuring 128.0 delivered against 128 requested is a detail I would not have checked.

1,818 key edges against 1,818 rendered edges, six runs, nothing dropped or invented. Together with the macOS 833, the two halves of this instrumentation now corroborate each other on two platforms.

The +3.5 ms offset is now a three-platform constant

You measured +3.35 to +3.76 ms; macOS saw +3.42 to +3.62; I saw the same on Windows. Three machines, three operating systems, both element classes, both speeds. That is no longer an artifact of anyone's box.

My raised-cosine lead still does not close the arithmetic (one full 5.0 ms ramp against a ~3.5 ms addend), so it stays a lead. What your data adds is that the offset is constant across the WASAPI pull path too, which is the strongest argument yet that it is the probe's segmentation and not the audio path — and it keeps the falsifiable prediction: it should track shapingMs if anyone varies it.

The Linux leg, and why the null result is the useful one

This is the leg I most needed and least expected, because it is the only one where the 81 lines outside #ifdef Q_OS_WIN could regress and neither of us had run it.

The arms agree in both configurations, so there is no regression. What I want to underline is the explicit-selection row, because it is the one that could have been misread as one: the fallback to QAudioSink push at 24 kHz is ugly, but you established it is byte-for-byte the merge-base behaviour and traced why — a Qt/Pulse name ("Built-in Audio Analog Stereo") can never appear in PortAudio's all-ALSA candidate list. That is #4978, pre-existing, and not something this PR introduced or worsened. Reporting a bad-looking result together with the proof it is not yours is the harder half.

On vkamp_connection_test — independent corroboration

Your correction that the control arm's pass inside the full run was the fluke, not the failures, is right, and I can add a third data point from outside this PR entirely.

I hit the same failure today on an unrelated branch of mine (a GUI label fix touching neither audio nor sockets) and ran it in isolation on this box: 5/5 fail. Same as your 5/5 on both arms. So it fails on three machines, on at least three unrelated branches, and it is not a #5201 artifact on anyone's hardware. On my box it is a socket test and my AV is the likely local cause; whatever the mechanism, it is not this PR.

Base drift — checked, and your control still holds

main has moved two commits since your 56edc21b control (now 3096621e). Neither touches the sidetone or PortAudio path: the GHE applet's only CMakeLists.txt change is three added source files. So both benches remain valid against current main and there is no need to re-run either.

What remains not exercised

Being precise, since you were:

• PortAudio start-failure → QAudioSink fallback on Windows — still unbuilt on any arm, by all three of us.
• ASIO — no SDK on your box or mine.
• Sink-side push-vs-pull on Windows — not comparable, since the probe does not compile at the merge-base.
• Both arms carry the cwKeyDownChanged echo, so absolute figures stay provisional against fix(cw): stop echoing iambic keyer edges back into the sidetone gate (#4976) #5128, and feat(cw): carry the CWX keyer's scheduled edge instants to the sidetone (#4977) #5129 is still open.

What is now established across macOS, Windows and Linux: the shared sink changes cause no measurable timing regression, and on Windows the sink this PR compiles resolves the right endpoint by ID and renders every element the keyer produces.

@jeremymturner — this one has three-platform bench evidence and four green checks whenever you have a moment.

[nigelfenton]

Rebase check against v26.9.1, and a connection to #5135 that isn't on this thread yet

Checked whether the sidetone work that shipped in v26.9.1 disturbs this PR. It does not — and one of those changes explicitly hands its Windows half to this PR.

No conflict, and the reason is ancestry rather than luck. #5135 (623cd691, merged 27 Aug) touches CwSidetonePortAudioSink.cpp, the same file as this PR, so it looked like a collision. It isn't: git merge-base --is-ancestor 623cd691 56edc21b passes, so #5135 was already in this branch's base when I opened it. git merge-tree against today's origin/main reports the only files changed on both sides as .github/workflows/ci.yml and CMakeLists.txt, with no conflict hunks. GitHub agrees the branch is mergeable.

Worth noting for anyone else grepping for it: the squashed commit's subject cites the issue (#5123), not the PR number, so git log --grep='#5135' finds nothing on main.

The part that matters for review. @skerker's commit message split a piece of work out and named this PR as where it lands:

The multi-partial WASAPI-preference branch (#ifdef Q_OS_WIN, :105-112) returns a substituted device without setting partialMatchName — the same reporting hole on a different branch of the function (found in review by @ten9876, measured by @NF0T). It is split into a follow-up: the file does not compile in any stock Windows build today (HAVE_PORTAUDIO is undefined there), and #5201 restructures that arbitration on Windows (endpoint-ID match, exact-collection with the WASAPI preference) on the box that can run it.

That hole is still open on main today. findPortAudioOutputDevice() returns wasapiCandidates[0].idx from inside the #ifdef Q_OS_WIN block without touching partialMatchName, so on the Linux side an operator's substituted device is named in the sidetone summary and the support bundle, and on the Windows side the same substitution is reported only as one qCInfo line.

This PR closes it. partialMatchName is set on the WASAPI branch (CwSidetonePortAudioSink.cpp:195-196) alongside the lone-partial branch (:171-172), and start() folds both into m_fallbackOccurred / m_fallbackReason (:334-343). So the two branches of the function now report substitutions the same way, which was the asymmetry #5135 deliberately left behind.

I'm flagging this rather than assuming it's known, because it changes what this PR is: not a standalone Windows build fix, but the follow-up half of a fix that shipped in v26.9.1 with its Windows arm deferred to the machine that can run it.

One thing I'd rather raise myself than have found. This PR adds 25 lines to .github/workflows/ci.yml. That is Tier 2 (@aethersdr/infrastructure), not the Tier 3 roster I've just been added to — so this needs an infrastructure approver alongside a source one, and being on @aethersdr/reviewers doesn't make me eligible to approve either half of my own PR.

#5135's message calls .github/workflows/ Tier-2 infrastructure and says it deliberately avoided touching ci.yml for that reason. Mine adds the PortAudio setup step so HAVE_PORTAUDIO is actually defined in the Windows CI job — without it check-windows compiles the same nothing it compiles today and the gate proves nothing about this code. That's the argument for it being here rather than in a follow-up, but it's a Tier 2 call and not mine to make. If the preference is to land the source changes now and take the CI wiring separately, say so and I'll split it.

Status. No blocking findings outstanding. @skerker benched macOS, Windows and Linux on hardware I don't have (three platforms, twelve-plus keyed runs into a dummy load), which closed three of the four items on this PR's "Not exercised" list. CI green on 81875ab.

One caveat on those benches, since it's now true and wasn't when they were run: #5128 (iambic keyer edges echoing into the sidetone gate) shipped in v26.9.1. @skerker deliberately kept that echo in both arms so the two SHAs stayed comparable, which was the right call then. It does mean a re-bench today would be against a different baseline — it doesn't invalidate the A/B, since both arms carried it equally, but anyone re-running these numbers should know the ground moved.

Ancestry checked with --is-ancestor rather than read off the changelog; a squash-merged change is not an ancestor just because its number appears in release notes.

[jeremymturner]

Can we ensure that Claude tags the right Jeremy?Thank you!jeremymturnerOn Aug 28, 2026, at 12:49, Nigel Fenton ***@***.***> wrote:nigelfenton left a comment (aethersdr/AetherSDR#5201) @skerker — thank you. That is three platforms, twelve-plus keyed runs into a dummy load, and two of them on hardware I do not have. Between them these two comments close three of the four items on this PR's "Not exercised" list, which I had expected to have to argue about rather than have answered. What the Windows leg settles It is an independent replication, not a re-run of mine. Different machine, different operator, different radio (FLEX-8400 against my 6300), and the dit means land at 43.721 ms against my 43.51 ms with cycle R→R at 80.040 ms against my 80.002 ms. That is the measurement I reported, reproduced by someone else. The three-layer transport proof is the part I would not have thought to build. A startup line saying hostApi= Windows WASAPI is the sort of evidence I have been burned by before — this PR exists because a sink that compiled and started was still inaudible, so "it said it started" is exactly the claim I distrust most. Scanning both binaries for the strings each sink emits (CwSidetonePortAudioSink and Windows WASAPI absent from the control), then zero portaudio matches across 31,026 control log lines and zero CwSidetoneQAudioSink: started on the PR arm, settles it at a level a log line cannot. The control build cannot run the pull sink; the object-count difference (0 vs 1 compilations of CwSidetonePortAudioSink.cpp.obj) says why. underflows=0 overflows=0 across 169,523 callbacks, with the displaced elements present in the same stream, is the counter I added at your prompting doing the job it was added for — ruling out a device deadline miss as the explanation for the tail. And framesPerBuffer measuring 128.0 delivered against 128 requested is a detail I would not have checked. 1,818 key edges against 1,818 rendered edges, six runs, nothing dropped or invented. Together with the macOS 833, the two halves of this instrumentation now corroborate each other on two platforms. The +3.5 ms offset is now a three-platform constant You measured +3.35 to +3.76 ms; macOS saw +3.42 to +3.62; I saw the same on Windows. Three machines, three operating systems, both element classes, both speeds. That is no longer an artifact of anyone's box. My raised-cosine lead still does not close the arithmetic (one full 5.0 ms ramp against a ~3.5 ms addend), so it stays a lead. What your data adds is that the offset is constant across the WASAPI pull path too, which is the strongest argument yet that it is the probe's segmentation and not the audio path — and it keeps the falsifiable prediction: it should track shapingMs if anyone varies it. The Linux leg, and why the null result is the useful one This is the leg I most needed and least expected, because it is the only one where the 81 lines outside #ifdef Q_OS_WIN could regress and neither of us had run it. The arms agree in both configurations, so there is no regression. What I want to underline is the explicit-selection row, because it is the one that could have been misread as one: the fallback to QAudioSink push at 24 kHz is ugly, but you established it is byte-for-byte the merge-base behaviour and traced why — a Qt/Pulse name ("Built-in Audio Analog Stereo") can never appear in PortAudio's all-ALSA candidate list. That is #4978, pre-existing, and not something this PR introduced or worsened. Reporting a bad-looking result together with the proof it is not yours is the harder half. On vkamp_connection_test — independent corroboration Your correction that the control arm's pass inside the full run was the fluke, not the failures, is right, and I can add a third data point from outside this PR entirely. I hit the same failure today on an unrelated branch of mine (a GUI label fix touching neither audio nor sockets) and ran it in isolation on this box: 5/5 fail. Same as your 5/5 on both arms. So it fails on three machines, on at least three unrelated branches, and it is not a #5201 artifact on anyone's hardware. On my box it is a socket test and my AV is the likely local cause; whatever the mechanism, it is not this PR. Base drift — checked, and your control still holds main has moved two commits since your 56edc21 control (now 3096621). Neither touches the sidetone or PortAudio path: the GHE applet's only CMakeLists.txt change is three added source files. So both benches remain valid against current main and there is no need to re-run either. What remains not exercised Being precise, since you were: PortAudio start-failure → QAudioSink fallback on Windows — still unbuilt on any arm, by all three of us. ASIO — no SDK on your box or mine. Sink-side push-vs-pull on Windows — not comparable, since the probe does not compile at the merge-base. Both arms carry the cwKeyDownChanged echo, so absolute figures stay provisional against #5128, and #5129 is still open. What is now established across macOS, Windows and Linux: the shared sink changes cause no measurable timing regression, and on Windows the sink this PR compiles resolves the right endpoint by ID and renders every element the keyer produces. @jeremymturner — this one has three-platform bench evidence and four green checks whenever you have a moment. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>