From 01f61dfada6a2b54f1a848c038ce904cff583f8d Mon Sep 17 00:00:00 2001 From: shafeq Date: Sat, 2 May 2026 09:48:05 +0800 Subject: [PATCH] fix(export): return temp path from buffer-mode FFmpeg muxer (>2 GiB) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exports of recordings whose muxed output exceeds 2 GiB failed with RangeError [ERR_FS_FILE_TOO_LARGE]: Node's fs.readFile rejects files larger than kIoMaxLength (2 ** 31 - 1). The legacy export pipeline hit this in muxExportedVideoAudioBuffer, which called await fs.readFile(finalized.outputPath) to ship the muxed bytes back to the renderer. Mirror the path-based contract that mux-exported-video-audio-from-path already uses: - muxExportedVideoAudioBuffer now returns { outputPath, metrics } and collects byte size via fs.stat instead of fs.readFile. The unmuxed intermediate is still cleaned up; the muxed output is left for the IPC handler to register and the renderer to finalize. - The mux-exported-video-audio IPC handler registers the muxed output via registerOwnedExportPath and returns { tempPath, metrics }. - preload.ts and electron-env.d.ts: tempPath replaces data in the renderer-facing return type. - videoExporter.ts and modernVideoExporter.ts (the finalizeExportWithFfmpegAudio fallback paths) now return { tempFilePath } so VideoEditor's existing finalize-exported-video flow handles the move — the same path the modern stream-mode export already takes. The renderer already preferred tempFilePath over blob in VideoEditor.tsx for MP4 saves (with the explicit comment "avoids ever allocating a multi-GiB ArrayBuffer in the renderer"), so this just removes the buffer-mode regression for large legacy exports. Adds electron/ipc/export/native-video.test.ts asserting the new contract: muxExportedVideoAudioBuffer returns a path, never calls fs.readFile, and still records muxedVideoBytes via stat. Closes #380 --- electron/electron-env.d.ts | 2 +- electron/ipc/export/native-video.test.ts | 69 ++++++++++++++++++++++++ electron/ipc/export/native-video.ts | 42 ++++++++++----- electron/ipc/register/export.ts | 7 ++- electron/preload.ts | 2 +- src/lib/exporter/modernVideoExporter.ts | 7 +-- src/lib/exporter/videoExporter.ts | 8 +-- 7 files changed, 113 insertions(+), 24 deletions(-) create mode 100644 electron/ipc/export/native-video.test.ts diff --git a/electron/electron-env.d.ts b/electron/electron-env.d.ts index 8b634980b..170641aec 100644 --- a/electron/electron-env.d.ts +++ b/electron/electron-env.d.ts @@ -246,7 +246,7 @@ interface Window { }, ) => Promise<{ success: boolean; - data?: Uint8Array; + tempPath?: string; error?: string; metrics?: RendererFfmpegAudioMuxMetrics; }>; diff --git a/electron/ipc/export/native-video.test.ts b/electron/ipc/export/native-video.test.ts new file mode 100644 index 000000000..de103dcf8 --- /dev/null +++ b/electron/ipc/export/native-video.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("electron", () => ({ + app: { + getPath: vi.fn(() => "/tmp"), + }, +})); + +vi.mock("../ffmpeg/binary", () => ({ + getFfmpegBinaryPath: vi.fn(() => "/usr/bin/ffmpeg"), +})); + +vi.mock("../state", () => ({ + cachedNativeVideoEncoder: null, + setCachedNativeVideoEncoder: vi.fn(), +})); + +const fsMocks = vi.hoisted(() => ({ + writeFile: vi.fn(async () => undefined), + readFile: vi.fn(), + stat: vi.fn(async () => ({ size: 5_000_000_000 })), + unlink: vi.fn(async () => undefined), +})); + +vi.mock("node:fs/promises", () => ({ + default: fsMocks, + ...fsMocks, +})); + +const execFileMock = vi.hoisted(() => + vi.fn((_cmd: string, _args: string[], _opts: unknown, cb: (err: Error | null) => void) => { + cb(null); + return { stdout: "", stderr: "" } as unknown; + }), +); + +vi.mock("node:child_process", () => ({ + execFile: execFileMock, + spawn: vi.fn(), +})); + +import { muxExportedVideoAudioBuffer } from "./native-video"; + +describe("muxExportedVideoAudioBuffer", () => { + it("returns the muxed output path without reading the muxed file into memory", async () => { + const videoData = new ArrayBuffer(64); + const result = await muxExportedVideoAudioBuffer(videoData, { audioMode: "none" }); + + // Path-based contract: caller (IPC handler) registers ownership and + // hands the path to the renderer's finalize-exported-video flow. + expect(typeof result.outputPath).toBe("string"); + expect(result.outputPath.length).toBeGreaterThan(0); + // The 2 GiB bug was a fs.readFile of the muxed output. The fix relies on + // stat-only metric collection — readFile must stay unused. + expect(fsMocks.readFile).not.toHaveBeenCalled(); + // We still record byte size so export metrics survive the change. + expect(result.metrics.muxedVideoBytes).toBe(5_000_000_000); + }); + + it("preserves the input temp path when audioMode='none' (no re-mux)", async () => { + const videoData = new ArrayBuffer(32); + const result = await muxExportedVideoAudioBuffer(videoData, { audioMode: "none" }); + + // muxNativeVideoExportAudio short-circuits when audioMode === "none" and + // returns the input path unchanged. We surface that so the renderer can + // finalize the same temp file the buffer was written to. + expect(result.outputPath).toMatch(/recordly-export-video-/); + }); +}); diff --git a/electron/ipc/export/native-video.ts b/electron/ipc/export/native-video.ts index a92053698..0167ab714 100644 --- a/electron/ipc/export/native-video.ts +++ b/electron/ipc/export/native-video.ts @@ -492,6 +492,8 @@ export async function muxExportedVideoAudioBuffer( `recordly-export-video-${Date.now()}-${Math.random().toString(36).slice(2, 8)}.mp4`, ); const metrics: NativeVideoAudioMuxMetrics = {}; + let succeeded = false; + let outputPath = tempVideoPath; try { const tempVideoWriteStartedAt = getNowMs(); @@ -500,23 +502,35 @@ export async function muxExportedVideoAudioBuffer( metrics.tempVideoBytes = videoData.byteLength; const finalized = await muxNativeVideoExportAudio(tempVideoPath, options); Object.assign(metrics, finalized.metrics); - const muxedVideoReadStartedAt = getNowMs(); - const muxedData = await fs.readFile(finalized.outputPath); - metrics.muxedVideoReadMs = getNowMs() - muxedVideoReadStartedAt; - metrics.muxedVideoBytes = muxedData.byteLength; + outputPath = finalized.outputPath; + // Record byte size via stat instead of reading the whole file into a + // Buffer — fs.readFile throws ERR_FS_FILE_TOO_LARGE on >2 GiB outputs. + try { + const stat = await fs.stat(outputPath); + metrics.muxedVideoBytes = stat.size; + } catch { + // Stat failures are non-fatal; size is purely metric data. + } + succeeded = true; return { - data: new Uint8Array(muxedData), + outputPath, metrics, }; } finally { - await Promise.allSettled([ - removeTemporaryExportFile(tempVideoPath), - removeTemporaryExportFile( - path.join( - path.dirname(tempVideoPath), - `${path.basename(tempVideoPath, path.extname(tempVideoPath))}-final.mp4`, - ), - ), - ]); + // Always remove the unmuxed intermediate when the muxer wrote a separate + // file. Only remove the muxed output on failure — on success the caller + // owns it and is responsible for moving/deleting it. + const cleanupTargets: string[] = []; + if (outputPath !== tempVideoPath) { + cleanupTargets.push(tempVideoPath); + } + if (!succeeded) { + cleanupTargets.push(outputPath); + } + if (cleanupTargets.length > 0) { + await Promise.allSettled( + cleanupTargets.map((target) => removeTemporaryExportFile(target)), + ); + } } } diff --git a/electron/ipc/register/export.ts b/electron/ipc/register/export.ts index 63b6f6df7..7518d0dc1 100644 --- a/electron/ipc/register/export.ts +++ b/electron/ipc/register/export.ts @@ -382,9 +382,14 @@ export function registerExportHandlers() { async (_, videoData: ArrayBuffer, options?: NativeVideoExportFinishOptions) => { try { const result = await muxExportedVideoAudioBuffer(videoData, options ?? {}); + // Register the muxed output so finalize-exported-video / discard- + // exported-temp accept it. Returning a temp path (instead of the + // muxed bytes) keeps us off Node's >2 GiB fs.readFile cap and + // avoids a redundant copy through the renderer. + registerOwnedExportPath(result.outputPath); return { success: true, - data: result.data, + tempPath: result.outputPath, metrics: result.metrics, }; } catch (error) { diff --git a/electron/preload.ts b/electron/preload.ts index c9e464f2a..422ce2c59 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -202,7 +202,7 @@ contextBridge.exposeInMainWorld("electronAPI", { ) => { return ipcRenderer.invoke("mux-exported-video-audio", videoData, options) as Promise<{ success: boolean; - data?: Uint8Array; + tempPath?: string; error?: string; metrics?: NativeVideoAudioMuxMetrics; }>; diff --git a/src/lib/exporter/modernVideoExporter.ts b/src/lib/exporter/modernVideoExporter.ts index 9f4393ed9..229351bd9 100644 --- a/src/lib/exporter/modernVideoExporter.ts +++ b/src/lib/exporter/modernVideoExporter.ts @@ -1244,17 +1244,18 @@ export class ModernVideoExporter { this.finalizationStageMs.ffmpegAudioMuxBreakdown = result.metrics; } - if (!result.success || !result.data) { + if (!result.success || !result.tempPath) { return { success: false, error: result.error || "Failed to mux exported audio with FFmpeg", }; } - const videoBytes = result.data.slice(); + // Returning a temp path (instead of buffering the muxed bytes back into + // the renderer) is what keeps >2 GiB exports off Node's fs.readFile cap. return { success: true, - blob: new Blob([videoBytes.buffer], { type: "video/mp4" }), + tempFilePath: result.tempPath, }; } diff --git a/src/lib/exporter/videoExporter.ts b/src/lib/exporter/videoExporter.ts index bcc887caf..f8581910d 100644 --- a/src/lib/exporter/videoExporter.ts +++ b/src/lib/exporter/videoExporter.ts @@ -981,7 +981,7 @@ export class VideoExporter { this.finalizationStageMs.ffmpegAudioMuxBreakdown = result.metrics; } - if (!result.success || !result.data) { + if (!result.success || !result.tempPath) { return { success: false, error: result.error || "Failed to mux exported audio with FFmpeg", @@ -989,11 +989,11 @@ export class VideoExporter { }; } - const blobData = new Uint8Array(result.data.byteLength); - blobData.set(result.data); + // Returning a temp path (instead of buffering the muxed bytes back into + // the renderer) is what keeps >2 GiB exports off Node's fs.readFile cap. return { success: true, - blob: new Blob([blobData.buffer], { type: "video/mp4" }), + tempFilePath: result.tempPath, metrics: this.buildExportMetrics(), }; }