diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index 075a26e9b..1a79405b6 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; @@ -743,13 +764,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 +891,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); } @@ -1117,33 +1134,6 @@ impl Drop for Stream { self.inner.dropping.set(true); self.trigger.wakeup(); self.thread.take().unwrap().join().unwrap(); - - // State-based drop behavior: drain if playing, drop if paused. This allows audio to - // complete naturally when stopping during playback, but provides immediate termination - // when already paused. - match self.inner.channel.state() { - alsa::pcm::State::Running => { - // Audio is actively playing - attempt graceful drain. - if let Ok(()) = self.inner.channel.drain() { - // TODO: Use SND_PCM_WAIT_DRAIN (-10002) when alsa-rs supports it properly, - // although it requires ALSA 1.2.8+ which may not be available everywhere. - // For now, calculate timeout based on buffer latency. - let buffer_duration_ms = ((self.inner.period_frames as f64 * 1000.0) - / self.inner.conf.sample_rate.0 as f64) - as u32; - - // This is safe: snd_pcm_wait() checks device state first and returns - // immediately with error codes like -ENODEV for disconnected devices. - let _ = self.inner.channel.wait(Some(buffer_duration_ms)); - } - // If drain fails or device has errors, stream terminates naturally - } - _ => { - // Not actively playing (paused, stopped, etc.) - immediate drop and discard any - // buffered audio data for immediate termination. - let _ = self.inner.channel.drop(); - } - } } } @@ -1287,19 +1277,29 @@ fn set_hw_params_from_format( hw_params.set_rate(config.sample_rate.0, alsa::ValueOr::Nearest)?; 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)?; + // Configure buffer and period size based on buffer size request + match config.buffer_size { + BufferSize::Fixed(buffer_frames) => { + // 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. + // + // Set buffer_size first to constrain total latency. Without this, some + // implementations (notably PipeWire-ALSA) allocate very large buffers (~1M frames), + // defeating the latency control that Fixed buffer size provides. + hw_params.set_buffer_size_near((2 * buffer_frames) as _)?; + // Then set period_size to control callback granularity and ensure the buffer + // is divided into two periods for double-buffering. + hw_params.set_period_size_near(buffer_frames as _, alsa::ValueOr::Nearest)?; + } + 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 +1320,35 @@ 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())?; + // Set avail_min based on stream direction. For playback, "avail" means space available + // for writing (buffer_size - frames_queued). For capture, "avail" means data available + // for reading (frames_captured). These opposite semantics require different values. + let target_avail = match stream_type { + alsa::Direction::Playback => { + // Wake when buffer level drops to one period remaining (avail >= buffer - period). + // This maintains double-buffering by refilling when we're down to one period. + buffer - period + } + alsa::Direction::Capture => { + // Wake when one period of data is available to read (avail >= period). + // Using buffer - period here would cause excessive latency as capture would + // wait for nearly the entire buffer to fill before reading. + period + } + }; + sw_params.set_avail_min(target_avail as alsa::pcm::Frames)?; + period as usize * config.channels as usize };