test recorder ring semantics; extract mix_tick

1 files changed, 187 insertions, 50 deletions

diff --git a/src/recording/mod.rs b/src/recording/mod.rs
index 6ee4b11..74ec516 100644
--- a/src/recording/mod.rs
+++ b/src/recording/mod.rs
@@ -28,7 +28,6 @@
 
 use std::process::Stdio;
 use std::sync::{Arc, Mutex, Weak};
-use std::time::Instant;
 
 use poise::serenity_prelude as serenity;
 use songbird::{Event, EventContext, EventHandler};
@@ -53,7 +52,6 @@ struct Inner {
     head: usize,
     /// Monotonic count of 20 ms frames written since recording started.
     frames_written: u64,
-    started: Instant,
 }
 
 impl Recorder {
@@ -71,7 +69,6 @@ impl Recorder {
                 ring: vec![0i16; window_samples],
                 head: 0,
                 frames_written: 0,
-                started: Instant::now(),
             }),
             sample_rate,
             samples_per_frame,
@@ -83,11 +80,17 @@ impl Recorder {
         self.sample_rate
     }
 
-    /// Snapshot `min(elapsed, window, written)` seconds of audio, oldest→newest,
-    /// as little-endian mono i16 bytes (the `s16le` format ffmpeg consumes).
+    /// Snapshot the last `min(frames_written, window)` frames of audio,
+    /// oldest→newest, as little-endian mono i16 bytes (the `s16le` format
+    /// ffmpeg consumes).
     ///
     /// Read-only: the ring is left intact (flush-and-keep, per the `/record`
     /// design — successive calls may overlap).
+    ///
+    /// `frames_written` is authoritative: the ring holds exactly the last
+    /// `min(frames_written, TICKS_PER_SEC * window_seconds)` frames, each
+    /// written by one real 20 ms tick. A wall-clock term would drift from the
+    /// mixer whenever ticks stall or bunch; the ring is the ground truth.
     pub fn flush(&self) -> Vec<u8> {
         let inner = match self.inner.lock() {
             Ok(g) => g,
@@ -97,33 +100,67 @@ impl Recorder {
             }
         };
         let len = self.window_samples;
-        let spf = self.samples_per_frame;
-        let written_samples = inner
+        let d = inner
             .frames_written
-            .saturating_mul(spf as u64)
+            .saturating_mul(self.samples_per_frame as u64)
             .min(len as u64) as usize;
-        let elapsed_samples =
-            (inner.started.elapsed().as_secs_f64() * self.sample_rate as f64) as usize;
-        let d = written_samples.min(elapsed_samples).min(len);
         if d == 0 {
             return Vec::new();
         }
         // Oldest live sample sits at `head - d` (mod len); read `d` forward.
         let start = (inner.head + len - d) % len;
-        let mut samples: Vec<i16> = Vec::with_capacity(d);
+        let mut bytes = Vec::with_capacity(d * 2);
         if start + d <= len {
-            samples.extend_from_slice(&inner.ring[start..start + d]);
+            push_le_i16(&mut bytes, &inner.ring[start..start + d]);
         } else {
             let first = len - start;
-            samples.extend_from_slice(&inner.ring[start..]);
-            samples.extend_from_slice(&inner.ring[..d - first]);
-        }
-        let mut bytes = Vec::with_capacity(samples.len() * 2);
-        for s in &samples {
-            bytes.extend_from_slice(&s.to_le_bytes());
+            push_le_i16(&mut bytes, &inner.ring[start..]);
+            push_le_i16(&mut bytes, &inner.ring[..d - first]);
         }
         bytes
     }
