audio(win): fix RT-safety + start-handshake bugs in the cpal backend
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>
This commit is contained in:
+39
-6
@@ -45,11 +45,14 @@ pub struct PushResampler {
|
||||
}
|
||||
|
||||
impl PushResampler {
|
||||
/// Build a resampler from `in_rate` to `out_rate` (both in Hz, must be > 0).
|
||||
/// Build a resampler from `in_rate` to `out_rate` (both in Hz). Rates are
|
||||
/// clamped to `>= 1` so `step` is always finite and non-zero: a zero `step`
|
||||
/// would make [`push`](Self::push)'s `while self.next < 1.0` loop forever. The
|
||||
/// cpal backend's `resolve()` also rejects such rates up front, so this is
|
||||
/// belt-and-suspenders against a future caller (review W7).
|
||||
pub fn new(in_rate: u32, out_rate: u32) -> Self {
|
||||
debug_assert!(in_rate > 0 && out_rate > 0);
|
||||
Self {
|
||||
step: in_rate as f64 / out_rate as f64,
|
||||
step: in_rate.max(1) as f64 / out_rate.max(1) as f64,
|
||||
next: 0.0,
|
||||
prev: 0.0,
|
||||
started: false,
|
||||
@@ -108,11 +111,12 @@ pub struct StereoPullResampler {
|
||||
}
|
||||
|
||||
impl StereoPullResampler {
|
||||
/// Build a resampler from `in_rate` to `out_rate` (both in Hz, must be > 0).
|
||||
/// Build a resampler from `in_rate` to `out_rate` (both in Hz). Rates are
|
||||
/// clamped to `>= 1` so `step` is finite and non-zero — otherwise
|
||||
/// [`next`](Self::next)'s `while self.frac >= 1.0` could spin (review W7).
|
||||
pub fn new(in_rate: u32, out_rate: u32) -> Self {
|
||||
debug_assert!(in_rate > 0 && out_rate > 0);
|
||||
Self {
|
||||
step: in_rate as f64 / out_rate as f64,
|
||||
step: in_rate.max(1) as f64 / out_rate.max(1) as f64,
|
||||
frac: 0.0,
|
||||
prev: (0.0, 0.0),
|
||||
cur: (0.0, 0.0),
|
||||
@@ -271,4 +275,33 @@ mod tests {
|
||||
// At step 2.0 we consume ~2 input frames per output frame.
|
||||
assert!(idx > emitted, "consumed {idx} input, emitted {emitted} output");
|
||||
}
|
||||
|
||||
/// A zero rate must not produce a zero `step` (which would spin `push`'s inner
|
||||
/// `while self.next < 1.0` forever). Clamping makes the call terminate (W7).
|
||||
#[test]
|
||||
fn push_zero_rate_does_not_spin() {
|
||||
let mut r = PushResampler::new(0, 48_000);
|
||||
let mut count = 0usize;
|
||||
// Feed two samples; with a clamped non-zero step this returns promptly.
|
||||
r.push(0.0, |_| count += 1);
|
||||
r.push(1.0, |_| count += 1);
|
||||
// Reaching here at all is the assertion (no hang); some output is produced.
|
||||
assert!(count >= 1);
|
||||
}
|
||||
|
||||
/// A zero output rate must not make the pull resampler's segment-advance loop
|
||||
/// spin. Clamping keeps `step` finite so `next` terminates (W7).
|
||||
#[test]
|
||||
fn pull_zero_out_rate_does_not_spin() {
|
||||
let mut r = StereoPullResampler::new(48_000, 0);
|
||||
let frames = [(0.0, 0.0), (1.0, 1.0), (2.0, 2.0)];
|
||||
let mut idx = 0;
|
||||
let got = r.next(|| {
|
||||
let v = frames.get(idx).copied();
|
||||
idx += 1;
|
||||
v
|
||||
});
|
||||
// Terminates and yields the primed frame instead of hanging.
|
||||
assert!(got.is_some());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user