From fa3d785997a1d387442b0c5a728753acb9d776b7 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Tue, 2 Jun 2026 05:41:25 -0400 Subject: [PATCH] refactor(core): extract pure mixer fns + add saturation unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pull the audio mixer's inline math out of the async mixer-task closure into three pure, testable functions — mix_frames (sample-by-sample sum with i16 saturation), apply_volume (per-peer scale + clamp, unity-skip fast path), and frame_level (normalized RMS for the UI meter). Behavior-preserving: the loop now calls them and the full suite still passes. Adds 12 #[cfg(test)] unit tests, notably the saturation guards: a loud mix or a volume boost clamps to i16::MAX/MIN rather than wrapping (a plain cast would wrap a 2x-full-scale sum to a large negative value). Also covers sum, ragged peer-frame lengths, no-peers silence, volume unity/zero/half, and RMS bounds. Co-Authored-By: Claude Opus 4.8 --- src/core/mod.rs | 156 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 137 insertions(+), 19 deletions(-) diff --git a/src/core/mod.rs b/src/core/mod.rs index 8afc34d..9836c2d 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -111,6 +111,45 @@ fn arm_grace_timer( timers_guard.insert(peer_id, handle); } +/// Scale a frame in place by a per-peer volume factor, saturating to the i16 +/// range. A volume within `f32::EPSILON` of 1.0 is treated as unity and skipped, +/// matching the mixer hot path that avoids touching unmodified frames. +fn apply_volume(frame: &mut [i16], vol: f32) { + if (vol - 1.0).abs() <= f32::EPSILON { + return; + } + for sample in frame.iter_mut() { + *sample = (*sample as f32 * vol).clamp(i16::MIN as f32, i16::MAX as f32) as i16; + } +} + +/// Normalized RMS level of a frame in `[0.0, 1.0]` (32768 = full scale), for the +/// UI level meter. An empty frame reads as 0.0. +fn frame_level(frame: &[i16]) -> f32 { + let sum_sq: f32 = frame.iter().map(|&x| (x as f32).powi(2)).sum(); + let rms = (sum_sq / frame.len().max(1) as f32).sqrt(); + (rms / 32768.0).clamp(0.0, 1.0) +} + +/// Sum per-peer frames sample-by-sample into one `frame_len`-sample output, +/// saturating each summed sample to the i16 range so a loud mix clips rather than +/// wrapping. Peers shorter than `frame_len` contribute 0 past their end; an empty +/// peer set yields a silent frame. +fn mix_frames(peer_frames: &[Vec], frame_len: usize) -> Vec { + let mut mixed = vec![0i16; frame_len]; + if peer_frames.is_empty() { + return mixed; + } + for (i, out) in mixed.iter_mut().enumerate() { + let sum: i32 = peer_frames + .iter() + .map(|f| f.get(i).copied().unwrap_or(0) as i32) + .sum(); + *out = sum.clamp(i16::MIN as i32, i16::MAX as i32) as i16; + } + mixed +} + /// Handles the transport's per-peer link-state stream (`ConnEvent`): arms/cancels /// reconnect grace timers, tracks which peers we've linked with, and forwards /// link state to the UI. Pulled out of the conn-event task as a unit so the @@ -564,32 +603,16 @@ async fn run_core_loop( }; let vol = current_volumes.get(&peer_id).copied().unwrap_or(1.0); - if (vol - 1.0).abs() > f32::EPSILON { - for sample in frame.iter_mut() { - *sample = (*sample as f32 * vol).clamp(i16::MIN as f32, i16::MAX as f32) as i16; - } - } + apply_volume(&mut frame, vol); - let sum_sq: f32 = frame.iter().map(|&x| (x as f32).powi(2)).sum(); - let rms = (sum_sq / frame.len().max(1) as f32).sqrt(); - let level = (rms / 32768.0).clamp(0.0, 1.0); let peak = level_peaks.entry(peer_id).or_insert(0.0); - *peak = peak.max(level); + *peak = peak.max(frame_level(&frame)); peer_frames.push(frame); } } - let mut mixed = vec![0i16; FRAME_SAMPLES]; - if !peer_frames.is_empty() { - for (i, out) in mixed.iter_mut().enumerate() { - let sum: i32 = peer_frames - .iter() - .map(|f| f.get(i).copied().unwrap_or(0) as i32) - .sum(); - *out = sum.clamp(i16::MIN as i32, i16::MAX as i32) as i16; - } - } + let mixed = mix_frames(&peer_frames, FRAME_SAMPLES); let frame_to_send = if is_deafened_clone.load(Ordering::Relaxed) { vec![0i16; FRAME_SAMPLES] @@ -775,3 +798,98 @@ async fn run_core_loop( Ok(()) } + +#[cfg(test)] +mod tests { + use super::{apply_volume, frame_level, mix_frames}; + + #[test] + fn mix_of_no_peers_is_silence() { + let mixed = mix_frames(&[], 4); + assert_eq!(mixed, vec![0i16; 4]); + } + + #[test] + fn single_peer_passes_through_unchanged() { + let frame = vec![100, -200, 300, -400]; + let mixed = mix_frames(std::slice::from_ref(&frame), 4); + assert_eq!(mixed, frame); + } + + #[test] + fn two_peers_sum_sample_by_sample() { + let a = vec![100, -200, 300, -400]; + let b = vec![50, 200, -100, 400]; + let mixed = mix_frames(&[a, b], 4); + assert_eq!(mixed, vec![150, 0, 200, 0]); + } + + #[test] + fn loud_positive_mix_saturates_not_wraps() { + // Two near-full-scale positive frames sum to ~2x i16::MAX. A plain i16 + // cast would wrap to a large negative value; the mixer must clamp to MAX. + let a = vec![30_000; 4]; + let b = vec![30_000; 4]; + let mixed = mix_frames(&[a, b], 4); + assert_eq!(mixed, vec![i16::MAX; 4]); + } + + #[test] + fn loud_negative_mix_saturates_to_min() { + let a = vec![i16::MIN; 4]; + let b = vec![i16::MIN; 4]; + let mixed = mix_frames(&[a, b], 4); + assert_eq!(mixed, vec![i16::MIN; 4]); + } + + #[test] + fn shorter_peer_frame_contributes_zero_past_its_end() { + let full = vec![100, 100, 100, 100]; + let short = vec![10, 20]; // only first two samples + let mixed = mix_frames(&[full, short], 4); + assert_eq!(mixed, vec![110, 120, 100, 100]); + } + + #[test] + fn volume_unity_is_a_noop() { + let mut frame = vec![100, -200, 300, -400]; + apply_volume(&mut frame, 1.0); + assert_eq!(frame, vec![100, -200, 300, -400]); + } + + #[test] + fn volume_zero_mutes() { + let mut frame = vec![100, -200, 300, -400]; + apply_volume(&mut frame, 0.0); + assert_eq!(frame, vec![0, 0, 0, 0]); + } + + #[test] + fn volume_half_scales_samples() { + let mut frame = vec![100, -200, 300, -400]; + apply_volume(&mut frame, 0.5); + // 100*0.5=50, -200*0.5=-100, 300*0.5=150, -400*0.5=-200 (exact in f32 here) + assert_eq!(frame, vec![50, -100, 150, -200]); + } + + #[test] + fn volume_boost_saturates_not_wraps() { + // 20000 * 4.0 = 80000, well past i16::MAX — must clamp, not wrap. + let mut frame = vec![20_000, -20_000, 20_000, -20_000]; + apply_volume(&mut frame, 4.0); + assert_eq!(frame, vec![i16::MAX, i16::MIN, i16::MAX, i16::MIN]); + } + + #[test] + fn frame_level_of_silence_is_zero() { + assert_eq!(frame_level(&[0, 0, 0, 0]), 0.0); + assert_eq!(frame_level(&[]), 0.0); + } + + #[test] + fn frame_level_of_full_scale_is_about_one() { + let full = vec![i16::MAX; 64]; + let level = frame_level(&full); + assert!(level > 0.99 && level <= 1.0, "full-scale level was {level}"); + } +}