diff --git a/electron/ipc/recording/mac.ts b/electron/ipc/recording/mac.ts index 01951345d..599e4ecdb 100644 --- a/electron/ipc/recording/mac.ts +++ b/electron/ipc/recording/mac.ts @@ -31,6 +31,7 @@ import { } from "./diagnostics"; import { emitRecordingInterrupted } from "./events"; import { getFinalMacCompanionAudioPath } from "./macCompanionAudio"; +import { isUnfinalizedMp4 } from "./mp4Integrity"; import { pruneAutoRecordings } from "./prune"; export function waitForNativeCaptureStart(process: ChildProcessWithoutNullStreams) { @@ -279,6 +280,15 @@ export async function recoverNativeMacCaptureOutput() { return null; } + // The capture helper writes through AVAssetWriter, so a helper that died + // mid-recording leaves bytes on disk that no decoder can open. Recovering + // with such a file hands the editor an undecodable stream instead of telling + // the user the recording failed. + if (await isUnfinalizedMp4(candidatePath)) { + console.error("[mac-recover] Capture file was never finalized:", candidatePath); + return null; + } + try { if (systemAudioPath || microphonePath) { try { diff --git a/electron/ipc/recording/mp4Integrity.test.ts b/electron/ipc/recording/mp4Integrity.test.ts new file mode 100644 index 000000000..cf648ff2d --- /dev/null +++ b/electron/ipc/recording/mp4Integrity.test.ts @@ -0,0 +1,123 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { classifyMp4Layout, isUnfinalizedMp4, readMp4TopLevelBoxes } from "./mp4Integrity"; + +function box(type: string, size: number, payload = 0): Buffer { + const header = Buffer.alloc(8 + payload); + header.writeUInt32BE(size, 0); + header.write(type, 4, "latin1"); + return header; +} + +describe("classifyMp4Layout", () => { + it("treats a file with a moov index as finalized", () => { + expect( + classifyMp4Layout( + [ + { type: "ftyp", size: 28 }, + { type: "mdat", size: 3809329 }, + { type: "moov", size: 1634 }, + ], + 3810991, + ), + ).toBe("finalized"); + }); + + it("flags the interrupted-writer signature as unfinalized", () => { + expect( + classifyMp4Layout( + [ + { type: "ftyp", size: 28 }, + { type: "wide", size: 8 }, + { type: "mdat", size: 0 }, + ], + 45989367, + ), + ).toBe("unfinalized"); + }); + + it("keeps a moov-bearing file finalized even when mdat runs to the end", () => { + expect( + classifyMp4Layout( + [ + { type: "ftyp", size: 28 }, + { type: "moov", size: 1634 }, + { type: "mdat", size: 0 }, + ], + 4096, + ), + ).toBe("finalized"); + }); + + it("does not guess when the layout is unfamiliar", () => { + expect(classifyMp4Layout([{ type: "ftyp", size: 28 }], 28)).toBe("unknown"); + expect( + classifyMp4Layout( + [ + { type: "ftyp", size: 28 }, + { type: "moof", size: 512 }, + ], + 540, + ), + ).toBe("unknown"); + }); + + it("does not classify an empty or unreadable box table", () => { + expect(classifyMp4Layout([], 1024)).toBe("unknown"); + expect(classifyMp4Layout([{ type: "mdat", size: 0 }], 0)).toBe("unknown"); + }); +}); + +describe("readMp4TopLevelBoxes / isUnfinalizedMp4", () => { + let dir: string; + + beforeAll(async () => { + dir = await mkdtemp(path.join(tmpdir(), "recordly-mp4-")); + }); + + afterAll(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + it("walks the box table of a finalized file", async () => { + const file = path.join(dir, "finalized.mp4"); + await writeFile( + file, + Buffer.concat([box("ftyp", 16, 8), box("mdat", 24, 16), box("moov", 16, 8)]), + ); + + expect(await readMp4TopLevelBoxes(file)).toEqual([ + { type: "ftyp", size: 16 }, + { type: "mdat", size: 24 }, + { type: "moov", size: 16 }, + ]); + expect(await isUnfinalizedMp4(file)).toBe(false); + }); + + it("stops at an open-ended mdat and reports the file as unfinalized", async () => { + const file = path.join(dir, "interrupted.mp4"); + await writeFile( + file, + Buffer.concat([ + box("ftyp", 16, 8), + box("wide", 8), + box("mdat", 0), + Buffer.alloc(4096, 7), + ]), + ); + + expect(await readMp4TopLevelBoxes(file)).toEqual([ + { type: "ftyp", size: 16 }, + { type: "wide", size: 8 }, + { type: "mdat", size: 0 }, + ]); + expect(await isUnfinalizedMp4(file)).toBe(true); + }); + + it("reports missing files as not-unfinalized so callers keep their fallback", async () => { + expect(await isUnfinalizedMp4(path.join(dir, "nope.mp4"))).toBe(false); + }); +}); diff --git a/electron/ipc/recording/mp4Integrity.ts b/electron/ipc/recording/mp4Integrity.ts new file mode 100644 index 000000000..dfc401cfa --- /dev/null +++ b/electron/ipc/recording/mp4Integrity.ts @@ -0,0 +1,100 @@ +import fs from "node:fs/promises"; + +export interface Mp4Box { + type: string; + size: number; +} + +export type Mp4Layout = "finalized" | "unfinalized" | "unknown"; + +/** + * An MP4 written by AVAssetWriter (the macOS capture helper) or by the Windows + * capture helper only becomes playable once the writer finalizes it: the `mdat` + * gets its real size and a `moov` index is appended. A helper that dies + * mid-recording leaves `ftyp` + an open-ended `mdat` and no `moov` — bytes on + * disk that no player can open. + * + * Classification is deliberately conservative: only the exact interrupted-writer + * signature is reported as `unfinalized`. Anything unexpected is `unknown`, so + * callers keep their existing behaviour rather than rejecting a file that might + * be perfectly fine. + */ +export function classifyMp4Layout(boxes: Mp4Box[], fileSize: number): Mp4Layout { + if (boxes.length === 0 || fileSize <= 0) { + return "unknown"; + } + + if (boxes.some((box) => box.type === "moov")) { + return "finalized"; + } + + const lastBox = boxes[boxes.length - 1]; + if (lastBox.type === "mdat" && lastBox.size === 0) { + // size 0 means "this box runs to the end of the file" — the placeholder a + // writer patches on finish. + return "unfinalized"; + } + + return "unknown"; +} + +/** + * Reads the top-level box table without loading the file: each header is 8 bytes + * and points at the next one, so even a multi-gigabyte capture costs a handful of + * reads. + */ +export async function readMp4TopLevelBoxes(filePath: string, maxBoxes = 32): Promise { + const handle = await fs.open(filePath, "r"); + try { + const { size } = await handle.stat(); + const boxes: Mp4Box[] = []; + const header = Buffer.alloc(16); + let offset = 0; + + while (offset < size && boxes.length < maxBoxes) { + const { bytesRead } = await handle.read(header, 0, 16, offset); + if (bytesRead < 8) { + break; + } + + const declaredSize = header.readUInt32BE(0); + const type = header.toString("latin1", 4, 8); + boxes.push({ type, size: declaredSize }); + + let boxSize = declaredSize; + if (declaredSize === 1) { + if (bytesRead < 16) { + break; + } + boxSize = Number(header.readBigUInt64BE(8)); + } else if (declaredSize === 0) { + // Runs to end of file — nothing can follow it. + break; + } + + if (boxSize < 8) { + break; + } + offset += boxSize; + } + + return boxes; + } finally { + await handle.close(); + } +} + +/** + * True only when the file is positively identified as a capture that was never + * finalized. Unreadable or unusual files return false so callers fall back to + * their previous behaviour. + */ +export async function isUnfinalizedMp4(filePath: string): Promise { + try { + const boxes = await readMp4TopLevelBoxes(filePath); + const { size } = await fs.stat(filePath); + return classifyMp4Layout(boxes, size) === "unfinalized"; + } catch { + return false; + } +} diff --git a/electron/ipc/register/recording.ts b/electron/ipc/register/recording.ts index 06a33f84f..845d07f5c 100644 --- a/electron/ipc/register/recording.ts +++ b/electron/ipc/register/recording.ts @@ -68,6 +68,7 @@ import { waitForNativeCaptureStart, waitForNativeCaptureStop, } from "../recording/mac"; +import { isUnfinalizedMp4 } from "../recording/mp4Integrity"; import { resolveRecordedVideoStoragePath } from "../recording/storagePath"; import { attachWindowsCaptureLifecycle, @@ -1201,7 +1202,25 @@ export function registerRecordingHandlers( error: String(error), }); - // Try to recover: if the target file exists on disk, finalize with it + // Try to recover: if the target file exists on disk, finalize with it. + // Existence alone is not enough — a helper that died mid-capture leaves + // an unfinalized MP4 (open `mdat`, no `moov` index) that no decoder can + // open, and passing it on shows the user a garbled editor preview + // instead of telling them the recording failed. + if (fallbackPath && (await isUnfinalizedMp4(fallbackPath))) { + console.error( + "[stop-native-screen-recording] Capture file was never finalized:", + fallbackPath, + ); + return { + success: false, + message: + "The recording was interrupted before it could be finalized. The raw capture data is still on disk.", + error: String(error), + unfinalizedPath: fallbackPath, + }; + } + if (fallbackPath) { try { await fs.access(fallbackPath); diff --git a/electron/native/ScreenCaptureKitRecorder.swift b/electron/native/ScreenCaptureKitRecorder.swift index 1e2a397aa..e322969d3 100644 --- a/electron/native/ScreenCaptureKitRecorder.swift +++ b/electron/native/ScreenCaptureKitRecorder.swift @@ -18,6 +18,10 @@ struct CaptureConfig: Codable { let targetCaptureFPS = 60 let maxInlineAudioTailExtension = CMTime(seconds: 2.0, preferredTimescale: 600) +/// How long finalization waits for a backed-up encoder queue before giving up on +/// the optional tail frame: 100 polls x 10 ms = 1 s. +let writerReadinessPollAttempts = 100 +let writerReadinessPollInterval: UInt64 = 10_000_000 final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { private let queue = DispatchQueue(label: "recordly.screencapturekit.video") @@ -309,7 +313,9 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { return } - guard let videoInput = videoInput, videoInput.isReadyForMoreMediaData else { return } + guard let videoInput = videoInput, + assetWriter?.status == .writing, + videoInput.isReadyForMoreMediaData else { return } if firstSampleTime == .zero { firstSampleTime = sampleBuffer.presentationTimeStamp @@ -328,21 +334,21 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { if outputType == .audio { guard let systemAudioInput else { return } - appendAudioSampleBuffer(sampleBuffer, to: systemAudioInput, firstSampleTime: &firstSystemAudioSampleTime, presentationTime: presentationTime) + appendAudioSampleBuffer(sampleBuffer, to: systemAudioInput, of: systemAudioWriter, firstSampleTime: &firstSystemAudioSampleTime, presentationTime: presentationTime) // Also write system audio to the inline video track if let inlineAudioInput, inlineAudioInput.isReadyForMoreMediaData { - appendAudioSampleBuffer(sampleBuffer, to: inlineAudioInput, firstSampleTime: &firstInlineAudioSampleTime, presentationTime: presentationTime) + appendAudioSampleBuffer(sampleBuffer, to: inlineAudioInput, of: assetWriter, firstSampleTime: &firstInlineAudioSampleTime, presentationTime: presentationTime) } return } if outputType.rawValue == microphoneOutputTypeRawValue { if let microphoneOnlyInput { - appendAudioSampleBuffer(sampleBuffer, to: microphoneOnlyInput, firstSampleTime: &firstMicrophoneSampleTime, presentationTime: presentationTime) + appendAudioSampleBuffer(sampleBuffer, to: microphoneOnlyInput, of: microphoneOnlyWriter, firstSampleTime: &firstMicrophoneSampleTime, presentationTime: presentationTime) } // Write mic to inline video track only if there's no system audio (avoids double-writing) if !capturesSystemAudio, let inlineAudioInput, inlineAudioInput.isReadyForMoreMediaData { - appendAudioSampleBuffer(sampleBuffer, to: inlineAudioInput, firstSampleTime: &firstInlineAudioSampleTime, presentationTime: presentationTime) + appendAudioSampleBuffer(sampleBuffer, to: inlineAudioInput, of: assetWriter, firstSampleTime: &firstInlineAudioSampleTime, presentationTime: presentationTime) } return } @@ -370,7 +376,16 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { stream = nil isRecording = false - if let originalBuffer = lastSampleBuffer, let videoInput = videoInput { + // The tail frame only gives the last captured frame its full duration, so + // it must never put the file at risk. Appending to an input whose encoder + // queue is still backed up — routine after a long high-resolution capture — + // raises an Objective-C exception that Swift cannot catch, aborting the + // helper before `finishWriting()` and leaving an mdat with no moov atom: + // an unplayable recording. Wait briefly for the queue to drain, then skip + // the frame rather than lose the recording. + if let originalBuffer = lastSampleBuffer, + let videoInput = videoInput, + await waitUntilReady(videoInput, of: assetWriter) { let additionalTime = lastVideoPresentationTime + frameDuration(for: originalBuffer) let timing = CMSampleTimingInfo(duration: originalBuffer.duration, presentationTimeStamp: additionalTime, decodeTimeStamp: originalBuffer.decodeTimeStamp) if let additionalSampleBuffer = try? CMSampleBuffer(copying: originalBuffer, withNewTiming: [timing]) { @@ -378,19 +393,29 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { } } + // `endSession`, `markAsFinished` and `finishWriting` all raise when the + // writer is no longer in the `.writing` state (a mid-capture failure, for + // example a full disk), which would abort the helper the same way. let videoEndTime = lastVideoPresentationTime + (lastSampleBuffer.map { frameDuration(for: $0) } ?? .zero) let endTime = resolvedCaptureEndTime(videoEndTime: videoEndTime) - assetWriter?.endSession(atSourceTime: endTime) - videoInput?.markAsFinished() - inlineAudioInput?.markAsFinished() - await assetWriter?.finishWriting() + if let assetWriter, assetWriter.status == .writing { + assetWriter.endSession(atSourceTime: endTime) + videoInput?.markAsFinished() + inlineAudioInput?.markAsFinished() + await assetWriter.finishWriting() + } - systemAudioInput?.markAsFinished() - await systemAudioWriter?.finishWriting() + if let systemAudioWriter, systemAudioWriter.status == .writing { + systemAudioInput?.markAsFinished() + await systemAudioWriter.finishWriting() + } - microphoneOnlyInput?.markAsFinished() - await microphoneOnlyWriter?.finishWriting() + if let microphoneOnlyWriter, microphoneOnlyWriter.status == .writing { + microphoneOnlyInput?.markAsFinished() + await microphoneOnlyWriter.finishWriting() + } + let finalizeFailure: Error? = assetWriter.flatMap { $0.status == .completed ? nil : ($0.error ?? unfinalizedWriterError(status: $0.status)) } let path = outputURL?.path ?? "" assetWriter = nil videoInput = nil @@ -420,9 +445,42 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { capturesMicrophone = false writesSystemAudioToSeparateTrack = false writesMicrophoneToSeparateTrack = false + + // Report a half-written file as a failure instead of handing the editor a + // path it cannot decode. + if let finalizeFailure { + throw finalizeFailure + } + return path } + /// Waits briefly for an input's encoder queue to drain. Returns false when the + /// input stays backed up or its writer is no longer accepting data, in which + /// case the caller must skip the append: `AVAssetWriterInput.append` raises an + /// uncatchable Objective-C exception in both cases. + private func waitUntilReady(_ input: AVAssetWriterInput, of writer: AVAssetWriter?) async -> Bool { + guard let writer else { return false } + + var attemptsRemaining = writerReadinessPollAttempts + while writer.status == .writing { + if input.isReadyForMoreMediaData { + return true + } + guard attemptsRemaining > 0 else { return false } + attemptsRemaining -= 1 + try? await Task.sleep(nanoseconds: writerReadinessPollInterval) + } + + return false + } + + private func unfinalizedWriterError(status: AVAssetWriter.Status) -> Error { + NSError(domain: "RecordlyCapture", code: 10, userInfo: [ + NSLocalizedDescriptionKey: "Recording could not be finalized (writer status \(status.rawValue))", + ]) + } + private func adjustedPresentationTime(for sampleBuffer: CMSampleBuffer, outputType: SCStreamOutputType) -> CMTime? { if isPaused { return nil @@ -497,8 +555,10 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { return videoEndTime + CMTimeMinimum(tailExtension, maxInlineAudioTailExtension) } - private func appendAudioSampleBuffer(_ sampleBuffer: CMSampleBuffer, to input: AVAssetWriterInput, firstSampleTime: inout CMTime?, presentationTime: CMTime) { - guard input.isReadyForMoreMediaData else { return } + private func appendAudioSampleBuffer(_ sampleBuffer: CMSampleBuffer, to input: AVAssetWriterInput, of writer: AVAssetWriter?, firstSampleTime: inout CMTime?, presentationTime: CMTime) { + // A writer that failed mid-capture (a full disk, say) raises on every + // further append, which would abort the helper and lose the whole file. + guard writer?.status == .writing, input.isReadyForMoreMediaData else { return } if firstSampleTime == nil { firstSampleTime = presentationTime diff --git a/electron/native/bin/darwin-arm64/recordly-screencapturekit-helper b/electron/native/bin/darwin-arm64/recordly-screencapturekit-helper index 259f18848..c972c4204 100755 Binary files a/electron/native/bin/darwin-arm64/recordly-screencapturekit-helper and b/electron/native/bin/darwin-arm64/recordly-screencapturekit-helper differ diff --git a/electron/native/bin/darwin-x64/recordly-screencapturekit-helper b/electron/native/bin/darwin-x64/recordly-screencapturekit-helper index 09e0ed5d6..2d4105613 100755 Binary files a/electron/native/bin/darwin-x64/recordly-screencapturekit-helper and b/electron/native/bin/darwin-x64/recordly-screencapturekit-helper differ