-
Notifications
You must be signed in to change notification settings - Fork 0
[02/09] audio: capture lifecycle fix + ALSA stderr suppression #124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f779e4a
b92498b
aa08f41
2d12d08
4a44dac
b0c61f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,2 @@ | ||
| pub mod vad_adapter; | ||
| pub mod vad_processor; | ||
|
|
||
| // Re-export modules from coldvox-audio crate | ||
| pub use coldvox_audio::{ | ||
| capture::CaptureStats, | ||
| chunker::{AudioChunker, ChunkerConfig, ResamplerQuality}, | ||
| frame_reader::FrameReader, | ||
| ring_buffer::{AudioProducer, AudioRingBuffer}, | ||
| }; | ||
|
|
||
| pub use coldvox_audio::AudioFrame; | ||
| pub use vad_adapter::*; | ||
| pub use vad_processor::*; |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -86,7 +86,7 @@ impl VadProcessor { | |||||
| timestamp_ms, | ||||||
| energy_db, | ||||||
| } => { | ||||||
| info!( | ||||||
| debug!( | ||||||
| "VAD: Speech started at {}ms (energy: {:.2} dB)", | ||||||
| timestamp_ms, energy_db | ||||||
| ); | ||||||
|
|
@@ -96,7 +96,7 @@ impl VadProcessor { | |||||
| duration_ms, | ||||||
| energy_db, | ||||||
| } => { | ||||||
| info!( | ||||||
| debug!( | ||||||
| "VAD: Speech ended at {}ms (duration: {}ms, energy: {:.2} dB)", | ||||||
| timestamp_ms, duration_ms, energy_db | ||||||
| ); | ||||||
|
|
@@ -126,14 +126,14 @@ impl VadProcessor { | |||||
|
|
||||||
| self.frames_processed += 1; | ||||||
|
|
||||||
| if self.frames_processed % 100 == 0 { | ||||||
| if self.frames_processed.is_multiple_of(100) { | ||||||
| tracing::debug!( | ||||||
| "VAD: Received {} frames, processing active", | ||||||
| self.frames_processed | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| if self.frames_processed % 1000 == 0 { | ||||||
| if self.frames_processed.is_multiple_of(1000) { | ||||||
|
||||||
| if self.frames_processed.is_multiple_of(1000) { | |
| if self.frames_processed % 1000 == 0 { |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -413,3 +413,6 @@ pub mod text_injection; | |
| #[cfg(feature = "tui")] | ||
| pub mod tui; | ||
| pub mod vad; | ||
|
|
||
| #[cfg(test)] | ||
| pub mod test_utils; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,8 +48,9 @@ pub struct AudioCaptureThread { | |
| impl AudioCaptureThread { | ||
| pub fn spawn( | ||
| config: AudioConfig, | ||
| audio_producer: AudioProducer, | ||
| audio_producer: Arc<Mutex<AudioProducer>>, | ||
| device_name: Option<String>, | ||
| enable_device_monitor: bool, | ||
|
Comment on lines
48
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The spawn signature now requires an Useful? React with 👍 / 👎. |
||
| ) -> Result< | ||
|
Comment on lines
49
to
54
|
||
| ( | ||
| Self, | ||
|
|
@@ -59,7 +60,11 @@ impl AudioCaptureThread { | |
| ), | ||
| AudioError, | ||
| > { | ||
| let running = Arc::new(AtomicBool::new(false)); | ||
| // Start in running state so the device monitor thread stays alive | ||
| // until we explicitly stop via `stop()`. Previously this was false, | ||
| // causing the monitor to exit immediately and the capture loop to | ||
| // detect a closed channel and terminate early. | ||
| let running = Arc::new(AtomicBool::new(true)); | ||
| let shutdown = running.clone(); | ||
| let device_config = Arc::new(RwLock::new(None::<DeviceConfig>)); | ||
| let device_config_clone = device_config.clone(); | ||
|
|
@@ -72,14 +77,21 @@ impl AudioCaptureThread { | |
| let (device_event_tx, device_event_rx) = tokio::sync::broadcast::channel(32); | ||
| let device_event_tx_clone = device_event_tx.clone(); | ||
|
|
||
| // Start device monitor | ||
| let (device_monitor, mut monitor_rx) = DeviceMonitor::new(Duration::from_millis(500))?; | ||
| // Start device monitor with 2-second interval to reduce false positives from CPAL enumeration glitches | ||
| let monitor_running = running.clone(); | ||
| let monitor_handle = device_monitor.start_monitoring(monitor_running); | ||
| let (monitor_rx_opt, monitor_handle) = if enable_device_monitor { | ||
| let (device_monitor, monitor_rx) = DeviceMonitor::new(Duration::from_secs(2))?; | ||
| let handle = device_monitor.start_monitoring(monitor_running); | ||
| (Some(monitor_rx), Some(handle)) | ||
| } else { | ||
| tracing::debug!("Device monitor disabled; skipping hotplug polling"); | ||
| (None, None) | ||
| }; | ||
|
|
||
| let handle = thread::Builder::new() | ||
| .name("audio-capture".to_string()) | ||
| .spawn(move || { | ||
| let mut monitor_rx = monitor_rx_opt; | ||
| let mut capture = match AudioCapture::new(config, audio_producer, running.clone()) { | ||
| Ok(c) => c.with_config_channel(config_tx_clone) | ||
| .with_device_event_channel(device_event_tx_clone), | ||
|
|
@@ -144,40 +156,42 @@ impl AudioCaptureThread { | |
| let mut restart_reason = "unknown"; | ||
|
|
||
| // Check for device monitor events | ||
| match monitor_rx.try_recv() { | ||
| Ok(event) => { | ||
| tracing::debug!("Device event: {:?}", event); | ||
| let _ = capture.device_event_tx.as_ref().map(|tx| tx.send(event.clone())); | ||
|
|
||
| match event { | ||
| DeviceEvent::CurrentDeviceDisconnected { name } => { | ||
| if capture.current_device_name.as_ref() == Some(&name) { | ||
| tracing::warn!("Current device {} disconnected, attempting recovery", name); | ||
| if let Some(rx) = monitor_rx.as_mut() { | ||
| match rx.try_recv() { | ||
| Ok(event) => { | ||
| tracing::debug!("Device event: {:?}", event); | ||
| let _ = capture.device_event_tx.as_ref().map(|tx| tx.send(event.clone())); | ||
|
|
||
| match event { | ||
| DeviceEvent::CurrentDeviceDisconnected { name } => { | ||
| if capture.current_device_name.as_ref() == Some(&name) { | ||
| tracing::warn!("Current device {} disconnected, attempting recovery", name); | ||
| needs_restart = true; | ||
| restart_reason = "device disconnected"; | ||
| } | ||
| } | ||
| DeviceEvent::DeviceAdded { name } => { | ||
| tracing::info!("New device available: {}", name); | ||
| // Could implement automatic switching to preferred devices here | ||
| } | ||
| DeviceEvent::DeviceSwitchRequested { target } => { | ||
| tracing::info!("Manual device switch requested to: {}", target); | ||
| needs_restart = true; | ||
| restart_reason = "device disconnected"; | ||
| restart_reason = "manual device switch requested"; | ||
| } | ||
| _ => {} | ||
| } | ||
| DeviceEvent::DeviceAdded { name } => { | ||
| tracing::info!("New device available: {}", name); | ||
| // Could implement automatic switching to preferred devices here | ||
| } | ||
| DeviceEvent::DeviceSwitchRequested { target } => { | ||
| tracing::info!("Manual device switch requested to: {}", target); | ||
| needs_restart = true; | ||
| restart_reason = "manual device switch requested"; | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
| Err(tokio::sync::broadcast::error::TryRecvError::Empty) => { | ||
| // No events, continue | ||
| } | ||
| Err(tokio::sync::broadcast::error::TryRecvError::Lagged(_)) => { | ||
| tracing::warn!("Device monitor events lagged, some events may have been missed"); | ||
| } | ||
| Err(tokio::sync::broadcast::error::TryRecvError::Closed) => { | ||
| tracing::error!("Device monitor channel closed"); | ||
| break; | ||
| Err(tokio::sync::broadcast::error::TryRecvError::Empty) => { | ||
| // No events, continue | ||
| } | ||
| Err(tokio::sync::broadcast::error::TryRecvError::Lagged(_)) => { | ||
| tracing::warn!("Device monitor events lagged, some events may have been missed"); | ||
| } | ||
| Err(tokio::sync::broadcast::error::TryRecvError::Closed) => { | ||
| tracing::error!("Device monitor channel closed"); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -240,7 +254,7 @@ impl AudioCaptureThread { | |
| thread::sleep(Duration::from_millis(100)); | ||
| } | ||
|
|
||
| tracing::info!("Audio capture thread shutting down."); | ||
| tracing::debug!("Audio capture thread shutting down."); | ||
| capture.stop(); | ||
| }) | ||
| .map_err(|e| AudioError::Fatal(format!("Failed to spawn audio thread: {}", e)))?; | ||
|
|
@@ -264,7 +278,7 @@ impl AudioCaptureThread { | |
| Self { | ||
| handle, | ||
| shutdown, | ||
| device_monitor_handle: Some(monitor_handle), | ||
| device_monitor_handle: monitor_handle, | ||
| }, | ||
| cfg, | ||
| config_rx, | ||
|
|
@@ -303,13 +317,13 @@ pub struct CaptureStats { | |
| impl AudioCapture { | ||
| pub fn new( | ||
| config: AudioConfig, | ||
| audio_producer: AudioProducer, | ||
| audio_producer: Arc<Mutex<AudioProducer>>, | ||
| running: Arc<AtomicBool>, | ||
| ) -> Result<Self, AudioError> { | ||
| let self_ = Self { | ||
| device_manager: DeviceManager::new()?, | ||
| stream: None, | ||
| audio_producer: Arc::new(Mutex::new(audio_producer)), | ||
| audio_producer, | ||
| watchdog: WatchdogTimer::new(Duration::from_secs(5)), | ||
| silence_detector: SilenceDetector::new(config.silence_threshold), | ||
| stats: Arc::new(CaptureStats::default()), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,10 +23,18 @@ impl SilenceDetector { | |
| let sum: i64 = samples.iter().map(|&s| s as i64 * s as i64).sum(); | ||
| let rms = ((sum / samples.len() as i64) as f64).sqrt() as i16; | ||
|
|
||
| // Log RMS every time to see actual audio levels (use trace level to avoid spam) | ||
| tracing::trace!( | ||
| "SilenceDetector: RMS={}, threshold={}, samples={}", | ||
| rms, | ||
| self.threshold, | ||
| samples.len() | ||
| ); | ||
|
Comment on lines
+27
to
+32
|
||
|
|
||
| if rms < self.threshold { | ||
| if self.silence_start.is_none() { | ||
| self.silence_start = Some(Instant::now()); | ||
| tracing::debug!( | ||
| tracing::info!( | ||
| "SilenceDetector: Silence started (RMS {} < threshold {})", | ||
| rms, | ||
| self.threshold | ||
|
Comment on lines
+37
to
40
|
||
|
|
@@ -36,7 +44,7 @@ impl SilenceDetector { | |
| } else { | ||
| if self.silence_start.is_some() { | ||
| let duration = self.silence_duration(); | ||
| tracing::debug!( | ||
| tracing::info!( | ||
| "SilenceDetector: Silence ended after {:?} (RMS {} >= threshold {})", | ||
| duration, | ||
| rms, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Primitive integers do not have is_multiple_of in stable Rust; this will not compile. Use a modulo check instead: self.frames_processed % 100 == 0.