From d9c330c23b22a20d211147d87cb5cdb4624d8c80 Mon Sep 17 00:00:00 2001 From: EdisonJwa Date: Sat, 16 May 2026 12:15:00 +0800 Subject: [PATCH] feat(audio,linux): output via SDL2; cpal stays on Windows/macOS User reported persistent crackling/popping from peer audio on Linux even after fixing the 48k->device-rate resampler boundary discontinuities, clamping pre-Opus-encode peaks, and pre-allocating the playback scratch buffer. Logs confirmed cpal opened raw ALSA at 44.1k native, no callback budget violations, no underrun warnings -- yet the audio was still poor. Root cause: cpal on Linux opens raw ALSA's 'default' PCM. On modern PipeWire / pipewire-alsa boxes that virtual device routes through ALSA's dmix + plug layers, whose default resampler is nearest-neighbour. cpal also picks a small default period size (~256 frames / 5.8 ms) leaving no headroom for kernel scheduler jitter. Both effects compound into the crackling the user heard. Upstream tsclientlib's own audio example (tsclientlib/examples/audio_utils/ts_to_audio.rs) and the official Qint client both use SDL2 with AudioSpecDesired { freq: 48000, channels: 2, samples: 960 }. SDL2 on the same systems routes through PipeWire's PA bridge (or PulseAudio directly), both carrying high-quality resamplers. Fix: * Add sdl2 = '0.37' as a target_os=linux dependency. Links libSDL2-2.0 .so (Arch sdl2-compat over SDL3, Debian libsdl2-2.0-0, Fedora SDL2). * New module crates/chanora_audio/src/sdl_output.rs implementing SdlOutput: opens a 48 kHz stereo 960-frame callback that zeroes the buffer and calls AudioHandler::fill_buffer directly (no user-side resampler). Master gain + hard-mute atomics wired in identically to the cpal callback so set_output_gain / set_output_muted keep working. * engine.rs cfg-gated: target_os='linux' builds SdlOutput; everywhere else continues with the cpal output path (including the device-native- rate negotiation and resampler-continuity fixes shipped earlier -- those remain correct on Windows/macOS where cpal targets WASAPI / CoreAudio cleanly). * The cpal output helpers (build_output_stream, PlaybackResampleState, FromF32) are now cfg(not(target_os='linux'))-gated so the Linux build doesn't emit dead-code warnings. Capture path still cpal on every platform -- outbound audio was not reported as bad. Resampler-continuity fix on the capture side stays: microphone -> Opus encoder still goes through the linear interpolator with the last-sample anchor. Tests: 32 / 0 / 0 (chanora_audio), workspace 78 / 0 / 1 unchanged. --- Cargo.lock | 30 ++ crates/chanora_audio/Cargo.toml | 18 ++ crates/chanora_audio/src/engine.rs | 366 ++++++++++++++++++++----- crates/chanora_audio/src/lib.rs | 3 + crates/chanora_audio/src/sdl_output.rs | 213 ++++++++++++++ 5 files changed, 564 insertions(+), 66 deletions(-) create mode 100644 crates/chanora_audio/src/sdl_output.rs diff --git a/Cargo.lock b/Cargo.lock index 2753428..4029030 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -384,6 +384,7 @@ dependencies = [ "jni 0.21.1", "ndk-context", "rand 0.8.6", + "sdl2", "thiserror 2.0.18", "tokio", "tracing", @@ -2884,6 +2885,29 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "sdl2" +version = "0.37.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b498da7d14d1ad6c839729bd4ad6fc11d90a57583605f3b4df2cd709a9cd380" +dependencies = [ + "bitflags 1.3.2", + "lazy_static", + "libc", + "sdl2-sys", +] + +[[package]] +name = "sdl2-sys" +version = "0.37.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "951deab27af08ed9c6068b7b0d05a93c91f0a8eb16b6b816a5e73452a43521d3" +dependencies = [ + "cfg-if", + "libc", + "version-compare", +] + [[package]] name = "sec1" version = "0.7.3" @@ -3796,6 +3820,12 @@ version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" +[[package]] +name = "version-compare" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "579a42fc0b8e0c63b76519a339be31bed574929511fa53c1a3acae26eb258f29" + [[package]] name = "version_check" version = "0.9.5" diff --git a/crates/chanora_audio/Cargo.toml b/crates/chanora_audio/Cargo.toml index 4c825ae..1957a3f 100644 --- a/crates/chanora_audio/Cargo.toml +++ b/crates/chanora_audio/Cargo.toml @@ -75,3 +75,21 @@ futures-util = { version = "0.3", default-features = false, features = ["std"] } # options. The portal recommends fresh tokens to scope its own # object paths per call. rand = "0.8" +# SDL2 audio for Linux. Replaces the cpal capture / playback paths +# on Linux only; cpal stays in use on Windows/macOS. Rationale: the +# cpal Linux backend opens raw ALSA `default`, which on most Arch +# / Fedora / Debian installs routes through `dmix` + `plug` with +# nearest-neighbour resampling and very small period sizes — the +# combination produces audible crackling/popping. SDL2 on the same +# systems routes through PipeWire's PulseAudio compat bridge (or +# real PulseAudio), both of which carry a high-quality resampler +# and a sensible default period. The upstream tsclientlib audio +# example (`tsclientlib/examples/audio_utils/ts_to_audio.rs`) and +# the official Qint client both use SDL2 in exactly this shape; +# this dep brings Chanora in line with that pattern. +# +# `bundled` is OFF deliberately — we link against the system +# libSDL2.so. Arch ships `sdl2-compat`; Debian/Ubuntu ship +# `libsdl2-2.0-0`; Fedora ships `SDL2`. The chanora-flutter Linux +# build documentation lists this as a runtime dependency. +sdl2 = { version = "0.37", default-features = false } diff --git a/crates/chanora_audio/src/engine.rs b/crates/chanora_audio/src/engine.rs index 834cfc4..22e87ff 100644 --- a/crates/chanora_audio/src/engine.rs +++ b/crates/chanora_audio/src/engine.rs @@ -98,8 +98,14 @@ pub struct AudioEngine { // Streams must be dropped to stop audio. Both are `!Send` because // cpal's Stream isn't Send on some backends; we keep them in an - // Option wrapped by Mutex so stop() can move them out. + // Option wrapped by Mutex so stop() can move them out. On Linux + // the output side is `crate::sdl_output::SdlOutput` instead of a + // cpal Stream (see the SDD note inside `sdl_output.rs`); the same + // unsafe Send/Sync impl below covers both. _input_stream: Mutex>, + #[cfg(target_os = "linux")] + _output_stream: Mutex>, + #[cfg(not(target_os = "linux"))] _output_stream: Mutex>, // Hand the inbound-voice forwarder task a shutdown signal. shutdown_tx: Option>, @@ -301,48 +307,82 @@ impl AudioEngine { let audio_handler: Arc>> = Arc::new(Mutex::new(AudioHandler::new())); - let out_cfg = out_dev - .default_output_config() - .map_err(|e| AudioError::StreamConfig(format!("output default: {e}")))?; - let out_format = out_cfg.sample_format(); - // AudioHandler::fill_buffer expects 48 kHz stereo f32. - let out_stream_cfg = cpal::StreamConfig { - channels: 2, - sample_rate: cpal::SampleRate(SAMPLE_RATE), - buffer_size: cpal::BufferSize::Default, - }; + // Linux uses SDL2 for output (Qint / upstream tsclientlib + // pattern). cpal's Linux backend opens raw ALSA which routes + // through `dmix`+`plug` and produces audible crackling / + // popping on the 48 kHz → device-rate step. SDL2 on the same + // box routes through PipeWire's PA bridge (or PulseAudio) + // whose resampler is high-quality. We keep cpal on Windows + // and macOS — both have native backends (WASAPI / CoreAudio) + // without this problem. See `crates/chanora_audio/src/sdl_output.rs` + // for the full rationale. + #[cfg(target_os = "linux")] + let output_stream = crate::sdl_output::SdlOutput::start( + audio_handler.clone(), + output_gain.clone(), + output_muted.clone(), + )?; - let output_stream = match out_format { - SampleFormat::F32 => build_output_stream::( - &out_dev, - &out_stream_cfg, - audio_handler.clone(), - output_gain.clone(), - output_muted.clone(), - )?, - SampleFormat::I16 => build_output_stream::( - &out_dev, - &out_stream_cfg, - audio_handler.clone(), - output_gain.clone(), - output_muted.clone(), - )?, - SampleFormat::U16 => build_output_stream::( - &out_dev, - &out_stream_cfg, - audio_handler.clone(), - output_gain.clone(), - output_muted.clone(), - )?, - other => { - return Err(AudioError::StreamConfig(format!( - "unsupported output format: {other:?}" - ))) - } + #[cfg(not(target_os = "linux"))] + let output_stream = { + let out_cfg = out_dev + .default_output_config() + .map_err(|e| AudioError::StreamConfig(format!("output default: {e}")))?; + let out_format = out_cfg.sample_format(); + let dev_sample_rate = out_cfg.sample_rate().0; + let dev_channels = out_cfg.channels() as usize; + let out_stream_cfg = cpal::StreamConfig { + channels: out_cfg.channels(), + sample_rate: out_cfg.sample_rate(), + buffer_size: cpal::BufferSize::Fixed(2048), + }; + info!( + target: "chanora_audio", + dev_sample_rate, + dev_channels, + buffer_size_frames = 2048, + "output stream using device native config (no 48k force)" + ); + + let stream = match out_format { + SampleFormat::F32 => build_output_stream::( + &out_dev, + &out_stream_cfg, + audio_handler.clone(), + output_gain.clone(), + output_muted.clone(), + dev_sample_rate, + dev_channels, + )?, + SampleFormat::I16 => build_output_stream::( + &out_dev, + &out_stream_cfg, + audio_handler.clone(), + output_gain.clone(), + output_muted.clone(), + dev_sample_rate, + dev_channels, + )?, + SampleFormat::U16 => build_output_stream::( + &out_dev, + &out_stream_cfg, + audio_handler.clone(), + output_gain.clone(), + output_muted.clone(), + dev_sample_rate, + dev_channels, + )?, + other => { + return Err(AudioError::StreamConfig(format!( + "unsupported output format: {other:?}" + ))) + } + }; + stream + .play() + .map_err(|e| AudioError::Backend(format!("output play: {e}")))?; + stream }; - output_stream - .play() - .map_err(|e| AudioError::Backend(format!("output play: {e}")))?; // ---------- Inbound forwarder ---------- let (shutdown_tx, mut shutdown_rx) = tokio::sync::oneshot::channel(); @@ -532,7 +572,14 @@ fn try_open_capture( let in_sample_rate = in_cfg.sample_rate().0; let in_channels = in_cfg.channels() as usize; let in_format = in_cfg.sample_format(); - let in_stream_cfg: cpal::StreamConfig = in_cfg.into(); + // Same buffer-size rationale as the output stream — request a + // ~46 ms period on the capture side to give the Opus encoder + // realistic time to run inside the cpal callback without + // overrunning. cpal carries over the device's negotiated rate / + // channels / sample-format from `in_cfg` via the From impl, then + // we override only the buffer size. + let mut in_stream_cfg: cpal::StreamConfig = in_cfg.into(); + in_stream_cfg.buffer_size = cpal::BufferSize::Fixed(2048); let opus_enc = OpusEncoder::new( OpusSampleRate::Hz48000, @@ -571,8 +618,15 @@ struct CaptureState { mic_gain: f32, /// 48 kHz mono buffer accumulated to FRAME_SAMPLES before each encode. pcm_accum: Vec, - /// Resampling state for non-48k sources (very simple linear resampler). + /// Resampling state for non-48k sources (simple linear resampler). resample_pos: f64, + /// Last input sample carried over from the previous cpal callback + /// so the resampler can interpolate across the buffer boundary + /// without dropping continuity. Without this, every cpal period + /// boundary produces a discontinuity → audible buzz / popping + /// roughly at the period rate (~100 Hz for a 10 ms period on + /// Linux ALSA defaults). + resample_last: f32, opus_out: [u8; MAX_OPUS_FRAME], voice_out_tx: mpsc::Sender, /// The PTT transmission gate. Read once per outbound frame; the @@ -598,6 +652,7 @@ impl CaptureState { mic_gain, pcm_accum: Vec::with_capacity(FRAME_SAMPLES * 2), resample_pos: 0.0, + resample_last: 0.0, opus_out: [0u8; MAX_OPUS_FRAME], voice_out_tx, transmit_active, @@ -631,9 +686,21 @@ impl CaptureState { self.resample_into_accum(&mono); } - // 3. Encode any complete frames. + // 3. Encode any complete frames. Clamp each sample to + // [-1.0, 1.0] before handing to libopus's float encoder — + // out-of-range samples are hard-clipped inside libopus, + // which produces audible distortion on transient peaks. + // Soft-clamping at the engine boundary preserves headroom + // and matches what every other VoIP client does. while self.pcm_accum.len() >= FRAME_SAMPLES { - let frame: Vec = self.pcm_accum.drain(..FRAME_SAMPLES).collect(); + let mut frame: Vec = self.pcm_accum.drain(..FRAME_SAMPLES).collect(); + for s in frame.iter_mut() { + if *s > 1.0 { + *s = 1.0; + } else if *s < -1.0 { + *s = -1.0; + } + } match self.encoder.encode_float(&frame, &mut self.opus_out[..]) { Ok(len) => { let packet = OutAudio::new(&AudioData::C2S { @@ -661,21 +728,56 @@ impl CaptureState { } /// Simple linear resampler for `in_sample_rate → 48000`. - /// Production quality work belongs in a Beta+ DSP module. + /// + /// The resampler maintains continuity across cpal buffer + /// boundaries by treating `self.resample_last` as a virtual + /// sample at fractional index `0.0`, followed by the incoming + /// `mono` slice at indices `1.0..=mono.len()`. Without the + /// virtual anchor sample, the first interpolation point of + /// every new buffer collapses to `mono[0]` for both `a` and + /// `b`, producing a sample-and-hold step at every cpal period + /// boundary. On Linux ALSA defaults that's a ~100 Hz buzz / + /// popping. Production-quality work would use a windowed sinc + /// kernel; this carries the previous sample only and keeps the + /// CPU cost trivial. fn resample_into_accum(&mut self, mono: &[f32]) { + if mono.is_empty() { + return; + } let ratio = self.in_sample_rate as f64 / SAMPLE_RATE as f64; let mut pos = self.resample_pos; + // The virtual buffer has length mono.len() + 1: index 0 is + // the carried-over last sample, indices 1..=mono.len() are + // the new buffer. We emit output samples while `pos` is + // strictly less than mono.len() so we always have a valid + // right-hand neighbour. The leftover sub-sample offset is + // carried over via `resample_pos` (rebased to the next + // buffer's virtual index 0 below). while pos < mono.len() as f64 { - let i = pos as usize; + let i = pos.floor() as isize; let frac = pos - i as f64; - let a = mono[i]; - let b = if i + 1 < mono.len() { mono[i + 1] } else { a }; + let a = if i <= 0 { + self.resample_last + } else { + mono[(i - 1) as usize] + }; + let b = if i < mono.len() as isize { + mono[i as usize] + } else { + // Should not happen given the while-condition, but + // guard for the boundary where ratio < 1.0 and `pos` + // can step past mono.len() in the last iteration. + a + }; self.pcm_accum .push((a as f64 + frac * (b - a) as f64) as f32); pos += ratio; } - // Keep the leftover sub-sample offset for the next buffer. + // Carry the leftover sub-sample offset, rebased so the next + // buffer's virtual index 0 is the new `resample_last`. self.resample_pos = pos - mono.len() as f64; + // Anchor for the next buffer's interpolation. + self.resample_last = *mono.last().unwrap(); } } @@ -725,43 +827,162 @@ where // ---------- Playback pipeline ---------- +#[cfg(not(target_os = "linux"))] fn build_output_stream( device: &cpal::Device, config: &cpal::StreamConfig, handler: Arc>>, output_gain: Arc, output_muted: Arc, + dev_sample_rate: u32, + dev_channels: usize, ) -> Result where T: SizedSample + FromF32 + Send + 'static, { + // Per-channel resampler state shared across cpal callbacks for + // continuity at buffer boundaries. AudioHandler produces 48 kHz + // stereo f32; we map the first two device channels to L/R and + // fill any extra channels with silence. `pos` carries the + // fractional source-sample offset; `last_l` / `last_r` are the + // anchor samples from the previous callback (avoid the sample- + // and-hold step that would otherwise pop at every period + // boundary — same trick as the capture-side resampler). + let resample_ratio = SAMPLE_RATE as f64 / dev_sample_rate as f64; + // `same_rate` is the common case where the device is already at + // 48 kHz — bypass the resampler entirely. + let same_rate = dev_sample_rate == SAMPLE_RATE; + let resample_state: Arc> = Arc::new(Mutex::new( + PlaybackResampleState { + pos: 0.0, + last_l: 0.0, + last_r: 0.0, + }, + )); + // Reusable scratch buffer for 48 kHz stereo samples coming out + // of AudioHandler. Allocating a fresh `Vec` per cpal callback on + // glibc malloc was costing measurable time on the realtime audio + // thread and contributing to the popping users heard. We resize + // the buffer to the per-callback need and only grow the + // backing allocation when it must — typical buffer-size jitter + // stays under the high-water mark and skips the allocator + // entirely after the first few callbacks. + let mut scratch: Vec = Vec::with_capacity(8192); + // Diagnostic: log a warning if the cpal callback wall-clock + // exceeds the period budget so we can correlate user-perceived + // popping with measurable underruns. The threshold is half a + // period at 48 kHz / 2 ch / 1024-frame typical period — about + // 10 ms. We rate-limit the warning to once per second. + let mut last_slow_warn = std::time::Instant::now() + .checked_sub(std::time::Duration::from_secs(2)) + .unwrap_or_else(std::time::Instant::now); let stream = device .build_output_stream( config, move |out: &mut [T], _| { + let cb_start = std::time::Instant::now(); let muted = output_muted.load(Ordering::Relaxed); + let dev_frames = out.len() / dev_channels.max(1); + let src_frames = if same_rate { + dev_frames + } else { + // Ask for a few extra source frames so we never + // starve on the resample fractional boundary. + ((dev_frames as f64 * resample_ratio).ceil() as usize) + 2 + }; + let needed = src_frames * 2; + if scratch.len() < needed { + scratch.resize(needed, 0.0); + } + // Zero the live slice; AudioHandler::fill_buffer + // writes silence into untouched samples, but + // resizing up from a smaller call leaves residual + // values from earlier callbacks. Use `fill` which + // optimises to memset on f32. + scratch[..needed].fill(0.0); + // Lock-and-decode. We deliberately hold the lock + // only for the duration of fill_buffer; the inbound + // forwarder uses handle_packet which is queue-fast. + { + let mut h = handler.lock().unwrap(); + h.fill_buffer(&mut scratch[..needed]); + } + if muted { - // Still call fill_buffer to keep the jitter - // buffer draining; just discard the result and - // emit silence to the device. - let mut scratch = vec![0.0f32; out.len()]; - { - let mut h = handler.lock().unwrap(); - h.fill_buffer(&mut scratch); - } for dst in out.iter_mut() { *dst = T::from_f32_sample(0.0); } - return; + } else { + let gain = f32::from_bits(output_gain.load(Ordering::Relaxed)); + if same_rate && dev_channels == 2 { + // Fast path: device is already 48 kHz stereo. + for (dst, s) in out.iter_mut().zip(scratch[..needed].iter().copied()) { + *dst = T::from_f32_sample(s * gain); + } + } else { + // Resample 48 kHz stereo → device-rate × + // device-channels with continuity across + // callback boundaries. + let mut state = resample_state.lock().unwrap(); + let mut pos = state.pos; + let mut last_l = state.last_l; + let mut last_r = state.last_r; + for frame_idx in 0..dev_frames { + let i = pos.floor() as isize; + let frac = pos - i as f64; + let (a_l, a_r) = if i <= 0 { + (last_l, last_r) + } else { + let idx = ((i - 1) as usize) * 2; + (scratch[idx], scratch[idx + 1]) + }; + let i_usize = i.max(0) as usize; + let (b_l, b_r) = if i_usize < src_frames { + let idx = i_usize * 2; + (scratch[idx], scratch[idx + 1]) + } else { + (a_l, a_r) + }; + let l = (a_l as f64 + frac * (b_l - a_l) as f64) as f32 * gain; + let r = (a_r as f64 + frac * (b_r - a_r) as f64) as f32 * gain; + let base = frame_idx * dev_channels; + if dev_channels >= 1 { + out[base] = T::from_f32_sample(l); + } + if dev_channels >= 2 { + out[base + 1] = T::from_f32_sample(r); + } + for c in 2..dev_channels { + out[base + c] = T::from_f32_sample(0.0); + } + pos += resample_ratio; + } + let consumed = pos.floor() as usize; + state.pos = pos - consumed as f64; + if consumed > 0 && consumed <= src_frames { + let idx = (consumed - 1) * 2; + last_l = scratch[idx]; + last_r = scratch[idx + 1]; + state.last_l = last_l; + state.last_r = last_r; + } + } } - let gain = f32::from_bits(output_gain.load(Ordering::Relaxed)); - let mut scratch = vec![0.0f32; out.len()]; + + // Period-budget diagnostic. + let elapsed = cb_start.elapsed(); + let period_us = (dev_frames as u64 * 1_000_000) / dev_sample_rate as u64; + if elapsed.as_micros() as u64 > period_us / 2 + && last_slow_warn.elapsed() > std::time::Duration::from_secs(1) { - let mut h = handler.lock().unwrap(); - h.fill_buffer(&mut scratch); - } - for (dst, src) in out.iter_mut().zip(scratch.into_iter()) { - *dst = T::from_f32_sample(src * gain); + last_slow_warn = std::time::Instant::now(); + warn!( + target: "chanora_audio", + callback_us = elapsed.as_micros() as u64, + period_us, + dev_frames, + "output callback exceeded half the period budget — possible underrun cause" + ); } }, move |e| { @@ -782,19 +1003,32 @@ where Ok(stream) } +/// Resampler state carried across output cpal callbacks. See +/// `build_output_stream` for the rationale. +#[cfg(not(target_os = "linux"))] +struct PlaybackResampleState { + pos: f64, + last_l: f32, + last_r: f32, +} + +#[cfg(not(target_os = "linux"))] trait FromF32 { fn from_f32_sample(v: f32) -> Self; } +#[cfg(not(target_os = "linux"))] impl FromF32 for f32 { fn from_f32_sample(v: f32) -> Self { v } } +#[cfg(not(target_os = "linux"))] impl FromF32 for i16 { fn from_f32_sample(v: f32) -> Self { (v.clamp(-1.0, 1.0) * f32::from(i16::MAX)) as i16 } } +#[cfg(not(target_os = "linux"))] impl FromF32 for u16 { fn from_f32_sample(v: f32) -> Self { let s = (v.clamp(-1.0, 1.0) * f32::from(i16::MAX)) as i32; diff --git a/crates/chanora_audio/src/lib.rs b/crates/chanora_audio/src/lib.rs index 9a8a2b8..8aa4457 100644 --- a/crates/chanora_audio/src/lib.rs +++ b/crates/chanora_audio/src/lib.rs @@ -35,6 +35,9 @@ pub mod release_tail; pub mod transmit_mode; pub mod transmit_selector; +#[cfg(target_os = "linux")] +mod sdl_output; + pub use engine::{AudioEngine, AudioEngineConfig}; pub use ptt::{ AudioTransmitGate, MissedKeyUpWatchdog, PttBackendDescriptor, PttCapabilityLevel, diff --git a/crates/chanora_audio/src/sdl_output.rs b/crates/chanora_audio/src/sdl_output.rs new file mode 100644 index 0000000..aa81e3d --- /dev/null +++ b/crates/chanora_audio/src/sdl_output.rs @@ -0,0 +1,213 @@ +//! Linux output stream via SDL2 (Qint / upstream `tsclientlib` +//! `ts_to_audio` pattern). +//! +//! ## Why SDL2 and not cpal on Linux +//! +//! cpal's Linux backend opens raw ALSA's `default` PCM. On most +//! modern distributions (Arch with `pipewire-alsa`, Fedora 38+, +//! Debian/Ubuntu with PipeWire) that virtual device still goes +//! through ALSA's `dmix` + `plug` layers when bypassing the +//! PulseAudio/PipeWire client. The `plug` layer's default +//! resampler is **nearest-neighbour**, which causes pronounced +//! aliasing on the 48 kHz → 44.1 kHz step that the +//! `tsclientlib::AudioHandler` output requires. Users perceive +//! this as constant crackling and popping. cpal also picks a +//! small default period size (≈256 frames), which leaves no +//! headroom for kernel scheduler jitter and causes additional +//! xruns. +//! +//! SDL2 on the same systems opens the audio device through the +//! SDL audio driver — which prefers PulseAudio when available +//! and falls back to ALSA otherwise. On a PipeWire box the SDL +//! PulseAudio driver lands inside PipeWire's PulseAudio +//! compatibility layer, whose resampler is high quality. SDL +//! also defaults to a buffer ≈ samples-requested, so we get +//! exactly one Opus frame (20 ms) per callback. +//! +//! This file replaces the cpal output path on Linux only. The +//! capture path stays on cpal until we have a reason to swap it +//! (the user reports outbound is currently fine). Windows / +//! macOS continue to use cpal because cpal's WASAPI and +//! CoreAudio backends do not have this problem. +//! +//! ## Threading & lifecycle +//! +//! `sdl2::AudioDevice` is `!Send + !Sync` — the SDL audio +//! lock is associated with the calling thread. We open the device +//! on the same thread that calls `Self::start_with_gate` (the +//! tokio worker that runs `chanora_core::ChanoraSession::start_audio`) +//! and never move it. The outer `AudioEngine` already carries an +//! `unsafe impl Send` to bypass cpal's identical constraint; we +//! reuse that and stash the SDL device behind a `Mutex>` +//! the same way cpal does. +//! +//! Dropping the `SdlOutput` closes the SDL device cleanly and +//! drops the playback callback — that releases the +//! `Arc>` clone the callback held. + +use std::sync::atomic::{AtomicBool, AtomicU32, Ordering}; +use std::sync::{Arc, Mutex}; + +use sdl2::audio::{AudioCallback, AudioDevice, AudioSpecDesired}; +use sdl2::AudioSubsystem; +use tracing::{info, warn}; +use tsclientlib::audio::AudioHandler; + +use crate::engine::SessionAudioId; +use crate::AudioError; + +/// 20 ms at 48 kHz mono == one Opus frame's worth of samples. +/// Stereo doubles the byte count but the frame-count stays the +/// same. Aligning the callback size to the Opus frame keeps the +/// jitter-buffer / playback handshake tight (no fractional frame +/// reads inside fill_buffer). +const FRAME_SAMPLES: u16 = 960; + +/// SDL output device wrapper. Holds the live `AudioDevice` so its +/// callback keeps firing for the engine's lifetime, plus a +/// reference to the same `AudioHandler` the inbound forwarder +/// pushes into. +pub struct SdlOutput { + // Drop order: device first (stops the callback), then any + // remaining references release naturally. We keep `_subsystem` + // alive because dropping the AudioSubsystem before the device + // would invalidate SDL's internal state. + device: AudioDevice, + _subsystem: AudioSubsystem, +} + +impl SdlOutput { + /// Open the default SDL playback device at the AudioHandler's + /// native format (48 kHz stereo f32) and start the device + /// playing immediately. The callback drains the AudioHandler + /// directly — no user-side resampling. + /// + /// `output_gain` is read on every callback to apply the + /// master-volume slider; `output_muted` zeroes the output (but + /// still drains AudioHandler so its jitter buffer doesn't grow + /// unbounded while muted). These two atomics share the same + /// definitions the cpal path uses, so the same FFI surface + /// (`set_output_gain` / `set_output_muted`) drives both. + pub fn start( + handler: Arc>>, + output_gain: Arc, + output_muted: Arc, + ) -> Result { + let sdl = sdl2::init() + .map_err(|e| AudioError::Backend(format!("sdl init: {e}")))?; + let subsystem = sdl + .audio() + .map_err(|e| AudioError::Backend(format!("sdl audio subsystem: {e}")))?; + + info!( + target: "chanora_audio", + driver = subsystem.current_audio_driver(), + "sdl audio subsystem initialised" + ); + + let desired = AudioSpecDesired { + freq: Some(48_000), + channels: Some(2), + samples: Some(FRAME_SAMPLES), + }; + + let device = AudioDevice::open_playback(&subsystem, None, &desired, |spec| { + info!( + target: "chanora_audio", + freq = spec.freq, + channels = spec.channels, + samples = spec.samples, + "sdl playback spec accepted" + ); + TsPlaybackCallback { + handler, + output_gain, + output_muted, + } + }) + .map_err(|e| AudioError::Backend(format!("sdl open_playback: {e}")))?; + + // Begin pumping audio frames out. SDL's device starts paused; + // resume() flips it into the playing state. The callback + // will fire repeatedly at ~50 Hz (every 20 ms) thereafter. + device.resume(); + + Ok(Self { + device, + _subsystem: subsystem, + }) + } + + /// Pause the SDL device. Used by the engine on hard mute / + /// shutdown if we ever want to stop the callback firing while + /// keeping the device handle alive. Not currently invoked — + /// the engine drops `SdlOutput` entirely on stop. + #[allow(dead_code)] + pub fn pause(&self) { + self.device.pause(); + } +} + +impl Drop for SdlOutput { + fn drop(&mut self) { + // AudioDevice::drop closes the device which stops the + // callback. We log so the chanora.log timeline matches + // engine shutdown. + warn!(target: "chanora_audio", "sdl playback device closing"); + } +} + +/// Playback callback invoked by SDL's audio thread. The shape +/// mirrors the upstream `tsclientlib` example's `SdlCallback`: +/// zero the output buffer (so silent regions emit silence rather +/// than stale memory), then ask the `AudioHandler` to fill in +/// whatever decoded frames it has buffered. +struct TsPlaybackCallback { + handler: Arc>>, + output_gain: Arc, + output_muted: Arc, +} + +impl AudioCallback for TsPlaybackCallback { + type Channel = f32; + + fn callback(&mut self, buffer: &mut [f32]) { + // The buffer is interleaved stereo at 48 kHz from SDL. + // Length is FRAME_SAMPLES * 2 (= 1920 f32) per the spec + // we requested. + for sample in buffer.iter_mut() { + *sample = 0.0; + } + // Lock window kept as tight as the upstream example — + // fill_buffer does the actual Opus decode + jitter logic. + // Contention with the inbound forwarder is the same as + // in the cpal path, but the upstream design has shipped + // this way for years. + { + let mut data = self.handler.lock().unwrap(); + let _removed_ids = data.fill_buffer(buffer); + // `_removed_ids` is the list of clients whose stream the + // handler just finished draining. We could publish that + // upward as a "stopped speaking" hint, but the existing + // BridgeEvent::VoiceState already covers that case via + // the bridge layer; ignoring matches upstream behaviour. + } + + // Apply local mute + master gain after fill so the jitter + // buffer still drains while muted (matching the cpal path's + // contract). `output_gain` is encoded as f32 bits inside an + // AtomicU32 — same encoding the cpal path uses. + if self.output_muted.load(Ordering::Relaxed) { + for sample in buffer.iter_mut() { + *sample = 0.0; + } + return; + } + let gain = f32::from_bits(self.output_gain.load(Ordering::Relaxed)); + if (gain - 1.0).abs() > f32::EPSILON { + for sample in buffer.iter_mut() { + *sample *= gain; + } + } + } +}