diff --git a/CHANGELOG.md b/CHANGELOG.md index b7ed6a779..43ac57bfb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -169,6 +169,9 @@ 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 on when device raced to an error state without delivering POLLERR. +- **ALSA**: Fix `supported_configs()` reporting twice as large buffer size rather than period size. - **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 a05eb2d8d..be9363ea3 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)? @@ -393,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()); } @@ -564,11 +539,14 @@ impl Device { }) .collect::>(); - 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, - }; + let buffer_size = hw_params_period_size_min_max(&hw_params) + .and_then(|(min, max)| { + Some(SupportedBufferSize::Range { + min: min.max(1).try_into().ok()?, + max: max.try_into().unwrap_or(FrameCount::MAX), + }) + }) + .unwrap_or(SupportedBufferSize::Unknown); let mut output = Vec::with_capacity( supported_formats.len() * supported_channels.len() * sample_rates.len(), @@ -580,7 +558,7 @@ impl Device { channels, min_sample_rate: min_rate, max_sample_rate: max_rate, - buffer_size: buffer_size_range, + buffer_size, sample_format, }); } @@ -1061,9 +1039,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()), } } @@ -1090,7 +1066,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); } @@ -1117,9 +1111,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, @@ -1175,13 +1167,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 => { @@ -1245,13 +1235,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 => { @@ -1446,22 +1434,15 @@ 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 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>( @@ -1571,9 +1552,34 @@ 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 { - 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)?; + let buffer_frames = buffer_frames 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. + if let Some((min_period, max_period)) = hw_params_period_size_min_max(&hw_params) { + if !(min_period..=max_period).contains(&buffer_frames) { + return Err(Error::with_message( + ErrorKind::UnsupportedConfig, + format!("Buffer size {buffer_frames} is not in the supported range {min_period}..={max_period}"), + )); + } + } + + let double_buffer = DEFAULT_PERIODS * buffer_frames; + if let Ok(max_buffer) = hw_params.get_buffer_size_max() { + if max_buffer > 0 && double_buffer > max_buffer { + let effective_max = max_buffer / DEFAULT_PERIODS; + return Err(Error::with_message( + ErrorKind::UnsupportedConfig, + format!("Buffer size {buffer_frames} exceeds the maximum supported value of {effective_max}"), + )); + } + } + + hw_params.set_buffer_size_near(double_buffer)?; + hw_params.set_period_size_near(buffer_frames, alsa::ValueOr::Nearest)?; } // Apply hardware parameters @@ -1583,7 +1589,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)?; @@ -1661,7 +1667,7 @@ impl From for Error { 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::EPIPE => ErrorKind::Xrun.into(), libc::ENOSYS => Error::with_message(ErrorKind::UnsupportedOperation, err.to_string()), _ => Error::with_message(ErrorKind::BackendError, err.to_string()), }