+
+    /// Absorb one 20 ms tick: zero the window about to be written (silence
+    /// placement), saturating-add each speaker's decoded mono i16 frame, then
+    /// advance `head` by one frame.
+    ///
+    /// Split out from the `EventHandler` so tests can drive the ring without
+    /// constructing songbird's `#[non_exhaustive]` `VoiceTick`.
+    pub fn mix_tick(&self, speakers: &[Vec<i16>]) {
+        let mut inner = match self.inner.lock() {
+            Ok(g) => g,
+            Err(_) => return,
+        };
+        let spf = self.samples_per_frame;
+        let len = self.window_samples;
+
+        // Zero the window about to be written. This is the silence-placement
+        // step: it clears samples that aged out one window-length ago, so a
+        // tick with no speakers writes pure silence rather than stale audio.
+        for i in 0..spf {
+            let pos = (inner.head + i) % len;
+            inner.ring[pos] = 0;
+        }
+        // Mix every speaking SSRC's decoded mono i16 into the window.
+        // `saturating_add` prevents wraparound when several speakers overlap
+        // (the only real failure mode on the hot path); multi-speaker overlaps
+        // may clip slightly, the correct trade for a 50 Hz × N-packet mix.
+        for audio in speakers {
+            let n = audio.len().min(spf);
+            for i in 0..n {
+                let pos = (inner.head + i) % len;
+                inner.ring[pos] = inner.ring[pos].saturating_add(audio[i]);
+            }
+        }
+        inner.head = (inner.head + spf) % len;
+        inner.frames_written += 1;
+    }
+}
+
+fn push_le_i16(out: &mut Vec<u8>, samples: &[i16]) {
+    for s in samples {
+        out.extend_from_slice(&s.to_le_bytes());
+    }
 }
 
 /// `songbird::EventHandler` registered on each `Call` while recording is
@@ -140,7 +177,6 @@ impl RecorderEventHandler {
         }
     }
 }
-
 #[async_trait::async_trait]
 impl EventHandler for RecorderEventHandler {
     async fn act(&self, ctx: &EventContext<'_>) -> Option<Event> {
@@ -154,36 +190,12 @@ impl EventHandler for RecorderEventHandler {
             // until songbird drops the Call and us along with it.
             return None;
         };
-        let spf = rec.samples_per_frame;
-        let len = rec.window_samples;
-        let mut inner = match rec.inner.lock() {
-            Ok(g) => g,
-            Err(_) => return None,
-        };
-
-        // Zero the window about to be written. This is the silence-placement
-        // step: it clears samples that aged out one window-length ago, so a
-        // tick with no speakers writes pure silence rather than stale audio.
-        for i in 0..spf {
-            let pos = (inner.head + i) % len;
-            inner.ring[pos] = 0;
-        }
-        // Mix every speaking SSRC's decoded mono i16 into the window.
-        // `saturating_add` prevents wraparound when several speakers overlap
-        // (the only real failure mode on the hot path); multi-speaker overlaps
-        // may clip slightly, the correct trade for a 50 Hz × N-packet mix.
-        for vd in tick.speaking.values() {
-            if let Some(audio) = vd.decoded_voice.as_ref() {
-                let n = audio.len().min(spf);
-                for i in 0..n {
-                    let pos = (inner.head + i) % len;
-                    inner.ring[pos] = inner.ring[pos].saturating_add(audio[i]);
-                }
-            }
-        }
-        inner.head = (inner.head + spf) % len;
-        inner.frames_written += 1;
-        let _ = rec; // drop the strong ref obtained via upgrade()
+        let frames: Vec<Vec<i16>> = tick
+            .speaking
+            .values()
+            .filter_map(|vd| vd.decoded_voice.clone())
+            .collect();
+        rec.mix_tick(&frames);
         None // keep listening
     }
 }
