From b0fdd4e05889e8c18ff3cc40d9955feca72851ce Mon Sep 17 00:00:00 2001 From: Mollusk Date: Fri, 19 Jun 2026 16:54:26 -0400 Subject: [PATCH] audio(win): filter choose_config to drivable formats (Codex B3/B5 re-review P3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/audio/cpal_impl.rs | 45 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/src/audio/cpal_impl.rs b/src/audio/cpal_impl.rs index f73c030..4ae5b7d 100644 --- a/src/audio/cpal_impl.rs +++ b/src/audio/cpal_impl.rs @@ -516,6 +516,26 @@ fn find_device_by_name(host: &cpal::Host, output: bool, name: &str) -> Option Err(...)` arm), and a zero-channel range would later be +/// rejected by [`resolve`]'s geometry check. Filtering both here keeps +/// [`choose_config`] from *ranking* a range it can't drive ahead of a usable one and +/// then hard-failing the start instead of trying the next candidate (Codex B5 +/// re-review, P3). +fn usable_range(r: &cpal::SupportedStreamConfigRange) -> bool { + r.channels() > 0 && format_supported(r.sample_format()) +} + +/// Sample formats the capture/playback stream builders accept. Pure, so it's +/// unit-testable independently of the cpal range types. +fn format_supported(fmt: SampleFormat) -> bool { + matches!( + fmt, + SampleFormat::F32 | SampleFormat::I16 | SampleFormat::U16 + ) +} + /// Pick a sample rate inside both a device's supported `[r_min, r_max]` span and the /// backend's drivable `[MIN_DEVICE_RATE, MAX_DEVICE_RATE]` window, preferring /// [`SAMPLE_RATE`] when it's reachable and otherwise the nearest in-window bound. @@ -558,7 +578,7 @@ fn choose_config(device: &Device, output: bool) -> Result| { ranges .iter() - .find(|r| supports_48k(r) && channels.is_none_or(|c| r.channels() == c)) + .find(|r| usable_range(r) && supports_48k(r) && channels.is_none_or(|c| r.channels() == c)) .cloned() }; @@ -581,7 +601,7 @@ fn choose_config(device: &Device, output: bool) -> Result| -> Option<(cpal::SupportedStreamConfigRange, u32)> { ranges .iter() - .filter(|r| channels.is_none_or(|c| r.channels() == c)) + .filter(|r| usable_range(r) && channels.is_none_or(|c| r.channels() == c)) .filter_map(|r| { bounded_rate(r.min_sample_rate().0, r.max_sample_rate().0) .map(|rate| (r.clone(), rate)) @@ -1363,6 +1383,27 @@ mod tests { ); } + #[test] + fn format_supported_matches_the_stream_builders() { + // Exactly the three the build_input/build_output match arms accept. + for f in [SampleFormat::F32, SampleFormat::I16, SampleFormat::U16] { + assert!(format_supported(f), "{f:?} should be drivable"); + } + // Everything else cpal can expose must be filtered out before ranking, or a + // start could pick it and then hit the `unsupported sample format` arm (P3). + for f in [ + SampleFormat::I8, + SampleFormat::U8, + SampleFormat::I32, + SampleFormat::U32, + SampleFormat::I64, + SampleFormat::U64, + SampleFormat::F64, + ] { + assert!(!format_supported(f), "{f:?} must not be reported drivable"); + } + } + #[test] fn ensure_idle_allows_an_idle_slot() { let mut s = SlotState::Idle;