From bf43198e3df97f84b704deb114ff302977f9224c Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Fri, 16 Oct 2026 01:51:27 -0400 Subject: [PATCH] Ring: reading never goes past a drop, so a Dropout's silence lands where the audio was lost (#734) TakeWriter asks the ring whether a drop is ready, then reads. A drop is not ready until audio is stored after it, so when that audio arrived between the two steps the read ran past the drop and the silence went in after it: the Dropout Marker sat at 0.11 s, not 0.1 s (the MarkerListTests.dropoutInTheList flake), and the Stems carried the silence and the audio in the wrong order. SampleRing.consume now stops at the next logged drop. Written red first (readStopsAtTheDrop, deterministic: no timing). fullLog modeled a reader that ignores drops, so it now lets the drops pile up instead; same intent. Claude-Session: https://claude.ai/code/session_01Wx6Spsz6BcTiKDzUUP9CzN Assisted-by: Claude Code:claude-sonnet-5-5 --- .../Sources/Recording/SampleRing.swift | 13 ++++- .../RecordingTests/SampleRingDropTests.swift | 52 ++++++++++++++++--- 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/Packages/ShowRecorderKit/Sources/Recording/SampleRing.swift b/Packages/ShowRecorderKit/Sources/Recording/SampleRing.swift index 03b3c2e9..a0185dd2 100644 --- a/Packages/ShowRecorderKit/Sources/Recording/SampleRing.swift +++ b/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) 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) diff --git a/Packages/ShowRecorderKit/Tests/RecordingTests/SampleRingDropTests.swift b/Packages/ShowRecorderKit/Tests/RecordingTests/SampleRingDropTests.swift index 6c5fb43b..6e6514df 100644 --- a/Packages/ShowRecorderKit/Tests/RecordingTests/SampleRingDropTests.swift +++ b/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") }