@@ -306,3 +318,128 @@ pub async fn on_bot_voice_state(
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Decode the `s16le` byte stream `flush` produces back into i16 samples.
+    fn to_i16(bytes: &[u8]) -> Vec<i16> {
+        bytes
+            .chunks_exact(2)
+            .map(|c| i16::from_le_bytes([c[0], c[1]]))
+            .collect()
+    }
+
+    #[test]
+    fn empty_recorder_flushes_nothing() {
+        let rec = Recorder::new(48_000, 1);
+        assert!(rec.flush().is_empty());
+    }
+
+    #[test]
+    fn single_speaker_values_survive() {
+        let rec = Recorder::new(48_000, 1);
+        let spf = rec.samples_per_frame;
+        // 10 ticks of a constant 100.
+        for _ in 0..10 {
+            rec.mix_tick(&[vec![100i16; spf]]);
+        }
+        let out = to_i16(&rec.flush());
+        assert_eq!(out.len(), 10 * spf);
+        assert!(out.iter().all(|&s| s == 100));
+    }
+
+    #[test]
+    fn silent_tick_writes_silence_not_stale_audio() {
+        let rec = Recorder::new(48_000, 1);
+        let spf = rec.samples_per_frame;
+        // Fill the whole 50-frame window with 100s.
+        for _ in 0..TICKS_PER_SEC {
+            rec.mix_tick(&[vec![100i16; spf]]);
+        }
+        // A tick with no speakers must age out one frame of the 100s…
+        rec.mix_tick(&[]);
+        let out = to_i16(&rec.flush());
+        assert_eq!(out.len(), TICKS_PER_SEC * spf);
+        // …and write 960 pure-silence samples at the *newest* end, keeping
+        // 49 frames of the old audio as the oldest content.
+        assert!(out[out.len() - spf..].iter().all(|&s| s == 0));
+        assert!(out[..(TICKS_PER_SEC - 1) * spf].iter().all(|&s| s == 100));
+    }
+
+    #[test]
+    fn two_speakers_mix_saturating() {
+        let rec = Recorder::new(48_000, 1);
+        let spf = rec.samples_per_frame;
+        rec.mix_tick(&[vec![20000i16; spf], vec![10000i16; spf]]);
+        let out = to_i16(&rec.flush());
+        assert!(out.iter().all(|&s| s == 30000));
+
+        // Overflow saturates instead of wrapping.
+        let rec = Recorder::new(48_000, 1);
+        rec.mix_tick(&[vec![30000i16; spf], vec![30000i16; spf]]);
+        let out = to_i16(&rec.flush());
+        assert!(out.iter().all(|&s| s == i16::MAX));
+    }
+
+    #[test]
+    fn ring_wraps_and_flush_orders_oldest_to_newest() {
+        // 2-frame window keeps the wrap tight and the assertion unambiguous.
+        // window_samples = 2 * spf ⇒ a whole number of 20 ms frames.
+        let rec = Recorder::new(48_000, 0); // 0 s → 0 samples; not a whole frame…
+        // …so build one manually with a 2-frame window instead.
+        drop(rec);
+        let spf = 960usize;
+        let rec = Recorder {
+            inner: Mutex::new(Inner {
+                ring: vec![0i16; 2 * spf],
+                head: 0,
+                frames_written: 0,
+            }),
+            sample_rate: 48_000,
+            samples_per_frame: spf,
+            window_samples: 2 * spf,
+        };
+        // Frame A = 7s, then frame B = 9s, then frame C = 11s (overwrites A).
+        rec.mix_tick(&[vec![7i16; spf]]);
+        rec.mix_tick(&[vec![9i16; spf]]);
+        rec.mix_tick(&[vec![11i16; spf]]);
+        let out = to_i16(&rec.flush());
+        assert_eq!(out.len(), 2 * spf);
+        // Oldest→newest must be B, C — the wrapped-over A is gone.
+        assert!(out[..spf].iter().all(|&s| s == 9));
+        assert!(out[spf..].iter().all(|&s| s == 11));
+    }
+
+    #[test]
+    fn flush_is_read_only() {
+        let rec = Recorder::new(48_000, 1);
+        let spf = rec.samples_per_frame;
+        rec.mix_tick(&[vec![50i16; spf]]);
+        let first = to_i16(&rec.flush());
+        let second = to_i16(&rec.flush());
+        assert_eq!(first, second);
+    }
+
+    #[tokio::test]
+    async fn encode_mp3_produces_mp3_frames() {
+        // 1 s of a 440 Hz sine at 48 kHz mono.
+        let sample_rate = 48_000u32;
+        let mut pcm = Vec::with_capacity(sample_rate as usize * 2);
+        for n in 0..sample_rate {
+            let t = n as f64 / sample_rate as f64;
+            let s = (2.0 * std::f64::consts::PI * 440.0 * t).sin() * 8000.0;
+            pcm.extend_from_slice(&(s as i16).to_le_bytes());
+        }
+        let mp3 = encode_mp3(&pcm, sample_rate)
+            .await
+            .expect("mp3 encode failed");
+        assert!(!mp3.is_empty(), "ffmpeg produced no mp3 bytes");
+        // MP3 frame sync: 0xFF followed by a byte with the upper 3 bits set
+        // (0xE0 mask), or an ID3 tag header.
+        let has_frame = mp3.windows(2).any(|w| w[0] == 0xFF && (w[1] & 0xE0) == 0xE0);
+        let has_id3 = mp3.starts_with(b"ID3");
+        assert!(has_frame || has_id3, "output is not an mp3 stream");
+    }
+}