diff --git a/Cargo.lock b/Cargo.lock index b9056ae..90d3158 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -356,7 +356,6 @@ dependencies = [ "ndk-context", "rand 0.8.6", "reqwest", - "rtrb", "sdl2", "thiserror 2.0.18", "tokio", @@ -2663,12 +2662,6 @@ dependencies = [ "windows-sys 0.52.0", ] -[[package]] -name = "rtrb" -version = "0.3.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4ade083ccbb4bf536df69d1f6432cc23deb7acccff86b183f3923a6fd56a1153" - [[package]] name = "rusqlite" version = "0.32.1" diff --git a/apps/chanora_flutter/pubspec.yaml b/apps/chanora_flutter/pubspec.yaml index 6c1b6af..d5ed1f8 100644 --- a/apps/chanora_flutter/pubspec.yaml +++ b/apps/chanora_flutter/pubspec.yaml @@ -16,7 +16,7 @@ publish_to: 'none' # Remove this line if you wish to publish to pub.dev # https://developer.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html # In Windows, build-name is used as the major, minor, and patch parts # of the product and file versions while build-number is used as the build suffix. -version: 1.0.0-rc.8+74 +version: 1.0.0-rc.8+75 environment: sdk: ^3.11.5 diff --git a/crates/chanora_audio/Cargo.toml b/crates/chanora_audio/Cargo.toml index 9c44dd5..64eff23 100644 --- a/crates/chanora_audio/Cargo.toml +++ b/crates/chanora_audio/Cargo.toml @@ -58,24 +58,6 @@ tokio = { version = "1", features = ["sync", "rt", "macros", "time"] } # for AudioUnit construction + property access. coreaudio-rs = "0.14" -# Lock-free SPSC ring buffer to decouple the VPIO render callback -# (consumer, runs on iOS's audio thread with strict realtime -# constraints) from the AudioHandler decoder (producer, runs on a -# tokio worker). Without this, the render callback calls -# AudioHandler::fill_buffer directly under a Mutex, and any time -# the decoder is mid-burst the callback either blocks or gets a -# partial-fill that produces clicks at the partial-fill boundary -# plus choppy fragments from the missing tail. iOS's VPIO calls -# the render callback at irregular intervals (we logged -# `frames_changes=60+ per 100 callbacks` = the buffer size flips -# on ~60% of callbacks); decoupling the two via a stable-rate -# ring buffer is the standard fix used by every production VoIP -# audio engine. `rtrb` 0.3.4 (mgeier, 6.8M downloads) is the -# realtime-safe SPSC ring buffer the Rust audio community -# converged on \u2014 lock-free push / pop with no allocation on -# the audio thread. -rtrb = "0.3" - [target.'cfg(target_os = "android")'.dependencies] # JNI bindings to flip Android's AudioManager into MODE_IN_COMMUNICATION # when the voice-comm preset is requested. ndk_context is initialised diff --git a/crates/chanora_audio/src/ios_voice_unit.rs b/crates/chanora_audio/src/ios_voice_unit.rs index 4f42075..ab90b24 100644 --- a/crates/chanora_audio/src/ios_voice_unit.rs +++ b/crates/chanora_audio/src/ios_voice_unit.rs @@ -77,7 +77,6 @@ use std::sync::atomic::{AtomicBool, AtomicU32, Ordering}; use std::sync::{Arc, Mutex}; -use std::time::Duration; use audiopus::coder::Encoder as OpusEncoder; use audiopus::{ @@ -88,7 +87,6 @@ use coreaudio::audio_unit::audio_format::LinearPcmFlags; use coreaudio::audio_unit::render_callback::{self, data}; use coreaudio::audio_unit::{AudioUnit, Element, SampleFormat, Scope, StreamFormat}; use coreaudio::audio_unit::IOType; -use rtrb::{Consumer, Producer, RingBuffer}; use tokio::sync::mpsc; use tracing::{debug, error, info, warn}; use tsclientlib::audio::AudioHandler; @@ -112,29 +110,6 @@ const FRAME_SAMPLES_MONO: usize = 960; /// shared `crate::framing` module. const MAX_OPUS_FRAME: usize = 1275; -/// Ring buffer capacity between the AudioHandler decoder -/// (producer, tokio task) and the VPIO render callback -/// (consumer, iOS audio thread). Sized for 200 ms of mono i16 -/// content at 48 kHz = 9600 samples. Rationale: -/// * Producer pushes one 20 ms chunk (FRAME_SAMPLES_MONO = 960 -/// mono i16 samples) per tick at 50 Hz. -/// * Consumer pulls whatever VPIO asks for (typically 960 or -/// 1104 mono samples per callback at irregular intervals). -/// * 200 ms = 10 producer ticks = enough headroom to absorb -/// iOS callback-size jitter (we logged frames_changes=60 -/// per 100 callbacks at +72) without underrunning when the -/// producer briefly stalls. -/// * Larger than 200 ms adds perceptible latency to the user -/// voice path (200 ms is already noticeable; doubling it -/// would hurt conversational responsiveness). -const RING_BUFFER_SAMPLES: usize = 9600; - -/// Producer task tick interval. Matches the Opus packet rate -/// (50 Hz = 20 ms) so each tick drains exactly one Opus-frame's -/// worth of decoded samples from AudioHandler. Smaller intervals -/// would burn CPU on lock acquisition; larger would risk -/// underrun bursts. -const PRODUCER_TICK_MS: u64 = 20; /// Sample rate every layer above us assumes. Matches the Opus /// encoder rate, the `tsclientlib::AudioHandler` mix rate, and the @@ -326,20 +301,13 @@ impl IosCaptureState { } } -/// Live iOS VPIO AudioUnit wrapper. Construct + start = audio +/// Live iOS audio unit wrapper. Construct + start = audio /// flowing; drop = audio stopped. pub struct IosVoiceUnit { - // Drop order: stop the unit first (severs callbacks), then - // signal the producer task to exit (its consumer-side - // counterpart in the render callback is gone, so any further - // push would just fill the ring), then drop the wrapper so - // `AudioComponentInstanceDispose` runs. + // Drop = stop the audio unit (severs render callback). The + // wrapper's own Drop calls AudioComponentInstanceDispose + // after stop returns. unit: AudioUnit, - /// One-shot channel to signal the producer task to stop. - /// `Option` so `Drop` can `take()` it without owning `&mut self` - /// through Send semantics. Sending fails if the receiver is - /// already dropped (task exited) \u2014 harmless, we ignore. - producer_shutdown_tx: Option>, } impl IosVoiceUnit { @@ -514,363 +482,113 @@ impl IosVoiceUnit { // contract every other backend follows (matches // SdlOutput::callback and the cpal output stream). // - // Build the playback pipeline. + // Build the playback pipeline (direct fill_buffer in + // render callback; matches tsclientlib's reference SDL + // example at + // tsclientlib/examples/audio_utils/ts_to_audio.rs). // - // Architecture (decoupled producer / consumer; per the - // external review at +72 that identified iOS render-callback - // jitter as the root cause of voice + clicks + chopiness): + // The earlier ring-buffer attempt (rc.8+73..+74) decoupled + // AudioHandler from the render callback via a 50 Hz + // producer task + SPSC ring buffer. The +74 diagnostics + // showed that approach was making things worse: the + // producer drained AudioHandler at 50 Hz, but iOS VPIO + // calls our render callback at ~43.5 Hz (consuming 1440 + // mono samples per 23 ms call). With consumer slightly + // slower than producer in chunks-per-second but each + // consumer pull being larger, the ring averaged out empty + // — fill_buffer was returning silence 65-84% of ticks + // because we drained it too aggressively before packets + // arrived. Linux/SDL's same pattern works fine because + // SDL calls fill_buffer at exactly the device callback + // rate. // - // Producer task (tokio, 50 Hz timer): - // loop { - // lock AudioHandler - // fill_buffer(scratch_stereo_f32, 1920 samples = 20 ms stereo) - // release lock - // downmix L+R -> mono i16 (960 samples) - // ring_buffer.push_slice(mono_i16) - // sleep 20 ms - // } - // - // Consumer (VPIO render callback, audio thread): - // loop { - // num_frames = args.data.buffer.len() - // pop num_frames i16 samples from ring_buffer into args.data.buffer - // zero-fill tail if ring buffer was short (underrun) - // apply muted/gain post-process - // } - // - // Why this fixes the +72 "voice + clicks + choppy" symptom: - // - // * iOS VPIO calls the render callback at irregular - // intervals (logged frames_changes >= 60 per 100 - // callbacks at +72) and with varying num_frames. - // Calling fill_buffer directly under those conditions - // causes AudioHandler to repeatedly enter its - // `buffering_samples` state (returns &[] = silence) - // when the request size is misaligned with its 20 ms - // internal frame size. Silent gaps in the middle of - // the output buffer create discontinuities = audible - // clicks. - // - // * The ring buffer absorbs the rate mismatch. Producer - // always asks for a stable 20 ms chunk (aligned with - // AudioHandler's internal Opus packet size). Consumer - // pulls whatever iOS asks for whenever iOS schedules - // it; the ring buffer's 200 ms depth covers jitter. - // - // Locking: rtrb is lock-free SPSC. The audio thread - // (consumer) never blocks. The producer task can block on - // the AudioHandler mutex but that's contention with the - // inbound forwarder (handle_packet), not with the audio - // thread \u2014 acceptable. - // - // Allocation: ring buffer allocated once at engine start. - // Audio thread never touches the allocator. Producer task - // allocates one scratch_stereo Vec and re-uses across - // ticks. - let (mut ring_producer, mut ring_consumer): (Producer, Consumer) = - RingBuffer::new(RING_BUFFER_SAMPLES); - - // Spawn the producer task. Owns: - // - Arc> clone (shared with the - // inbound forwarder). - // - Producer end of the ring buffer. - // - Shutdown receiver. - let handler_for_producer = handler.clone(); - let (producer_shutdown_tx, mut producer_shutdown_rx) = - tokio::sync::oneshot::channel::<()>(); - // Use the consumer's `slots()` (= free slots = unwritten) - // to compute current ring fill level (= RING_BUFFER_SAMPLES - // - free). But the consumer end is in the render closure, - // not accessible here. rtrb exposes `Producer::slots()` - // which returns the FREE count, same arithmetic: - // fill_samples = RING_BUFFER_SAMPLES - producer.slots() - // fill_ms = fill_samples * 1000 / 48000 - // No locks needed; slots() is a relaxed atomic read. - tokio::spawn(async move { - let mut scratch_stereo = vec![0.0_f32; FRAME_SAMPLES_MONO * 2]; - let mut interval = tokio::time::interval(Duration::from_millis(PRODUCER_TICK_MS)); - interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); - // Per-tick counters (rolled up every PRODUCER_LOG_TICKS - // ticks = ~5 s of wall time). All u64 to avoid overflow - // across hours of session. - let mut producer_ticks: u64 = 0; - let mut produced_chunks: u64 = 0; - let mut fill_buffer_calls: u64 = 0; - let mut fill_buffer_zero_returns: u64 = 0; - let mut ring_full_drops: u64 = 0; - // Min/max ring level WITHIN the current logging - // window. Reset to extremes at each log emission so - // each window's report describes that window's - // behaviour. - let mut ring_min_samples: usize = usize::MAX; - let mut ring_max_samples: usize = 0; - // Accumulated scratch envelope across the window so - // we can emit window-average peak + RMS without - // doing a O(n) scan in the hot path \u2014 only an - // O(n) scan per producer tick (already affordable). - let mut window_peak_f32: f32 = 0.0; - let mut window_sumsq_f64: f64 = 0.0; - let mut window_sample_count: u64 = 0; - const PRODUCER_LOG_TICKS: u64 = 250; // 250 * 20 ms = 5 s - loop { - tokio::select! { - _ = &mut producer_shutdown_rx => { - debug!( - target: "chanora_audio", - produced_chunks, - fill_buffer_calls, - fill_buffer_zero_returns, - ring_full_drops, - "ios VPIO producer task shutting down" - ); - break; - } - _ = interval.tick() => { - producer_ticks = producer_ticks.wrapping_add(1); - // Measure ring level BEFORE the push so - // the diagnostic shows whether the - // consumer is keeping up. Slots = free - // slots; fill = capacity - free. - let free_before = ring_producer.slots(); - let fill_before = RING_BUFFER_SAMPLES.saturating_sub(free_before); - if fill_before < ring_min_samples { - ring_min_samples = fill_before; - } - if fill_before > ring_max_samples { - ring_max_samples = fill_before; - } - - // Zero scratch first (fill_buffer is - // additive over per-talker streams; it - // does NOT clear). - for s in scratch_stereo.iter_mut() { - *s = 0.0; - } - { - let mut h = handler_for_producer.lock().unwrap(); - let _removed = h.fill_buffer(&mut scratch_stereo); - } - fill_buffer_calls = fill_buffer_calls.wrapping_add(1); - - // Measure scratch envelope. fill_buffer - // either filled with real content or - // left it at zero (we pre-zeroed). A - // zero-peak scratch means the handler - // had nothing to play \u2014 either no - // talker active, no packets in queue, or - // queue stuck in buffering_samples - // state. - let mut tick_peak: f32 = 0.0; - let mut tick_sumsq: f64 = 0.0; - for &s in scratch_stereo.iter() { - let a = s.abs(); - if a > tick_peak { - tick_peak = a; - } - tick_sumsq += (s as f64) * (s as f64); - } - if tick_peak == 0.0 { - fill_buffer_zero_returns = - fill_buffer_zero_returns.wrapping_add(1); - } - if tick_peak > window_peak_f32 { - window_peak_f32 = tick_peak; - } - window_sumsq_f64 += tick_sumsq; - window_sample_count = - window_sample_count.wrapping_add(scratch_stereo.len() as u64); - - // Check ring has room then push. - if free_before < FRAME_SAMPLES_MONO { - ring_full_drops = ring_full_drops.wrapping_add(1); - // Don't push; ring would block / wrap. - } else { - for i in 0..FRAME_SAMPLES_MONO { - let l = scratch_stereo[i * 2]; - let r = scratch_stereo[i * 2 + 1]; - let mono_f32 = (l + r) * 0.5; - let clamped = mono_f32.clamp(-1.0, 1.0); - let sample = (clamped * i16::MAX as f32) as i16; - let _ = ring_producer.push(sample); - } - produced_chunks = produced_chunks.wrapping_add(1); - } - - // Emit rolled-up window diagnostic every - // ~5 s. All fields together let us tell - // exactly where audio is being lost: - // - // producer_ticks : how many times we - // ticked (should be - // ~250 per 5 s window; - // fewer = tokio - // scheduler stalled). - // produced_chunks : pushes into ring - // (= ticks - drops). - // fill_buffer_calls : AudioHandler queries - // (= ticks; always - // equal unless tokio - // panicked mid-loop). - // fill_buffer_zero_returns - // : ticks where fill_buffer - // left scratch at all - // zeros. High value = - // AudioHandler has no - // content to play. - // ring_full_drops : pushes skipped because - // ring was full (consumer - // slow). - // ring_min/max_samples : depth envelope across - // window. Should sit - // stable around the - // target_level - // (FRAME_SAMPLES_MONO). - // window_peak_f32 : max scratch sample - // across window. - // window_rms_f32 : RMS across all scratch - // samples in window. - if producer_ticks.is_multiple_of(PRODUCER_LOG_TICKS) { - let window_rms = if window_sample_count > 0 { - (window_sumsq_f64 / window_sample_count as f64).sqrt() as f32 - } else { - 0.0 - }; - let ring_min_ms = - (ring_min_samples * 1000) as f64 / 48000.0; - let ring_max_ms = - (ring_max_samples * 1000) as f64 / 48000.0; - info!( - target: "chanora_audio", - producer_ticks, - produced_chunks, - fill_buffer_calls, - fill_buffer_zero_returns, - ring_full_drops, - ring_min_samples, - ring_max_samples, - ring_min_ms = format!("{:.1}", ring_min_ms), - ring_max_ms = format!("{:.1}", ring_max_ms), - window_peak_f32, - window_rms_f32 = window_rms, - "ios VPIO producer task diagnostic sample" - ); - // Reset window-scope counters. - ring_min_samples = usize::MAX; - ring_max_samples = 0; - window_peak_f32 = 0.0; - window_sumsq_f64 = 0.0; - window_sample_count = 0; - } - } - } - } - }); - - // Render callback (consumer). Pulls mono i16 samples from - // the ring buffer into the VPIO output buffer. + // Revert to direct call: render callback locks + // AudioHandler, asks for `num_frames` stereo frames, and + // immediately downmixes to i16 mono into the output + // buffer. Same as Linux/SDL, just stereo-f32 -> mono-i16 + // converted at the boundary. + let mut scratch_stereo: Vec = Vec::with_capacity(2048); + let handler_for_render = handler.clone(); let output_gain_for_render = output_gain.clone(); let output_muted_for_render = output_muted.clone(); + // Diagnostic counters (sampled every 100 callbacks ~= 2 s). let mut cb_count: u64 = 0; let mut last_num_frames: usize = 0; let mut num_frames_changes: u32 = 0; - let mut underruns: u64 = 0; - let mut underrun_samples: u64 = 0; + let mut callbacks_with_audio: u64 = 0; + let mut callbacks_with_silence: u64 = 0; unit.set_render_callback(move |args: render_callback::Args>| { let out: &mut [i16] = args.data.buffer; let num_frames = out.len(); - - // Measure ring buffer fill level BEFORE the read so - // the diagnostic shows whether the producer is keeping - // up. Consumer::slots() returns the number of - // AVAILABLE (= readable) samples; the inverse - // (free space) lives on the Producer end. - let ring_avail_before = ring_consumer.slots(); - - // Pop up to num_frames samples from the ring buffer - // into the output. Anything not filled stays at - // whatever was there (we zero-fill the tail - // explicitly below to avoid stale-buffer clicks). - let mut filled: usize = 0; - while filled < num_frames { - match ring_consumer.pop() { - Ok(sample) => { - out[filled] = sample; - filled += 1; - } - Err(_) => { - // Ring empty (producer behind or no audio). - // Zero-fill the rest \u2014 same contract as - // the cpal/SDL paths (silence on underrun - // is acceptable; click on stale memory is - // not). - for s in out[filled..].iter_mut() { - *s = 0; - } - underruns = underruns.wrapping_add(1); - underrun_samples = - underrun_samples.wrapping_add((num_frames - filled) as u64); - break; - } - } + // AudioHandler produces 48 kHz stereo f32 (= num_frames * 2 floats). + let needed = num_frames * 2; + if scratch_stereo.len() < needed { + scratch_stereo.resize(needed, 0.0); } - let zero_filled_this_call = num_frames - filled; - - // Post-process: mute then gain. Gain is applied - // CONSUMER-SIDE (not producer-side) so user-driven - // volume changes take effect on the next callback - // rather than after the current ring contents drain - // (\u2264 200 ms latency). - if output_muted_for_render.load(Ordering::Relaxed) { - for s in out.iter_mut() { - *s = 0; - } - return Ok(()); + // Zero the live slice. AudioHandler::fill_buffer is + // additive (does NOT clear); residual values from + // earlier callbacks (when scratch was bigger) would + // leak through otherwise. + scratch_stereo[..needed].fill(0.0); + // Lock + fill. Same pattern as Linux/SDL output. + { + let mut h = handler_for_render.lock().unwrap(); + let _removed = h.fill_buffer(&mut scratch_stereo[..needed]); } + + // Downmix stereo f32 -> mono i16 with master gain. + // (l + r) * 0.5 preserves total signal energy with + // 3 dB headroom against sum-of-correlated-peaks + // clipping. Hard-clip i16 cast at the boundary. let gain = f32::from_bits(output_gain_for_render.load(Ordering::Relaxed)); - if (gain - 1.0).abs() > f32::EPSILON { - for s in out.iter_mut() { - let scaled = (*s as f32) * gain; - *s = scaled.clamp(i16::MIN as f32, i16::MAX as f32) as i16; + let muted = output_muted_for_render.load(Ordering::Relaxed); + let mut peak_out: i16 = 0; + for (i, dst) in out.iter_mut().enumerate() { + if muted { + *dst = 0; + continue; + } + let l = scratch_stereo[i * 2]; + let r = scratch_stereo[i * 2 + 1]; + let mono_f32 = (l + r) * 0.5 * gain; + let clamped = mono_f32.clamp(-1.0, 1.0); + let sample = (clamped * i16::MAX as f32) as i16; + *dst = sample; + let a = sample.unsigned_abs() as i16; + if a > peak_out { + peak_out = a; } } - // Diagnostic sampling (~2 Hz). + // Track audio-vs-silence for the diagnostic. + if peak_out > 0 { + callbacks_with_audio = callbacks_with_audio.wrapping_add(1); + } else { + callbacks_with_silence = callbacks_with_silence.wrapping_add(1); + } + + // Diagnostic sampling. if last_num_frames != 0 && last_num_frames != num_frames { num_frames_changes = num_frames_changes.wrapping_add(1); } last_num_frames = num_frames; cb_count = cb_count.wrapping_add(1); if cb_count.is_multiple_of(100) { - let mut peak_out: i16 = 0; - let mut clipped: u32 = 0; - for &s in out.iter() { - let a = s.unsigned_abs() as i16; - if a > peak_out { - peak_out = a; - } - if s == i16::MAX || s == i16::MIN || s == -i16::MAX { - clipped += 1; - } - } info!( target: "chanora_audio", cb = cb_count, num_frames, frames_changes = num_frames_changes, - ring_avail_before, - read_frames = filled, - zero_filled = zero_filled_this_call, - underruns, - underrun_samples, + callbacks_with_audio, + callbacks_with_silence, peak_out_i16 = peak_out, - clip_count_i16 = clipped, gain, - "ios VPIO render callback diagnostic sample (ring-buf consumer)" + "ios audio unit render callback diagnostic sample (direct fill_buffer)" ); } Ok(()) }) - .map_err(|e| AudioError::Backend(format!("vpio set render callback: {e}")))?; + .map_err(|e| AudioError::Backend(format!("audio unit set render callback: {e}")))?; // Finalise the unit — allocates internal buffers per the // stream formats we set above. After initialize() most @@ -936,30 +654,19 @@ impl IosVoiceUnit { Ok(Self { unit, - producer_shutdown_tx: Some(producer_shutdown_tx), }) } } impl Drop for IosVoiceUnit { fn drop(&mut self) { - // Stop the AudioUnit first so the render callback no longer - // fires (consumer side of the ring buffer is now idle). + // Stop the audio unit so the render callback no longer + // fires. The coreaudio-rs wrapper's own Drop calls + // AudioComponentInstanceDispose afterwards. if let Err(e) = self.unit.stop() { - warn!(target: "chanora_audio", error = %e, "vpio stop on drop failed"); + warn!(target: "chanora_audio", error = %e, "ios audio unit stop on drop failed"); } else { - info!(target: "chanora_audio", "ios VPIO audio unit stopped"); - } - // Signal the producer task to exit. Sending fails (Err) - // if the receiver is already dropped \u2014 harmless, - // ignored. The task observes the shutdown signal at its - // next select! iteration (every PRODUCER_TICK_MS = 20 ms - // at most) and exits its loop, dropping the Producer - // end of the ring buffer which makes the Consumer's - // pop() return Err on subsequent reads (now irrelevant - // because the audio unit is stopped). - if let Some(tx) = self.producer_shutdown_tx.take() { - let _ = tx.send(()); + info!(target: "chanora_audio", "ios audio unit stopped"); } } }