diff --git a/Packages/ShowRecorderKit/Sources/BroadcastWave/StemWriter.swift b/Packages/ShowRecorderKit/Sources/BroadcastWave/StemWriter.swift index a3f1418c..cdc6bae9 100644 --- a/Packages/ShowRecorderKit/Sources/BroadcastWave/StemWriter.swift +++ b/Packages/ShowRecorderKit/Sources/BroadcastWave/StemWriter.swift @@ -65,7 +65,7 @@ public final class StemWriter: StemSink { self.url = url self.rf64Threshold = rf64Threshold guard FileManager.default.createFile(atPath: url.path, contents: nil) else { - throw CocoaError(.fileWriteUnknown, userInfo: [NSFilePathErrorKey: url.path]) + throw CocoaError(.fileReadCorruptFile, userInfo: [NSFilePathErrorKey: url.path]) } handle = try FileHandle(forWritingTo: url) let header = Self.header(info: info) @@ -155,7 +155,14 @@ public final class StemWriter: StemSink { /// Reopens the Stem at `url` to write into it again: `overwrite` over its Gaps, `append` after its /// last whole sample. The header is kept as it is; only the sizes are rewritten at `finalize`. public static func reopen(url: URL) throws -> StemWriter { - let frames = try StemFile.inspect(url).framesOnDisk + let stem = try StemFile.inspect(url) + // A Stem a DAW re-saved can start its audio later than this writer's header does. Writing at this writer's offsets + // would cut it short and write over the start of its audio (#881): refused, nothing is touched. + guard stem.dataStart == UInt64(dataStart) else { + throw CocoaError(.fileWriteUnknown, userInfo: [ + NSFilePathErrorKey: url.path, NSLocalizedDescriptionKey: "\(url.lastPathComponent) was saved by another program and can't be edited here."]) + } + let frames = stem.framesOnDisk // A resumed Stem keeps its own header, so this Info is only used to find where the sizes and // Marker region are, and is the same for every Stem. let info = Info(sampleRate: 48_000, description: "", originator: "", timeReference: 0, originationDate: Date(timeIntervalSince1970: 0)) diff --git a/Packages/ShowRecorderKit/Sources/Recording/ChannelRenamer.swift b/Packages/ShowRecorderKit/Sources/Recording/ChannelRenamer.swift index e6de2af3..2fe01bf1 100644 --- a/Packages/ShowRecorderKit/Sources/Recording/ChannelRenamer.swift +++ b/Packages/ShowRecorderKit/Sources/Recording/ChannelRenamer.swift @@ -60,13 +60,14 @@ public enum ChannelRenamer { let file = StemFileName.make(usbChannel: channel.usbChannel, sourceName: name) for stemFolder in stemFolders ?? [folder] { let old = stemFolder.appending(path: channel.stemFile), new = stemFolder.appending(path: file) - if file != channel.stemFile { - guard !fm.fileExists(atPath: new.path) else { throw CocoaError(.fileWriteFileExists, userInfo: [NSFilePathErrorKey: new.path]) } - try fm.moveItem(at: old, to: new) + if file != channel.stemFile, fm.fileExists(atPath: new.path) { + throw CocoaError(.fileWriteFileExists, userInfo: [NSFilePathErrorKey: new.path]) } - let stem = try StemWriter.reopen(url: new) + // Before it is moved: a Stem another program saved is refused, and left under its name (#881). + let stem = try StemWriter.reopen(url: old) try stem.setDescription(name) try stem.finalize() + if file != channel.stemFile { try fm.moveItem(at: old, to: new) } } take.usbChannels[index].stemFile = file take.usbChannels[index].name = name diff --git a/Packages/ShowRecorderKit/Tests/BroadcastWaveTests/StemReadTests.swift b/Packages/ShowRecorderKit/Tests/BroadcastWaveTests/StemReadTests.swift index eec40228..6a5144f0 100644 --- a/Packages/ShowRecorderKit/Tests/BroadcastWaveTests/StemReadTests.swift +++ b/Packages/ShowRecorderKit/Tests/BroadcastWaveTests/StemReadTests.swift @@ -78,4 +78,36 @@ struct StemReadTests { #expect(after.prefix(StemWriter.dataStart - 4)[8...] == before.prefix(StemWriter.dataStart - 4)[8...]) #expect(try StemWriter.readSamples(url: url, frames: 0..<1_500) == Self.samples(0..<1_500)) } + + /// What a DAW does on a re-save: its own chunk ahead of the audio, so the audio starts later than ours. + func resavedByADAW(_ url: URL) throws { + var bytes = [UInt8](try Data(contentsOf: url)) + let dataChunk = StemWriter.dataStart - 8 + let junk: [UInt8] = Array("JUNK".utf8) + [32, 0, 0, 0] + [UInt8](repeating: 0, count: 32) + bytes.insert(contentsOf: junk, at: dataChunk) + try Data(bytes).write(to: url) + } + + @Test("A Stem a DAW re-saved, with its audio starting later than ours, isn't reopened: nothing is written to it (#881)") + func reopenRefusesAnotherDataStart() throws { + let url = try write("resaved.wav", frames: 1_000) + try resavedByADAW(url) + let before = try Data(contentsOf: url) + #expect(try StemFile.inspect(url).dataStart != UInt64(StemWriter.dataStart), "the fixture moved the audio") + + #expect(throws: (any Error).self) { try StemWriter.reopen(url: url) } + + #expect(try Data(contentsOf: url) == before) + } + + @Test("A Stem our writer made and promoted to RF64 is reopened: the guard doesn't refuse our own files (#881)") + func reopenAcceptsOurRF64() throws { + let url = try write("ours.wav", frames: 20_000, rf64Threshold: 20_000 + UInt64(StemWriter.markerRegionSize)) + #expect(try Data(contentsOf: url).prefix(4).elementsEqual("RF64".utf8)) + + let writer = try StemWriter.reopen(url: url) + + #expect(writer.frameCount == 20_000) + try writer.finalize() + } } diff --git a/Packages/ShowRecorderKit/Tests/RecordingTests/ChannelRenamerTests.swift b/Packages/ShowRecorderKit/Tests/RecordingTests/ChannelRenamerTests.swift index 04718f76..bd6006b9 100644 --- a/Packages/ShowRecorderKit/Tests/RecordingTests/ChannelRenamerTests.swift +++ b/Packages/ShowRecorderKit/Tests/RecordingTests/ChannelRenamerTests.swift @@ -1,4 +1,5 @@ import AudioIO +import BroadcastWave import Destinations import Foundation @testable import Recording @@ -64,6 +65,25 @@ struct ChannelRenamerTests { } } + @Test("A Stem another program re-saved isn't renamed or written: the file keeps its name and bytes, and the Take.json keeps the old name (#881)") + func resavedStemIsLeftAlone() async throws { + try await recordTake() + let stem = take(device).appending(path: "02 USB 02.wav") + var bytes = [UInt8](try Data(contentsOf: stem)) + let dataChunk = StemWriter.dataStart - 8 + bytes.insert(contentsOf: Array("JUNK".utf8) + [32, 0, 0, 0] + [UInt8](repeating: 0, count: 32), at: dataChunk) + try Data(bytes).write(to: stem) + let before = try Data(contentsOf: stem) + + let outcome = ChannelRenamer.rename(names: [2: "Lead Vocal"], in: [.device: take(device)], allowsProExtras: true) + + #expect(outcome.result == .noCopyUpdated) + #expect(outcome.failed.map(\.copy) == [.device]) + #expect(try Data(contentsOf: stem) == before, "the Stem is as the other program left it, under its old name") + #expect(!FileManager.default.fileExists(atPath: take(device).appending(path: "02 Lead Vocal.wav").path)) + #expect(try metadata(take(device)).usbChannels[1].name != "Lead Vocal") + } + @Test("The report and the Reaper project show the new name") func reportAndProject() async throws { try await recordTake() diff --git a/Packages/ShowRecorderKit/Tests/RecordingTests/MarkerRenamerTests.swift b/Packages/ShowRecorderKit/Tests/RecordingTests/MarkerRenamerTests.swift index 0102c8d0..58a50e22 100644 --- a/Packages/ShowRecorderKit/Tests/RecordingTests/MarkerRenamerTests.swift +++ b/Packages/ShowRecorderKit/Tests/RecordingTests/MarkerRenamerTests.swift @@ -1,4 +1,5 @@ import AudioIO +import BroadcastWave import Destinations import Foundation @testable import Recording @@ -63,6 +64,22 @@ struct MarkerRenamerTests { #expect(after.map(\.timeReference) == before.map(\.timeReference)) } + @Test("A Stem another program re-saved is left as it is, and the outcome names it (#881)") + func resavedStemIsLeftAlone() async throws { + try await recordTake() + let stem = take(device).appending(path: "02 USB 02.wav") + var bytes = [UInt8](try Data(contentsOf: stem)) + bytes.insert(contentsOf: Array("JUNK".utf8) + [32, 0, 0, 0] + [UInt8](repeating: 0, count: 32), at: StemWriter.dataStart - 8) + try Data(bytes).write(to: stem) + let before = try Data(contentsOf: stem) + + let outcome = MarkerRenamer.rename(markerAt: 1, to: "Chorus", in: [.device: take(device)], allowsProExtras: true) + + #expect(outcome.failed.map(\.copy) == [.device]) + #expect(outcome.failed.first?.reason.contains("02 USB 02.wav") == true, "\(outcome.failed)") + #expect(try Data(contentsOf: stem) == before) + } + @Test("With a Copy not reachable, the others are renamed and the outcome names the Copy that wasn't") func oneCopyMissing() async throws { try await recordTake()