From 6a3264245b2a6350ffeb3d4af9adbdb372aaa6b1 Mon Sep 17 00:00:00 2001 From: nanameru <4869nanataitai@gmail.com> Date: Wed, 2 Sep 2026 23:12:48 +1000 Subject: [PATCH] fix(mac): recover valid recording after stop failure (Refs #4) --- electron/electron-env.d.ts | 1 + electron/ipc/handlers.ts | 53 +++++++- .../recording/nativeMacCaptureStop.test.ts | 101 +++++++++++++++ electron/recording/nativeMacCaptureStop.ts | 122 ++++++++++++++++++ test-board.yaml | 22 ++++ 5 files changed, 294 insertions(+), 5 deletions(-) create mode 100644 electron/recording/nativeMacCaptureStop.test.ts create mode 100644 electron/recording/nativeMacCaptureStop.ts create mode 100644 test-board.yaml diff --git a/electron/electron-env.d.ts b/electron/electron-env.d.ts index e140a4e37..9c960aba5 100644 --- a/electron/electron-env.d.ts +++ b/electron/electron-env.d.ts @@ -179,6 +179,7 @@ interface Window { session?: import("../src/lib/recordingSession").RecordingSession; message?: string; discarded?: boolean; + recovered?: boolean; error?: string; }>; attachNativeMacWebcamRecording: (payload: { diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index aa2014670..63eddebf2 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -77,6 +77,7 @@ import { findPipeWireCursorHelperPath } from "../native-bridge/cursor/recording/ import type { CursorRecordingSession } from "../native-bridge/cursor/recording/session"; import { toHelperRect } from "../native-bridge/helperCoordinates"; import { scoreDeviceNameMatch } from "../recording/deviceNameMatching"; +import { resolveNativeMacCaptureStop } from "../recording/nativeMacCaptureStop"; import { isSalvageableFragmentedCapture, NATIVE_WINDOWS_SALVAGEABLE_OUTPUT_BYTES, @@ -1527,6 +1528,35 @@ function waitForNativeMacCaptureStop(proc: ChildProcessWithoutNullStreams) { }); } +function hasNativeMacCaptureExited(proc: ChildProcessWithoutNullStreams) { + return proc.exitCode !== null || proc.signalCode !== null; +} + +function waitForNativeMacCaptureExit(proc: ChildProcessWithoutNullStreams, timeoutMs = 5_000) { + if (hasNativeMacCaptureExited(proc)) { + return Promise.resolve(true); + } + + return new Promise((resolve) => { + const timer = setTimeout(() => { + cleanup(); + resolve(false); + }, timeoutMs); + const onExit = () => { + cleanup(); + resolve(true); + }; + const cleanup = () => { + clearTimeout(timer); + proc.off("close", onExit); + proc.off("exit", onExit); + }; + + proc.once("close", onExit); + proc.once("exit", onExit); + }); +} + function setCurrentRecordingSessionState(session: RecordingSession | null) { currentRecordingSession = session; currentVideoPath = session?.screenVideoPath ?? null; @@ -3087,11 +3117,21 @@ export function registerIpcHandlers( completeNativeMacCursorPauseRange(); const stoppedPathPromise = waitForNativeMacCaptureStop(proc); proc.stdin.write("stop\n"); - const stoppedPath = await stoppedPathPromise; - const screenVideoPath = stoppedPath || preferredPath; - if (!screenVideoPath) { - throw new Error("Native macOS capture did not return an output path."); + const stopResolution = await resolveNativeMacCaptureStop({ + preferredPath, + waitForStop: () => stoppedPathPromise, + waitForExit: () => waitForNativeMacCaptureExit(proc), + }); + if (stopResolution.recovered) { + console.warn("[native-sck] stop failed but the completed MP4 was recovered", { + error: + stopResolution.stopError instanceof Error + ? stopResolution.stopError.message + : String(stopResolution.stopError), + path: preferredPath, + }); } + const { path: screenVideoPath, recovered } = stopResolution; if (cursorCaptureMode === "editable-overlay") { await stopCursorRecording(); @@ -3132,7 +3172,10 @@ export function registerIpcHandlers( success: true, path: screenVideoPath, session, - message: "Native macOS recording session stored successfully", + recovered, + message: recovered + ? "Native macOS recording recovered from a failed stop" + : "Native macOS recording session stored successfully", }; } catch (error) { console.error("Failed to stop native macOS recording:", error); diff --git a/electron/recording/nativeMacCaptureStop.test.ts b/electron/recording/nativeMacCaptureStop.test.ts new file mode 100644 index 000000000..107a53516 --- /dev/null +++ b/electron/recording/nativeMacCaptureStop.test.ts @@ -0,0 +1,101 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { isSalvageableNativeMacCapture, resolveNativeMacCaptureStop } from "./nativeMacCaptureStop"; + +let dir: string; +const validVideoFixture = path.resolve("website/static/video/webcam.mp4"); + +beforeEach(async () => { + dir = await fs.mkdtemp(path.join(os.tmpdir(), "native-mac-stop-")); +}); + +afterEach(async () => { + await fs.rm(dir, { recursive: true, force: true }); +}); + +function atom(type: string, payloadBytes: number) { + const result = Buffer.alloc(8 + payloadBytes); + result.writeUInt32BE(result.length, 0); + result.write(type, 4, 4, "ascii"); + return result; +} + +async function writeMp4(name: string, atoms: Buffer[]): Promise { + const filePath = path.join(dir, name); + await fs.writeFile(filePath, Buffer.concat(atoms)); + return filePath; +} + +describe("isSalvageableNativeMacCapture", () => { + it("accepts an MP4 with a parseable video stream after helper exit", async () => { + await expect(isSalvageableNativeMacCapture(validVideoFixture, true)).resolves.toBe(true); + }); + + it("rejects the former false positive with atom names but no video stream", async () => { + const filePath = await writeMp4("empty-shell.mp4", [ + atom("ftyp", 24), + atom("mdat", 2048), + atom("moov", 256), + ]); + await expect(isSalvageableNativeMacCapture(filePath, true)).resolves.toBe(false); + }); + + it("does not inspect or admit a file while the helper may still be writing", async () => { + await expect(isSalvageableNativeMacCapture(validVideoFixture, false)).resolves.toBe(false); + }); + + it("returns false when the expected output is missing", async () => { + await expect(isSalvageableNativeMacCapture(path.join(dir, "missing.mp4"), true)).resolves.toBe( + false, + ); + }); +}); + +describe("resolveNativeMacCaptureStop", () => { + it("keeps the acknowledged stop path unchanged", async () => { + const waitForExit = vi.fn(async () => true); + const isSalvageable = vi.fn(async () => true); + await expect( + resolveNativeMacCaptureStop({ + preferredPath: "/recordings/preferred.mp4", + waitForStop: async () => "/recordings/acknowledged.mp4", + waitForExit, + isSalvageable, + }), + ).resolves.toEqual({ path: "/recordings/acknowledged.mp4", recovered: false }); + expect(waitForExit).not.toHaveBeenCalled(); + expect(isSalvageable).not.toHaveBeenCalled(); + }); + + it("returns the preferred path when failed stop output is safely recoverable", async () => { + const stopError = new Error("helper closed before stopped event"); + await expect( + resolveNativeMacCaptureStop({ + preferredPath: validVideoFixture, + waitForStop: async () => { + throw stopError; + }, + waitForExit: async () => true, + }), + ).resolves.toEqual({ path: validVideoFixture, recovered: true, stopError }); + }); + + it.each([ + ["helper is still alive", false, true], + ["output is invalid", true, false], + ])("preserves the original stop failure when %s", async (_label, helperExited, valid) => { + const stopError = new Error("stop failed"); + await expect( + resolveNativeMacCaptureStop({ + preferredPath: "/recordings/incomplete.mp4", + waitForStop: async () => { + throw stopError; + }, + waitForExit: async () => helperExited, + isSalvageable: async () => valid, + }), + ).rejects.toBe(stopError); + }); +}); diff --git a/electron/recording/nativeMacCaptureStop.ts b/electron/recording/nativeMacCaptureStop.ts new file mode 100644 index 000000000..28da99f84 --- /dev/null +++ b/electron/recording/nativeMacCaptureStop.ts @@ -0,0 +1,122 @@ +import fs from "node:fs/promises"; +import { createFile } from "mp4box"; + +const MIN_SALVAGEABLE_MP4_BYTES = 1024; +const PARSE_CHUNK_BYTES = 1024 * 1024; + +type ParsedMovie = { + hasMoov: boolean; + duration: number; + timescale: number; + videoTracks: Array<{ + codec: string; + duration: number; + timescale: number; + nb_samples: number; + video?: { width: number; height: number }; + }>; +}; + +type PositionedArrayBuffer = ArrayBuffer & { fileStart: number }; + +/** + * Parses the MP4 incrementally, without retaining `mdat`, and requires a real + * video sample table. Atom names alone are insufficient: a zero-filled `moov` + * shell looks superficially complete but cannot be opened by the editor. + */ +async function hasReadableVideoStream(filePath: string): Promise { + let handle: Awaited> | null = null; + try { + handle = await fs.open(filePath, "r"); + const stat = await handle.stat(); + if (!stat.isFile() || stat.size < MIN_SALVAGEABLE_MP4_BYTES) return false; + + const parser = createFile(false); + let movie: ParsedMovie | null = null; + let parseFailed = false; + parser.onReady = (info) => { + movie = info as ParsedMovie; + }; + parser.onError = () => { + parseFailed = true; + }; + + let offset = 0; + while (offset < stat.size) { + const bytesToRead = Math.min(PARSE_CHUNK_BYTES, stat.size - offset); + const chunk = Buffer.allocUnsafe(bytesToRead); + const { bytesRead } = await handle.read(chunk, 0, bytesToRead, offset); + if (bytesRead !== bytesToRead) return false; + const arrayBuffer = chunk.buffer.slice( + chunk.byteOffset, + chunk.byteOffset + bytesRead, + ) as PositionedArrayBuffer; + arrayBuffer.fileStart = offset; + parser.appendBuffer(arrayBuffer, offset + bytesRead === stat.size); + offset += bytesRead; + } + parser.flush(); + + if (parseFailed || !movie) return false; + const parsedMovie = movie as ParsedMovie; + return ( + parsedMovie.hasMoov && + parsedMovie.duration > 0 && + parsedMovie.timescale > 0 && + parsedMovie.videoTracks.some( + (track) => + track.codec.length > 0 && + track.duration > 0 && + track.timescale > 0 && + track.nb_samples > 0 && + (track.video?.width ?? 0) > 0 && + (track.video?.height ?? 0) > 0, + ) + ); + } catch { + return false; + } finally { + await handle?.close().catch(() => undefined); + } +} + +/** A file is never inspected while the helper may still be mutating it. */ +export async function isSalvageableNativeMacCapture( + filePath: string | null, + helperExited: boolean, +): Promise { + if (!filePath || !helperExited) return false; + return hasReadableVideoStream(filePath); +} + +export type NativeMacCaptureStopResolution = { + path: string; + recovered: boolean; + stopError?: unknown; +}; + +/** + * Keeps the normal acknowledgement path unchanged. If acknowledgement fails, + * recovery is allowed only after helper exit and successful media parsing. + */ +export async function resolveNativeMacCaptureStop(options: { + preferredPath: string | null; + waitForStop: () => Promise; + waitForExit: () => Promise; + isSalvageable?: (filePath: string | null, helperExited: boolean) => Promise; +}): Promise { + try { + return { path: await options.waitForStop(), recovered: false }; + } catch (stopError) { + const helperExited = await options.waitForExit(); + const isSalvageable = options.isSalvageable ?? isSalvageableNativeMacCapture; + if ( + !helperExited || + !(await isSalvageable(options.preferredPath, helperExited)) || + !options.preferredPath + ) { + throw stopError; + } + return { path: options.preferredPath, recovered: true, stopError }; + } +} diff --git a/test-board.yaml b/test-board.yaml new file mode 100644 index 000000000..873369954 --- /dev/null +++ b/test-board.yaml @@ -0,0 +1,22 @@ +version: 1 +project: + name: "openscreen" + test_command: "npm run test" + repo: "nanameru/openscreen" + +source_roots: + - src + - electron + - tests + +cases: + - id: TC-001 + title: "macOS録画の保存救済: helper停止通知失敗時に完成済みMP4を救済する" + feature: "macOS録画の保存救済" + scenario: "helper停止通知失敗時に完成済みMP4を救済する" + status: pass + priority: medium + type: regression + source: [electron/recording/nativeMacCaptureStop.ts] + test_file: "electron/recording/nativeMacCaptureStop.test.ts" + issues: [4]