From 727e395fcf0b7db2910b2e572e402234e8c8acbe Mon Sep 17 00:00:00 2001 From: neurot1cal Date: Mon, 25 May 2026 17:53:22 -0700 Subject: [PATCH 1/4] fix: stream long recordings to disk and patch WebM duration on save Recordings longer than ~10 minutes silently fail to save (#616). The renderer buffers the whole WebM as a Blob[], then on stop makes several in-memory copies (fixWebmDuration -> arrayBuffer -> Buffer.from) before writing. A long 1080p recording duplicates hundreds of MB several times in the renderer, exceeds Electron's memory limit, and the renderer crashes silently with no file saved. Two changes: 1. Stream chunks to disk (originally @Amanuel2x's contribution in #617). Open an fs.WriteStream in the main process at recording start and send each ~1s ondataavailable chunk straight to disk over two new IPC calls (open-recording-stream, append-recording-chunk), so the renderer never holds more than a single chunk. A full in-memory fallback is preserved for environments where the IPC stream cannot open. 2. Patch the WebM Duration header on disk after the stream closes. Browser MediaRecorder writes WebM with no Duration element, so streamed files save with duration=N/A and the editor's seek bar, timeline, and any scrub/trim break. A new electron/recording/webm-duration.ts module rewrites the Duration element, writing to a temp file and renaming in place so a crash mid-write cannot corrupt the recording. Streaming is opt-in: the screen recorder and the browser-only webcam recorder stream to disk; native-capture webcam sidecars (Windows, macOS) keep buffering in-memory, since their finalize path reads the recorder blob directly to attach the webcam track. Verified: tsc --noEmit clean; biome clean; vitest 166/166. Closes #616 Supersedes #617 Co-Authored-By: Amanuel Co-Authored-By: Claude Opus 4.7 (1M context) --- electron/electron-env.d.ts | 8 ++ electron/ipc/handlers.ts | 92 ++++++++++++++++- electron/preload.ts | 6 ++ electron/recording/webm-duration.ts | 97 ++++++++++++++++++ src/hooks/useScreenRecorder.ts | 148 +++++++++++++++++++++------- src/lib/recordingSession.ts | 8 ++ 6 files changed, 322 insertions(+), 37 deletions(-) create mode 100644 electron/recording/webm-duration.ts diff --git a/electron/electron-env.d.ts b/electron/electron-env.d.ts index 1b82992..cbe43be 100644 --- a/electron/electron-env.d.ts +++ b/electron/electron-env.d.ts @@ -81,6 +81,14 @@ interface Window { message?: string; error?: string; }>; + openRecordingStream: ( + recordingId: number, + fileName: string, + ) => Promise<{ success: boolean; error?: string }>; + appendRecordingChunk: ( + recordingId: number, + chunk: ArrayBuffer, + ) => Promise<{ success: boolean; error?: string }>; getRecordedVideoPath: () => Promise<{ success: boolean; path?: string; diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 669f1dd..f10cbbc 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -1,6 +1,6 @@ import { type ChildProcessWithoutNullStreams, spawn } from "node:child_process"; import { EventEmitter } from "node:events"; -import { constants as fsConstants } from "node:fs"; +import { createWriteStream, constants as fsConstants, type WriteStream } from "node:fs"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -40,6 +40,7 @@ import { RECORDINGS_DIR } from "../main"; import { createCursorRecordingSession } from "../native-bridge/cursor/recording/factory"; import { requestMacCursorAccessibilityAccess } from "../native-bridge/cursor/recording/macNativeCursorRecordingSession"; import type { CursorRecordingSession } from "../native-bridge/cursor/recording/session"; +import { patchWebmDurationOnDisk } from "../recording/webm-duration"; import { registerNativeBridgeHandlers } from "./nativeBridge"; const PROJECT_FILE_EXTENSION = "openscreen"; @@ -2141,6 +2142,47 @@ export function registerIpcHandlers( }, ); + // Streaming chunk writers — keyed by recordingId. Chunks are appended directly + // to disk as they arrive from ondataavailable so the renderer never holds the + // full video in memory. + const activeWriteStreams = new Map(); + + ipcMain.handle( + "open-recording-stream", + async ( + _, + recordingId: number, + fileName: string, + ): Promise<{ success: boolean; error?: string }> => { + try { + const filePath = resolveRecordingOutputPath(fileName); + const ws = createWriteStream(filePath, { flags: "w" }); + activeWriteStreams.set(recordingId, ws); + return { success: true }; + } catch (error) { + return { success: false, error: String(error) }; + } + }, + ); + + ipcMain.handle( + "append-recording-chunk", + async ( + _, + recordingId: number, + chunk: ArrayBuffer, + ): Promise<{ success: boolean; error?: string }> => { + const ws = activeWriteStreams.get(recordingId); + if (!ws) return { success: false, error: "No active stream for recordingId " + recordingId }; + return new Promise((resolve) => { + ws.write(Buffer.from(chunk), (err) => { + if (err) resolve({ success: false, error: err.message }); + else resolve({ success: true }); + }); + }); + }, + ); + ipcMain.handle("store-recorded-session", async (_, payload: StoreRecordedSessionInput) => { try { return await storeRecordedSessionFiles(payload); @@ -2161,12 +2203,56 @@ export function registerIpcHandlers( : Date.now(); const cursorCaptureMode = normalizeCursorCaptureMode(payload.cursorCaptureMode); const screenVideoPath = resolveRecordingOutputPath(payload.screen.fileName); - await fs.writeFile(screenVideoPath, Buffer.from(payload.screen.videoData)); + + // Close the streaming write stream if one was used; otherwise fall back to + // writing the full buffer (short recordings that never opened a stream). + const screenWs = activeWriteStreams.get(createdAt); + let screenStreamed = false; + if (screenWs) { + await new Promise((resolve, reject) => + screenWs.end((err?: Error | null) => (err ? reject(err) : resolve())), + ); + activeWriteStreams.delete(createdAt); + screenStreamed = true; + } else if (payload.screen.videoData && payload.screen.videoData.byteLength > 0) { + await fs.writeFile(screenVideoPath, Buffer.from(payload.screen.videoData)); + } let webcamVideoPath: string | undefined; + let webcamStreamed = false; if (payload.webcam) { webcamVideoPath = resolveRecordingOutputPath(payload.webcam.fileName); - await fs.writeFile(webcamVideoPath, Buffer.from(payload.webcam.videoData)); + const webcamWs = activeWriteStreams.get(createdAt + 1); // webcam stream keyed as recordingId+1 + if (webcamWs) { + await new Promise((resolve, reject) => + webcamWs.end((err?: Error | null) => (err ? reject(err) : resolve())), + ); + activeWriteStreams.delete(createdAt + 1); + webcamStreamed = true; + } else if (payload.webcam.videoData && payload.webcam.videoData.byteLength > 0) { + await fs.writeFile(webcamVideoPath, Buffer.from(payload.webcam.videoData)); + } + } + + // Streamed files lack the WebM Duration header (renderer no longer holds the + // blob to patch). Patch on disk so the editor's seek bar and timeline work. + // Best-effort: log on failure but don't block, since the file is still playable. + if ( + screenStreamed && + typeof payload.durationMs === "number" && + Number.isFinite(payload.durationMs) && + payload.durationMs > 0 + ) { + await patchWebmDurationOnDisk(screenVideoPath, payload.durationMs); + } + if ( + webcamStreamed && + webcamVideoPath && + typeof payload.durationMs === "number" && + Number.isFinite(payload.durationMs) && + payload.durationMs > 0 + ) { + await patchWebmDurationOnDisk(webcamVideoPath, payload.durationMs); } const session: RecordingSession = webcamVideoPath diff --git a/electron/preload.ts b/electron/preload.ts index 933ce9d..a7283d8 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -64,6 +64,12 @@ contextBridge.exposeInMainWorld("electronAPI", { storeRecordedSession: (payload: StoreRecordedSessionInput) => { return ipcRenderer.invoke("store-recorded-session", payload); }, + openRecordingStream: (recordingId: number, fileName: string) => { + return ipcRenderer.invoke("open-recording-stream", recordingId, fileName); + }, + appendRecordingChunk: (recordingId: number, chunk: ArrayBuffer) => { + return ipcRenderer.invoke("append-recording-chunk", recordingId, chunk); + }, getRecordedVideoPath: () => { return ipcRenderer.invoke("get-recorded-video-path"); diff --git a/electron/recording/webm-duration.ts b/electron/recording/webm-duration.ts new file mode 100644 index 0000000..5b2c197 --- /dev/null +++ b/electron/recording/webm-duration.ts @@ -0,0 +1,97 @@ +import fs from "node:fs/promises"; +import { fixParsedWebmDuration } from "@fix-webm-duration/fix"; +import { WebmFile } from "@fix-webm-duration/parser"; + +export type DurationPatchResult = + | { patched: true } + | { patched: false; reason: "no-section" | "already-valid" | "io-error" | "internal" }; + +/** + * Patch the WebM Duration header on a finalized recording file. + * + * Browser MediaRecorder writes WebM with no Duration EBML element. With the + * streaming-to-disk path the renderer never holds the blob, so the historical + * `fixWebmDuration(blob, durationMs)` call can't run. Patching on disk after + * `WriteStream.end()` produces an equivalent result: the editor's seek bar and + * timeline read a real duration instead of `N/A`. + * + * Atomic by design: writes the patched bytes to `.duration-patch.tmp` + * and renames in place. If the process crashes mid-rewrite, the original file + * survives intact, so the user never loses their recording to a partial write. + * + * Best-effort by intent: any failure (read, parse, write) logs and returns a + * non-`patched` result rather than throwing. The file is still playable without + * the patch (decoders walk frames sequentially); the only cost is that the + * editor's seek bar and timeline break until it is patched. + * + * Memory: reads the whole file into a main-process Buffer, the same footprint + * as the pre-streaming renderer path, just on the side without V8's heap cap. + */ +export async function patchWebmDurationOnDisk( + filePath: string, + durationMs: number, +): Promise { + try { + const fileBytes = await fs.readFile(filePath); + const webm = new WebmFile(new Uint8Array(fileBytes)); + + const patched = fixParsedWebmDuration(webm, durationMs, { logger: false }); + if (!patched) { + // fixParsedWebmDuration returns false for: missing Segment, missing + // Info, or a Duration that is already valid. The first two mean a + // malformed (most likely truncated) file; the third is a no-op. + const reason = inferUnpatchedReason(webm); + if (reason === "no-section") { + console.warn( + `[webm-duration] no Segment/Info section in ${filePath}; file may be truncated`, + ); + } + return { patched: false, reason }; + } + + if (!webm.source) { + console.error(`[webm-duration] patched but source missing for ${filePath}`); + return { patched: false, reason: "internal" }; + } + + const tmpPath = `${filePath}.duration-patch.tmp`; + const patchedBytes = Buffer.from( + webm.source.buffer, + webm.source.byteOffset, + webm.source.byteLength, + ); + try { + await fs.writeFile(tmpPath, patchedBytes); + await fs.rename(tmpPath, filePath); + return { patched: true }; + } catch (writeError) { + console.error(`[webm-duration] failed to write patched ${filePath}:`, writeError); + // Best-effort cleanup of the temp file; if unlink also fails, leave it. + // The original recording is untouched because the rename never ran. + await fs.unlink(tmpPath).catch(() => undefined); + return { patched: false, reason: "io-error" }; + } + } catch (error) { + console.error(`[webm-duration] failed to patch ${filePath}:`, error); + return { patched: false, reason: "io-error" }; + } +} + +/** + * Distinguish "no Segment/Info section" (malformed/truncated file) from "Info + * present but Duration already valid" (patch unnecessary). + * + * The IDs are the length-descriptor-stripped form that @fix-webm-duration/parser + * uses as its lookup keys (Segment `0x8538067`, Info `0x549a966`), verified + * against the parser's `src/lib/sections.js` — not the canonical 4-byte EBML + * IDs (`0x18538067` / `0x1549A966`), which this parser's `getSectionById` would + * never match. + */ +function inferUnpatchedReason(webm: WebmFile): "no-section" | "already-valid" { + const segment = webm.getSectionById?.(0x8538067); + if (!segment) return "no-section"; + const info = ( + segment as unknown as { getSectionById?: (id: number) => unknown } + ).getSectionById?.(0x549a966); + return info ? "already-valid" : "no-section"; +} diff --git a/src/hooks/useScreenRecorder.ts b/src/hooks/useScreenRecorder.ts index 14e55b3..665ae5e 100644 --- a/src/hooks/useScreenRecorder.ts +++ b/src/hooks/useScreenRecorder.ts @@ -77,6 +77,7 @@ type UseScreenRecorderReturn = { type RecorderHandle = { recorder: MediaRecorder; recordedBlobPromise: Promise; + streaming: boolean; }; type NativeWindowsRecordingHandle = { @@ -92,26 +93,88 @@ type NativeMacRecordingHandle = { paused: boolean; }; -function createRecorderHandle(stream: MediaStream, options: MediaRecorderOptions): RecorderHandle { +function createRecorderHandle( + stream: MediaStream, + options: MediaRecorderOptions, + recordingId?: number, + fileName?: string, +): RecorderHandle { const recorder = new MediaRecorder(stream, options); - const chunks: Blob[] = []; const mimeType = options.mimeType || "video/webm"; + + // Stream chunks to disk only when a target (recordingId + fileName) is given. + // The main screen recorder and the browser-only webcam recorder pass a target + // so long recordings never buffer the whole video in the renderer (the #616 + // fix). Native-capture webcam sidecars omit the target and buffer in-memory, + // because their finalize path reads recordedBlobPromise directly to attach the + // webcam file; an empty streamed blob would silently drop their webcam track. + const streamTarget = + recordingId !== undefined && fileName !== undefined ? { recordingId, fileName } : null; + + const pendingChunks: ArrayBuffer[] = []; + let streamReady = false; + let streamFailed = streamTarget === null; + + if (streamTarget) { + const streamOpenPromise = + window.electronAPI?.openRecordingStream?.(streamTarget.recordingId, streamTarget.fileName) ?? + Promise.resolve({ success: false }); + + streamOpenPromise.then((result) => { + if (result.success) { + streamReady = true; + for (const chunk of pendingChunks) { + void window.electronAPI.appendRecordingChunk(streamTarget.recordingId, chunk); + } + pendingChunks.length = 0; + } else { + streamFailed = true; + } + }); + } + + const fallbackChunks: Blob[] = []; + const recordedBlobPromise = new Promise((resolve, reject) => { recorder.ondataavailable = (event: BlobEvent) => { - if (event.data && event.data.size > 0) { - chunks.push(event.data); + if (!event.data || event.data.size === 0) return; + + if (streamFailed) { + fallbackChunks.push(event.data); + return; } + + void event.data.arrayBuffer().then((buf) => { + if (streamFailed) { + fallbackChunks.push(new Blob([buf], { type: mimeType })); + return; + } + if (streamReady && streamTarget) { + void window.electronAPI.appendRecordingChunk(streamTarget.recordingId, buf); + } else { + pendingChunks.push(buf); + } + }); }; + recorder.onerror = () => { reject(new Error("Recording failed")); }; + recorder.onstop = () => { - resolve(new Blob(chunks, { type: mimeType })); + if (streamFailed) { + // Not streaming, or the stream failed to open — return the full + // in-memory blob (the buffered fallback). + resolve(new Blob(fallbackChunks, { type: mimeType })); + } else { + // Streaming succeeded — the main process already has the data on disk. + resolve(new Blob([], { type: mimeType })); + } }; }); recorder.start(RECORDER_TIMESLICE_MS); - return { recorder, recordedBlobPromise }; + return { recorder, recordedBlobPromise, streaming: !streamFailed }; } export function useScreenRecorder(): UseScreenRecorderReturn { @@ -361,34 +424,44 @@ export function useScreenRecorder(): UseScreenRecorderReturn { window.electronAPI?.discardCursorTelemetry(activeRecordingId); return; } - if (screenBlob.size === 0) { + // When streaming succeeded the blob is empty — the data is already on disk. + if (!activeScreenRecorder.streaming && screenBlob.size === 0) { return; } - const fixedScreenBlob = await fixWebmDuration(screenBlob, duration); - let fixedWebcamBlob: Blob | null = null; - if (activeWebcamRecorder) { - const webcamBlob = await activeWebcamRecorder.recordedBlobPromise.catch(() => null); - if (webcamBlob && webcamBlob.size > 0) { - fixedWebcamBlob = await fixWebmDuration(webcamBlob, duration); - } - } - const screenFileName = `${RECORDING_FILE_PREFIX}${activeRecordingId}${VIDEO_FILE_EXTENSION}`; const webcamFileName = `${RECORDING_FILE_PREFIX}${activeRecordingId}${WEBCAM_FILE_SUFFIX}${VIDEO_FILE_EXTENSION}`; + + // Only fix duration / convert to ArrayBuffer if we have in-memory data. + let screenVideoData: ArrayBuffer = new ArrayBuffer(0); + if (!activeScreenRecorder.streaming && screenBlob.size > 0) { + const fixedScreenBlob = await fixWebmDuration(screenBlob, duration); + screenVideoData = await fixedScreenBlob.arrayBuffer(); + } + + let webcamVideoData: ArrayBuffer | undefined; + if (activeWebcamRecorder) { + const webcamBlob = await activeWebcamRecorder.recordedBlobPromise.catch(() => null); + if (!activeWebcamRecorder.streaming && webcamBlob && webcamBlob.size > 0) { + const fixedWebcamBlob = await fixWebmDuration(webcamBlob, duration); + webcamVideoData = await fixedWebcamBlob.arrayBuffer(); + } else if (activeWebcamRecorder.streaming) { + webcamVideoData = new ArrayBuffer(0); + } + } + const result = await window.electronAPI.storeRecordedSession({ screen: { - videoData: await fixedScreenBlob.arrayBuffer(), + videoData: screenVideoData, fileName: screenFileName, }, - webcam: fixedWebcamBlob - ? { - videoData: await fixedWebcamBlob.arrayBuffer(), - fileName: webcamFileName, - } - : undefined, + webcam: + webcamVideoData !== undefined + ? { videoData: webcamVideoData, fileName: webcamFileName } + : undefined, createdAt: activeRecordingId, cursorCaptureMode, + durationMs: duration, }); if (!result.success) { @@ -1336,13 +1409,18 @@ export function useScreenRecorder(): UseScreenRecorderReturn { recordingId.current = Date.now(); const activeRecordingId = recordingId.current; - screenRecorder.current = createRecorderHandle(stream.current, { - mimeType, - videoBitsPerSecond, - ...(hasAudio - ? { audioBitsPerSecond: systemAudioTrack ? AUDIO_BITRATE_SYSTEM : AUDIO_BITRATE_VOICE } - : {}), - }); + screenRecorder.current = createRecorderHandle( + stream.current, + { + mimeType, + videoBitsPerSecond, + ...(hasAudio + ? { audioBitsPerSecond: systemAudioTrack ? AUDIO_BITRATE_SYSTEM : AUDIO_BITRATE_VOICE } + : {}), + }, + activeRecordingId, + `${RECORDING_FILE_PREFIX}${activeRecordingId}${VIDEO_FILE_EXTENSION}`, + ); screenRecorder.current.recorder.addEventListener( "error", () => { @@ -1352,10 +1430,12 @@ export function useScreenRecorder(): UseScreenRecorderReturn { ); if (webcamStream.current) { - webcamRecorder.current = createRecorderHandle(webcamStream.current, { - mimeType, - videoBitsPerSecond: Math.min(videoBitsPerSecond, BITRATE_BASE), - }); + webcamRecorder.current = createRecorderHandle( + webcamStream.current, + { mimeType, videoBitsPerSecond: Math.min(videoBitsPerSecond, BITRATE_BASE) }, + activeRecordingId + 1, + `${RECORDING_FILE_PREFIX}${activeRecordingId}${WEBCAM_FILE_SUFFIX}${VIDEO_FILE_EXTENSION}`, + ); } accumulatedDurationMs.current = 0; diff --git a/src/lib/recordingSession.ts b/src/lib/recordingSession.ts index f5ebf9c..12a6afd 100644 --- a/src/lib/recordingSession.ts +++ b/src/lib/recordingSession.ts @@ -20,6 +20,14 @@ export interface StoreRecordedSessionInput { webcam?: RecordedVideoAssetInput; createdAt?: number; cursorCaptureMode?: CursorCaptureMode; + /** + * Recording wall-clock duration in milliseconds. Used by the main process + * to patch the WebM Duration header on streamed recordings, since the + * renderer no longer holds the bytes. Browser MediaRecorder writes WebM + * with no/zero duration; without this patch, the editor's seek bar and + * timeline break for any recording that took the streaming path. + */ + durationMs?: number; } export function normalizeCursorCaptureMode(value: unknown): CursorCaptureMode | undefined { From f3c5b8a65d554216ba7e2182571f7b5c675705a2 Mon Sep 17 00:00:00 2001 From: neurot1cal Date: Tue, 26 May 2026 16:09:39 -0700 Subject: [PATCH 2/4] fix: harden streaming lifecycle and lift it out of the IPC god-module Addresses the review feedback on #658 (CodeRabbit + Codex) and the structural notes from the quality pass. Correctness: - Compute the recorder's streaming state at finalize time, not at construction. A stream that fails to open is now reported as not-streamed, so its buffered chunks are saved as a complete in-memory fallback instead of being dropped (was total data loss on open failure). - Await every in-flight chunk write before onstop resolves, so the main process never closes the write stream while a final chunk is still in flight (was truncating the tail of a recording under load). - Open the disk write stream by awaiting its 'open' event, so a bad path or permission error rejects up front instead of being acknowledged as success and then silently dropping bytes. - Close the stream and remove the partial file when a streamed recording is discarded or fails, so cancelled/failed runs don't leak descriptors or orphan partial recordings. - Surface a mid-stream write failure as a rejected recording rather than saving a silently truncated file. Structure: - Extract the streaming concern into electron/ipc/recordingStream.ts (RecordingStreamRegistry) and src/hooks/recorderHandle.ts, out of the 2.8k-line handlers.ts and the screen-recorder hook. - Key write streams by output file name, removing the implicit recordingId/+1 contract that spanned the IPC boundary. - Collapse the duplicated screen/webcam finalize blocks into one helper and the repeated duration-validity guard into one check; patch the screen and webcam durations in parallel. Adds unit tests for the registry (real temp-dir fs) and the recorder handle state machine (open-failure fallback, in-order writes awaited before stop, mid-stream failure). Extends the vitest include glob to collect electron-side tests. Verified: tsc --noEmit clean; biome clean; vitest 180/180. Co-Authored-By: Claude Opus 4.7 (1M context) --- electron/electron-env.d.ts | 8 +- electron/ipc/handlers.ts | 139 ++++++++---------- electron/ipc/recordingStream.test.ts | 84 +++++++++++ electron/ipc/recordingStream.ts | 147 ++++++++++++++++++++ electron/preload.ts | 11 +- src/hooks/recorderHandle.test.ts | 201 +++++++++++++++++++++++++++ src/hooks/recorderHandle.ts | 140 +++++++++++++++++++ src/hooks/useScreenRecorder.ts | 118 +++------------- vitest.config.ts | 2 +- 9 files changed, 658 insertions(+), 192 deletions(-) create mode 100644 electron/ipc/recordingStream.test.ts create mode 100644 electron/ipc/recordingStream.ts create mode 100644 src/hooks/recorderHandle.test.ts create mode 100644 src/hooks/recorderHandle.ts diff --git a/electron/electron-env.d.ts b/electron/electron-env.d.ts index cbe43be..b86c4ff 100644 --- a/electron/electron-env.d.ts +++ b/electron/electron-env.d.ts @@ -81,14 +81,12 @@ interface Window { message?: string; error?: string; }>; - openRecordingStream: ( - recordingId: number, - fileName: string, - ) => Promise<{ success: boolean; error?: string }>; + openRecordingStream: (fileName: string) => Promise<{ success: boolean; error?: string }>; appendRecordingChunk: ( - recordingId: number, + fileName: string, chunk: ArrayBuffer, ) => Promise<{ success: boolean; error?: string }>; + closeRecordingStream: (fileName: string) => Promise<{ success: boolean; error?: string }>; getRecordedVideoPath: () => Promise<{ success: boolean; path?: string; diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index f10cbbc..d367728 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -1,6 +1,6 @@ import { type ChildProcessWithoutNullStreams, spawn } from "node:child_process"; import { EventEmitter } from "node:events"; -import { createWriteStream, constants as fsConstants, type WriteStream } from "node:fs"; +import { constants as fsConstants } from "node:fs"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -42,6 +42,7 @@ import { requestMacCursorAccessibilityAccess } from "../native-bridge/cursor/rec import type { CursorRecordingSession } from "../native-bridge/cursor/recording/session"; import { patchWebmDurationOnDisk } from "../recording/webm-duration"; import { registerNativeBridgeHandlers } from "./nativeBridge"; +import { RecordingStreamRegistry, registerRecordingStreamHandlers } from "./recordingStream"; const PROJECT_FILE_EXTENSION = "openscreen"; const SHORTCUTS_FILE = path.join(app.getPath("userData"), "shortcuts.json"); @@ -266,6 +267,30 @@ function resolveRecordingOutputPath(fileName: string): string { return path.join(RECORDINGS_DIR, parsedPath.base); } +function isValidDurationMs(value: number | undefined): value is number { + return typeof value === "number" && Number.isFinite(value) && value > 0; +} + +/** + * Finalize a single recording file: if it was streamed to disk, flush and close + * the stream; otherwise (a short recording, or the stream failed to open and the + * renderer fell back to in-memory buffering) write the buffered bytes. Returns + * whether the file was streamed, which the caller uses to decide whether the + * WebM duration needs patching on disk. + */ +async function finalizeRecordingFile( + registry: RecordingStreamRegistry, + fileName: string, + filePath: string, + videoData?: ArrayBuffer, +): Promise { + const streamed = await registry.finalize(fileName); + if (!streamed && videoData && videoData.byteLength > 0) { + await fs.writeFile(filePath, Buffer.from(videoData)); + } + return streamed; +} + async function getApprovedProjectSession( project: unknown, projectFilePath?: string, @@ -2142,46 +2167,11 @@ export function registerIpcHandlers( }, ); - // Streaming chunk writers — keyed by recordingId. Chunks are appended directly - // to disk as they arrive from ondataavailable so the renderer never holds the - // full video in memory. - const activeWriteStreams = new Map(); - - ipcMain.handle( - "open-recording-stream", - async ( - _, - recordingId: number, - fileName: string, - ): Promise<{ success: boolean; error?: string }> => { - try { - const filePath = resolveRecordingOutputPath(fileName); - const ws = createWriteStream(filePath, { flags: "w" }); - activeWriteStreams.set(recordingId, ws); - return { success: true }; - } catch (error) { - return { success: false, error: String(error) }; - } - }, - ); - - ipcMain.handle( - "append-recording-chunk", - async ( - _, - recordingId: number, - chunk: ArrayBuffer, - ): Promise<{ success: boolean; error?: string }> => { - const ws = activeWriteStreams.get(recordingId); - if (!ws) return { success: false, error: "No active stream for recordingId " + recordingId }; - return new Promise((resolve) => { - ws.write(Buffer.from(chunk), (err) => { - if (err) resolve({ success: false, error: err.message }); - else resolve({ success: true }); - }); - }); - }, - ); + // On-disk write streams for in-progress recordings, keyed by output file name. + // Chunks are appended as they arrive from ondataavailable so the renderer + // never buffers the full video in memory (the #616 fix). + const recordingStreams = new RecordingStreamRegistry(); + registerRecordingStreamHandlers(ipcMain, recordingStreams, resolveRecordingOutputPath); ipcMain.handle("store-recorded-session", async (_, payload: StoreRecordedSessionInput) => { try { @@ -2203,56 +2193,37 @@ export function registerIpcHandlers( : Date.now(); const cursorCaptureMode = normalizeCursorCaptureMode(payload.cursorCaptureMode); const screenVideoPath = resolveRecordingOutputPath(payload.screen.fileName); - - // Close the streaming write stream if one was used; otherwise fall back to - // writing the full buffer (short recordings that never opened a stream). - const screenWs = activeWriteStreams.get(createdAt); - let screenStreamed = false; - if (screenWs) { - await new Promise((resolve, reject) => - screenWs.end((err?: Error | null) => (err ? reject(err) : resolve())), - ); - activeWriteStreams.delete(createdAt); - screenStreamed = true; - } else if (payload.screen.videoData && payload.screen.videoData.byteLength > 0) { - await fs.writeFile(screenVideoPath, Buffer.from(payload.screen.videoData)); - } + const screenStreamed = await finalizeRecordingFile( + recordingStreams, + payload.screen.fileName, + screenVideoPath, + payload.screen.videoData, + ); let webcamVideoPath: string | undefined; let webcamStreamed = false; if (payload.webcam) { webcamVideoPath = resolveRecordingOutputPath(payload.webcam.fileName); - const webcamWs = activeWriteStreams.get(createdAt + 1); // webcam stream keyed as recordingId+1 - if (webcamWs) { - await new Promise((resolve, reject) => - webcamWs.end((err?: Error | null) => (err ? reject(err) : resolve())), - ); - activeWriteStreams.delete(createdAt + 1); - webcamStreamed = true; - } else if (payload.webcam.videoData && payload.webcam.videoData.byteLength > 0) { - await fs.writeFile(webcamVideoPath, Buffer.from(payload.webcam.videoData)); - } + webcamStreamed = await finalizeRecordingFile( + recordingStreams, + payload.webcam.fileName, + webcamVideoPath, + payload.webcam.videoData, + ); } - // Streamed files lack the WebM Duration header (renderer no longer holds the - // blob to patch). Patch on disk so the editor's seek bar and timeline work. - // Best-effort: log on failure but don't block, since the file is still playable. - if ( - screenStreamed && - typeof payload.durationMs === "number" && - Number.isFinite(payload.durationMs) && - payload.durationMs > 0 - ) { - await patchWebmDurationOnDisk(screenVideoPath, payload.durationMs); - } - if ( - webcamStreamed && - webcamVideoPath && - typeof payload.durationMs === "number" && - Number.isFinite(payload.durationMs) && - payload.durationMs > 0 - ) { - await patchWebmDurationOnDisk(webcamVideoPath, payload.durationMs); + // Streamed files lack the WebM Duration header (the renderer no longer holds + // the blob to patch). Patch on disk so the editor's seek bar and timeline + // work. Best-effort and independent per file, so the patches run together. + if (isValidDurationMs(payload.durationMs)) { + const patches: Promise[] = []; + if (screenStreamed) { + patches.push(patchWebmDurationOnDisk(screenVideoPath, payload.durationMs)); + } + if (webcamStreamed && webcamVideoPath) { + patches.push(patchWebmDurationOnDisk(webcamVideoPath, payload.durationMs)); + } + await Promise.all(patches); } const session: RecordingSession = webcamVideoPath diff --git a/electron/ipc/recordingStream.test.ts b/electron/ipc/recordingStream.test.ts new file mode 100644 index 0000000..776fcf1 --- /dev/null +++ b/electron/ipc/recordingStream.test.ts @@ -0,0 +1,84 @@ +import { mkdtemp, readFile, rm, stat } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { RecordingStreamRegistry } from "./recordingStream"; + +describe("RecordingStreamRegistry", () => { + let dir: string; + const pathFor = (name: string) => path.join(dir, name); + + beforeEach(async () => { + dir = await mkdtemp(path.join(tmpdir(), "openscreen-stream-")); + }); + + afterEach(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + it("streams chunks to disk in order and reports streamed on finalize", async () => { + const registry = new RecordingStreamRegistry(); + await registry.open("rec.webm", pathFor("rec.webm")); + await registry.append("rec.webm", Buffer.from("hello ")); + await registry.append("rec.webm", Buffer.from("world")); + + const streamed = await registry.finalize("rec.webm"); + + expect(streamed).toBe(true); + expect(await readFile(pathFor("rec.webm"), "utf8")).toBe("hello world"); + // A second finalize has nothing to close. + expect(await registry.finalize("rec.webm")).toBe(false); + }); + + it("reports not-streamed when no stream was opened", async () => { + const registry = new RecordingStreamRegistry(); + expect(await registry.finalize("missing.webm")).toBe(false); + expect(registry.has("missing.webm")).toBe(false); + }); + + it("rejects open when the target path is not writable (open is awaited, not assumed)", async () => { + const registry = new RecordingStreamRegistry(); + // Parent directory does not exist, so createWriteStream emits 'error' on open. + await expect( + registry.open("rec.webm", path.join(dir, "does-not-exist", "rec.webm")), + ).rejects.toThrow(); + // A failed open must not register a stream the renderer would treat as live. + expect(registry.has("rec.webm")).toBe(false); + }); + + it("rejects append when no stream is open", async () => { + const registry = new RecordingStreamRegistry(); + await expect(registry.append("rec.webm", Buffer.from("x"))).rejects.toThrow( + /No active recording stream/, + ); + }); + + it("discard closes the stream and removes the partial file", async () => { + const registry = new RecordingStreamRegistry(); + await registry.open("rec.webm", pathFor("rec.webm")); + await registry.append("rec.webm", Buffer.from("partial")); + + await registry.discard("rec.webm", pathFor("rec.webm")); + + expect(registry.has("rec.webm")).toBe(false); + await expect(stat(pathFor("rec.webm"))).rejects.toThrow(); + // Nothing left to finalize after a discard. + expect(await registry.finalize("rec.webm")).toBe(false); + }); + + it("discard tolerates a missing file", async () => { + const registry = new RecordingStreamRegistry(); + await expect(registry.discard("never.webm", pathFor("never.webm"))).resolves.toBeUndefined(); + }); + + it("opening the same file twice replaces the prior stream", async () => { + const registry = new RecordingStreamRegistry(); + await registry.open("rec.webm", pathFor("rec.webm")); + await registry.append("rec.webm", Buffer.from("first")); + await registry.open("rec.webm", pathFor("rec.webm")); + await registry.append("rec.webm", Buffer.from("second")); + await registry.finalize("rec.webm"); + + expect(await readFile(pathFor("rec.webm"), "utf8")).toBe("second"); + }); +}); diff --git a/electron/ipc/recordingStream.ts b/electron/ipc/recordingStream.ts new file mode 100644 index 0000000..3dce5b9 --- /dev/null +++ b/electron/ipc/recordingStream.ts @@ -0,0 +1,147 @@ +import { createWriteStream, type WriteStream } from "node:fs"; +import { unlink } from "node:fs/promises"; +import type { IpcMain } from "electron"; + +/** + * Owns the lifecycle of on-disk write streams for in-progress recordings, keyed + * by the recording's output file name. Browser MediaRecorder chunks are appended + * here as they arrive so a long recording never buffers the whole video in the + * renderer (the #616 fix). + * + * The file name is the key because it is the one value the renderer and main + * process already exchange and it is globally unique per recording, so there is + * no derived/offset key to keep in sync across the IPC boundary. + */ +export class RecordingStreamRegistry { + private readonly streams = new Map(); + + /** + * Open a write stream and resolve only once the OS confirms it is writable. + * Resolving on the `open` event (rather than on `createWriteStream` returning) + * means a bad path or permission error rejects here instead of surfacing as a + * silent chunk drop later, so the renderer's fallback can take over. + */ + async open(fileName: string, filePath: string): Promise { + await this.endStream(fileName); + + const ws = createWriteStream(filePath, { flags: "w" }); + await new Promise((resolve, reject) => { + const onError = (error: Error) => reject(error); + ws.once("error", onError); + ws.once("open", () => { + ws.removeListener("error", onError); + resolve(); + }); + }); + // Keep a listener for the stream's lifetime so a late error logs rather + // than crashing the main process with an unhandled 'error' event. Per-write + // failures still surface through the `append` callback below. + ws.on("error", (error) => { + console.error(`[recording-stream] ${fileName}:`, error); + }); + + this.streams.set(fileName, ws); + } + + has(fileName: string): boolean { + return this.streams.has(fileName); + } + + /** Append a chunk; rejects if no stream is open or the write fails. */ + async append(fileName: string, chunk: Buffer): Promise { + const ws = this.streams.get(fileName); + if (!ws) { + throw new Error(`No active recording stream for ${fileName}`); + } + await new Promise((resolve, reject) => { + ws.write(chunk, (error) => (error ? reject(error) : resolve())); + }); + } + + /** + * Flush and close the stream, keeping the file. Returns whether a stream was + * open — i.e. whether the recording was streamed to disk (true) or needs its + * in-memory buffer written by the caller (false). + */ + async finalize(fileName: string): Promise { + const ws = this.streams.get(fileName); + if (!ws) { + return false; + } + this.streams.delete(fileName); + await new Promise((resolve, reject) => { + ws.end((error?: Error | null) => (error ? reject(error) : resolve())); + }); + return true; + } + + /** + * Close the stream (if any) and delete the partial file. Used when a streamed + * recording is discarded or fails before a successful save, so cancelled runs + * don't leak file descriptors or orphan partial recordings on disk. + */ + async discard(fileName: string, filePath: string): Promise { + await this.endStream(fileName); + await unlink(filePath).catch(() => undefined); + } + + private async endStream(fileName: string): Promise { + const ws = this.streams.get(fileName); + if (!ws) { + return; + } + this.streams.delete(fileName); + await new Promise((resolve) => ws.end(() => resolve())); + } +} + +/** + * Register the streaming IPC handlers. Thin wrappers that translate the + * registry's throw-on-failure contract into the `{ success, error }` shape the + * renderer expects. + */ +export function registerRecordingStreamHandlers( + ipcMain: IpcMain, + registry: RecordingStreamRegistry, + resolveRecordingOutputPath: (fileName: string) => string, +): void { + ipcMain.handle( + "open-recording-stream", + async (_, fileName: string): Promise<{ success: boolean; error?: string }> => { + try { + await registry.open(fileName, resolveRecordingOutputPath(fileName)); + return { success: true }; + } catch (error) { + return { success: false, error: String(error) }; + } + }, + ); + + ipcMain.handle( + "append-recording-chunk", + async ( + _, + fileName: string, + chunk: ArrayBuffer, + ): Promise<{ success: boolean; error?: string }> => { + try { + await registry.append(fileName, Buffer.from(chunk)); + return { success: true }; + } catch (error) { + return { success: false, error: String(error) }; + } + }, + ); + + ipcMain.handle( + "close-recording-stream", + async (_, fileName: string): Promise<{ success: boolean; error?: string }> => { + try { + await registry.discard(fileName, resolveRecordingOutputPath(fileName)); + return { success: true }; + } catch (error) { + return { success: false, error: String(error) }; + } + }, + ); +} diff --git a/electron/preload.ts b/electron/preload.ts index a7283d8..4b69740 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -64,11 +64,14 @@ contextBridge.exposeInMainWorld("electronAPI", { storeRecordedSession: (payload: StoreRecordedSessionInput) => { return ipcRenderer.invoke("store-recorded-session", payload); }, - openRecordingStream: (recordingId: number, fileName: string) => { - return ipcRenderer.invoke("open-recording-stream", recordingId, fileName); + openRecordingStream: (fileName: string) => { + return ipcRenderer.invoke("open-recording-stream", fileName); }, - appendRecordingChunk: (recordingId: number, chunk: ArrayBuffer) => { - return ipcRenderer.invoke("append-recording-chunk", recordingId, chunk); + appendRecordingChunk: (fileName: string, chunk: ArrayBuffer) => { + return ipcRenderer.invoke("append-recording-chunk", fileName, chunk); + }, + closeRecordingStream: (fileName: string) => { + return ipcRenderer.invoke("close-recording-stream", fileName); }, getRecordedVideoPath: () => { diff --git a/src/hooks/recorderHandle.test.ts b/src/hooks/recorderHandle.test.ts new file mode 100644 index 0000000..3551b7d --- /dev/null +++ b/src/hooks/recorderHandle.test.ts @@ -0,0 +1,201 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createRecorderHandle } from "./recorderHandle"; + +type ElectronAPI = Window["electronAPI"]; + +const tick = () => new Promise((resolve) => setTimeout(resolve, 0)); +const decode = (buffer: ArrayBuffer) => new TextDecoder().decode(new Uint8Array(buffer)); + +/** Minimal MediaRecorder stand-in the tests can drive directly. */ +class FakeMediaRecorder { + ondataavailable: ((event: BlobEvent) => void) | null = null; + onstop: (() => void) | null = null; + onerror: (() => void) | null = null; + state: "inactive" | "recording" = "inactive"; + + start(): void { + this.state = "recording"; + } + + stop(): void { + this.state = "inactive"; + this.onstop?.(); + } + + emit(data: Blob): void { + this.ondataavailable?.({ data } as BlobEvent); + } +} + +function stubElectronAPI(api: Partial): void { + window.electronAPI = api as unknown as ElectronAPI; +} + +function driver(handle: { recorder: MediaRecorder }): FakeMediaRecorder { + return handle.recorder as unknown as FakeMediaRecorder; +} + +describe("createRecorderHandle", () => { + beforeEach(() => { + vi.stubGlobal("MediaRecorder", FakeMediaRecorder); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + window.electronAPI = undefined as unknown as ElectronAPI; + }); + + it("streams chunks to disk in arrival order and resolves an empty blob", async () => { + const appended: string[] = []; + const openRecordingStream = vi.fn(async () => ({ success: true })); + const appendRecordingChunk = vi.fn(async (_fileName: string, buffer: ArrayBuffer) => { + appended.push(decode(buffer)); + return { success: true }; + }); + stubElectronAPI({ openRecordingStream, appendRecordingChunk }); + + const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }, "rec.webm"); + const fake = driver(handle); + + fake.emit(new Blob(["a"])); // arrives before open resolves -> buffered + await tick(); // open resolves -> buffered chunk flushes, mode becomes streaming + fake.emit(new Blob(["b"])); + fake.emit(new Blob(["c"])); + fake.stop(); + + const blob = await handle.recordedBlobPromise; + expect(openRecordingStream).toHaveBeenCalledWith("rec.webm"); + expect(appended).toEqual(["a", "b", "c"]); + expect(blob.size).toBe(0); + expect(handle.isStreaming()).toBe(true); + }); + + it("falls back to a complete in-memory blob when the stream fails to open", async () => { + const openRecordingStream = vi.fn(async () => ({ success: false, error: "nope" })); + const appendRecordingChunk = vi.fn(async () => ({ success: true })); + stubElectronAPI({ openRecordingStream, appendRecordingChunk }); + + const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }, "rec.webm"); + const fake = driver(handle); + + fake.emit(new Blob(["a"])); + await tick(); // open resolves false -> buffering, keep everything in memory + fake.emit(new Blob(["bc"])); + fake.stop(); + + const blob = await handle.recordedBlobPromise; + expect(appendRecordingChunk).not.toHaveBeenCalled(); + expect(handle.isStreaming()).toBe(false); + expect(blob.size).toBe(3); + expect(decode(await blob.arrayBuffer())).toBe("abc"); + }); + + it("waits for in-flight chunk writes before stop resolves (no truncation)", async () => { + let releaseAppend: () => void = () => undefined; + const appendGate = new Promise((resolve) => { + releaseAppend = resolve; + }); + const appendRecordingChunk = vi.fn(async () => { + await appendGate; + return { success: true }; + }); + stubElectronAPI({ + openRecordingStream: vi.fn(async () => ({ success: true })), + appendRecordingChunk, + }); + + const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }, "rec.webm"); + const fake = driver(handle); + + await tick(); // open resolves + fake.emit(new Blob(["a"])); // write blocks on the gate + fake.stop(); + + let resolved = false; + void handle.recordedBlobPromise.then(() => { + resolved = true; + }); + await tick(); + expect(resolved).toBe(false); // must not resolve while the write is in flight + + releaseAppend(); + await handle.recordedBlobPromise; + expect(resolved).toBe(true); + expect(appendRecordingChunk).toHaveBeenCalledTimes(1); + }); + + it("rejects when a chunk fails to write mid-stream", async () => { + stubElectronAPI({ + openRecordingStream: vi.fn(async () => ({ success: true })), + appendRecordingChunk: vi.fn(async () => ({ success: false, error: "disk full" })), + closeRecordingStream: vi.fn(async () => ({ success: true })), + }); + + const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }, "rec.webm"); + const fake = driver(handle); + + await tick(); + fake.emit(new Blob(["a"])); + fake.stop(); + + await expect(handle.recordedBlobPromise).rejects.toThrow(/disk full/); + expect(handle.isStreaming()).toBe(false); + }); + + it("buffers in memory and never opens a stream when no file name is given", async () => { + const openRecordingStream = vi.fn(async () => ({ success: true })); + stubElectronAPI({ + openRecordingStream, + appendRecordingChunk: vi.fn(async () => ({ success: true })), + }); + + const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }); + const fake = driver(handle); + + fake.emit(new Blob(["xy"])); + await tick(); + fake.stop(); + + const blob = await handle.recordedBlobPromise; + expect(openRecordingStream).not.toHaveBeenCalled(); + expect(handle.isStreaming()).toBe(false); + expect(blob.size).toBe(2); + }); + + it("discard closes the disk stream for a streamed recording", async () => { + const closeRecordingStream = vi.fn(async () => ({ success: true })); + stubElectronAPI({ + openRecordingStream: vi.fn(async () => ({ success: true })), + appendRecordingChunk: vi.fn(async () => ({ success: true })), + closeRecordingStream, + }); + + const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }, "rec.webm"); + const fake = driver(handle); + await tick(); + fake.emit(new Blob(["a"])); + fake.stop(); + await handle.recordedBlobPromise; + + await handle.discard(); + expect(closeRecordingStream).toHaveBeenCalledWith("rec.webm"); + }); + + it("discard is a no-op when the stream never opened", async () => { + const closeRecordingStream = vi.fn(async () => ({ success: true })); + stubElectronAPI({ + openRecordingStream: vi.fn(async () => ({ success: false })), + appendRecordingChunk: vi.fn(async () => ({ success: true })), + closeRecordingStream, + }); + + const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }, "rec.webm"); + const fake = driver(handle); + await tick(); + fake.stop(); + await handle.recordedBlobPromise; + + await handle.discard(); + expect(closeRecordingStream).not.toHaveBeenCalled(); + }); +}); diff --git a/src/hooks/recorderHandle.ts b/src/hooks/recorderHandle.ts new file mode 100644 index 0000000..3c63120 --- /dev/null +++ b/src/hooks/recorderHandle.ts @@ -0,0 +1,140 @@ +const RECORDER_TIMESLICE_MS = 1000; + +export type RecorderHandle = { + recorder: MediaRecorder; + /** + * Resolves once the recording has fully drained. For a streamed recording the + * blob is empty (the bytes are already on disk); for an in-memory recording it + * holds the full WebM. Rejects if a chunk failed to write to disk mid-stream, + * so a truncated recording surfaces as an error instead of a silent partial save. + */ + recordedBlobPromise: Promise; + /** + * Whether the recording's bytes went to disk via the streaming path. Computed + * at finalize time rather than construction, so a stream that fails to open is + * correctly reported as not-streamed and its in-memory fallback is used. + */ + isStreaming: () => boolean; + /** + * Close the disk stream (if one opened) and delete its partial file. Called + * when a recording is discarded or fails before a successful save, so cancelled + * runs don't leak the stream or orphan a partial file. No-op for in-memory + * recorders. + */ + discard: () => Promise; +}; + +/** + * Wrap a MediaRecorder, optionally streaming its chunks to disk. + * + * When `fileName` is given, chunks are written to disk in arrival order through + * the main process as they arrive, so a long recording never buffers the whole + * video in the renderer (the #616 fix). Until the disk stream confirms it is + * open, chunks are held in memory; if the open fails, that buffer becomes a + * complete in-memory fallback so nothing is lost. Native-capture webcam sidecars + * omit `fileName` and always buffer in memory, since their finalize path reads + * the blob directly to attach the webcam track. + */ +export function createRecorderHandle( + stream: MediaStream, + options: MediaRecorderOptions, + fileName?: string, +): RecorderHandle { + const recorder = new MediaRecorder(stream, options); + const mimeType = options.mimeType || "video/webm"; + const api = window.electronAPI; + + // Chunks held in memory: everything before the stream opens, plus everything + // when not streaming at all. On a successful open these flush to disk and are + // dropped; on open failure they remain as the complete fallback recording. + const memoryChunks: Blob[] = []; + let mode: "pending" | "streaming" | "buffering" = fileName ? "pending" : "buffering"; + let streamOpened = false; + let appendError: Error | null = null; + + // Serialize chunk writes so they land on disk in arrival order, and so stop + // can await every in-flight write before the main process closes the stream + // (otherwise a late chunk arrives after close and truncates the recording). + let writeChain: Promise = Promise.resolve(); + const enqueueWrite = (chunk: Blob) => { + writeChain = writeChain.then(async () => { + if (appendError || !fileName || !api?.appendRecordingChunk) { + return; + } + const buffer = await chunk.arrayBuffer(); + const result = await api.appendRecordingChunk(fileName, buffer); + if (!result.success) { + appendError = new Error(result.error ?? "Failed to write recording chunk to disk"); + } + }); + }; + + const openPromise: Promise<{ success: boolean; error?: string }> = + fileName && api?.openRecordingStream + ? api.openRecordingStream(fileName) + : Promise.resolve({ success: false }); + + void openPromise.then((result) => { + if (result.success) { + streamOpened = true; + mode = "streaming"; + for (const chunk of memoryChunks) { + enqueueWrite(chunk); + } + memoryChunks.length = 0; + } else { + mode = "buffering"; + } + }); + + const recordedBlobPromise = new Promise((resolve, reject) => { + recorder.ondataavailable = (event: BlobEvent) => { + if (!event.data || event.data.size === 0) { + return; + } + if (mode === "streaming") { + enqueueWrite(event.data); + } else { + // "pending" (stream not open yet) or "buffering" (not streaming). + memoryChunks.push(event.data); + } + }; + + recorder.onerror = () => { + reject(new Error("Recording failed")); + }; + + recorder.onstop = () => { + resolve(finalizeBlob()); + }; + }); + + async function finalizeBlob(): Promise { + // Wait for the open attempt to settle so its flush (or fallback switch) has + // been applied, then for every queued write to land, so we never resolve + // while chunks are still in flight to the about-to-close disk stream. + await openPromise.catch(() => undefined); + await writeChain; + if (appendError) { + throw appendError; + } + if (mode === "streaming") { + return new Blob([], { type: mimeType }); + } + return new Blob(memoryChunks, { type: mimeType }); + } + + async function discard(): Promise { + if (streamOpened && fileName && api?.closeRecordingStream) { + await api.closeRecordingStream(fileName); + } + } + + recorder.start(RECORDER_TIMESLICE_MS); + return { + recorder, + recordedBlobPromise, + isStreaming: () => mode === "streaming" && !appendError, + discard, + }; +} diff --git a/src/hooks/useScreenRecorder.ts b/src/hooks/useScreenRecorder.ts index 665ae5e..d6c03f1 100644 --- a/src/hooks/useScreenRecorder.ts +++ b/src/hooks/useScreenRecorder.ts @@ -13,6 +13,7 @@ import { } from "@/lib/nativeWindowsRecording"; import type { CursorCaptureMode, RecordedVideoAssetInput } from "@/lib/recordingSession"; import { requestCameraAccess } from "@/lib/requestCameraAccess"; +import { createRecorderHandle, type RecorderHandle } from "./recorderHandle"; const TARGET_FRAME_RATE = 60; const MIN_FRAME_RATE = 30; @@ -34,7 +35,6 @@ const DEFAULT_HEIGHT = 1080; const CODEC_ALIGNMENT = 2; -const RECORDER_TIMESLICE_MS = 1000; const BITS_PER_MEGABIT = 1_000_000; const CHROME_MEDIA_SOURCE = "desktop"; const RECORDING_FILE_PREFIX = "recording-"; @@ -74,12 +74,6 @@ type UseScreenRecorderReturn = { setCursorCaptureMode: (mode: CursorCaptureMode) => void; }; -type RecorderHandle = { - recorder: MediaRecorder; - recordedBlobPromise: Promise; - streaming: boolean; -}; - type NativeWindowsRecordingHandle = { recordingId: number; finalizing: boolean; @@ -93,90 +87,6 @@ type NativeMacRecordingHandle = { paused: boolean; }; -function createRecorderHandle( - stream: MediaStream, - options: MediaRecorderOptions, - recordingId?: number, - fileName?: string, -): RecorderHandle { - const recorder = new MediaRecorder(stream, options); - const mimeType = options.mimeType || "video/webm"; - - // Stream chunks to disk only when a target (recordingId + fileName) is given. - // The main screen recorder and the browser-only webcam recorder pass a target - // so long recordings never buffer the whole video in the renderer (the #616 - // fix). Native-capture webcam sidecars omit the target and buffer in-memory, - // because their finalize path reads recordedBlobPromise directly to attach the - // webcam file; an empty streamed blob would silently drop their webcam track. - const streamTarget = - recordingId !== undefined && fileName !== undefined ? { recordingId, fileName } : null; - - const pendingChunks: ArrayBuffer[] = []; - let streamReady = false; - let streamFailed = streamTarget === null; - - if (streamTarget) { - const streamOpenPromise = - window.electronAPI?.openRecordingStream?.(streamTarget.recordingId, streamTarget.fileName) ?? - Promise.resolve({ success: false }); - - streamOpenPromise.then((result) => { - if (result.success) { - streamReady = true; - for (const chunk of pendingChunks) { - void window.electronAPI.appendRecordingChunk(streamTarget.recordingId, chunk); - } - pendingChunks.length = 0; - } else { - streamFailed = true; - } - }); - } - - const fallbackChunks: Blob[] = []; - - const recordedBlobPromise = new Promise((resolve, reject) => { - recorder.ondataavailable = (event: BlobEvent) => { - if (!event.data || event.data.size === 0) return; - - if (streamFailed) { - fallbackChunks.push(event.data); - return; - } - - void event.data.arrayBuffer().then((buf) => { - if (streamFailed) { - fallbackChunks.push(new Blob([buf], { type: mimeType })); - return; - } - if (streamReady && streamTarget) { - void window.electronAPI.appendRecordingChunk(streamTarget.recordingId, buf); - } else { - pendingChunks.push(buf); - } - }); - }; - - recorder.onerror = () => { - reject(new Error("Recording failed")); - }; - - recorder.onstop = () => { - if (streamFailed) { - // Not streaming, or the stream failed to open — return the full - // in-memory blob (the buffered fallback). - resolve(new Blob(fallbackChunks, { type: mimeType })); - } else { - // Streaming succeeded — the main process already has the data on disk. - resolve(new Blob([], { type: mimeType })); - } - }; - }); - - recorder.start(RECORDER_TIMESLICE_MS); - return { recorder, recordedBlobPromise, streaming: !streamFailed }; -} - export function useScreenRecorder(): UseScreenRecorderReturn { const t = useScopedT("editor"); const [recording, setRecording] = useState(false); @@ -418,6 +328,10 @@ export function useScreenRecorder(): UseScreenRecorderReturn { window.electronAPI?.setRecordingState(false); void (async () => { + // Set once the recording is safely stored. Until then any disk stream + // is still open, so the finally block closes it and removes the partial + // file on the discard or error paths. + let savedToDisk = false; try { const screenBlob = await activeScreenRecorder.recordedBlobPromise; if (discardRecordingId.current === activeRecordingId) { @@ -425,16 +339,17 @@ export function useScreenRecorder(): UseScreenRecorderReturn { return; } // When streaming succeeded the blob is empty — the data is already on disk. - if (!activeScreenRecorder.streaming && screenBlob.size === 0) { + if (!activeScreenRecorder.isStreaming() && screenBlob.size === 0) { return; } const screenFileName = `${RECORDING_FILE_PREFIX}${activeRecordingId}${VIDEO_FILE_EXTENSION}`; const webcamFileName = `${RECORDING_FILE_PREFIX}${activeRecordingId}${WEBCAM_FILE_SUFFIX}${VIDEO_FILE_EXTENSION}`; - // Only fix duration / convert to ArrayBuffer if we have in-memory data. + // Only fix duration / convert to ArrayBuffer for in-memory data; + // streamed recordings are patched on disk by the main process. let screenVideoData: ArrayBuffer = new ArrayBuffer(0); - if (!activeScreenRecorder.streaming && screenBlob.size > 0) { + if (!activeScreenRecorder.isStreaming() && screenBlob.size > 0) { const fixedScreenBlob = await fixWebmDuration(screenBlob, duration); screenVideoData = await fixedScreenBlob.arrayBuffer(); } @@ -442,10 +357,10 @@ export function useScreenRecorder(): UseScreenRecorderReturn { let webcamVideoData: ArrayBuffer | undefined; if (activeWebcamRecorder) { const webcamBlob = await activeWebcamRecorder.recordedBlobPromise.catch(() => null); - if (!activeWebcamRecorder.streaming && webcamBlob && webcamBlob.size > 0) { + if (!activeWebcamRecorder.isStreaming() && webcamBlob && webcamBlob.size > 0) { const fixedWebcamBlob = await fixWebmDuration(webcamBlob, duration); webcamVideoData = await fixedWebcamBlob.arrayBuffer(); - } else if (activeWebcamRecorder.streaming) { + } else if (activeWebcamRecorder.isStreaming()) { webcamVideoData = new ArrayBuffer(0); } } @@ -468,6 +383,8 @@ export function useScreenRecorder(): UseScreenRecorderReturn { console.error("Failed to store recording session:", result.message); return; } + // store-recorded-session has flushed and closed the disk streams. + savedToDisk = true; if (result.session) { await window.electronAPI.setCurrentRecordingSession(result.session); @@ -479,6 +396,13 @@ export function useScreenRecorder(): UseScreenRecorderReturn { } catch (error) { console.error("Error saving recording:", error); } finally { + if (!savedToDisk) { + // Discarded, or failed before a successful save — close any + // dangling disk streams and remove their partial files so a + // cancelled or failed run doesn't leak a descriptor or orphan. + await activeScreenRecorder.discard().catch(() => undefined); + await activeWebcamRecorder?.discard().catch(() => undefined); + } if (finalizingRecordingId.current === activeRecordingId) { finalizingRecordingId.current = null; } @@ -1418,7 +1342,6 @@ export function useScreenRecorder(): UseScreenRecorderReturn { ? { audioBitsPerSecond: systemAudioTrack ? AUDIO_BITRATE_SYSTEM : AUDIO_BITRATE_VOICE } : {}), }, - activeRecordingId, `${RECORDING_FILE_PREFIX}${activeRecordingId}${VIDEO_FILE_EXTENSION}`, ); screenRecorder.current.recorder.addEventListener( @@ -1433,7 +1356,6 @@ export function useScreenRecorder(): UseScreenRecorderReturn { webcamRecorder.current = createRecorderHandle( webcamStream.current, { mimeType, videoBitsPerSecond: Math.min(videoBitsPerSecond, BITRATE_BASE) }, - activeRecordingId + 1, `${RECORDING_FILE_PREFIX}${activeRecordingId}${WEBCAM_FILE_SUFFIX}${VIDEO_FILE_EXTENSION}`, ); } diff --git a/vitest.config.ts b/vitest.config.ts index 9108f69..5a52a9b 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -5,7 +5,7 @@ export default defineConfig({ test: { globals: true, environment: "jsdom", - include: ["src/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}"], + include: ["{src,electron}/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}"], exclude: ["src/**/*.browser.test.{ts,tsx}"], }, resolve: { From 36d7d2bdd034f410163a1c82b5b005a6b4c20961 Mon Sep 17 00:00:00 2001 From: neurot1cal Date: Tue, 26 May 2026 16:28:50 -0700 Subject: [PATCH 3/4] fix: tighten streaming failure handling from re-review Addresses the CodeRabbit + Codex re-review of the prior commit. - Normalize a rejected append (channel/handler error, not just a { success: false } result) into appendError, so the write queue never rejects and isStreaming() stays consistent after a failure (CodeRabbit). - Handle a rejected open-stream IPC the same as a failed open: fall back to in-memory buffering instead of leaving the recorder stuck "pending" with an unhandled rejection (CodeRabbit). - Discard a streamed webcam whose write failed even when the screen save succeeds. The cleanup gate is now per-recorder, so a webcam omitted from a successful screen-only save no longer leaks its stream and partial file (Codex). Adds tests for the rejected-append and rejected-open paths. Verified: tsc --noEmit clean; biome clean; vitest 182/182. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/hooks/recorderHandle.test.ts | 43 ++++++++++++++++++++++++++++++++ src/hooks/recorderHandle.ts | 42 ++++++++++++++++++++----------- src/hooks/useScreenRecorder.ts | 27 ++++++++++++-------- 3 files changed, 87 insertions(+), 25 deletions(-) diff --git a/src/hooks/recorderHandle.test.ts b/src/hooks/recorderHandle.test.ts index 3551b7d..adb5370 100644 --- a/src/hooks/recorderHandle.test.ts +++ b/src/hooks/recorderHandle.test.ts @@ -90,6 +90,29 @@ describe("createRecorderHandle", () => { expect(decode(await blob.arrayBuffer())).toBe("abc"); }); + it("falls back to in-memory buffering when the open IPC call rejects", async () => { + const openRecordingStream = vi.fn(async () => { + throw new Error("ipc channel closed"); + }); + stubElectronAPI({ + openRecordingStream, + appendRecordingChunk: vi.fn(async () => ({ success: true })), + }); + + const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }, "rec.webm"); + const fake = driver(handle); + + fake.emit(new Blob(["a"])); + await tick(); // open rejects -> treated as a failed open, keep buffering + fake.emit(new Blob(["b"])); + fake.stop(); + + const blob = await handle.recordedBlobPromise; + expect(handle.isStreaming()).toBe(false); + expect(blob.size).toBe(2); + expect(decode(await blob.arrayBuffer())).toBe("ab"); + }); + it("waits for in-flight chunk writes before stop resolves (no truncation)", async () => { let releaseAppend: () => void = () => undefined; const appendGate = new Promise((resolve) => { @@ -142,6 +165,26 @@ describe("createRecorderHandle", () => { expect(handle.isStreaming()).toBe(false); }); + it("treats a rejected append the same as a failed write", async () => { + stubElectronAPI({ + openRecordingStream: vi.fn(async () => ({ success: true })), + appendRecordingChunk: vi.fn(async () => { + throw new Error("kernel said no"); + }), + closeRecordingStream: vi.fn(async () => ({ success: true })), + }); + + const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }, "rec.webm"); + const fake = driver(handle); + + await tick(); + fake.emit(new Blob(["a"])); + fake.stop(); + + await expect(handle.recordedBlobPromise).rejects.toThrow(/kernel said no/); + expect(handle.isStreaming()).toBe(false); + }); + it("buffers in memory and never opens a stream when no file name is given", async () => { const openRecordingStream = vi.fn(async () => ({ success: true })); stubElectronAPI({ diff --git a/src/hooks/recorderHandle.ts b/src/hooks/recorderHandle.ts index 3c63120..d547bf9 100644 --- a/src/hooks/recorderHandle.ts +++ b/src/hooks/recorderHandle.ts @@ -61,10 +61,17 @@ export function createRecorderHandle( if (appendError || !fileName || !api?.appendRecordingChunk) { return; } - const buffer = await chunk.arrayBuffer(); - const result = await api.appendRecordingChunk(fileName, buffer); - if (!result.success) { - appendError = new Error(result.error ?? "Failed to write recording chunk to disk"); + // Capture both outcomes — a `{ success: false }` result and an outright + // rejection (channel/handler error) — into appendError, so writeChain + // never rejects and isStreaming() stays consistent after a failure. + try { + const buffer = await chunk.arrayBuffer(); + const result = await api.appendRecordingChunk(fileName, buffer); + if (!result.success) { + appendError = new Error(result.error ?? "Failed to write recording chunk to disk"); + } + } catch (error) { + appendError = error instanceof Error ? error : new Error(String(error)); } }); }; @@ -74,18 +81,25 @@ export function createRecorderHandle( ? api.openRecordingStream(fileName) : Promise.resolve({ success: false }); - void openPromise.then((result) => { - if (result.success) { - streamOpened = true; - mode = "streaming"; - for (const chunk of memoryChunks) { - enqueueWrite(chunk); + void openPromise.then( + (result) => { + if (result.success) { + streamOpened = true; + mode = "streaming"; + for (const chunk of memoryChunks) { + enqueueWrite(chunk); + } + memoryChunks.length = 0; + } else { + mode = "buffering"; } - memoryChunks.length = 0; - } else { + }, + () => { + // The IPC call itself rejected (channel or handler error). Treat it the + // same as a failed open: keep buffering in memory so nothing is lost. mode = "buffering"; - } - }); + }, + ); const recordedBlobPromise = new Promise((resolve, reject) => { recorder.ondataavailable = (event: BlobEvent) => { diff --git a/src/hooks/useScreenRecorder.ts b/src/hooks/useScreenRecorder.ts index d6c03f1..f5fb920 100644 --- a/src/hooks/useScreenRecorder.ts +++ b/src/hooks/useScreenRecorder.ts @@ -328,10 +328,11 @@ export function useScreenRecorder(): UseScreenRecorderReturn { window.electronAPI?.setRecordingState(false); void (async () => { - // Set once the recording is safely stored. Until then any disk stream - // is still open, so the finally block closes it and removes the partial - // file on the discard or error paths. - let savedToDisk = false; + // Each disk stream must end up either saved or explicitly discarded. + // store-recorded-session finalizes the streams included in a successful + // save; the finally block discards everything else. + let storeSucceeded = false; + let webcamIncludedInSave = false; try { const screenBlob = await activeScreenRecorder.recordedBlobPromise; if (discardRecordingId.current === activeRecordingId) { @@ -364,6 +365,7 @@ export function useScreenRecorder(): UseScreenRecorderReturn { webcamVideoData = new ArrayBuffer(0); } } + webcamIncludedInSave = webcamVideoData !== undefined; const result = await window.electronAPI.storeRecordedSession({ screen: { @@ -383,8 +385,8 @@ export function useScreenRecorder(): UseScreenRecorderReturn { console.error("Failed to store recording session:", result.message); return; } - // store-recorded-session has flushed and closed the disk streams. - savedToDisk = true; + // store-recorded-session has flushed and closed the saved streams. + storeSucceeded = true; if (result.session) { await window.electronAPI.setCurrentRecordingSession(result.session); @@ -396,12 +398,15 @@ export function useScreenRecorder(): UseScreenRecorderReturn { } catch (error) { console.error("Error saving recording:", error); } finally { - if (!savedToDisk) { - // Discarded, or failed before a successful save — close any - // dangling disk streams and remove their partial files so a - // cancelled or failed run doesn't leak a descriptor or orphan. + // Discard any recorder whose data was not part of a successful save + // — a discarded run, a failed save, or a webcam whose disk write + // failed (so it was omitted while the screen still saved) — so no + // stream or partial file is left open or orphaned. + if (!storeSucceeded) { await activeScreenRecorder.discard().catch(() => undefined); - await activeWebcamRecorder?.discard().catch(() => undefined); + } + if (activeWebcamRecorder && !(storeSucceeded && webcamIncludedInSave)) { + await activeWebcamRecorder.discard().catch(() => undefined); } if (finalizingRecordingId.current === activeRecordingId) { finalizingRecordingId.current = null; From 5c5cab69034cb2108b3a7d78d06a035024d623ec Mon Sep 17 00:00:00 2001 From: neurot1cal Date: Tue, 26 May 2026 16:39:53 -0700 Subject: [PATCH 4/4] fix: don't stream when the append IPC is unavailable Codex re-review: if openRecordingStream exists but appendRecordingChunk does not (renderer/main version skew), the recorder would open the stream and switch to streaming mode, but every append silently no-ops and the save ends up empty. Require both IPC methods before streaming; otherwise fall back to in-memory buffering. Adds a regression test. Verified: tsc --noEmit clean; biome clean; vitest 183/183. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/hooks/recorderHandle.test.ts | 20 ++++++++++++++++++++ src/hooks/recorderHandle.ts | 8 +++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/hooks/recorderHandle.test.ts b/src/hooks/recorderHandle.test.ts index adb5370..3f16437 100644 --- a/src/hooks/recorderHandle.test.ts +++ b/src/hooks/recorderHandle.test.ts @@ -205,6 +205,26 @@ describe("createRecorderHandle", () => { expect(blob.size).toBe(2); }); + it("buffers in memory when appendRecordingChunk is unavailable (version skew)", async () => { + const openRecordingStream = vi.fn(async () => ({ success: true })); + // appendRecordingChunk intentionally omitted to simulate renderer/main skew. + stubElectronAPI({ openRecordingStream }); + + const handle = createRecorderHandle({} as MediaStream, { mimeType: "video/webm" }, "rec.webm"); + const fake = driver(handle); + + fake.emit(new Blob(["a"])); + await tick(); + fake.emit(new Blob(["b"])); + fake.stop(); + + const blob = await handle.recordedBlobPromise; + // Never even attempts to open the stream when it can't append to it. + expect(openRecordingStream).not.toHaveBeenCalled(); + expect(handle.isStreaming()).toBe(false); + expect(blob.size).toBe(2); + }); + it("discard closes the disk stream for a streamed recording", async () => { const closeRecordingStream = vi.fn(async () => ({ success: true })); stubElectronAPI({ diff --git a/src/hooks/recorderHandle.ts b/src/hooks/recorderHandle.ts index d547bf9..e98000e 100644 --- a/src/hooks/recorderHandle.ts +++ b/src/hooks/recorderHandle.ts @@ -76,8 +76,14 @@ export function createRecorderHandle( }); }; + // Require BOTH stream IPC methods before attempting to stream. If only + // openRecordingStream exists (renderer/main version skew), streaming would + // open but every append would silently no-op, saving an empty file — so in + // that case fall through to in-memory buffering instead. const openPromise: Promise<{ success: boolean; error?: string }> = - fileName && api?.openRecordingStream + fileName !== undefined && + typeof api?.openRecordingStream === "function" && + typeof api?.appendRecordingChunk === "function" ? api.openRecordingStream(fileName) : Promise.resolve({ success: false });