fix(alsa): buffering behavior on PipeWire-ALSA

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<SupportedStreamConfigRange>;
 pub type SupportedOutputConfigs = VecIntoIter<SupportedStreamConfigRange>;
@@ -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,22 +1277,25 @@ 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.
+    // 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 {
+        hw_params.set_buffer_size_near((2 * buffer_frames) as _)?;
         hw_params.set_period_size_near(buffer_frames as _, alsa::ValueOr::Nearest)?;
     }
 
-    // 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)?;
 
+    // For BufferSize::Default, trim the buffer to 2 periods for double-buffering
+    if config.buffer_size == BufferSize::Default {
+        if let Ok(period) = hw_params.get_period_size() {
+            hw_params.set_buffer_size_near(2 * period)?;
+            pcm_handle.hw_params(&hw_params)?;
+        }
+    }
+
     Ok(hw_params.can_pause())
 }
 
@@ -1320,18 +1313,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
     };