From 9fb917b910e0fc22f1584fe1c3cb196b1f53f849 Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Tue, 4 Aug 2026 23:07:16 +0200 Subject: [PATCH] fix(recording): stream the native webcam to disk instead of losing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On the macOS and Linux native capture paths the webcam handle was built with no file name, which selects in-memory buffering. Nothing reached disk during capture, and finalize had to flatten the whole clip into one ArrayBuffer to hand it across IPC. Past ~2GB — a take of roughly 20 minutes at BITRATE_BASE — that allocation throws. The throw was swallowed twice over: fixWebmDuration catches its own FileReader failure and returns the unpatched blob, then the finalize catch logged to console and returned undefined, which made the attach guard skip attachNative*WebcamRecording entirely. The session was written screen-only and the editor opened as if nothing had happened, with the camera simply absent. A 23-minute take lost its webcam this way; shorter takes in the same app session saved fine. Pass the webcam file name on both native paths so chunks stream to disk as they arrive, the way the legacy path and the Windows helper already do. Finalize now branches on isStreaming(): a streamed clip hands over its name alone and the main process closes the stream and patches the WebM duration on disk, so nothing multi-gigabyte is ever flattened or sent across IPC. Buffered short takes keep the existing behaviour. Every failure now comes back with a reason and reaches the user as a toast. Silently discarding a completed take is the worst available outcome, and it was the one that shipped. Because the bytes now land on disk during capture, a webcam stream that isn't folded into a saved session is closed and its partial file removed — otherwise a discarded or failed take orphans a half-written .webm. The macOS and Linux finalizers were line-for-line copies, so the shared logic moves into finalizeWebcamAsset() rather than being duplicated again. Its tests pin both halves of the fix: that a streamed clip is never read into memory, and that a failure is never silent. Fixes #253 --- electron/electron-env.d.ts | 3 + electron/ipc/handlers.ts | 46 ++++++-- electron/preload.ts | 2 + src/hooks/useScreenRecorder.ts | 198 +++++++++++++++++++++------------ src/hooks/webcamAsset.test.ts | 110 ++++++++++++++++++ 5 files changed, 279 insertions(+), 80 deletions(-) create mode 100644 src/hooks/webcamAsset.test.ts diff --git a/electron/electron-env.d.ts b/electron/electron-env.d.ts index e5381999b..9e1fb7d0d 100644 --- a/electron/electron-env.d.ts +++ b/electron/electron-env.d.ts @@ -178,6 +178,8 @@ interface Window { recordingId: number; webcam: import("../src/lib/recordingSession").RecordedVideoAssetInput; cursorCaptureMode?: import("../src/lib/recordingSession").CursorCaptureMode; + durationMs?: number; + webcamOffsetMs?: number; }) => Promise<{ success: boolean; path?: string; @@ -233,6 +235,7 @@ interface Window { recordingId: number; webcam: import("../src/lib/recordingSession").RecordedVideoAssetInput; cursorCaptureMode?: import("../src/lib/recordingSession").CursorCaptureMode; + durationMs?: number; webcamOffsetMs?: number; }) => Promise<{ success: boolean; diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 3800671e5..5fae3cbeb 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -439,6 +439,12 @@ type AttachNativeMacWebcamRecordingInput = { recordingId?: number; webcam?: RecordedVideoAssetInput; cursorCaptureMode?: CursorCaptureMode; + /** + * Webcam clip duration (ms), head start included. A streamed webcam file carries + * no Duration header and the renderer no longer holds the blob to patch, so the + * main process repairs the container on disk with this value. + */ + durationMs?: number; /** See {@link ProjectMedia.webcamOffsetMs}. */ webcamOffsetMs?: number; }; @@ -2788,6 +2794,13 @@ export function registerIpcHandlers( } }); + // On-disk write streams for in-progress recordings, keyed by output file name. + // Chunks append as they arrive so the renderer never buffers the full video (#616). + // Declared here because both the webcam attach below and store-recorded-session + // finalize through the same registry. + const recordingStreams = new RecordingStreamRegistry(); + registerRecordingStreamHandlers(ipcMain, recordingStreams, resolveRecordingOutputPath); + /** * Writes a browser-recorded webcam clip next to a natively-recorded screen * video and rewrites the session manifest to include both. @@ -2817,7 +2830,7 @@ export function registerIpcHandlers( await fs.access(screenVideoPath, fsConstants.R_OK); - if (!payload.webcam?.fileName || !payload.webcam.videoData) { + if (!payload.webcam?.fileName) { return { success: false, error: `Native ${platformLabel} webcam attachment is missing video data.`, @@ -2825,7 +2838,31 @@ export function registerIpcHandlers( } const webcamVideoPath = resolveRecordingOutputPath(payload.webcam.fileName); - await fs.writeFile(webcamVideoPath, Buffer.from(payload.webcam.videoData)); + // A streamed webcam arrives with an empty buffer: its bytes are already on + // disk, so close the stream and keep the file rather than writing it here. + // Nothing multi-gigabyte crosses IPC or gets flattened into one Buffer (#253). + const webcamStreamed = await finalizeRecordingFile( + recordingStreams, + payload.webcam.fileName, + webcamVideoPath, + payload.webcam.videoData, + ); + // Mirrors finalizeRecordingFile's own condition, so this fires exactly when + // it wrote nothing and the session would point at a file that isn't there. + if ( + !webcamStreamed && + !(payload.webcam.videoData && payload.webcam.videoData.byteLength > 0) + ) { + return { + success: false, + error: `Native ${platformLabel} webcam attachment is missing video data.`, + }; + } + // Streamed files lack the WebM Duration header, which the editor needs to + // scale its timeline. Best-effort: a failed repair leaves the clip intact. + if (webcamStreamed && isValidDurationMs(payload.durationMs)) { + await repairRecordingContainer(webcamVideoPath, payload.durationMs); + } const createdAt = typeof payload.recordingId === "number" && Number.isFinite(payload.recordingId) @@ -2892,11 +2929,6 @@ export function registerIpcHandlers( }, ); - // On-disk write streams for in-progress recordings, keyed by output file name. - // Chunks append as they arrive so the renderer never buffers the full video (#616). - const recordingStreams = new RecordingStreamRegistry(); - registerRecordingStreamHandlers(ipcMain, recordingStreams, resolveRecordingOutputPath); - ipcMain.handle("store-recorded-session", async (_, payload: StoreRecordedSessionInput) => { try { return await storeRecordedSessionFiles(payload); diff --git a/electron/preload.ts b/electron/preload.ts index 04b2427ae..8e018ed8e 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -209,6 +209,7 @@ contextBridge.exposeInMainWorld("electronAPI", { recordingId: number; webcam: { fileName: string; videoData: ArrayBuffer }; cursorCaptureMode?: import("../src/lib/recordingSession").CursorCaptureMode; + durationMs?: number; webcamOffsetMs?: number; }) => { return ipcRenderer.invoke("attach-native-linux-webcam-recording", payload); @@ -242,6 +243,7 @@ contextBridge.exposeInMainWorld("electronAPI", { recordingId: number; webcam: { fileName: string; videoData: ArrayBuffer }; cursorCaptureMode?: import("../src/lib/recordingSession").CursorCaptureMode; + durationMs?: number; webcamOffsetMs?: number; }) => { return ipcRenderer.invoke("attach-native-mac-webcam-recording", payload); diff --git a/src/hooks/useScreenRecorder.ts b/src/hooks/useScreenRecorder.ts index f5ec47faa..7a35fca77 100644 --- a/src/hooks/useScreenRecorder.ts +++ b/src/hooks/useScreenRecorder.ts @@ -143,6 +143,48 @@ export function webcamOffsetMsFrom( return -Math.round(nativeStartedAtMs - webcamStartedAtMs); } +/** + * Turn a finished webcam recorder into the asset the native attach IPC wants, or + * into the reason it cannot be saved. Shared by the macOS and Linux finalizers, + * which differ only in the name they log under. + * + * A streamed recording resolves an empty blob by design — its bytes are already + * on disk — so it hands over the file name alone and the main process closes the + * stream and patches the duration there. Only a buffered recording is read into + * memory, and flattening one of those into a single ArrayBuffer is exactly what + * used to throw past ~2 GB and cost the user the whole camera track (#253). + * + * Never resolves to "nothing happened": every failure comes back with a reason, + * because the screen recording still saves and a silent drop just opens the + * editor with the camera mysteriously absent. + */ +export async function finalizeWebcamAsset( + webcamRecorder: RecorderHandle, + fileName: string, + durationMs: number, + platformLabel: string, +): Promise<{ asset?: RecordedVideoAssetInput; error?: string }> { + try { + if (webcamRecorder.recorder.state !== "inactive") { + webcamRecorder.recorder.stop(); + } + // Rejects on a mid-stream write failure, so a truncated recording lands in + // the catch below rather than passing for a good one. + const webcamBlob = await webcamRecorder.recordedBlobPromise; + if (webcamRecorder.isStreaming()) { + return { asset: { videoData: new ArrayBuffer(0), fileName } }; + } + if (!webcamBlob || webcamBlob.size === 0) { + return { error: "the webcam produced no data" }; + } + const fixedWebcamBlob = await fixWebmDuration(webcamBlob, durationMs); + return { asset: { videoData: await fixedWebcamBlob.arrayBuffer(), fileName } }; + } catch (error) { + console.error(`Failed to finalize native ${platformLabel} webcam recording:`, error); + return { error: error instanceof Error ? error.message : String(error) }; + } +} + export function useScreenRecorder(): UseScreenRecorderReturn { const t = useScopedT("editor"); const [recording, setRecording] = useState(false); @@ -580,38 +622,23 @@ export function useScreenRecorder(): UseScreenRecorderReturn { if (activeWebcamRecorder && webcamRecorder.current === activeWebcamRecorder) { webcamRecorder.current = null; } - const webcamAssetPromise = (async (): Promise => { - if (!activeWebcamRecorder) { - return undefined; - } - - try { - if (activeWebcamRecorder.recorder.state !== "inactive") { - activeWebcamRecorder.recorder.stop(); - } - const webcamBlob = await activeWebcamRecorder.recordedBlobPromise; - if (!webcamBlob || webcamBlob.size === 0) { - return undefined; - } - // The webcam MediaRecorder started before the native recording did (see - // webcamOffsetMs on NativeMacRecordingHandle), so its real content is - // longer than the screen's active `duration` by that same head start. - // Patching the WebM's declared duration to the screen's shorter duration - // would make that extra leading footage unseekable in a standard