Fix: a click in the shared game sound when packets are dropped after a burst

crates/share-worker/src/sound.rs

@@ -148,7 +148,13 @@ impl SoundReceiver {
     /// The next 10 ms: decoded, rebuilt, concealed or silent.
     pub fn next_packet(&mut self) -> Vec<f32> {
         let mut out = vec![0.0; PACKET_SAMPLES];
-        let decoded = match self.jitter.pop() {
+        let playout = self.jitter.pop();
+        for dropped in self.jitter.take_dropped() {
+            // Unheard: only so that the decoder carries on from where the sound now is.
+            let _ = self.decoder.decode_float(&dropped, &mut out, false);
+        }
+        out.fill(0.0);
+        let decoded = match playout {
             Playout::Silence => return out,
             Playout::Packet(payload) => self.decoder.decode_float(&payload, &mut out, false),
             Playout::Recover(next) => self.decoder.decode_float(&next, &mut out, true),
@@ -330,6 +336,7 @@ pub fn capture(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use anyplay_share::audio::MIN_DEPTH;
 
     /// Peak level of a stretch of samples.
     fn peak(samples: &[f32]) -> f32 {
@@ -393,6 +400,34 @@ mod tests {
         assert_eq!(receiver.stats().lost, 1);
     }
 
+    #[test]
+    fn packets_dropped_after_a_burst_do_not_make_a_click() {
+        let mut sender = SoundSender::new(0).unwrap();
+        let datagrams = sender.push(&tone(48_000)).unwrap();
+        // Playing, then a stall and a burst: the oldest packets are dropped to catch up. Where
+        // the tone is cut depends on the burst; at none of them may it play louder than 0.25.
+        for burst in MIN_DEPTH + 5..30 {
+            let mut receiver = SoundReceiver::new().unwrap();
+            for datagram in &datagrams[..MIN_DEPTH] {
+                receiver.insert(datagram);
+            }
+            for _ in 0..MIN_DEPTH {
+                receiver.next_packet();
+            }
+            for datagram in &datagrams[MIN_DEPTH..MIN_DEPTH + burst] {
+                receiver.insert(datagram);
+            }
+            let after: Vec<f32> = (0..MIN_DEPTH)
+                .flat_map(|_| receiver.next_packet())
+                .collect();
+            assert!(
+                receiver.stats().skipped > 0,
+                "burst {burst}: nothing dropped"
+            );
+            assert!(peak(&after) < 0.27, "burst {burst}: peak {}", peak(&after));
+        }
+    }
+
     #[test]
     fn the_resampler_keeps_48_khz_and_doubles_for_96() {
         let input: Vec<f32> = (0..8).flat_map(|i| [i as f32, -(i as f32)]).collect();

crates/share/src/audio.rs

@@ -137,6 +137,8 @@ pub struct AudioJitter {
     concealed: u32,
     calm: u32,
     stats: AudioStats,
+    /// What the latest `pop` dropped to catch up, oldest first.
+    dropped: Vec<Vec<u8>>,
 }
 
 impl Default for AudioJitter {
@@ -159,6 +161,7 @@ impl AudioJitter {
             concealed: 0,
             calm: 0,
             stats: AudioStats::default(),
+            dropped: Vec::new(),
         }
     }
 
@@ -218,6 +221,7 @@ impl AudioJitter {
 
     /// What to play for the next 10 ms.
     pub fn pop(&mut self) -> Playout {
+        self.dropped.clear();
         let Some(next) = self.next else {
             // Waiting until the buffer is as deep as it should be.
             let Some((&first, _)) = self.packets.first_key_value() else {
@@ -232,7 +236,8 @@ impl AudioJitter {
         // Too far behind the newest packet: drop the oldest, back to the depth.
         if self.packets.len() > self.depth + SLACK {
             while self.packets.len() > self.depth {
-                if let Some((sequence, _)) = self.packets.pop_first() {
+                if let Some((sequence, payload)) = self.packets.pop_first() {
+                    self.dropped.push(payload);
                     self.stats.skipped += 1;
                     self.next = Some(sequence + 1);
                 }
@@ -266,6 +271,14 @@ impl AudioJitter {
         }
     }
 
+    /// The packets the latest [`pop`](Self::pop) dropped to catch up, oldest first. They go
+    /// through the decoder before what `pop` answered, unheard: a decoder that never saw them
+    /// joins two stretches of sound that were not next to each other, and the join overshoots
+    /// (a click, up to half again as loud as the sound).
+    pub fn take_dropped(&mut self) -> Vec<Vec<u8>> {
+        std::mem::take(&mut self.dropped)
+    }
+
     /// The packet due is not here: it is counted lost, and remembered in case it comes late.
     fn miss(&mut self, sequence: i64) {
         self.next = Some(sequence + 1);
@@ -532,6 +545,10 @@ mod tests {
             "played {first} of 40: the delay was kept"
         );
         assert!(jitter.stats().skipped > 0);
+        let dropped = jitter.take_dropped();
+        assert_eq!(dropped.len() as u64, jitter.stats().skipped);
+        assert_eq!(dropped, (0..first).map(payload).collect::<Vec<_>>());
+        assert!(jitter.take_dropped().is_empty());
     }
 
     #[test]