From 8542c5f2f89f053b4858b3e4203a885383c2aa29 Mon Sep 17 00:00:00 2001 From: David Kleingeld Date: Thu, 11 Sep 2025 15:49:34 +0200 Subject: [PATCH] Documentation and error handing improvements Co-authored-by: Richard --- Cargo.lock | 1 + crates/audio/Cargo.toml | 1 + crates/audio/src/audio.rs | 3 +-- crates/audio/src/audio_settings.rs | 5 ++++- crates/audio/src/rodio_ext.rs | 7 ++++++- crates/livekit_client/src/livekit_client/playback.rs | 2 ++ .../livekit_client/src/livekit_client/playback/source.rs | 4 +--- 7 files changed, 16 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 43bcbf9672475253b976fbd7d01c0b2eb697e612..99a8f2fd739dabb2eedb0919de1e86c23ca8ac2d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1397,6 +1397,7 @@ dependencies = [ "serde", "settings", "smol", + "thiserror 2.0.12", "util", "workspace-hack", ] diff --git a/crates/audio/Cargo.toml b/crates/audio/Cargo.toml index 08e0df424dcdaa15cfd78fddaf5758fb9b8d7e0b..85274f651417f8df91e2f785056e5ee8da0220de 100644 --- a/crates/audio/Cargo.toml +++ b/crates/audio/Cargo.toml @@ -25,6 +25,7 @@ schemars.workspace = true serde.workspace = true settings.workspace = true smol.workspace = true +thiserror.workspace = true util.workspace = true workspace-hack.workspace = true diff --git a/crates/audio/src/audio.rs b/crates/audio/src/audio.rs index 511d00671ae99789610bac1f7e30b63ca29ac480..1478afa0c2b8bf5b5d633b11442f7038198109b0 100644 --- a/crates/audio/src/audio.rs +++ b/crates/audio/src/audio.rs @@ -174,8 +174,7 @@ impl Audio { .periodic_access(Duration::from_millis(100), move |agc_source| { agc_source.set_enabled(LIVE_SETTINGS.control_input_volume.load(Ordering::Relaxed)); }) - .replayable(REPLAY_DURATION) - .expect("REPLAY_DURATION is longer then 100ms"); + .replayable(REPLAY_DURATION)?; voip_parts .replays diff --git a/crates/audio/src/audio_settings.rs b/crates/audio/src/audio_settings.rs index 43edb8d60d96122d5515ec7274a6b5725b247ca0..ea0ea5f3558e015f5579cca43eeb8c529273cb52 100644 --- a/crates/audio/src/audio_settings.rs +++ b/crates/audio/src/audio_settings.rs @@ -59,6 +59,7 @@ impl Settings for AudioSettings { fn import_from_vscode(_vscode: &settings::VsCodeSettings, _current: &mut Self::FileContent) {} } +/// See docs on [LIVE_SETTINGS] pub(crate) struct LiveSettings { pub(crate) control_input_volume: AtomicBool, pub(crate) control_output_volume: AtomicBool, @@ -89,7 +90,9 @@ impl LiveSettings { } /// Allows access to settings from the audio thread. Updated by -/// observer of SettingsStore. +/// observer of SettingsStore. Needed because audio playback and recording are +/// real time and must each run in a dedicated OS thread, therefore we can not +/// use the background executor. pub(crate) static LIVE_SETTINGS: LiveSettings = LiveSettings { control_input_volume: AtomicBool::new(true), control_output_volume: AtomicBool::new(true), diff --git a/crates/audio/src/rodio_ext.rs b/crates/audio/src/rodio_ext.rs index 4e9430a0b9462448b879f653f9ddcb06ef892cdb..ba4e4ff0554dd3c9bc2a7e2691de270c0d00908b 100644 --- a/crates/audio/src/rodio_ext.rs +++ b/crates/audio/src/rodio_ext.rs @@ -9,7 +9,8 @@ use std::{ use crossbeam::queue::ArrayQueue; use rodio::{ChannelCount, Sample, SampleRate, Source}; -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] +#[error("Replay duration is too short must be >= 100ms")] pub struct ReplayDurationTooShort; pub trait RodioExt: Source + Sized { @@ -338,6 +339,7 @@ impl Iterator for Replayable { fn next(&mut self) -> Option { if let Some(sample) = self.inner.next() { self.buffer.push(sample); + // If the buffer is full send it if self.buffer.len() == self.chunk_size { self.tx.push_normal(std::mem::take(&mut self.buffer)); } @@ -422,6 +424,9 @@ impl Iterator for Replay { return None; } + // The queue does not support blocking on a next item. We want this queue as it + // is quite fast and provides a fixed size. We know how many samples are in a + // buffer so if we do not get one now we must be getting one after `sleep_duration`. std::thread::sleep(self.sleep_duration); } } diff --git a/crates/livekit_client/src/livekit_client/playback.rs b/crates/livekit_client/src/livekit_client/playback.rs index d1b2cee4aa1750ba4b8af3033e44b1fe9fbe78de..7c866113103a883e7e7a2d9d3f5651d833d7e637 100644 --- a/crates/livekit_client/src/livekit_client/playback.rs +++ b/crates/livekit_client/src/livekit_client/playback.rs @@ -186,6 +186,8 @@ impl AudioStack { let capture_task = if rodio_pipeline { info!("Using experimental.rodio_audio audio pipeline"); let voip_parts = audio::VoipParts::new(cx)?; + // Audio needs to run real-time and should never be paused. That is why we are using a + // normal std::thread and not a background task thread::spawn(move || { // microphone is non send on mac let microphone = audio::Audio::open_microphone(voip_parts)?; diff --git a/crates/livekit_client/src/livekit_client/playback/source.rs b/crates/livekit_client/src/livekit_client/playback/source.rs index 67bfe793902da94a114ca617ce5bfa33c68d02e7..f605b3d517cd816491f0eceadce5ac778ef75d21 100644 --- a/crates/livekit_client/src/livekit_client/playback/source.rs +++ b/crates/livekit_client/src/livekit_client/playback/source.rs @@ -12,9 +12,7 @@ fn frame_to_samplesbuffer(frame: AudioFrame) -> SamplesBuffer { let samples = SampleTypeConverter::<_, _>::new(samples); let samples: Vec = samples.collect(); SamplesBuffer::new( - // here be dragons - // NonZero::new(frame.num_channels as u16).expect("audio frame channels is nonzero"), - nz!(2), + nz!(2), // frame always has two channels NonZero::new(frame.sample_rate).expect("audio frame sample rate is nonzero"), samples, )