From 454c3225a857e3707aa4c801454e366663442d94 Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Wed, 20 May 2026 23:26:02 +0200 Subject: [PATCH 1/5] fix(alsa): prevent double opens with BufferSize::Fixed --- src/host/alsa/mod.rs | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index a05eb2d8d..22018fa25 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -361,31 +361,6 @@ impl Device { sample_format: SampleFormat, stream_type: alsa::Direction, ) -> Result { - // Validate buffer size if Fixed is specified. This is necessary because - // `set_period_size_near()` with `ValueOr::Nearest` will accept ANY value and return the - // "nearest" supported value, which could be wildly different (e.g., requesting 4096 frames - // might return 512 frames if that's "nearest"). - if let BufferSize::Fixed(requested_size) = conf.buffer_size { - // Note: We use `default_input_config`/`default_output_config` to get the buffer size - // range. This queries the CURRENT device (`self.pcm_id`), not the default device. The - // buffer size range is the same across all format configurations for a given device - // (see `supported_configs()`). - let supported_config = match stream_type { - alsa::Direction::Capture => self.default_input_config(), - alsa::Direction::Playback => self.default_output_config(), - }; - if let Ok(config) = supported_config { - if let SupportedBufferSize::Range { min, max } = config.buffer_size { - if !(min..=max).contains(&requested_size) { - return Err(Error::with_message( - ErrorKind::UnsupportedConfig, - format!("Buffer size {requested_size} is not in the supported range {min}..={max}"), - )); - } - } - } - } - let handle = { let _guard = ALSA_OPEN_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); alsa::pcm::PCM::new(&self.pcm_id, stream_type, true)? @@ -1571,6 +1546,17 @@ fn set_hw_params_from_format( // buffer_size = 2x and period_size = x. This provides consistent low-latency // behavior across different ALSA implementations and hardware. if let BufferSize::Fixed(buffer_frames) = config.buffer_size { + // Validate the requested size against the device's supported range using the same PCM + // handle we'll use for streaming. This avoids a second PCM open (which can disturb + // hardware clock state on some drivers) while still catching wildly out-of-range + // requests before set_period_size_near silently rounds them. + let (min_buffer, max_buffer) = hw_params_buffer_size_min_max(&hw_params); + if !(min_buffer..=max_buffer).contains(&buffer_frames) { + return Err(Error::with_message( + ErrorKind::UnsupportedConfig, + format!("Buffer size {buffer_frames} is not in the supported range {min_buffer}..={max_buffer}"), + )); + } hw_params.set_buffer_size_near(DEFAULT_PERIODS * buffer_frames as alsa::pcm::Frames)?; hw_params .set_period_size_near(buffer_frames as alsa::pcm::Frames, alsa::ValueOr::Nearest)?; From b57e74f6346d5fd6a518515089de4fa9c960592c Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Wed, 20 May 2026 23:41:58 +0200 Subject: [PATCH 2/5] feat(alsa): defend against hangs due to polling timeouts --- src/host/alsa/mod.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index 22018fa25..a029ba3bd 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -1065,7 +1065,25 @@ fn poll_for_period( let res = alsa::poll::poll(descriptors, *poll_timeout)?; if res == 0 { - // poll() returned 0: either a timeout or a spurious wakeup. Nothing to do. + // Timeout expired with no events. Query PCM state to handle cases where + // POLLERR/POLLHUP was not delivered before the timeout fired (e.g. some + // power-management suspend paths or VM/container ALSA shims). + match stream.handle.state() { + alsa::pcm::State::Disconnected => { + return Err(Error::with_message( + ErrorKind::DeviceNotAvailable, + "Device disconnected", + )); + } + // Xrun with POLLERR missed: recover the same way the POLLERR path does. + alsa::pcm::State::XRun => { + return Err(ErrorKind::Xrun.into()); + } + // Suspend with POLLHUP/POLLERR missed: attempt hardware resume. + alsa::pcm::State::Suspended => return try_resume(&stream.handle), + // No events and no error state: spurious wakeup, poll again. + _ => {} + } return Ok(Poll::Pending); } From 1504475683d2bc5c0e7f0c3f59f99ec11d2716d2 Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Thu, 21 May 2026 23:21:02 +0200 Subject: [PATCH 3/5] fix(alsa): enumeration and validation of buffers and sample rates --- CHANGELOG.md | 7 ++ src/host/alsa/mod.rs | 220 +++++++++++++++++++++++-------------------- src/lib.rs | 5 +- 3 files changed, 130 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7ed6a779..396e25f61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `BufferSize` now implements `Default` (returns `BufferSize::Default`). - `SupportedBufferSize` now implements `Default` (returns `SupportedBufferSize::Unknown`). - `SupportedStreamConfig` now implements `Copy`. +- DSD512 sample rates added to the common rate probe list. - **AAudio**: Streams now request `PERFORMANCE_MODE_LOW_LATENCY` when the `realtime` feature is enabled; stream error callback receives `ErrorKind::RealtimeDenied` if not granted. - **AAudio**: `Device` now implements `PartialEq`, `Eq`, `Hash`, and `Debug`. @@ -169,6 +170,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **ALSA**: Fix silence template not being applied for DSD. - **ALSA**: Fix stream corruption on certain drivers with spurious wakeups. - **ALSA**: Fix callbacks firing before `build_*_stream` returns the `Stream` handle. +- **ALSA**: Fix `BufferSize::Fixed` validation opening the PCM device a second time. +- **ALSA**: Fix hang when device raced to an error state without delivering POLLERR. +- **ALSA**: Fix `supported_configs()` reporting buffer size instead of period size. +- **ALSA**: Fix `supported_configs()` using the same buffer range for all formats and channels. +- **ALSA**: Fix `supported_configs()` dropping sample rates outside of `COMMON_SAMPLE_RATES`. +- **ALSA**: Fix `BufferSize::Fixed(0)` being silently accepted. - **ASIO**: Fix enumeration returning only the first device when using `collect()`. - **ASIO**: Fix device enumeration and stream creation failing when called from spawned threads. - **ASIO**: Fix buffer size not resizing when the driver reports `kAsioBufferSizeChange`. diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index a029ba3bd..a7db4318a 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -8,7 +8,7 @@ extern crate alsa_sys; extern crate libc; use std::{ - cmp, fmt, + fmt, sync::{ atomic::{AtomicBool, Ordering}, Arc, Mutex, @@ -368,7 +368,7 @@ impl Device { let hw_params = set_hw_params_from_format(&handle, conf, sample_format)?; let (buffer_size, period_size) = set_sw_params_from_format(&handle, stream_type)?; - if buffer_size == 0 { + if buffer_size == 0 || period_size == 0 { return Err(ErrorKind::DeviceNotAvailable.into()); } @@ -494,68 +494,52 @@ impl Device { //SND_PCM_FORMAT_U18_3BE, ]; - // Collect supported formats, deduplicating since we test both LE and BE variants. - // If hardware supports both endiannesses (rare), we only report the format once. - let mut supported_formats = Vec::new(); - for &(sample_format, alsa_format) in FORMATS.iter() { - if hw_params.test_format(alsa_format).is_ok() - && !supported_formats.contains(&sample_format) - { - supported_formats.push(sample_format); - } - } - let min_rate = hw_params.get_rate_min()?; let max_rate = hw_params.get_rate_max()?; let sample_rates = if min_rate == max_rate || hw_params.test_rate(min_rate + 1).is_ok() { + // Fixed rate or continuous range. vec![(min_rate, max_rate)] } else { - let mut rates = Vec::new(); - for &sample_rate in COMMON_SAMPLE_RATES.iter() { - if hw_params.test_rate(sample_rate).is_ok() { - rates.push((sample_rate, sample_rate)); - } - } - - if rates.is_empty() { - vec![(min_rate, max_rate)] - } else { - rates - } + // Discrete rates: probe the standard list plus the hardware's own min and max so + // that rates outside `COMMON_SAMPLE_RATES` are not missed. + let mut probe: Vec = COMMON_SAMPLE_RATES.to_vec(); + probe.push(min_rate); + probe.push(max_rate); + probe.sort_unstable(); + probe.dedup(); + probe + .into_iter() + .filter(|&r| (min_rate..=max_rate).contains(&r) && hw_params.test_rate(r).is_ok()) + .map(|r| (r, r)) + .collect() }; let min_channels = hw_params.get_channels_min()?; - let max_channels = hw_params.get_channels_max()?; + let max_channels = hw_params.get_channels_max()?.min(32); // TODO: cap at 32 or too many configs - let max_channels = cmp::min(max_channels, 32); // TODO: limiting to 32 channels or too much stuff is returned - let supported_channels = (min_channels..=max_channels) - .filter_map(|num| { - if hw_params.test_channels(num).is_ok() { - Some(num as ChannelCount) - } else { - None - } - }) - .collect::>(); + let mut output = Vec::new(); + let mut seen_formats: Vec = Vec::new(); + for &(sample_format, alsa_format) in FORMATS.iter() { + if seen_formats.contains(&sample_format) || hw_params.test_format(alsa_format).is_err() + { + continue; + } + seen_formats.push(sample_format); - let (min_buffer_size, max_buffer_size) = hw_params_buffer_size_min_max(&hw_params); - let buffer_size_range = SupportedBufferSize::Range { - min: min_buffer_size, - max: max_buffer_size, - }; + for channels in min_channels..=max_channels { + if hw_params.test_channels(channels).is_err() { + continue; + } + let channels = channels as ChannelCount; + let buffer_size = supported_period_size_range(&pcm, alsa_format, channels); - let mut output = Vec::with_capacity( - supported_formats.len() * supported_channels.len() * sample_rates.len(), - ); - for &sample_format in supported_formats.iter() { - for &channels in supported_channels.iter() { for &(min_rate, max_rate) in sample_rates.iter() { output.push(SupportedStreamConfigRange { channels, min_sample_rate: min_rate, max_sample_rate: max_rate, - buffer_size: buffer_size_range, + buffer_size, sample_format, }); } @@ -579,7 +563,7 @@ impl Device { let mut formats: Vec<_> = { match self.supported_configs(stream_t) { // EINVAL when querying direction the device does not support (input-only or output-only) - Err(err) if err.kind() == ErrorKind::InvalidInput => { + Err(err) if err.kind() == ErrorKind::UnsupportedConfig => { let dir = match stream_t { alsa::Direction::Capture => "input", alsa::Direction::Playback => "output", @@ -1036,9 +1020,7 @@ fn try_resume(handle: &alsa::PCM) -> Result { // device is still resuming; poll again until it is ready. Err(e) if e.errno() == libc::EAGAIN => Ok(Poll::Pending), // hardware does not support soft resume; treat as xrun so the worker calls prepare() - Err(e) if e.errno() == libc::ENOSYS => { - Err(Error::with_message(ErrorKind::Xrun, e.to_string())) - } + Err(e) if e.errno() == libc::ENOSYS => Err(ErrorKind::Xrun.into()), Err(e) => Err(e.into()), } } @@ -1110,9 +1092,7 @@ fn poll_for_period( // POLLIN/POLLOUT: data is ready, fall through to process it. let (avail_frames, delay_frames) = match stream.handle.avail_delay() { // Xrun: recover via prepare() (+ start() for capture, handled by the worker). - Err(err) if err.errno() == libc::EPIPE => { - return Err(Error::with_message(ErrorKind::Xrun, err.to_string())) - } + Err(err) if err.errno() == libc::EPIPE => return Err(ErrorKind::Xrun.into()), // Suspend: try hardware resume first; fall back to prepare() if unsupported. Err(err) if err.errno() == libc::ESTRPIPE => return try_resume(&stream.handle), res => res, @@ -1168,13 +1148,11 @@ fn process_input( if frames_read == 0 { return Ok(()); } else { - return Err(Error::with_message(ErrorKind::Xrun, err.to_string())); + return Err(ErrorKind::Xrun.into()); } } // EPIPE = xrun: full underrun recovery (prepare + start) required. - Err(err) if err.errno() == libc::EPIPE => { - return Err(Error::with_message(ErrorKind::Xrun, err.to_string())) - } + Err(err) if err.errno() == libc::EPIPE => return Err(ErrorKind::Xrun.into()), // ESTRPIPE = hardware suspend: try soft resume first, falling back to underrun // recovery if the hardware doesn't support it. Err(err) if err.errno() == libc::ESTRPIPE => { @@ -1238,13 +1216,11 @@ fn process_output( if frames_written == 0 { return Ok(()); } else { - return Err(Error::with_message(ErrorKind::Xrun, err.to_string())); + return Err(ErrorKind::Xrun.into()); } } // EPIPE = xrun: full underrun recovery (prepare) required. - Err(err) if err.errno() == libc::EPIPE => { - return Err(Error::with_message(ErrorKind::Xrun, err.to_string())) - } + Err(err) if err.errno() == libc::EPIPE => return Err(ErrorKind::Xrun.into()), // ESTRPIPE = hardware suspend: try soft resume first, falling back to underrun // recovery if the hardware doesn't support it. Err(err) if err.errno() == libc::ESTRPIPE => { @@ -1439,22 +1415,52 @@ impl StreamTrait for Stream { } } -// Convert ALSA frames to FrameCount, clamping to valid range. -// ALSA Frames are i64 (64-bit) or i32 (32-bit). -fn clamp_frame_count(buffer_size: alsa::pcm::Frames) -> FrameCount { - buffer_size.max(1).try_into().unwrap_or(FrameCount::MAX) +fn supported_period_size_range( + pcm: &alsa::pcm::PCM, + alsa_format: alsa::pcm::Format, + channels: ChannelCount, +) -> SupportedBufferSize { + let Ok(p) = alsa::pcm::HwParams::any(pcm) else { + return SupportedBufferSize::Unknown; + }; + if p.set_access(alsa::pcm::Access::RWInterleaved).is_err() + || p.set_channels(channels as u32).is_err() + || p.set_format(alsa_format).is_err() + { + return SupportedBufferSize::Unknown; + } + let Some((min, max)) = hw_params_period_size_min_max(&p) else { + return SupportedBufferSize::Unknown; + }; + let min_frames = min.max(1); + // cpal double-buffers (ring = DEFAULT_PERIODS × period), so the achievable + // period maximum is also bounded by max_buffer / DEFAULT_PERIODS. + let effective_max = match p.get_buffer_size_max() { + Ok(max_buf) if max_buf > 0 => max.min(max_buf / DEFAULT_PERIODS), + _ => max, + }; + if effective_max >= min_frames { + let Ok(min) = min_frames.try_into() else { + return SupportedBufferSize::Unknown; + }; + SupportedBufferSize::Range { + min, + max: effective_max.try_into().unwrap_or(FrameCount::MAX), + } + } else { + SupportedBufferSize::Unknown + } } -fn hw_params_buffer_size_min_max(hw_params: &alsa::pcm::HwParams) -> (FrameCount, FrameCount) { - let min_buf = hw_params - .get_buffer_size_min() - .map(clamp_frame_count) - .unwrap_or(1); - let max_buf = hw_params - .get_buffer_size_max() - .map(clamp_frame_count) - .unwrap_or(FrameCount::MAX); - (min_buf, max_buf) +fn hw_params_period_size_min_max( + hw_params: &alsa::pcm::HwParams, +) -> Option<(alsa::pcm::Frames, alsa::pcm::Frames)> { + let min = hw_params.get_period_size_min().ok()?; + let max = hw_params.get_period_size_max().ok()?; + // min=0 means no hardware lower bound (PipeWire reports this on unconstrained params); + // it is handled in the caller by clamping to 1. max <= 0 is degenerate (or ULONG_MAX + // wrapping negative), so we return None in that case rather than a misleading range. + (max > 0 && max >= min).then_some((min, max)) } fn init_hw_params<'a>( @@ -1563,21 +1569,42 @@ fn set_hw_params_from_format( // When BufferSize::Fixed(x) is specified, we configure double-buffering with // buffer_size = 2x and period_size = x. This provides consistent low-latency // behavior across different ALSA implementations and hardware. - if let BufferSize::Fixed(buffer_frames) = config.buffer_size { - // Validate the requested size against the device's supported range using the same PCM + if let BufferSize::Fixed(period_size) = config.buffer_size { + if period_size == 0 { + return Err(Error::with_message( + ErrorKind::InvalidInput, + "Buffer size must be greater than 0", + )); + } + + let period_size = period_size as alsa::pcm::Frames; + + // Validate the requested size against the device's supported ranges using the same PCM // handle we'll use for streaming. This avoids a second PCM open (which can disturb // hardware clock state on some drivers) while still catching wildly out-of-range // requests before set_period_size_near silently rounds them. - let (min_buffer, max_buffer) = hw_params_buffer_size_min_max(&hw_params); - if !(min_buffer..=max_buffer).contains(&buffer_frames) { - return Err(Error::with_message( - ErrorKind::UnsupportedConfig, - format!("Buffer size {buffer_frames} is not in the supported range {min_buffer}..={max_buffer}"), - )); + if let Some((min_period, max_period)) = hw_params_period_size_min_max(&hw_params) { + if !(min_period..=max_period).contains(&period_size) { + return Err(Error::with_message( + ErrorKind::UnsupportedConfig, + format!("Buffer size {period_size} is not in the supported range {min_period}..={max_period}"), + )); + } + } + + let buffer_size = DEFAULT_PERIODS * period_size; + if let Ok(max_buffer) = hw_params.get_buffer_size_max() { + if max_buffer > 0 && buffer_size > max_buffer { + let effective_max = max_buffer / DEFAULT_PERIODS; + return Err(Error::with_message( + ErrorKind::UnsupportedConfig, + format!("Buffer size {period_size} exceeds the maximum supported value of {effective_max}"), + )); + } } - hw_params.set_buffer_size_near(DEFAULT_PERIODS * buffer_frames as alsa::pcm::Frames)?; - hw_params - .set_period_size_near(buffer_frames as alsa::pcm::Frames, alsa::ValueOr::Nearest)?; + + hw_params.set_buffer_size_near(buffer_size)?; + hw_params.set_period_size_near(period_size, alsa::ValueOr::Nearest)?; } // Apply hardware parameters @@ -1587,7 +1614,7 @@ fn set_hw_params_from_format( // PipeWire-ALSA picks a good period size but pairs it with many periods (huge buffer). // We need to re-initialize hw_params and set BOTH period and buffer to constrain properly. if config.buffer_size == BufferSize::Default { - if let Ok(period_size) = hw_params.get_period_size().map(|s| s as alsa::pcm::Frames) { + if let Ok(period_size) = hw_params.get_period_size() { // Re-initialize hw_params to clear previous constraints let hw_params = init_hw_params(pcm_handle, config, sample_format)?; @@ -1655,18 +1682,11 @@ fn canonical_pcm_id(pcm_id: &str) -> String { impl From for Error { fn from(err: alsa::Error) -> Self { match err.errno() { - libc::ENODEV | libc::ENOENT | LIBC_ENOTSUPP => { - Error::with_message(ErrorKind::DeviceNotAvailable, err.to_string()) - } - libc::EPERM | libc::EACCES => { - Error::with_message(ErrorKind::PermissionDenied, err.to_string()) - } - libc::EBUSY | libc::EAGAIN => { - Error::with_message(ErrorKind::DeviceBusy, err.to_string()) - } - libc::EINVAL => Error::with_message(ErrorKind::InvalidInput, err.to_string()), - libc::EPIPE => Error::with_message(ErrorKind::Xrun, err.to_string()), - libc::ENOSYS => Error::with_message(ErrorKind::UnsupportedOperation, err.to_string()), + libc::ENODEV | libc::ENOENT | LIBC_ENOTSUPP => ErrorKind::DeviceNotAvailable.into(), + libc::EPERM | libc::EACCES => ErrorKind::PermissionDenied.into(), + libc::EBUSY | libc::EAGAIN => ErrorKind::DeviceBusy.into(), + libc::EINVAL | libc::ENOSYS => ErrorKind::UnsupportedConfig.into(), + libc::EPIPE => ErrorKind::Xrun.into(), _ => Error::with_message(ErrorKind::BackendError, err.to_string()), } } diff --git a/src/lib.rs b/src/lib.rs index 749f4c16e..ac1cc668f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -975,6 +975,7 @@ impl From for StreamConfig { // of commonly used rates. This is always the case for WASAPI and is sometimes the case for ALSA. #[allow(dead_code)] pub(crate) const COMMON_SAMPLE_RATES: &[SampleRate] = &[ - 5512, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000, 88200, 96000, - 176400, 192000, 352800, 384000, 705600, 768000, 1411200, 1536000, + // Standard PCM rates + 5512, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000, 88200, 96000, 176400, + 192000, 352800, 384000, 705600, 768000, 1411200, 1536000, 2822400, 3072000, ]; From d8a6f36a32d68f54ed6595a7bbe225a31811a0c3 Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Sat, 23 May 2026 14:27:03 +0200 Subject: [PATCH 4/5] feat(alsa): enumerate up to 64 channels (AES10 maximum) --- CHANGELOG.md | 1 + src/host/alsa/mod.rs | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 396e25f61..31d4723a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **ALSA**: Streams no longer start automatically on creation; call `play()` manually. - **ALSA**: The `realtime` feature now skips RT promotion for ineligible PCM types (null, I/O plugins, wrapper types); `audio_thread_priority` promoted unconditionally. +- **ALSA**: Supported configurations are now enumerated up to 64 channels (AES10 maximum). - **ASIO**: `Device::driver`, `asio_streams`, and `current_callback_flag` are no longer `pub`. - **ASIO**: Timestamps now include driver-reported hardware latency. - **ASIO**: Hardware latency is now re-queried when the driver reports `kAsioLatenciesChanged`. diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index a7db4318a..597abec50 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -516,7 +516,11 @@ impl Device { }; let min_channels = hw_params.get_channels_min()?; - let max_channels = hw_params.get_channels_max()?.min(32); // TODO: cap at 32 or too many configs + // 64 = AES10 (MADI) maximum; also prevents spinning on plugins like plughw that report u32::MAX. + const CHANNEL_ENUM_CAP: u32 = 64; + let max_channels = hw_params + .get_channels_max()? + .min(min_channels.max(CHANNEL_ENUM_CAP)); let mut output = Vec::new(); let mut seen_formats: Vec = Vec::new(); @@ -1685,7 +1689,8 @@ impl From for Error { libc::ENODEV | libc::ENOENT | LIBC_ENOTSUPP => ErrorKind::DeviceNotAvailable.into(), libc::EPERM | libc::EACCES => ErrorKind::PermissionDenied.into(), libc::EBUSY | libc::EAGAIN => ErrorKind::DeviceBusy.into(), - libc::EINVAL | libc::ENOSYS => ErrorKind::UnsupportedConfig.into(), + libc::EINVAL => ErrorKind::UnsupportedConfig.into(), + libc::ENOSYS => ErrorKind::UnsupportedOperation.into(), libc::EPIPE => ErrorKind::Xrun.into(), _ => Error::with_message(ErrorKind::BackendError, err.to_string()), } From d6fe895e43cfb282deafb9eaa6c78ea74f7e411b Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Sat, 23 May 2026 20:06:36 +0200 Subject: [PATCH 5/5] fix(alsa): validate StreamConfig for zero channels, sample rate, and buffer size --- src/host/alsa/mod.rs | 60 ++++---- src/lib.rs | 348 ++++++++++++++++++++++++++----------------- 2 files changed, 242 insertions(+), 166 deletions(-) diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index 597abec50..85eed93be 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -94,6 +94,25 @@ const DEFAULT_PERIODS: alsa::pcm::Frames = 2; // Some ALSA plugins (e.g. alsaequal, certain USB drivers) are not reentrant. static ALSA_OPEN_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(()); +fn open_pcm(pcm_id: &str, direction: alsa::Direction) -> Result { + let _guard = ALSA_OPEN_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); + alsa::pcm::PCM::new(pcm_id, direction, true).map_err(|e| { + let e = Error::from(e); + if e.kind() == ErrorKind::UnsupportedConfig { + let dir = match direction { + alsa::Direction::Capture => "input", + alsa::Direction::Playback => "output", + }; + Error::with_message( + ErrorKind::UnsupportedOperation, + format!("Device does not support {dir}"), + ) + } else { + e + } + }) +} + // TODO: Not yet defined in rust-lang/libc crate const LIBC_ENOTSUPP: libc::c_int = 524; @@ -361,10 +380,9 @@ impl Device { sample_format: SampleFormat, stream_type: alsa::Direction, ) -> Result { - let handle = { - let _guard = ALSA_OPEN_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); - alsa::pcm::PCM::new(&self.pcm_id, stream_type, true)? - }; + crate::validate_stream_config(&conf)?; + + let handle = open_pcm(&self.pcm_id, stream_type)?; let hw_params = set_hw_params_from_format(&handle, conf, sample_format)?; let (buffer_size, period_size) = set_sw_params_from_format(&handle, stream_type)?; @@ -438,10 +456,7 @@ impl Device { &self, stream_t: alsa::Direction, ) -> Result, Error> { - let pcm = { - let _guard = ALSA_OPEN_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); - alsa::pcm::PCM::new(&self.pcm_id, stream_t, true)? - }; + let pcm = open_pcm(&self.pcm_id, stream_t)?; let hw_params = alsa::pcm::HwParams::any(&pcm)?; @@ -520,7 +535,8 @@ impl Device { const CHANNEL_ENUM_CAP: u32 = 64; let max_channels = hw_params .get_channels_max()? - .min(min_channels.max(CHANNEL_ENUM_CAP)); + .min(CHANNEL_ENUM_CAP) + .min(ChannelCount::MAX as u32); let mut output = Vec::new(); let mut seen_formats: Vec = Vec::new(); @@ -564,22 +580,9 @@ impl Device { // ALSA does not offer default stream formats, so instead we compare all supported formats by // the `SupportedStreamConfigRange::cmp_default_heuristics` order and select the greatest. fn default_config(&self, stream_t: alsa::Direction) -> Result { - let mut formats: Vec<_> = { - match self.supported_configs(stream_t) { - // EINVAL when querying direction the device does not support (input-only or output-only) - Err(err) if err.kind() == ErrorKind::UnsupportedConfig => { - let dir = match stream_t { - alsa::Direction::Capture => "input", - alsa::Direction::Playback => "output", - }; - return Err(Error::with_message( - ErrorKind::UnsupportedOperation, - format!("Device does not support {dir}"), - )); - } - Err(err) => return Err(err), - Ok(fmts) => fmts.collect(), - } + let mut formats: Vec<_> = match self.supported_configs(stream_t) { + Err(err) => return Err(err), + Ok(fmts) => fmts.collect(), }; formats.sort_by(|a, b| a.cmp_default_heuristics(b)); @@ -1574,13 +1577,6 @@ fn set_hw_params_from_format( // buffer_size = 2x and period_size = x. This provides consistent low-latency // behavior across different ALSA implementations and hardware. if let BufferSize::Fixed(period_size) = config.buffer_size { - if period_size == 0 { - return Err(Error::with_message( - ErrorKind::InvalidInput, - "Buffer size must be greater than 0", - )); - } - let period_size = period_size as alsa::pcm::Frames; // Validate the requested size against the device's supported ranges using the same PCM diff --git a/src/lib.rs b/src/lib.rs index ac1cc668f..cf11e08eb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -830,152 +830,232 @@ impl SupportedStreamConfigRange { } } -#[cfg(test)] -fn make_range( - channels: ChannelCount, - format: SampleFormat, - min: SampleRate, - max: SampleRate, -) -> SupportedStreamConfigRange { - SupportedStreamConfigRange { - buffer_size: SupportedBufferSize::Range { min: 256, max: 512 }, - channels, - min_sample_rate: min, - max_sample_rate: max, - sample_format: format, +impl From for StreamConfig { + fn from(conf: SupportedStreamConfig) -> Self { + conf.config() } } -#[test] -fn test_with_standard_sample_rate() { - let r = |min, max| make_range(2, SampleFormat::F32, min, max); - - assert_eq!( - r(1, 96_000).with_standard_sample_rate().sample_rate(), - SAMPLE_RATE_48K - ); - assert_eq!( - r(1, 44_100).with_standard_sample_rate().sample_rate(), - SAMPLE_RATE_CD - ); +#[allow(dead_code)] +pub(crate) fn validate_stream_config(config: &StreamConfig) -> Result<(), Error> { + if config.channels == 0 { + return Err(Error::with_message( + ErrorKind::InvalidInput, + "channel count must be at least 1", + )); + } + if config.sample_rate == 0 { + return Err(Error::with_message( + ErrorKind::InvalidInput, + "sample rate must be at least 1 Hz", + )); + } + if config.buffer_size == BufferSize::Fixed(0) { + return Err(Error::with_message( + ErrorKind::InvalidInput, + "buffer size must be greater than 0", + )); + } + Ok(()) } -#[test] -#[should_panic(expected = "no standard sample rate")] -fn test_with_standard_sample_rate_panics_when_no_standard_rate() { - make_range(2, SampleFormat::F32, 8_000, 32_000).with_standard_sample_rate(); -} +// If a backend does not provide an API for retrieving supported formats, we query it with a bunch +// of commonly used rates. This is always the case for WASAPI and is sometimes the case for ALSA. +#[allow(dead_code)] +pub(crate) const COMMON_SAMPLE_RATES: &[SampleRate] = &[ + // Standard PCM rates and DSD sample rates through DSD512 + 5512, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000, 88200, 96000, 176400, + 192000, 352800, 384000, 705600, 768000, 1411200, 1536000, 2822400, 3072000, 5644800, 6144000, + 11289600, 12288000, 22579200, 24576000, +]; -#[test] -fn test_try_with_standard_sample_rate() { - let r = |min, max| make_range(2, SampleFormat::F32, min, max); - - // 48 kHz available; first choice - assert_eq!( - r(1, 96_000) - .try_with_standard_sample_rate() - .map(|c| c.sample_rate()), - Some(SAMPLE_RATE_48K) - ); - - // 48 kHz not available but 44.1 kHz is; second choice - assert_eq!( - r(1, 44_100) - .try_with_standard_sample_rate() - .map(|c| c.sample_rate()), - Some(SAMPLE_RATE_CD) - ); - - // Neither preferred rate available - assert_eq!(r(8_000, 32_000).try_with_standard_sample_rate(), None); -} +#[cfg(test)] +mod tests { + use super::*; -#[test] -fn test_cmp_default_heuristics_format_order() { - use SampleFormat::*; + fn make_range( + channels: ChannelCount, + format: SampleFormat, + min: SampleRate, + max: SampleRate, + ) -> SupportedStreamConfigRange { + SupportedStreamConfigRange { + buffer_size: SupportedBufferSize::Range { min: 256, max: 512 }, + channels, + min_sample_rate: min, + max_sample_rate: max, + sample_format: format, + } + } - // Input is deliberately not sorted to prove the sort works. - let unsorted = [ - F32, I16, DsdU32, U64, I32, DsdU8, F64, U8, I24, U16, I64, U24, U32, I8, DsdU16, - ]; + #[test] + fn with_standard_sample_rate() { + let r = |min, max| make_range(2, SampleFormat::F32, min, max); + + assert_eq!( + r(1, 96_000).with_standard_sample_rate().sample_rate(), + SAMPLE_RATE_48K + ); + assert_eq!( + r(1, 44_100).with_standard_sample_rate().sample_rate(), + SAMPLE_RATE_CD + ); + } - let mut ranges: Vec<_> = unsorted - .iter() - .map(|&fmt| make_range(2, fmt, 1, 96_000)) - .collect(); - ranges.sort_by(|a, b| a.cmp_default_heuristics(b)); + #[test] + #[should_panic(expected = "no standard sample rate")] + fn with_standard_sample_rate_panics_when_no_standard_rate() { + make_range(2, SampleFormat::F32, 8_000, 32_000).with_standard_sample_rate(); + } - let sorted_formats: Vec = ranges.iter().map(|r| r.sample_format()).collect(); + #[test] + fn try_with_standard_sample_rate() { + let r = |min, max| make_range(2, SampleFormat::F32, min, max); + + // 48 kHz available; first choice + assert_eq!( + r(1, 96_000) + .try_with_standard_sample_rate() + .map(|c| c.sample_rate()), + Some(SAMPLE_RATE_48K) + ); + + // 48 kHz not available but 44.1 kHz is; second choice + assert_eq!( + r(1, 44_100) + .try_with_standard_sample_rate() + .map(|c| c.sample_rate()), + Some(SAMPLE_RATE_CD) + ); + + // Neither preferred rate available + assert_eq!(r(8_000, 32_000).try_with_standard_sample_rate(), None); + } - // Expected order from lowest to highest priority: - assert_eq!( - sorted_formats, - vec![DsdU8, DsdU16, DsdU32, U8, I8, U64, I64, U16, I16, U24, I24, U32, I32, F64, F32,] - ); -} + #[test] + fn cmp_default_heuristics_format_order() { + use SampleFormat::*; -#[test] -fn test_cmp_default_heuristics() { - let mut configs = [ - make_range(1, SampleFormat::F64, 1, 96_000), // mono; loses to all stereo - make_range(2, SampleFormat::DsdU8, 1, 96_000), // DSD; lowest format priority - make_range(2, SampleFormat::U8, 1, 96_000), - make_range(2, SampleFormat::I8, 1, 96_000), - make_range(2, SampleFormat::U16, 1, 96_000), - make_range(2, SampleFormat::I16, 1, 96_000), - make_range(2, SampleFormat::F32, 1, 22_050), // neither standard rate - make_range(2, SampleFormat::F32, 1, 44_100), // 44.1 kHz only - make_range(2, SampleFormat::F32, 48_000, 96_000), // 48 kHz only - make_range(2, SampleFormat::F32, 1, 96_000), // both standard rates - make_range(2, SampleFormat::F64, 1, 96_000), - ]; - - configs.sort_by(|a, b| a.cmp_default_heuristics(b)); - - // Results in ascending priority order (lowest first): - - // [0] Mono loses to every stereo entry regardless of format or rate. - assert_eq!(configs[0].channels(), 1); - assert_eq!(configs[0].sample_format(), SampleFormat::F64); - - // [1]–[5] Stereo entries ranked by format (DSD < 8-bit < 16-bit). - assert_eq!(configs[1].sample_format(), SampleFormat::DsdU8); - assert_eq!(configs[2].sample_format(), SampleFormat::U8); - assert_eq!(configs[3].sample_format(), SampleFormat::I8); - assert_eq!(configs[4].sample_format(), SampleFormat::U16); - assert_eq!(configs[5].sample_format(), SampleFormat::I16); - - // [6] F64 is outranked by F32. - assert_eq!(configs[6].sample_format(), SampleFormat::F64); - assert_eq!(configs[6].channels(), 2); - - // [7]–[10] Stereo F32 entries ranked by rate coverage. - assert_eq!(configs[7].sample_format(), SampleFormat::F32); - assert_eq!(configs[7].max_sample_rate(), 22_050); // neither standard rate - - assert_eq!(configs[8].sample_format(), SampleFormat::F32); - assert_eq!(configs[8].max_sample_rate(), 44_100); // 44.1 kHz only - - assert_eq!(configs[9].sample_format(), SampleFormat::F32); - assert_eq!(configs[9].min_sample_rate(), 48_000); // 48 kHz only (no 44.1 kHz) - assert_eq!(configs[9].max_sample_rate(), 96_000); - - assert_eq!(configs[10].sample_format(), SampleFormat::F32); - assert_eq!(configs[10].min_sample_rate(), 1); - assert_eq!(configs[10].max_sample_rate(), 96_000); // both standard rates -} + // Input is deliberately not sorted to prove the sort works. + let unsorted = [ + F32, I16, DsdU32, U64, I32, DsdU8, F64, U8, I24, U16, I64, U24, U32, I8, DsdU16, + ]; -impl From for StreamConfig { - fn from(conf: SupportedStreamConfig) -> Self { - conf.config() + let mut ranges: Vec<_> = unsorted + .iter() + .map(|&fmt| make_range(2, fmt, 1, 96_000)) + .collect(); + ranges.sort_by(|a, b| a.cmp_default_heuristics(b)); + + let sorted_formats: Vec = ranges.iter().map(|r| r.sample_format()).collect(); + + // Expected order from lowest to highest priority: + assert_eq!( + sorted_formats, + vec![DsdU8, DsdU16, DsdU32, U8, I8, U64, I64, U16, I16, U24, I24, U32, I32, F64, F32,] + ); } -} -// If a backend does not provide an API for retrieving supported formats, we query it with a bunch -// of commonly used rates. This is always the case for WASAPI and is sometimes the case for ALSA. -#[allow(dead_code)] -pub(crate) const COMMON_SAMPLE_RATES: &[SampleRate] = &[ - // Standard PCM rates - 5512, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000, 88200, 96000, 176400, - 192000, 352800, 384000, 705600, 768000, 1411200, 1536000, 2822400, 3072000, -]; + #[test] + fn cmp_default_heuristics() { + let mut configs = [ + make_range(1, SampleFormat::F64, 1, 96_000), // mono; loses to all stereo + make_range(2, SampleFormat::DsdU8, 1, 96_000), // DSD; lowest format priority + make_range(2, SampleFormat::U8, 1, 96_000), + make_range(2, SampleFormat::I8, 1, 96_000), + make_range(2, SampleFormat::U16, 1, 96_000), + make_range(2, SampleFormat::I16, 1, 96_000), + make_range(2, SampleFormat::F32, 1, 22_050), // neither standard rate + make_range(2, SampleFormat::F32, 1, 44_100), // 44.1 kHz only + make_range(2, SampleFormat::F32, 48_000, 96_000), // 48 kHz only + make_range(2, SampleFormat::F32, 1, 96_000), // both standard rates + make_range(2, SampleFormat::F64, 1, 96_000), + ]; + + configs.sort_by(|a, b| a.cmp_default_heuristics(b)); + + // Results in ascending priority order (lowest first): + + // [0] Mono loses to every stereo entry regardless of format or rate. + assert_eq!(configs[0].channels(), 1); + assert_eq!(configs[0].sample_format(), SampleFormat::F64); + + // [1]–[5] Stereo entries ranked by format (DSD < 8-bit < 16-bit). + assert_eq!(configs[1].sample_format(), SampleFormat::DsdU8); + assert_eq!(configs[2].sample_format(), SampleFormat::U8); + assert_eq!(configs[3].sample_format(), SampleFormat::I8); + assert_eq!(configs[4].sample_format(), SampleFormat::U16); + assert_eq!(configs[5].sample_format(), SampleFormat::I16); + + // [6] F64 is outranked by F32. + assert_eq!(configs[6].sample_format(), SampleFormat::F64); + assert_eq!(configs[6].channels(), 2); + + // [7]–[10] Stereo F32 entries ranked by rate coverage. + assert_eq!(configs[7].sample_format(), SampleFormat::F32); + assert_eq!(configs[7].max_sample_rate(), 22_050); // neither standard rate + + assert_eq!(configs[8].sample_format(), SampleFormat::F32); + assert_eq!(configs[8].max_sample_rate(), 44_100); // 44.1 kHz only + + assert_eq!(configs[9].sample_format(), SampleFormat::F32); + assert_eq!(configs[9].min_sample_rate(), 48_000); // 48 kHz only (no 44.1 kHz) + assert_eq!(configs[9].max_sample_rate(), 96_000); + + assert_eq!(configs[10].sample_format(), SampleFormat::F32); + assert_eq!(configs[10].min_sample_rate(), 1); + assert_eq!(configs[10].max_sample_rate(), 96_000); // both standard rates + } + + #[test] + fn validate_stream_config_rejects_zero_channels() { + let err = validate_stream_config(&StreamConfig { + channels: 0, + sample_rate: 44100, + buffer_size: BufferSize::Default, + }) + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::InvalidInput); + assert!(err.message().unwrap().contains("channel")); + } + + #[test] + fn validate_stream_config_rejects_zero_sample_rate() { + let err = validate_stream_config(&StreamConfig { + channels: 2, + sample_rate: 0, + buffer_size: BufferSize::Default, + }) + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::InvalidInput); + assert!(err.message().unwrap().contains("sample rate")); + } + + #[test] + fn validate_stream_config_rejects_fixed_buffer_zero() { + let err = validate_stream_config(&StreamConfig { + channels: 2, + sample_rate: 44100, + buffer_size: BufferSize::Fixed(0), + }) + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::InvalidInput); + assert!(err.message().unwrap().contains("buffer size")); + } + + #[test] + fn validate_stream_config_accepts_valid_configs() { + assert!(validate_stream_config(&StreamConfig { + channels: 2, + sample_rate: 44100, + buffer_size: BufferSize::Default, + }) + .is_ok()); + assert!(validate_stream_config(&StreamConfig { + channels: 1, + sample_rate: 1, + buffer_size: BufferSize::Fixed(1), + }) + .is_ok()); + } +}