mirror of
https://github.com/webadderallorg/Recordly.git
synced 2026-11-09 15:33:31 +00:00
fix(export): return temp path from buffer-mode FFmpeg muxer (>2 GiB)
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
This commit is contained in:
1 parent
1c461734d8
commit
aa024cf497
7 files changed
+113
-24
No files matched your search
Vendored
+1
-1
@@ -246,7 +246,7 @@ interface Window {
|
||||
},
|
||||
) => Promise<{
|
||||
success: boolean;
|
||||
data?: Uint8Array;
|
||||
tempPath?: string;
|
||||
error?: string;
|
||||
metrics?: RendererFfmpegAudioMuxMetrics;
|
||||
}>;
|
||||
|
||||
@@ -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-/);
|
||||
});
|
||||
});
|
||||
@@ -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)),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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) {
|
||||
|
||||
+1
-1
@@ -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;
|
||||
}>;
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -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(),
|
||||
};
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user