From 29fc77d7fd569127680972f3d801f05fac996ab0 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Fri, 23 Oct 2026 22:00:40 -0400 Subject: [PATCH] Delete code only tests used: Mix Hold undo, shared headamp, Pre-roll snapshot (#885) Removes MixHold.UndoSet, Plan.undo, Run, apply(_:to:), ChannelSource and channels(movedByHeadamp:sources:), Recorder.isKeepingPreRoll, and PreRollBuffer.snapshot(maxFrames:) with its Snapshot. The PreRollBuffer tests read through read(frames:); tests that only exercised the deleted code go too. No change anyone can see. Assisted-by: Claude Code:claude-sonnet-5-5 --- .../Sources/AutoGain/MixHold.swift | 100 +------------- .../Sources/Recording/Capture.swift | 2 +- .../Sources/Recording/PreRollBuffer.swift | 41 +----- .../Sources/Recording/Recorder.swift | 3 - .../Tests/AutoGainTests/MixHoldTests.swift | 130 +----------------- .../RecordingTests/PreRollBufferTests.swift | 65 +++------ .../RecordingTests/PreRollFillTests.swift | 9 +- 7 files changed, 29 insertions(+), 321 deletions(-) diff --git a/Packages/ShowRecorderKit/Sources/AutoGain/MixHold.swift b/Packages/ShowRecorderKit/Sources/AutoGain/MixHold.swift index 08a6cab6..24d8399c 100644 --- a/Packages/ShowRecorderKit/Sources/AutoGain/MixHold.swift +++ b/Packages/ShowRecorderKit/Sources/AutoGain/MixHold.swift @@ -149,8 +149,6 @@ public enum MixHold { public var changes: [Change] /// Why `gainChangeDb` is less than requested; nil when it isn't. public var limitedBy: Limit? - /// What undoes this plan: the gain move and every setting's value before it. - public var undo: UndoSet /// Each compensated setting's new value, to write with the gain. public var values: [Setting: Double] { @@ -158,72 +156,9 @@ public enum MixHold { } } - /// What restores a channel after Mix Hold: how far each headamp moved, to move it back (the gain itself is never - /// read back), and each compensated setting's value before the first change, to write back exactly. - public struct UndoSet: Equatable, Sendable { - public var gainMovedDb: [Int: Double] - public var beforeDb: [Setting: Double] - - public init(gainMovedDb: [Int: Double] = [:], beforeDb: [Setting: Double] = [:]) { - self.gainMovedDb = gainMovedDb - self.beforeDb = beforeDb - } - - /// Folds in a later undo set, so one set undoes a whole run: every step of the loop, on every channel. A - /// setting keeps its first before-value, and each headamp's moves add up. - public mutating func merge(_ later: UndoSet) { - gainMovedDb.merge(later.gainMovedDb, uniquingKeysWith: +) - beforeDb.merge(later.beforeDb, uniquingKeysWith: { first, _ in first }) - } - } - - /// Mix Hold over a whole auto-gain run: one channel's steps, or every channel's when the run covers them all. An - /// actuator plans each step from the state it has just read from the Mixer, writes the gain move with the plan's - /// values, and gives the loop `gainChangeDb` as how far it moved. So the loop's steps stay within the compensable - /// range, and stop with range reached when it runs out. - public struct Run: Sendable { - public var ranges: Ranges - public var scopes: Scopes - /// Undoes every step so far. - public private(set) var undo = UndoSet() - /// Why the latest step was limited; nil when it wasn't. - public private(set) var limitedBy: Limit? - - public init(ranges: Ranges, scopes: Scopes = .all) { - self.ranges = ranges - self.scopes = scopes - } - - /// Plans one step and keeps its undo set and limit. `channels` must be read fresh, just before the step. - public mutating func step(headamp: Int, gainChangeDb: Double, channels: [ChannelState], keyed: [KeyedDynamics] = []) -> Plan { - let plan = MixHold.plan(headamp: headamp, gainChangeDb: gainChangeDb, channels: channels, ranges: ranges, scopes: scopes, keyed: keyed) - undo.merge(plan.undo) - limitedBy = plan.limitedBy - return plan - } - } - - /// `channels` with `values` written in, as the Mixer would have them after writing a plan or an undo set. - public static func apply(_ values: [Setting: Double], to channels: [ChannelState]) -> [ChannelState] { - channels.map { state in - var state = state - for (setting, value) in values where setting.channel == state.channel { - switch setting.parameter { - case .gateThreshold: state.gateThresholdDb = value - case .compressorThreshold: state.compressorThresholdDb = value - case .mainFader: state.mainFaderDb = value - case .otherMain(let main): state.otherMainsDb[main] = value - case .send(let bus): - if let index = state.sends.firstIndex(where: { $0.bus == bus }) { state.sends[index].levelDb = value } - } - } - return state - } - } - /// The compensation for moving `headamp` by `gainChangeDb`, limited to the compensable range. /// - Parameters: - /// - channels: every channel the headamp moves (see `channels(movedByHeadamp:sources:)`), read fresh. + /// - channels: every channel the headamp moves, read fresh. /// - keyed: gates and compressors whose sidechain key is another channel, read fresh. Those keyed from a moved /// channel, on a channel that isn't moved, have their threshold moved +g like the moved channels' own. public static func plan( @@ -247,11 +182,7 @@ public enum MixHold { limit = range.lowerLimit } let changes = items.map { Change(setting: $0.setting, fromDb: $0.valueDb, toDb: $0.valueDb + $0.direction * g) } - let undo = UndoSet( - gainMovedDb: [headamp: g], - beforeDb: Dictionary(changes.map { ($0.setting, $0.fromDb) }, uniquingKeysWith: { first, _ in first }) - ) - return Plan(requestedDb: gainChangeDb, gainChangeDb: g, changes: changes, limitedBy: limit, undo: undo) + return Plan(requestedDb: gainChangeDb, gainChangeDb: g, changes: changes, limitedBy: limit) } /// How far the gain can move each way before a compensated setting reaches its end stop: Mix Hold's red lines. @@ -310,33 +241,6 @@ public enum MixHold { } } - /// Where a channel's signal comes from: its headamp, and the channel it is stereo-linked with, if any. - public struct ChannelSource: Equatable, Sendable { - public var channel: Int - public var headamp: Int - public var linkedChannel: Int? - public init(channel: Int, headamp: Int, linkedChannel: Int? = nil) { - self.channel = channel - self.headamp = headamp - self.linkedChannel = linkedChannel - } - } - - /// Every channel a change to `headamp` moves, in order: the channels it feeds (on the X32 one headamp can feed - /// several), their stereo-linked partners, and whatever those partners' headamps feed in turn. - public static func channels(movedByHeadamp headamp: Int, sources: [ChannelSource]) -> [Int] { - var headamps: Set = [headamp] - var moved: Set = [] - while true { - let fed = sources.filter { headamps.contains($0.headamp) } - let next = Set(fed.map(\.channel)).union(fed.compactMap(\.linkedChannel)) - guard next != moved else { break } - moved = next - headamps.formUnion(sources.filter { moved.contains($0.channel) }.map(\.headamp)) - } - return moved.sorted() - } - /// A setting Mix Hold moves: +1 with the gain (thresholds), -1 against it (faders and sends). struct Item { var setting: Setting diff --git a/Packages/ShowRecorderKit/Sources/Recording/Capture.swift b/Packages/ShowRecorderKit/Sources/Recording/Capture.swift index 432fdedd..d28a5acb 100644 --- a/Packages/ShowRecorderKit/Sources/Recording/Capture.swift +++ b/Packages/ShowRecorderKit/Sources/Recording/Capture.swift @@ -110,7 +110,7 @@ final class Capture: Sendable { var isDrained: Bool { rings.allSatisfy { $0.availableFrames == 0 } } /// - Parameter preRollSeconds: how much audio to keep before a Take starts; the buffer holds a second - /// more, so a snapshot of that length isn't cut short by the writer overwriting its oldest frames. + /// more, so a read of that length isn't cut short by the writer overwriting its oldest frames. /// - Parameter startHeadroomSeconds: room in the buffer for a Take that starts late (#915): the time between the press /// and the first live block, while the Drive's files are made. With it the buffer exists even when Pre-roll is off. It is /// allocated here, at Arm, never when a Take starts. diff --git a/Packages/ShowRecorderKit/Sources/Recording/PreRollBuffer.swift b/Packages/ShowRecorderKit/Sources/Recording/PreRollBuffer.swift index 5f3e4688..f75a2791 100644 --- a/Packages/ShowRecorderKit/Sources/Recording/PreRollBuffer.swift +++ b/Packages/ShowRecorderKit/Sources/Recording/PreRollBuffer.swift @@ -2,22 +2,16 @@ import AudioIO import Synchronization /// A rolling buffer of the most recent audio: it holds up to `capacity` frames of every channel and -/// overwrites the oldest. The real-time thread writes; any other thread can take a `snapshot` of the -/// last N frames while writing carries on. +/// overwrites the oldest. The real-time thread writes; any other thread can `read` a range of frames +/// while writing carries on. /// /// `write` never allocates, locks or blocks (ADR 0002). It is single-producer: only one thread writes. /// -/// A snapshot may copy frames the writer overwrites while it copies. The writer announces how far it is +/// A read may copy frames the writer overwrites while it copies. The writer announces how far it is /// about to write (`writing`) before it touches the storage, and the reader checks that mark after -/// copying and drops any frames that may have been overwritten. So a snapshot is never torn; when the -/// writer laps the reader it is shorter instead. +/// copying and refuses the read if any frames may have been overwritten. So a read is never torn; when +/// the writer laps the reader it returns nil instead. final class PreRollBuffer: @unchecked Sendable { - /// Frames from `startFrame` (counted from the first frame ever written) on, one array per channel. - struct Snapshot { - var startFrame: Int - var channels: [[Float]] - } - let channelCount: Int let capacity: Int private let storage: UnsafeMutablePointer @@ -79,29 +73,4 @@ final class PreRollBuffer: @unchecked Sendable { atomicMemoryFence(ordering: .sequentiallyConsistent) return writing.load(ordering: .acquiring) - capacity <= range.lowerBound ? channels : nil } - - /// The last `maxFrames` frames (or fewer, when less has been written or the writer lapped the copy). - func snapshot(maxFrames: Int) -> Snapshot { - let end = written.load(ordering: .acquiring) - let count = max(0, min(maxFrames, end, capacity)) - let start = end - count - var channels = (0.. 0 { - let offset = start % capacity - let first = min(count, capacity - offset) - for channel in 0.. 0 { channels = channels.map { Array($0.dropFirst(lost)) } } - return Snapshot(startFrame: firstValid, channels: channels) - } } diff --git a/Packages/ShowRecorderKit/Sources/Recording/Recorder.swift b/Packages/ShowRecorderKit/Sources/Recording/Recorder.swift index 6fd8fc99..9570506c 100644 --- a/Packages/ShowRecorderKit/Sources/Recording/Recorder.swift +++ b/Packages/ShowRecorderKit/Sources/Recording/Recorder.swift @@ -391,9 +391,6 @@ public final class Recorder { isArmed = false } - /// Whether the Armed recorder is keeping Pre-roll audio. - public var isKeepingPreRoll: Bool { (capture?.preRollSeconds ?? 0) > 0 } - /// The Pre-roll buffer of the Armed device: the last seconds of audio since Arming. Nil when not Armed, or when Pre-roll /// is off and the Capture keeps no room for a Take that starts late (#915). var armedPreRoll: PreRollBuffer? { capture?.preRoll } diff --git a/Packages/ShowRecorderKit/Tests/AutoGainTests/MixHoldTests.swift b/Packages/ShowRecorderKit/Tests/AutoGainTests/MixHoldTests.swift index 946b88b9..ac362519 100644 --- a/Packages/ShowRecorderKit/Tests/AutoGainTests/MixHoldTests.swift +++ b/Packages/ShowRecorderKit/Tests/AutoGainTests/MixHoldTests.swift @@ -129,27 +129,8 @@ struct MixHoldTests { #expect(plan.limitedBy == nil) } - @Test("A stereo-linked pair moves together: either channel's headamp brings in both") - func linkedPair() { - let sources = [ - MixHold.ChannelSource(channel: 1, headamp: 1, linkedChannel: 2), - MixHold.ChannelSource(channel: 2, headamp: 2, linkedChannel: 1), - MixHold.ChannelSource(channel: 3, headamp: 3), - ] - #expect(MixHold.channels(movedByHeadamp: 1, sources: sources) == [1, 2]) - #expect(MixHold.channels(movedByHeadamp: 2, sources: sources) == [1, 2]) - #expect(MixHold.channels(movedByHeadamp: 3, sources: sources) == [3]) - } - @Test("An X32 headamp feeding two channels compensates both, and the range is the tighter of the two") func headampFeedingTwo() { - let sources = [ - MixHold.ChannelSource(channel: 3, headamp: 5), - MixHold.ChannelSource(channel: 9, headamp: 5), - MixHold.ChannelSource(channel: 4, headamp: 6), - ] - #expect(MixHold.channels(movedByHeadamp: 5, sources: sources) == [3, 9]) - var other = channel(9) other.mainFaderDb = 7 let plan = MixHold.plan(headamp: 5, gainChangeDb: -5, channels: [channel(3), other], ranges: x32Ranges) @@ -159,38 +140,7 @@ struct MixHoldTests { #expect(plan.changes.contains(MixHold.Change(setting: .init(channel: 9, parameter: .mainFader), fromDb: 7, toDb: 10))) } - @Test("The undo set holds the before-values, so it restores every compensated setting exactly, and the gain move to reverse") - func undoRestores() { - var state = channel() - state.mainFaderDb = -15.3 - let before = [state] - let plan = MixHold.plan(headamp: 1, gainChangeDb: 4.7, channels: before, ranges: x32Ranges) - let after = MixHold.apply(plan.values, to: before) - #expect(after != before) - #expect(MixHold.apply(plan.undo.beforeDb, to: after) == before) - #expect(plan.undo.gainMovedDb == [1: 4.7]) - } - - @Test("Undo sets combine over a run: the first before-value of each setting wins, and each headamp's moves add up") - func undoCombines() { - // Two steps of the loop on channel 1 (headamp 1), and one on channel 2 (headamp 2), all in one run. - let original = [channel(1), channel(2)] - let first = MixHold.plan(headamp: 1, gainChangeDb: 6, channels: [original[0]], ranges: x32Ranges) - let afterFirst = MixHold.apply(first.values, to: original) - let second = MixHold.plan(headamp: 1, gainChangeDb: -2, channels: [afterFirst[0]], ranges: x32Ranges) - let afterSecond = MixHold.apply(second.values, to: afterFirst) - let third = MixHold.plan(headamp: 2, gainChangeDb: 3, channels: [afterSecond[1]], ranges: x32Ranges) - let end = MixHold.apply(third.values, to: afterSecond) - - var run = MixHold.UndoSet() - run.merge(first.undo) - run.merge(second.undo) - run.merge(third.undo) - #expect(MixHold.apply(run.beforeDb, to: end) == original) - #expect(run.gainMovedDb == [1: 4, 2: 3]) - } - - @Test("A threshold on another channel keyed from a moved channel moves +g too, with an undo before-value") + @Test("A threshold on another channel keyed from a moved channel moves +g too") func sidechainCompensated() { let keyed = [ MixHold.KeyedDynamics(channel: 5, dynamics: .compressor, thresholdDb: -25, keyChannel: 1), @@ -201,7 +151,6 @@ struct MixHoldTests { let setting = MixHold.Setting(channel: 5, parameter: .compressorThreshold) #expect(plan.changes.contains(MixHold.Change(setting: setting, fromDb: -25, toDb: -22))) #expect(plan.values[setting] == -22) - #expect(plan.undo.beforeDb[setting] == -25) #expect(!plan.changes.contains { $0.setting.channel == 6 }) } @@ -239,85 +188,8 @@ struct MixHoldTests { } } -/// A Mixer with one channel behind headamp 1 and a steady source: the recorded level follows the gain dB for dB. -/// The gain is private, as on the EVO; only the channel's other settings can be read. -final class FakeMixer { - private var gainDb: Double - let sourceDbfs: Double - var state: MixHold.ChannelState - private(set) var gainWrites: [Double] = [] - - init(gainDb: Double, sourceDbfs: Double, state: MixHold.ChannelState) { - self.gainDb = gainDb - self.sourceDbfs = sourceDbfs - self.state = state - } - - var levelDbfs: Double { min(sourceDbfs + gainDb, 0) } - - /// Writes a gain move and its compensation together, as #153 and #392 would. - func write(gainMoveDb: Double, values: [MixHold.Setting: Double]) { - gainDb += gainMoveDb - gainWrites.append(gainMoveDb) - state = MixHold.apply(values, to: [state])[0] - } -} - @Suite("Mix Hold in the auto-gain loop") struct MixHoldLoopTests { - @Test("Each step is limited to the compensable range, the reason is kept, and one undo set restores the whole run") - func loopLimitedByCompensation() async throws { - var start = channel() - start.mainFaderDb = 2 - let mixer = FakeMixer(gainDb: 30, sourceDbfs: -40, state: start) - var run = MixHold.Run(ranges: x32Ranges) - - let outcome = try await AutoGainRule().run( - measure: { mixer.levelDbfs }, - adjust: { requested in - // Read fresh before every step, never cached. - let plan = run.step(headamp: 1, gainChangeDb: requested, channels: [mixer.state]) - mixer.write(gainMoveDb: plan.gainChangeDb, values: plan.values) - return plan.gainChangeDb - }, - settle: {} - ) - - // -10 dBFS wants 8 dB less gain; the fader at +2 can only rise 8 dB to +10, so the first step is all of it... - #expect(outcome == .onTarget(levelDbfs: -18, changeDb: -8, passes: 1)) - #expect(mixer.state.mainFaderDb == 10) - #expect(run.limitedBy == nil) - - // ...and asking for more is limited to nothing, with the fader as the reason. - let more = run.step(headamp: 1, gainChangeDb: -3, channels: [mixer.state]) - #expect(more.gainChangeDb == 0) - #expect(run.limitedBy == MixHold.Limit(setting: .init(channel: 1, parameter: .mainFader), endDb: 10)) - - mixer.write(gainMoveDb: -(run.undo.gainMovedDb[1] ?? 0), values: run.undo.beforeDb) - #expect(mixer.state == start) - #expect(mixer.levelDbfs == -10) - } - - @Test("A fader at its end stops the loop with range reached") - func loopStopsAtTheEnd() async throws { - var start = channel() - start.mainFaderDb = 10 - let mixer = FakeMixer(gainDb: 30, sourceDbfs: -40, state: start) - var run = MixHold.Run(ranges: x32Ranges) - let outcome = try await AutoGainRule().run( - measure: { mixer.levelDbfs }, - adjust: { requested in - let plan = run.step(headamp: 1, gainChangeDb: requested, channels: [mixer.state]) - mixer.write(gainMoveDb: plan.gainChangeDb, values: plan.values) - return plan.gainChangeDb - }, - settle: {} - ) - #expect(outcome == .failed(.rangeReached(levelDbfs: -10), changeDb: 0)) - #expect(run.limitedBy?.setting.parameter == .mainFader) - #expect(mixer.state == start) - } - @Test("A gate keyed from an input two moved channels share is compensated once, not once per channel (#473)") func keyedOncePerSetting() { let ranges = MixHold.Ranges(fader: -90...10, send: -90...10, gateThreshold: -80...0, compressorThreshold: -60...0, offDb: -90) diff --git a/Packages/ShowRecorderKit/Tests/RecordingTests/PreRollBufferTests.swift b/Packages/ShowRecorderKit/Tests/RecordingTests/PreRollBufferTests.swift index d95eb492..69c71c76 100644 --- a/Packages/ShowRecorderKit/Tests/RecordingTests/PreRollBufferTests.swift +++ b/Packages/ShowRecorderKit/Tests/RecordingTests/PreRollBufferTests.swift @@ -27,62 +27,28 @@ struct PreRollBufferTests { (0..(false) } - @Test("A snapshot taken while the writer keeps going is never torn") + @Test("A read taken while the writer keeps going is never torn") func concurrentWriter() async throws { let buffer = PreRollBuffer(channelCount: 1, capacity: 20_000) let flag = Flag() @@ -104,18 +70,19 @@ struct PreRollBufferTests { } writer.start() - var snapshots = 0 + var reads = 0 var torn = 0 while !flag.done.load(ordering: .acquiring) { - let snapshot = buffer.snapshot(maxFrames: 20_000) - let samples = snapshot.channels[0] - if !samples.isEmpty { - snapshots += 1 - if samples.enumerated().contains(where: { $1 != Float(snapshot.startFrame + $0) }) { torn += 1 } + let end = buffer.totalWrittenFrames + let start = max(0, end - 19_500) // about one block short of the capacity: the writer often laps a read + // Nil when the writer lapped this read; what comes back must be the ramp, whole. + if let samples = buffer.read(frames: start.. 50) + #expect(reads > 50) #expect(torn == 0) } } diff --git a/Packages/ShowRecorderKit/Tests/RecordingTests/PreRollFillTests.swift b/Packages/ShowRecorderKit/Tests/RecordingTests/PreRollFillTests.swift index c8033509..ece461da 100644 --- a/Packages/ShowRecorderKit/Tests/RecordingTests/PreRollFillTests.swift +++ b/Packages/ShowRecorderKit/Tests/RecordingTests/PreRollFillTests.swift @@ -19,10 +19,10 @@ struct PreRollFillTests { try recorder.arm(device) deliver(72_000, to: device, block: 480, sample: Self.value) - let snapshot = try #require(recorder.armedPreRoll).snapshot(maxFrames: 48_000) - #expect(snapshot.startFrame == 24_000) - #expect(snapshot.channels[0] == (24_000..<72_000).map { Float($0) }) - #expect(snapshot.channels[1] == (24_000..<72_000).map { Float($0 + 1_000_000) }) + let buffer = try #require(recorder.armedPreRoll) + let held = try #require(buffer.read(frames: 24_000..<72_000)) + #expect(held[0] == (24_000..<72_000).map { Float($0) }) + #expect(held[1] == (24_000..<72_000).map { Float($0 + 1_000_000) }) } @Test("Arming again starts with an empty buffer") @@ -63,7 +63,6 @@ struct PreRollFillTests { let device = FakeAudioDevice(inputChannelCount: 1) let off = Recorder(deviceFolder: root, preRollSeconds: 0) try off.arm(device) - #expect(!off.isKeepingPreRoll) #expect(off.armedPreRoll != nil, "room for the start wait (#915)") off.disarm() #expect(off.armedPreRoll == nil)