diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 7f837ae19..4e9ba5429 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -93,6 +93,7 @@ import { import { patchWebmDurationOnDisk } from "../recording/webm-duration"; import { reindexRecordingOnDisk } from "../recording/webm-seek-index"; import { registerNativeBridgeHandlers } from "./nativeBridge"; +import { createNativeMacMidCaptureErrorWatch } from "./nativeMacMidCaptureErrorWatch"; import { registerRecordingPrefsHandlers } from "./recordingPrefs"; import { RecordingStreamRegistry, registerRecordingStreamHandlers } from "./recordingStream"; @@ -1485,8 +1486,17 @@ function inspectNativeMacCaptureOutput() { } } -function attachNativeMacCaptureOutputDrain(proc: ChildProcessWithoutNullStreams) { +function attachNativeMacCaptureOutputDrain( + proc: ChildProcessWithoutNullStreams, + onErrorDuringCapture: () => void, +) { let lineBuffer = ""; + // Hooked here rather than on `nativeMacCaptureEvents`, which the stop wait + // replays from the buffer: the drain sees each line once, live. + const watchForMidCaptureError = createNativeMacMidCaptureErrorWatch( + () => nativeMacCaptureProcess === proc, + onErrorDuringCapture, + ); const drain = (chunk: Buffer) => { const text = chunk.toString(); nativeMacCaptureOutput += text; @@ -1497,6 +1507,7 @@ function attachNativeMacCaptureOutputDrain(proc: ChildProcessWithoutNullStreams) const event = tryParseNativeHelperEvent(line.trim()); if (event) { dispatchNativeMacHelperEvent(event); + watchForMidCaptureError(event); } } }; @@ -2825,7 +2836,14 @@ export function registerIpcHandlers( stdio: ["pipe", "pipe", "pipe"], }); nativeMacCaptureProcess = proc; - attachNativeMacCaptureOutputDrain(proc); + // Drives the renderer's own stop, the same one the tray's Stop Recording + // sends: it clears the HUD and surfaces the helper's error as the result. + attachNativeMacCaptureOutputDrain(proc, () => { + const hudWindow = getMainWindow(); + if (hudWindow && !hudWindow.isDestroyed()) { + hudWindow.webContents.send("stop-recording-from-tray"); + } + }); await waitForNativeMacCaptureStart(proc); const captureStartedAtMs = Date.now(); diff --git a/electron/ipc/nativeMacMidCaptureErrorWatch.test.ts b/electron/ipc/nativeMacMidCaptureErrorWatch.test.ts new file mode 100644 index 000000000..c22b9c0e9 --- /dev/null +++ b/electron/ipc/nativeMacMidCaptureErrorWatch.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it, vi } from "vitest"; +import { createNativeMacMidCaptureErrorWatch } from "./nativeMacMidCaptureErrorWatch"; + +describe("createNativeMacMidCaptureErrorWatch", () => { + const error = { event: "error", code: "writer-failed-during-capture" }; + + it("fires on an error raised after recording started", () => { + const onError = vi.fn(); + const watch = createNativeMacMidCaptureErrorWatch(() => true, onError); + + watch({ event: "ready" }); + watch({ event: "recording-started" }); + watch(error); + + expect(onError).toHaveBeenCalledTimes(1); + }); + + it("leaves an error raised before recording started to the start wait", () => { + const onError = vi.fn(); + const watch = createNativeMacMidCaptureErrorWatch(() => true, onError); + + watch(error); + + expect(onError).not.toHaveBeenCalled(); + }); + + it("ignores events that are not errors", () => { + const onError = vi.fn(); + const watch = createNativeMacMidCaptureErrorWatch(() => true, onError); + + watch({ event: "recording-started" }); + watch({ event: "warning", code: "stop-capture-failed" }); + watch({ event: "recording-stopped" }); + + expect(onError).not.toHaveBeenCalled(); + }); + + it("ignores a helper that is no longer the current process", () => { + const onError = vi.fn(); + let current = true; + const watch = createNativeMacMidCaptureErrorWatch(() => current, onError); + + watch({ event: "recording-started" }); + current = false; + watch(error); + + expect(onError).not.toHaveBeenCalled(); + }); +}); diff --git a/electron/ipc/nativeMacMidCaptureErrorWatch.ts b/electron/ipc/nativeMacMidCaptureErrorWatch.ts new file mode 100644 index 000000000..4c62f6b7e --- /dev/null +++ b/electron/ipc/nativeMacMidCaptureErrorWatch.ts @@ -0,0 +1,24 @@ +/** + * Decides, one live helper event at a time, when the macOS helper raised an + * error mid-take. + * + * The start and stop waits only listen while they are pending, so an error the + * helper raises between them (a dead writer, a stream that stopped) used to sit + * in the buffer until the user pressed stop, while the HUD counted on for + * minutes (issue #621). Before `recording-started` the start wait owns the + * error, and a helper that is no longer the current process has nobody to stop. + */ +export function createNativeMacMidCaptureErrorWatch( + isCurrentProcess: () => boolean, + onError: () => void, +) { + let recordingStarted = false; + return (event: Record) => { + if (event.event === "recording-started") { + recordingStarted = true; + } + if (event.event === "error" && recordingStarted && isCurrentProcess()) { + onError(); + } + }; +} diff --git a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift index 00706a9f9..9059bac92 100644 --- a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift +++ b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift @@ -130,7 +130,11 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { private var didStartWriting = false private var didEmitRecordingStarted = false private var didReportWriterFailure = false - private var isStopping = false + /// The one shutdown, however many callers ask for it. A writer failure starts it on + /// its own task, and the `stop` Electron sends right after has to wait for that run: + /// the command loop exits the process as soon as `stop()` returns, which would cut + /// `finishWriter()` off before its terminal event. + private var shutdownTask: Task? private var isPaused = false private var pauseStartedAt: CMTime? private var totalPausedDuration = CMTime.zero @@ -182,17 +186,20 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { } func stop() async { - let shouldStop = stateQueue.sync { - if isStopping { - return false + let task = stateQueue.sync { () -> Task in + if let shutdownTask { + return shutdownTask } - isStopping = true - return true - } - if !shouldStop { - return + let task = Task { + await self.performStop() + } + shutdownTask = task + return task } + await task.value + } + private func performStop() async { do { try await stream?.stopCapture() } catch { @@ -208,7 +215,7 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { func pause() { let didPause = stateQueue.sync { - if isStopping || isPaused { + if shutdownTask != nil || isPaused { return false } @@ -227,7 +234,7 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { func resume() { let didResume = stateQueue.sync { - if isStopping || !isPaused { + if shutdownTask != nil || !isPaused { return false } @@ -297,6 +304,13 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { audioMixer?.beginTimeline(at: presentationTime) startAudioTicker() } + // Checked before the readiness gate, not only at a false append: a failed + // writer is not guaranteed to call itself ready, and an append that is never + // attempted can never report the failure (issue #621). + if writer.status == .failed { + reportWriterFailure("writer status") + return + } if videoInput.isReadyForMoreMediaData { let appended = videoInput.append(sampleBuffer) @@ -330,6 +344,12 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { /// settles and every failure becomes the "Saving..." hang instead of an error. /// This event answers "when did the writer die"; that one answers "did stopping /// work". Two questions, two codes. + /// + /// It also ends the capture, the way `didStopWithError` does. A failed writer + /// never recovers, so every frame after it is dropped; issue #621 is a take + /// whose writer died at 75 s while capture ran on for 22 more minutes. The + /// process stays up and still answers `stop`, so the Electron stop path works + /// unchanged and reads this event back as the reason. private func reportWriterFailure(_ stage: String) { guard !didReportWriterFailure, let writer else { return @@ -337,10 +357,14 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { didReportWriterFailure = true emitError( code: "writer-failed-during-capture", - message: "\(stage): " + message: "Recording stopped: the video file could not be written (\(stage): " + (writer.error.map { "\($0)" } - ?? "AVAssetWriter status \(writer.status.rawValue)"), + ?? "AVAssetWriter status \(writer.status.rawValue)") + + ").", ) + Task { + await stop() + } } private func ensureRequestedPermissions() throws { @@ -604,7 +628,16 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { leeway: .milliseconds(5) ) timer.setEventHandler { [weak self] in - self?.audioMixer?.tick() + guard let self else { + return + } + // A still screen delivers no complete frames, so the frame path alone + // could sit on a dead writer for as long as nothing on screen moves. + if self.writer?.status == .failed { + self.reportWriterFailure("writer status") + return + } + self.audioMixer?.tick() } audioTicker = timer timer.resume()