audio(win): filter choose_config to drivable formats (Codex B3/B5 re-review P3)
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>
This commit is contained in:
+43
-2
@@ -516,6 +516,26 @@ fn find_device_by_name(host: &cpal::Host, output: bool, name: &str) -> Option<De
|
||||
.find(|d| d.name().is_ok_and(|n| n == name))
|
||||
}
|
||||
|
||||
/// Whether the backend can actually open this config range. The workers only build
|
||||
/// `F32`/`I16`/`U16` streams ([`build_input`]/[`build_output`] — every other sample
|
||||
/// format hits the `other => 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<cpal::SupportedStreamC
|
||||
let pick = |channels: Option<u16>| {
|
||||
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<cpal::SupportedStreamC
|
||||
let pick_bounded = |channels: Option<u16>| -> 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;
|
||||
|
||||
Reference in New Issue
Block a user