Ring: reading never goes past a drop, so a Dropout's silence lands where the audio was lost (#734)

Packages/ShowRecorderKit/Sources/Recording/SampleRing.swift

@@ -133,11 +133,20 @@ final class SampleRing: @unchecked Sendable {
     }
 
     /// Consumer: hands up to `maxFrames` of each channel to `body` (in up to two contiguous pieces)
-    /// and then releases them. Returns the number of frames consumed.
+    /// and then releases them. Returns the number of frames consumed. Never reads past the next drop, even when audio
+    /// has been stored after the drop since the caller last looked at it: its silence belongs where it fell (#734).
     @discardableResult
     func consume(maxFrames: Int, _ body: (_ channel: Int, _ samples: UnsafeBufferPointer<Float>) throws -> Void) rethrows -> Int {
         let start = read.load(ordering: .relaxed)
-        let frames = min(maxFrames, written.load(ordering: .acquiring) - start)
+        // `written` first: a drop at or before it was logged before the audio that follows it was stored, so it is seen.
+        var end = written.load(ordering: .acquiring)
+        let nextDropIndex = dropsRead.load(ordering: .relaxed)
+        if nextDropIndex < dropsLogged.load(ordering: .acquiring) {
+            let fell = dropStorage[(nextDropIndex % dropCapacity) * 2]
+            // `max` is only defensive: a drop before `start` has audio after it, so the writer has already taken it.
+            if fell < end { end = max(fell, start) }
+        }
+        let frames = min(maxFrames, end - start)
         guard frames > 0 else { return 0 }
         let offset = start % capacity
         let first = min(frames, capacity - offset)

Packages/ShowRecorderKit/Tests/RecordingTests/SampleRingDropTests.swift

@@ -45,6 +45,43 @@ struct SampleRingDropTests {
         #expect(ring.nextDrop() != nil)
     }
 
+    @Test("Reading never goes past a drop, even when audio is stored after it between the writer's look and its read (#734)")
+    func readStopsAtTheDrop() {
+        let ring = SampleRing(channelCount: 1, capacity: 1_000)
+        write(800, to: ring)
+        write(300, to: ring)  // dropped, after 800 frames
+        drain(700, from: ring)
+        #expect(ring.nextDrop() == nil, "the writer looks here: nothing stored after the drop yet")
+        write(100, to: ring)  // arrives before the writer reads
+
+        let taken = ring.consume(maxFrames: 4_096) { _, _ in }
+        #expect(taken == 100, "only up to the drop at 800, not the 100 stored after it")
+        #expect(ring.nextDrop() == SampleRing.Drop(storedFrames: 800, length: 300))
+    }
+
+    @Test("A drop with nothing stored after it yet doesn't hold back the audio before it")
+    func pendingDropDoesNotRestrict() {
+        let ring = SampleRing(channelCount: 1, capacity: 1_000)
+        write(800, to: ring)
+        write(300, to: ring)  // dropped, nothing stored since
+
+        #expect(ring.consume(maxFrames: 4_096) { _, _ in } == 800)
+    }
+
+    @Test("Reading stops at each drop in turn, past the log's wraparound")
+    func manyDropsInTurn() {
+        let ring = SampleRing(channelCount: 1, capacity: 1_000, dropLogCapacity: 2)
+        for _ in 0..<5 {
+            write(100, to: ring)
+            write(950, to: ring)  // dropped
+            write(100, to: ring)  // stored after it
+            #expect(ring.consume(maxFrames: 4_096) { _, _ in } == 100, "up to the drop")
+            #expect(ring.nextDrop() != nil)
+            ring.consumeDrop()
+            #expect(ring.consume(maxFrames: 4_096) { _, _ in } == 100, "then the audio after it")
+        }
+    }
+
     @Test("Blocks dropped one after another are one drop; the last one is available once the producer has stopped")
     func backToBackMerge() {
         let ring = SampleRing(channelCount: 1, capacity: 1_000)
@@ -81,22 +118,21 @@ struct SampleRingDropTests {
     @Test("When the log is full the lost frames still add up exactly; the extra detail is only a total")
     func fullLog() {
         let ring = SampleRing(channelCount: 1, capacity: 1_000, dropLogCapacity: 2)
-        write(900, to: ring)
-        // Three separate drops, with a little audio stored between each (the reader keeps up with the audio only).
+        // Three separate drops, with a little audio stored between each, before the reader gets to any of them (it
+        // can't read past a drop, so they have to pile up in the ring).
         for _ in 0..<3 {
-            write(200, to: ring)  // dropped: 100 free
-            drain(900, from: ring)
-            write(900, to: ring)
+            write(100, to: ring)
+            write(950, to: ring)  // dropped: not enough room
         }
-        #expect(ring.overflowedFrames.load(ordering: .relaxed) == 600)
+        #expect(ring.overflowedFrames.load(ordering: .relaxed) == 2_850)
 
         var logged = 0
         while let drop = ring.nextDrop() {
             logged += drop.length
             ring.consumeDrop()
         }
-        #expect(logged == 400, "two drops fit the log")
-        #expect(ring.takeUnloggedDroppedFrames() == 200, "the third is kept as a total")
+        #expect(logged == 1_900, "two drops fit the log")
+        #expect(ring.takeUnloggedDroppedFrames() == 950, "the third is kept as a total")
         #expect(ring.takeUnloggedDroppedFrames() == 0, "and handed over once")
     }