From c0815bf40310a4e90bf79be2d6e9dc97c9c4dbd1 Mon Sep 17 00:00:00 2001 From: Mal Detair Date: Sat, 7 Feb 2026 21:28:16 +0100 Subject: [PATCH] fix: resolve 11 issues from full project review Critical: - Move NIH-plug theme setup from render callback to init callback (perf) - Fix LV2 dynamic_threshold=true making threshold port useless - Handle GlobalHotKeyManager init failure gracefully instead of panic Important: - Persist reference device selection in config across restarts - Mix (L+R)/2 for NIH-plug mono output instead of discarding right channel - Pre-create all 4 VAD instances to avoid allocation on audio thread - Log reference device enumeration errors instead of silently swallowing Suggestions: - Name audio processing threads for easier debugging - Cache eq_enabled/agc_enabled per-frame in process_updates() - Replace eprintln with log::warn in echo_cancel.rs Co-Authored-By: Claude Opus 4.6 --- crates/app/src/audio.rs | 18 ++++++++------ crates/app/src/config.rs | 6 +++++ crates/app/src/gui.rs | 40 ++++++++++++++++++++++--------- crates/core/src/echo_cancel.rs | 2 +- crates/core/src/processor.rs | 43 +++++++++++++++++----------------- crates/lv2/src/lib.rs | 2 +- crates/plugin/src/lib.rs | 11 +++++---- 7 files changed, 75 insertions(+), 47 deletions(-) diff --git a/crates/app/src/audio.rs b/crates/app/src/audio.rs index 12e0bb4..3a6df6a 100644 --- a/crates/app/src/audio.rs +++ b/crates/app/src/audio.rs @@ -99,9 +99,13 @@ impl AudioEngine { let dev = if ref_name == "default" { host.default_input_device() } else { - host.input_devices() - .ok() - .and_then(|mut devs| devs.find(|d| d.name().ok().as_deref() == Some(ref_name))) + match host.input_devices() { + Ok(mut devs) => devs.find(|d| d.name().ok().as_deref() == Some(ref_name)), + Err(e) => { + warn!("Failed to enumerate input devices for reference: {}", e); + None + } + } }; if let Some(d) = &dev { info!("Using reference device: {}", d.name().unwrap_or_default()); @@ -228,7 +232,7 @@ impl AudioEngine { let has_reference = echo_cancel_enabled && reference_stream.is_some(); - thread::spawn(move || { + thread::Builder::new().name("voidmic-audio".into()).spawn(move || { let mut input_frame = [0.0f32; FRAME_SIZE]; let mut output_frame = [0.0f32; FRAME_SIZE]; let mut ref_frame = [0.0f32; FRAME_SIZE]; @@ -306,7 +310,7 @@ impl AudioEngine { thread::sleep(Duration::from_micros(200)); } } - }); + }).context("Failed to spawn audio processing thread")?; input_stream.play()?; output_stream.play()?; @@ -428,7 +432,7 @@ impl OutputFilterEngine { let suppression_atomic = Arc::new(AtomicU32::new(suppression_strength.to_bits())); let suppression_for_thread = suppression_atomic.clone(); - thread::spawn(move || { + thread::Builder::new().name("voidmic-output-filter".into()).spawn(move || { let mut denoise = DenoiseState::new(); let mut input_frame = [0.0f32; FRAME_SIZE]; let mut output_frame = [0.0f32; FRAME_SIZE]; @@ -462,7 +466,7 @@ impl OutputFilterEngine { thread::sleep(Duration::from_micros(500)); } } - }); + }).context("Failed to spawn output filter thread")?; input_stream.play()?; output_stream.play()?; diff --git a/crates/app/src/config.rs b/crates/app/src/config.rs index fe771ec..1812245 100644 --- a/crates/app/src/config.rs +++ b/crates/app/src/config.rs @@ -54,6 +54,9 @@ pub struct AppConfig { #[serde(default = "default_agc_target")] pub agc_target_level: f32, + #[serde(default)] + pub last_reference: String, + // Phase 6 #[serde(default)] pub mini_mode: bool, @@ -117,6 +120,7 @@ impl Default for AppConfig { eq_high_gain: 0.0, agc_enabled: false, agc_target_level: default_agc_target(), + last_reference: String::new(), mini_mode: false, } } @@ -202,6 +206,7 @@ mod tests { eq_high_gain: 0.0, agc_enabled: false, agc_target_level: 0.7, + last_reference: String::new(), mini_mode: false, }; @@ -250,6 +255,7 @@ mod tests { eq_high_gain: 0.0, agc_enabled: true, agc_target_level: 0.8, + last_reference: "Monitor of Speakers".to_string(), mini_mode: true, }; diff --git a/crates/app/src/gui.rs b/crates/app/src/gui.rs index 80cd4fa..4bebcd6 100644 --- a/crates/app/src/gui.rs +++ b/crates/app/src/gui.rs @@ -132,7 +132,7 @@ struct VoidMicApp { selected_reference: String, // Global Hotkeys #[allow(dead_code)] // Manager must be kept alive - hotkey_manager: GlobalHotKeyManager, + hotkey_manager: Option, hotkey_id: Option, // Wizard State show_wizard: bool, @@ -190,10 +190,14 @@ impl VoidMicApp { .unwrap_or_else(|| "default".to_string()) }; - let default_ref = inputs - .first() - .cloned() - .unwrap_or_else(|| "default".to_string()); + let default_ref = if !config.last_reference.is_empty() && inputs.contains(&config.last_reference) { + config.last_reference.clone() + } else { + inputs + .first() + .cloned() + .unwrap_or_else(|| "default".to_string()) + }; let auto_start = config.auto_start_processing; let show_wizard = config.first_run; @@ -220,7 +224,13 @@ impl VoidMicApp { virtual_sink_cached: false, last_sink_check: std::time::Instant::now() - std::time::Duration::from_secs(5), selected_reference: default_ref, - hotkey_manager: GlobalHotKeyManager::new().unwrap(), + hotkey_manager: match GlobalHotKeyManager::new() { + Ok(m) => Some(m), + Err(e) => { + log::warn!("Failed to initialize global hotkey manager: {:?}", e); + None + } + }, hotkey_id: None, show_wizard, wizard_step: WizardStep::Welcome, @@ -231,11 +241,13 @@ impl VoidMicApp { }; // Register Hotkey - if let Ok(hotkey) = app.config.toggle_hotkey.parse::() { - if app.hotkey_manager.register(hotkey).is_ok() { - app.hotkey_id = Some(hotkey.id()); - } else { - log::warn!("Failed to register hotkey: {}", app.config.toggle_hotkey); + if let Some(ref manager) = app.hotkey_manager { + if let Ok(hotkey) = app.config.toggle_hotkey.parse::() { + if manager.register(hotkey).is_ok() { + app.hotkey_id = Some(hotkey.id()); + } else { + log::warn!("Failed to register hotkey: {}", app.config.toggle_hotkey); + } } } @@ -366,6 +378,7 @@ impl VoidMicApp { if self.config_dirty { self.config.last_input = self.selected_input.clone(); self.config.last_output = self.selected_output.clone(); + self.config.last_reference = self.selected_reference.clone(); // gate_threshold is already in config from slider updates self.config.save(); self.config_dirty = false; @@ -375,6 +388,7 @@ impl VoidMicApp { fn save_config_now(&mut self) { self.config.last_input = self.selected_input.clone(); self.config.last_output = self.selected_output.clone(); + self.config.last_reference = self.selected_reference.clone(); self.config.save(); } @@ -706,6 +720,7 @@ impl VoidMicApp { if self.config.echo_cancel_enabled || self.config.output_filter_enabled { ui.horizontal(|ui| { ui.label("Reference Input (Monitor):"); + let prev_ref = self.selected_reference.clone(); egui::ComboBox::from_id_salt("ref_combo") .selected_text(&self.selected_reference) .width(200.0) @@ -715,6 +730,9 @@ impl VoidMicApp { ui.selectable_value(&mut self.selected_reference, dev.clone(), dev); } }); + if self.selected_reference != prev_ref { + self.mark_config_dirty(); + } ui.label(egui::RichText::new("ℹ️ Select speaker monitor").size(10.0)); }); } diff --git a/crates/core/src/echo_cancel.rs b/crates/core/src/echo_cancel.rs index 02a4251..9f97ac5 100644 --- a/crates/core/src/echo_cancel.rs +++ b/crates/core/src/echo_cancel.rs @@ -42,7 +42,7 @@ impl EchoCanceller { .aec .process(mic_input, Some(speaker_ref), false, &mut self.output_buffer) { - eprintln!("AEC error: {:?}", e); + log::warn!("AEC error: {:?}", e); output.copy_from_slice(mic_input); // Fallback to raw input return false; } diff --git a/crates/core/src/processor.rs b/crates/core/src/processor.rs index 8fedf59..4a3ab68 100644 --- a/crates/core/src/processor.rs +++ b/crates/core/src/processor.rs @@ -214,7 +214,7 @@ pub struct VoidProcessor { eq: Vec, agc_limiter: LookaheadLimiter, noise_floor_tracker: NoiseFloorTracker, - vad: Vad, + vad_instances: [Vad; 4], // Pre-created for all VadMode variants to avoid RT allocation channels: usize, // State @@ -228,6 +228,8 @@ pub struct VoidProcessor { // Current Settings (Locally cached to avoid atomic load every sample) current_vad_mode: i32, + current_eq_enabled: bool, + current_agc_enabled: bool, current_eq_low: f32, current_eq_mid: f32, current_eq_high: f32, @@ -275,15 +277,12 @@ impl VoidProcessor { agc_target_level: f32, echo_cancel_enabled: bool, ) -> Self { - let vad = Vad::new_with_rate_and_mode( - webrtc_vad::SampleRate::Rate48kHz, - match vad_sensitivity { - 0 => VadMode::Quality, - 1 => VadMode::LowBitrate, - 2 => VadMode::Aggressive, - _ => VadMode::VeryAggressive, - }, - ); + let vad_instances = [ + Vad::new_with_rate_and_mode(webrtc_vad::SampleRate::Rate48kHz, VadMode::Quality), + Vad::new_with_rate_and_mode(webrtc_vad::SampleRate::Rate48kHz, VadMode::LowBitrate), + Vad::new_with_rate_and_mode(webrtc_vad::SampleRate::Rate48kHz, VadMode::Aggressive), + Vad::new_with_rate_and_mode(webrtc_vad::SampleRate::Rate48kHz, VadMode::VeryAggressive), + ]; let mut denoise = Vec::with_capacity(channels); let mut echo_canceller = Vec::with_capacity(channels); @@ -315,7 +314,7 @@ impl VoidProcessor { eq, agc_limiter: LookaheadLimiter::new(agc_target_level), noise_floor_tracker: NoiseFloorTracker::new(), - vad, + vad_instances, channels, gate_open: false, @@ -327,6 +326,8 @@ impl VoidProcessor { calibration_samples: Vec::with_capacity(300), // Pre-alloc for ~3s calibration current_vad_mode: vad_sensitivity, + current_eq_enabled: true, + current_agc_enabled: false, current_eq_low: eq_params.0, current_eq_mid: eq_params.1, current_eq_high: eq_params.2, @@ -361,14 +362,7 @@ impl VoidProcessor { // Check for settings updates let new_vad = self.vad_sensitivity.load(Ordering::Relaxed) as i32; if new_vad != self.current_vad_mode { - self.current_vad_mode = new_vad; - let vad_mode = match self.current_vad_mode { - 0 => VadMode::Quality, - 1 => VadMode::LowBitrate, - 2 => VadMode::Aggressive, - _ => VadMode::VeryAggressive, - }; - self.vad = Vad::new_with_rate_and_mode(webrtc_vad::SampleRate::Rate48kHz, vad_mode); + self.current_vad_mode = new_vad.clamp(0, 3); } if !self.eq.is_empty() { @@ -403,6 +397,10 @@ impl VoidProcessor { _ => {} } + // Cache EQ and AGC enabled state + self.current_eq_enabled = self.eq_enabled.load(Ordering::Relaxed); + self.current_agc_enabled = self.agc_enabled.load(Ordering::Relaxed); + // Check AGC settings let target_bits = self.agc_target.load(Ordering::Relaxed); let new_target = f32::from_bits(target_bits); @@ -517,7 +515,8 @@ impl VoidProcessor { for i in 0..FRAME_SIZE { vad_buffer[i] = (mono_mix[i] * 32767.0).clamp(-32768.0, 32767.0) as i16; } - let is_speech = self.vad.is_voice_segment(&vad_buffer).unwrap_or(false); + let vad_idx = self.current_vad_mode.clamp(0, 3) as usize; + let is_speech = self.vad_instances[vad_idx].is_voice_segment(&vad_buffer).unwrap_or(false); let attack_samples = (SAMPLE_RATE / 1000) * ATTACK_MS; let release_samples = (SAMPLE_RATE / 1000) * RELEASE_MS; @@ -560,7 +559,7 @@ impl VoidProcessor { } // Equalizer - if self.eq_enabled.load(Ordering::Relaxed) { + if self.current_eq_enabled { if let Some(eq) = self.eq.get_mut(i) { for sample in output_ch.iter_mut() { *sample = eq.process(*sample); @@ -577,7 +576,7 @@ impl VoidProcessor { } // AGC (Linked) - if self.agc_enabled.load(Ordering::Relaxed) { + if self.current_agc_enabled { self.agc_limiter.process_frame(output_frames); } } diff --git a/crates/lv2/src/lib.rs b/crates/lv2/src/lib.rs index 1246e5c..fdfb380 100644 --- a/crates/lv2/src/lib.rs +++ b/crates/lv2/src/lib.rs @@ -101,7 +101,7 @@ impl Plugin for VoidMic { None, suppression, threshold, - true, + false, // Use explicit threshold from control port, not dynamic ); for j in 0..FRAME_SIZE { diff --git a/crates/plugin/src/lib.rs b/crates/plugin/src/lib.rs index 48161a1..f47c166 100644 --- a/crates/plugin/src/lib.rs +++ b/crates/plugin/src/lib.rs @@ -133,9 +133,10 @@ impl Plugin for VoidMicPlugin { create_egui_editor( self.params.editor_state.clone(), gui_data, - |_, _| {}, - move |egui_ctx, setter, state| { + |egui_ctx, _| { theme::setup_custom_style(egui_ctx, true); + }, + move |egui_ctx, setter, state| { egui::CentralPanel::default().show(egui_ctx, |ui| { ui.heading("VoidMic Plugin"); @@ -306,10 +307,10 @@ impl Plugin for VoidMicPlugin { let l = rb_out.try_pop().unwrap_or(0.0); let r = rb_out.try_pop().unwrap_or(0.0); - if num_channels >= 1 { + if num_channels == 1 { + channel_data[0][i] = (l + r) * 0.5; + } else { channel_data[0][i] = l; - } // If output is mono, take left? Or mix? Left is safer. - if num_channels >= 2 { channel_data[1][i] = r; } }