Codex's xhigh re-review of 306bc29 confirmed B3 sound and bounded_rate
correct (no P1/P2), and caught one real P3: choose_config ranked supported
config ranges by sample rate + channel count only, but the stream builders
accept just F32/I16/U16 — cpal can also expose U8/I8/I32/U32/I64/U64/F64.
An unsupported-format range (or a zero-channel range) could therefore out-
rank a usable one, win selection, and then hard-fail in setup()'s
`other => Err(unsupported sample format)` arm without trying another
candidate. This was latent in the exact-48 kHz path too, not only B5's
bounded case 3.
Fix: a pure `format_supported` predicate + `usable_range` (nonzero channels
AND a drivable format), applied as a filter in BOTH the exact-48 kHz `pick`
and the bounded `pick_bounded`, so an undrivable range is never ranked. A
zero-channel range can no longer be logged as "using bounded …" and then
rejected by resolve. +1 unit test enumerating every cpal SampleFormat.
Verified: windows-gnu cargo check --release --lib --tests --bins clean, no
warnings; Linux paths untouched (cfg(windows)).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closes the two Windows-only follow-ups Codex deferred in the RT-audit
re-review (review-2026-06-19-cpal-rt-audit.md). Both are cfg(windows),
so they carry zero risk to the shared Linux audio path.
B3 — orphan-thread tombstone on a wedged start. On the FINISH_START_TIMEOUT
path the owner thread is detached (not joined) so start_*/stop can't hang;
previously the slot was left empty, so a retry against a permanently wedged
device spawned ANOTHER orphan worker holding its own COM/device handle, and
so on without bound. The slot is now a SlotState { Idle | Live | Wedged }:
- Each worker carries an `exited: Arc<AtomicBool>` flipped true by an
ExitGuard at the top of the thread body — fires on normal return, panic
unwind, or whenever the wedged driver call finally releases the thread.
- A timed-out start detaches its thread and leaves a `Wedged { exited }`
tombstone instead of an empty slot.
- `ensure_idle` (pure, unit-tested) rejects new starts while the orphan is
still alive, but clears the tombstone once `exited` flips, so the slot
becomes reusable after the device recovers. `stop` restores a still-live
tombstone rather than silently clearing it.
B5 — choose_config picks a bounded supported rate before the device default.
A device whose default rate is outside the drivable 8k–384k window but which
also exposes a usable in-window config was previously rejected by resolve().
New case 3 scans the supported config ranges for one overlapping the window
and drives it at a `bounded_rate` (48 kHz when reachable, else the nearest
in-window bound), preferring the native layout; the device default is now a
last resort. `bounded_rate` is pure and unit-tested.
6 new unit tests (bounded_rate x4, ensure_idle x2) — they're in the
cfg(windows) module, so they compile/run under the windows-gnu target, not
the Linux lib suite.
Verified: Linux cargo test --lib 326/0 + clippy --lib --tests clean (shared
paths untouched); windows-gnu cargo check --release --lib --tests --bins
clean, no warnings.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex's xhigh re-review of the prior cpal RT fixes confirmed W2/W3/W7/W4-diag
addressed (and validated the reserve-first ring-publish ordering), but found the
W1/W6 start-handshake fixes were partial. This closes the holes:
- B1 (P1): wait_for_stream_start checked the liveness flag before the error code,
so a callback that ran then failed in the same WASAPI cycle could still report
Ok on a dead stream. Readiness now (a) treats the error as terminal — checked
first each loop AND re-checked before returning Ok — and (b) requires
MIN_START_CALLBACKS (2) completed callbacks, not one, so a fire-once-then-die
stream is caught by the error/timeout path. The liveness signal is now a
callback counter (AtomicUsize) instead of a one-shot bool.
- B2 (P2): on the inner STREAM_START_TIMEOUT the owner sent Err and THEN dropped
the stream; since cpal Stream::drop joins its (wedged) WASAPI worker and
finish_start joins the owner on that Err, start_*/stop could still hang past the
backstop. The owner now drops the stream BEFORE reporting Err, so a wedged drop
withholds the Err and lets finish_start's timeout branch detach.
- B4 (P3): the two timeouts didn't compose — a slow-but-valid setup plus a slow
first callback could exceed the 6s backstop and be falsely failed. Raised
FINISH_START_TIMEOUT to 10s (setup budget + callback wait + cleanup slack) and
corrected the comment.
Deferred (logged in review-2026-06-19-cpal-rt-audit.md): B3 (orphan-thread
tombstone accounting on a permanent >10s driver wedge — rare, non-crashing, needs
a slot-state redesign) and B5 (choose_config picking a bounded supported rate for
an oddball sub-8k/over-384k default-rate device — rare; the safety validation
already prevents the panic/spin).
Verified: Linux cargo test --lib 326/0, clippy --all-targets clean; windows-gnu
cargo check --lib --tests --bins clean; windows-gnu release peerspeak.exe links.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses Codex's xhigh RT-audio audit of the new Windows cpal path (review
2026-06-19; all Windows-only, no Linux-path change):
- W1 (P1): start_capture/start_playback reported Ok as soon as cpal's play()
returned, but cpal's WASAPI play() only QUEUES IAudioClient::Start(); a later
Start failure left the UI joined-but-silent. Readiness is now driven by the
stream actually proving itself: the first RT data callback sets a started
flag (or the error callback sets an error code), and the owner thread waits
(bounded by STREAM_START_TIMEOUT) before reporting Ok.
- W2: both RT error callbacks ran format!+log_msg on the time-critical stream
thread. They now store a category in an AtomicU8 only; the owner / health
logger translate + log off the RT path.
- W3: the playback ring was published one interleaved sample at a time, letting
the RT consumer read a half-written L/R pair and letting a raced fetch_sub
wrap ring_fill to usize::MAX (wedging mixer pacing). Now reserves occupancy
before publishing and writes the whole frame with a single push_slice.
- W6: finish_start did an unbounded recv() while holding the slot mutex, so a
wedged driver hung start_* and any concurrent stop. Now recv_timeout with a
FINISH_START_TIMEOUT backstop; on timeout it signals + detaches (never joins).
- W7: OS-reported device geometry is validated in resolve() (channels>0, rate in
8k-384k) so 0 channels can't panic chunks_exact(0) and a 0/absurd rate can't
make an infinite/huge resample ratio. resample.rs constructors also clamp
rates >=1 (release-safe; +2 tests) instead of a debug-only assert.
- W4 (diagnostic half): the playout-health logger compared raw device samples
against the internal-stereo prefill target. The callback now records demand in
internal 48 kHz-stereo units (internal_demand) so the comparison is correct
for remapped/non-48k devices. The dynamic-target restructure stays deferred.
Deferred (logged in review-2026-06-19-cpal-rt-audit.md): W5 (bounded mixer->
worker channel) touches the shared Linux audio path and wants its own design +
regression pass; the W2 dynamic-target sizing needs a real WASAPI callback.
Verified: Linux cargo test --lib 326/0, clippy --all-targets clean; windows-gnu
cargo check --lib --tests --bins clean; windows-gnu release peerspeak.exe builds.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The GUI re-sends the saved network mode as part of its startup config-sync.
The SetNetworkMode handler unconditionally tore down + rebuilt the iroh
endpoint whenever idle, so every launch rebuilt the freshly-built stack for
an identical posture — a needless ~1s teardown+rebuild bounce visible in the
logs on both Linux and Windows/Wine (the 'start core loop -> shut down network
stack ~1s later' pattern from the Wine spike). Guard the rebuild on an actual
mode change; a real change still rebuilds exactly as before.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- src/audio/resample.rs: pure linear PushResampler (capture) +
StereoPullResampler (playback pull), 6 unit tests green on Linux.
- choose_config: prefer native 48kHz, else fall back to device default
config and convert at the boundary instead of hard-erroring.
- run_capture: resample device-rate mono -> 48kHz on the drain thread.
- i16<->f32 helpers. Playback build_output remap still TODO (Codex).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two safe, host-independent hardening steps from the Codex Windows-compat review.
W7 — when a saved input/output device name no longer resolves (WASAPI friendly
names can change across driver/endpoint changes), resolve() now logs the
fallback to the system default instead of switching devices silently — so a
"my audio went to the wrong device" report has a log line explaining why.
(cpal 0.15 exposes only the device name, so a stable hardware id isn't available
to persist; this surfaces the limitation rather than hiding it.)
W2 — the output RT callback now records the largest interleaved buffer length it
is ever asked for (a wait-free fetch_max into an atomic, kept off the log/alloc
path). The once-per-second health-logger reports that size and, if a callback
ever exceeds the prefill target (PLAYBACK_TARGET_SAMPLES), warns explicitly —
that's the exact signature of the WASAPI-shared-mode underrun-every-cycle bug.
This is the diagnostic a real-host test needs before committing to the
structural fix (larger target / fixed buffer); no behavior change.
Windows-only file (cfg(windows)); compile-verified via the windows-gnu
cross-build, not yet exercised on a real WASAPI host.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implemented by Codex (gpt-5.5); reviewed and committed by Claude.
W8 — chimes were played by shelling out to pw-play/paplay/aplay, which don't
exist on Windows, so every chime silently no-op'd there. spawn_player is now
cfg-split: Linux/unix keeps the existing player list; Windows plays the WAV via
PowerShell's System.Media.SoundPlayer (PlaySync on the existing detached thread).
Dependency-free, same fire-and-forget / silent-on-failure contract. Custom chime
paths are single-quote-escaped for the PowerShell command (helper + unit test).
Also adds docs/WINDOWS.md: a build/run/status guide (native MSVC + cross-compile
to -gnu, first-run firewall/UDP note, %APPDATA% paths, and the honest known-gaps
table — echo-cancel/screenshare/resampling/device-id/buffer-pacing).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three Windows-compatibility fixes from the Codex review. Implemented by Codex
(gpt-5.5); reviewed and committed by Claude.
W5 — echo cancellation is a Linux/PipeWire feature, but the toggle was shown and
live on Windows, so a Windows join tried `pactl` and errored before falling back.
Now `#[cfg(target_os = "linux")]` gates the core enable path (and the
ActiveSession guard field); on other targets the Settings + in-call controls
render as a disabled checkbox with a "not available on Windows yet" note.
W6 — pixelpass PATH lookup only tried `pixelpass`; on Windows it now also tries
`pixelpass.exe` via a cfg-selected candidate list (+ unit test).
W9 — the Linux audio stack (pipewire/pw_cli/echo_cancel/audio_probe + the
`PlatformAudioBackend` alias and device-enum re-export) was gated `cfg(unix)`;
tightened to `cfg(target_os = "linux")` so a hypothetical macOS build won't try
to compile PipeWire. cpal stays `cfg(windows)`. Genuinely-Unix file/key
permission code in lib.rs/identity.rs left as `cfg(unix)`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two correctness fixes for the cpal/WASAPI backend from the Codex Windows-compat
review, plus device logging.
W1 — start_capture/start_playback no longer return Ok before the stream exists.
The owning thread did device resolution, config selection, build_stream, and
play() and only *logged* failures, so a missing 48 kHz config / unsupported
format / WASAPI error left the UI in a joined-but-silent room. The worker now
reports readiness over a channel and start_* blocks on it via finish_start(),
returning the real AudioError on failure (and joining the dead worker).
W3 — the RT capture callback no longer allocates or sends on a channel. It now
only downmixes and wait-free-pushes mono samples into a preallocated lock-free
HeapRb; the owning thread drains that ring, frames it (the Vec allocation lives
off the RT path), and sends completed frames. A full ring increments an overrun
counter instead of blocking. Restores the no-alloc/no-block-in-callback contract
the PipeWire backend already honors.
Also logs the selected device name / sample format / channels / rate on stream
start (a review nice-to-have) and logs capture overruns when they occur.
Windows-only file (cfg(windows)); Linux build unaffected. Compile-verified via
the windows-gnu cross-build; not yet run on a real WASAPI host.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
iced's `window::settings::PlatformSpecific::application_id` only exists on
Linux (X11/Wayland use it to match the .desktop launcher icon); on Windows
the struct exposes a different field set, so the unconditional assignment
failed to compile for `*-pc-windows-*`. This was the first real Windows
compile blocker surfaced now that the port actually cross-compiles.
Move the field behind a `platform_specific_settings()` helper gated on
`target_os = "linux"`, with a defaults-only variant elsewhere. Linux build
unchanged (verified `cargo check`); the windows-gnu target now builds a
runnable .exe (verified launching under Wine: GUI renders, iroh network
stack + ring identity init, config/identity land in %APPDATA%).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Give the Windows device pickers a real device list (Phase 0/1 left pw_cli
returning nothing off-Linux) and generalize enumeration into a
platform-neutral interface.
- audio/mod.rs: move AudioDevice here (neutral home), gate pw_cli to
cfg(unix), and re-export enumerate_audio_devices per-platform (pw_cli on
unix, cpal_impl on windows). Also drop a now-stale "no-op stub" doc note.
- cpal_impl.rs: add enumerate_audio_devices() — iterate the cpal host's
input + output devices into AudioDevice (name == description == the cpal
friendly name, which is what resolve() matches target_node against, so a
saved selection round-trips), sorted by description.
- pw_cli.rs: use super::AudioDevice instead of a local copy; parsing +
tests unchanged.
- app/mod.rs: one-line import change; the device-picker logic is untouched.
Verified: shipped Linux state green (build --locked, clippy, 316/316,
pw_cli parse tests 6/6); the cpal enumerator compiles against real cpal via
the Linux/ALSA toggle. Runtime device listing on Windows is pending a real
host (M2/M3). WASAPI names are less stable than PipeWire node names, so a
saved device may not always round-trip (falls back to default).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
molluskandClaude Opus 4.8