diff --git a/src/host/alsa/enumerate.rs b/src/host/alsa/enumerate.rs index aec92520c..03ddd733a 100644 --- a/src/host/alsa/enumerate.rs +++ b/src/host/alsa/enumerate.rs @@ -3,9 +3,7 @@ use std::{ sync::{Arc, Mutex}, }; -use super::{ - alsa, {Device, DeviceHandles}, -}; +use super::{alsa, Device}; use crate::{BackendSpecificError, DevicesError}; /// ALSA's implementation for `Devices`. @@ -26,16 +24,13 @@ impl Devices { } } -unsafe impl Send for Devices {} -unsafe impl Sync for Devices {} - impl Iterator for Devices { type Item = Device; - fn next(&mut self) -> Option { + fn next(&mut self) -> Option { loop { let hint = self.hint_iter.next()?; - if let Ok(device) = Device::try_from(hint) { + if let Ok(device) = Self::Item::try_from(hint) { if self.enumerated_pcm_ids.insert(device.pcm_id.clone()) { return Some(device); } else { @@ -77,23 +72,15 @@ impl TryFrom for Device { type Error = BackendSpecificError; fn try_from(hint: alsa::device_name::Hint) -> Result { - let pcm_id = hint.name.ok_or_else(|| BackendSpecificError { + let pcm_id = hint.name.ok_or_else(|| Self::Error { description: "ALSA hint missing PCM ID".to_string(), })?; - // Try to open handles during enumeration - let handles = DeviceHandles::open(&pcm_id).unwrap_or_else(|_| { - // If opening fails during enumeration, create default handles - // The actual opening will be attempted when the device is used - DeviceHandles::default() - }); - // Include all devices from ALSA hints (matches `aplay -L` behavior) - // Even devices that can't be opened during enumeration are valid for selection Ok(Self { pcm_id: pcm_id.to_owned(), desc: hint.desc, - handles: Arc::new(Mutex::new(handles)), + handles: Arc::new(Mutex::new(Default::default())), }) } } diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index 075a26e9b..a39c34433 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -49,6 +49,27 @@ use crate::{ // // This mirrors the behavior documented in the cpal API where `BufferSize::Fixed(x)` // requests but does not guarantee a specific callback size. +// +// ## BufferSize::Default Behavior +// +// When `BufferSize::Default` is specified, cpal does NOT set explicit period size or +// period count constraints, allowing the device/driver to choose sensible defaults. +// +// **Why not set defaults?** Different audio systems have different behaviors: +// +// - **Native ALSA hardware**: Typically chooses reasonable defaults (e.g., 1024-2048 +// frame periods with 2-4 periods) +// +// - **PipeWire-ALSA plugin**: Allocates a large buffer (~1M frames at 48kHz) but uses +// small periods (~1024 frames). Critically, if you request `set_periods(2)` without +// specifying period size, PipeWire calculates period = buffer/2, resulting in +// pathologically large periods (~524K frames = 10 seconds). See issue #1029. +// +// By not constraining period configuration, PipeWire-ALSA can use its optimized defaults +// (small periods with many-period buffer), while native ALSA hardware uses its own defaults. +// +// **Startup latency**: Regardless of buffer size, cpal uses double-buffering for startup +// (start_threshold = 2 periods), ensuring low latency even with large multi-period buffers. pub type SupportedInputConfigs = VecIntoIter; pub type SupportedOutputConfigs = VecIntoIter; @@ -130,7 +151,7 @@ impl DeviceTrait for Device { { let stream_inner = self.build_stream_inner(conf, sample_format, alsa::Direction::Capture)?; - let stream = Stream::new_input( + let stream = Self::Stream::new_input( Arc::new(stream_inner), data_callback, error_callback, @@ -153,7 +174,7 @@ impl DeviceTrait for Device { { let stream_inner = self.build_stream_inner(conf, sample_format, alsa::Direction::Playback)?; - let stream = Stream::new_output( + let stream = Self::Stream::new_output( Arc::new(stream_inner), data_callback, error_callback, @@ -214,19 +235,6 @@ struct DeviceHandles { } impl DeviceHandles { - /// Create `DeviceHandles` for `name` and try to open a handle for both - /// directions. Returns `Ok` if either direction is opened successfully. - fn open(pcm_id: &str) -> Result { - let mut handles = Self::default(); - let playback_err = handles.try_open(pcm_id, alsa::Direction::Playback).err(); - let capture_err = handles.try_open(pcm_id, alsa::Direction::Capture).err(); - if let Some(err) = capture_err.and(playback_err) { - Err(err) - } else { - Ok(handles) - } - } - /// Get a mutable reference to the `Option` for a specific `stream_type`. /// If the `Option` is `None`, the `alsa::PCM` will be opened and placed in /// the `Option` before returning. If `handle_mut()` returns `Ok` the contained @@ -743,13 +751,6 @@ fn output_stream_worker( let mut ctxt = StreamWorkerContext::new(&timeout, stream, &rx); - // As first period, always write one buffer with equilibrium values. - // This ensures we start with a full period of silence, giving the user their - // requested latency while avoiding underruns on the first callback. - if let Err(err) = stream.channel.io_bytes().writei(&ctxt.transfer_buffer) { - error_callback(err.into()); - } - loop { let flow = poll_descriptors_and_prepare_buffer(&rx, stream, &mut ctxt).unwrap_or_else(|err| { @@ -877,7 +878,10 @@ fn poll_descriptors_and_prepare_buffer( }; let available_samples = avail_frames * stream.conf.channels as usize; - // Only go on if there is at least one period's worth of space available. + // ALSA can have spurious wakeups where poll returns but avail < avail_min. + // This is documented to occur with dmix (timer-driven) and other plugins. + // Verify we have room for at least one full period before processing. + // See: https://bugzilla.kernel.org/show_bug.cgi?id=202499 if available_samples < stream.period_samples { return Ok(PollDescriptorsFlow::Continue); } @@ -1072,7 +1076,7 @@ impl Stream { ); }) .unwrap(); - Stream { + Self { thread: Some(thread), inner, trigger: tx, @@ -1104,7 +1108,7 @@ impl Stream { ); }) .unwrap(); - Stream { + Self { thread: Some(thread), inner, trigger: tx, @@ -1288,18 +1292,23 @@ fn set_hw_params_from_format( hw_params.set_channels(config.channels as u32)?; // Configure period size based on buffer size request - // When BufferSize::Fixed(x) is specified, we request a period size of x frames - // to achieve approximately x-sized callbacks. ALSA may adjust this to the nearest - // supported value based on hardware constraints. - if let BufferSize::Fixed(buffer_frames) = config.buffer_size { - hw_params.set_period_size_near(buffer_frames as _, alsa::ValueOr::Nearest)?; + match config.buffer_size { + BufferSize::Fixed(buffer_frames) => { + // When BufferSize::Fixed(x) is specified, we request two periods with size of x frames + // to achieve approximately x-sized double-buffered callbacks. ALSA may adjust this to + // the nearest supported value based on hardware constraints. + hw_params.set_period_size_near(buffer_frames as _, alsa::ValueOr::Nearest)?; + // We shouldn't fail if the driver isn't happy here - some devices may use more periods. + let _ = hw_params.set_periods(2, alsa::ValueOr::Greater); + } + BufferSize::Default => { + // For BufferSize::Default, we don't set period size or count, allowing the device + // to choose sensible defaults. This is important for PipeWire-ALSA compatibility: + // setting periods=2 without a period size causes PipeWire to create massive periods + // (buffer_size/2), resulting in multi-second latency. See issue #1029. + } } - // We shouldn't fail if the driver isn't happy here. - // `default` pcm sometimes fails here, but there's no reason to as we - // provide a direction and 2 is strictly the minimum number of periods. - let _ = hw_params.set_periods(2, alsa::ValueOr::Greater); - // Apply hardware parameters pcm_handle.hw_params(&hw_params)?; @@ -1320,18 +1329,23 @@ fn set_sw_params_from_format( description: "initialization resulted in a null buffer".to_string(), }); } - sw_params.set_avail_min(period as alsa::pcm::Frames)?; - let start_threshold = match stream_type { alsa::Direction::Playback => { - // Start when ALSA buffer has enough data to maintain consistent playback - // while preserving user's expected latency across different period counts - buffer - period + // Always use 2-period double-buffering: one period playing from hardware, one + // period queued in the software buffer. This ensures consistent low latency + // regardless of the total buffer size. + 2 * period } alsa::Direction::Capture => 1, }; sw_params.set_start_threshold(start_threshold.try_into().unwrap())?; + // We want to wake when one period has been consumed from our 2-period target level. + // This prevents filling PipeWire-ALSA's huge buffer beyond 2 periods, which could lead + // to pathological latency (21+ seconds at 48 kHz with a 1M buffer). + let target_avail = buffer - period; + sw_params.set_avail_min(target_avail as alsa::pcm::Frames)?; + period as usize * config.channels as usize }; @@ -1351,7 +1365,7 @@ fn set_sw_params_from_format( impl From for BackendSpecificError { fn from(err: alsa::Error) -> Self { - BackendSpecificError { + Self { description: err.to_string(), } }