diff --git a/Cargo.toml b/Cargo.toml index 6086f9f..b765d36 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,11 @@ iced = { version = "0.14.0", features = ["canvas"] } iroh = "1.0.0-rc.0" iroh-gossip = "0.99.0" opus = "0.3.1" -pipewire = "0.9" +# v0_3_49 exposes `Buffer::requested()` (the graph's per-cycle quantum), used by +# the playback RT callback to fill exactly what the device asks for instead of +# pinning the buffer to a hard-coded 1024-frame quantum (crackle on non-1024 +# hardware). The field has existed in libpipewire since 0.3.49 (2022). +pipewire = { version = "0.9", features = ["v0_3_49"] } rand = "0.10.1" ringbuf = "0.5.0" serde = { version = "1.0.228", features = ["derive"] } diff --git a/src/audio/pipewire_impl.rs b/src/audio/pipewire_impl.rs index f4ad278..40a7cbd 100644 --- a/src/audio/pipewire_impl.rs +++ b/src/audio/pipewire_impl.rs @@ -224,6 +224,27 @@ fn run_capture(cmd_rx: pw::channel::Receiver<()>, tx: Sender>, target_n Ok(()) } +/// Frames the playback RT callback should produce this cycle. +/// +/// `requested` is the graph's per-cycle quantum from `Buffer::requested()` (0 if +/// the graph didn't set it — first buffers / non-driver cycles). `mapped_frames` +/// is how many frames the mapped buffer slice can physically hold. +/// +/// Honoring the requested quantum is what makes playback correct on any graph +/// quantum, instead of the old hard-pinned 1024 (which crackled when a machine's +/// `clock.quantum` wasn't 1024). Rules: +/// - never write past the mapped slice (clamp to `mapped_frames`); +/// - when `requested == 0`, fall back to a safe cap (≤ one 1024-frame quantum) — +/// never the whole slice, because over-pulling past the ring depth is exactly +/// the sustained crackle this replaces. Under-filling a cycle is a harmless +/// brief glitch (PipeWire pads). +fn frames_to_produce(requested: usize, mapped_frames: usize) -> usize { + /// Safe per-cycle fallback when the graph doesn't report a quantum. + const FALLBACK_FRAMES: usize = 1024; + let want = if requested > 0 { requested } else { FALLBACK_FRAMES }; + want.min(mapped_frames) +} + fn run_playback( cmd_rx: pw::channel::Receiver<()>, rx: Receiver>, @@ -317,13 +338,20 @@ fn run_playback( }) .process(|stream, user_data| { if let Some(mut buffer) = stream.dequeue_buffer() { + // Read the graph's requested quantum BEFORE the mutable + // `datas_mut()` borrow below (`requested()` borrows `&buffer`). + let requested = buffer.requested() as usize; let datas = buffer.datas_mut(); if !datas.is_empty() { let data = &mut datas[0]; let mut total_size = 0; if let Some(slice) = data.data() { let stride = 2; // S16LE Mono = 2 bytes per frame - let n_frames = slice.len() / stride; + // Fill exactly what the graph asked for this cycle (with + // a safe fallback), never the whole mapped slice — that + // over-pull past the ring depth was the original crackle. + let mapped_frames = slice.len() / stride; + let n_frames = frames_to_produce(requested, mapped_frames); // Diagnostic: record the quantum the device asked for and // count the callback. Wait-free, RT-safe. user_data.last_quantum.store(n_frames, Ordering::Relaxed); @@ -384,15 +412,19 @@ fn run_playback( .0 .into_inner(); - // Explicit Buffers param — THE fix for the playback crackle. Without it - // PipeWire handed this stream a ~256ms (12288-frame) maxsize buffer and - // only called `process` ~4x/sec; each callback then asked us to fill all - // 12288 frames, far more than the 200ms ring could ever hold, so ~half of - // every buffer was silence (the crackle). Pinning the buffer size to one - // graph quantum (1024 frames = 2048 bytes mono S16LE) makes the device hand - // us a ~1024-frame buffer ~47x/sec, which the ring satisfies comfortably and - // keeps `slice.len()/stride` equal to the quantum so we never over-pull. - const QUANTUM_FRAMES: i32 = 1024; + // Explicit Buffers param. History: without ANY Buffers param PipeWire handed + // this stream a ~256ms (12288-frame) maxsize buffer and called `process` + // only ~4x/sec; the callback then over-pulled the whole 12288-frame slice + // (far more than the 200ms ring holds → ~half silence → the crackle). The + // first fix pinned the size to exactly one 1024-frame quantum, which only + // works when the graph quantum IS 1024 — on other hardware (quantum 512 or + // 2048) the pinned slice mismatches the device's per-cycle demand and the + // crackle returns. The correct fix: give the buffer enough room for any + // plausible quantum (the desktop's quantum-limit is 8192) and let the RT + // callback fill exactly `Buffer::requested()` frames per cycle (see + // `frames_to_produce`). `requested()`, not the buffer size, now governs + // per-cycle output, so this is a generous max rather than a hard pin. + const MAX_QUANTUM_FRAMES: i32 = 8192; const STRIDE: i32 = 2; // S16LE mono = 2 bytes/frame let buffers_obj = pw::spa::pod::Object { type_: pw::spa::utils::SpaTypes::ObjectParamBuffers.as_raw(), @@ -414,7 +446,7 @@ fn run_playback( ), pw::spa::pod::Property::new( pw::spa::sys::SPA_PARAM_BUFFERS_size, - pw::spa::pod::Value::Int(QUANTUM_FRAMES * STRIDE), + pw::spa::pod::Value::Int(MAX_QUANTUM_FRAMES * STRIDE), ), pw::spa::pod::Property::new( pw::spa::sys::SPA_PARAM_BUFFERS_stride, @@ -516,3 +548,39 @@ fn run_playback( Ok(()) } + +#[cfg(test)] +mod tests { + use super::frames_to_produce; + + #[test] + fn requested_in_range_is_honored() { + // The graph's requested quantum is produced verbatim when it fits. + assert_eq!(frames_to_produce(512, 8192), 512); + assert_eq!(frames_to_produce(1024, 8192), 1024); + assert_eq!(frames_to_produce(2048, 8192), 2048); + } + + #[test] + fn requested_over_mapped_is_clamped() { + // Never write past the mapped slice, even if the graph asks for more. + assert_eq!(frames_to_produce(8192, 1024), 1024); + assert_eq!(frames_to_produce(2048, 2048), 2048); + } + + #[test] + fn zero_requested_uses_safe_fallback() { + // requested()==0 (first buffers / non-driver cycles) → capped fallback, + // never the whole mapped slice (that's the over-pull crackle). + assert_eq!(frames_to_produce(0, 8192), 1024); + // ...still clamped to a small mapped slice. + assert_eq!(frames_to_produce(0, 256), 256); + } + + #[test] + fn degenerate_zero_mapped_is_zero() { + // No buffer to write into → produce nothing, no panic / underflow. + assert_eq!(frames_to_produce(0, 0), 0); + assert_eq!(frames_to_produce(1024, 0), 0); + } +}