diff --git a/packages/producer/src/services/render/audioPadTrim.test.ts b/packages/producer/src/services/render/audioPadTrim.test.ts index 89e31a694..8e686180e 100644 --- a/packages/producer/src/services/render/audioPadTrim.test.ts +++ b/packages/producer/src/services/render/audioPadTrim.test.ts @@ -44,17 +44,28 @@ describe("buildPadTrimAudioArgs", () => { const concatArgs = plan.steps[1]!.args; expect(plan.steps[1]!.kind).toBe("pad-concat"); expect(concatArgs).toContain("concat"); - expect(concatArgs[concatArgs.indexOf("-i") + 1]).toBe("pipe:0"); + // The concat script is passed via a real file (NOT `pipe:0`). Feeding + // it through stdin makes FFmpeg's URL joiner prepend `pipe:` to bare + // absolute paths in the script — the demuxer then tries to open e.g. + // `pipe:/tmp/foo.aac` and fails with "Impossible to open pipe:/…". + // Materializing to a file matches `assemble.ts`'s concat convention. + expect(plan.steps[1]!.concatListPath).toBe("/tmp/out.aac.concat-list.txt"); + expect(concatArgs[concatArgs.indexOf("-i") + 1]).toBe("/tmp/out.aac.concat-list.txt"); expect(concatArgs[concatArgs.indexOf("-c:a") + 1]).toBe("copy"); expect(concatArgs[concatArgs.length - 1]).toBe("/tmp/out.aac"); // Concat script MUST use bare paths, NOT `file://` URLs. FFmpeg 8.x // on Windows can't open `file:///C:/…` URLs from the concat demuxer // (field-signal ts=1784169914 / 1784177061 / 1784177375). Regression - // pin: the `file://` scheme prefix must never appear in the stdin. - expect(plan.steps[1]!.stdin).toContain("file '/tmp/in.aac'"); - expect(plan.steps[1]!.stdin).toContain("file '/tmp/out.aac.pad-silence.aac'"); - expect(plan.steps[1]!.stdin).not.toContain("file://"); - expect(plan.cleanupPaths).toEqual(["/tmp/out.aac.pad-silence.aac"]); + // pin: the `file://` scheme prefix must never appear in the concat + // list content. + expect(plan.steps[1]!.concatListContent).toContain("file '/tmp/in.aac'"); + expect(plan.steps[1]!.concatListContent).toContain("file '/tmp/out.aac.pad-silence.aac'"); + expect(plan.steps[1]!.concatListContent).not.toContain("file://"); + // Cleanup includes BOTH the silence tail and the concat list script. + expect(plan.cleanupPaths).toEqual([ + "/tmp/out.aac.pad-silence.aac", + "/tmp/out.aac.concat-list.txt", + ]); const reencodedSourceStep = plan.steps.find( (step) => @@ -112,7 +123,7 @@ describe("buildPadTrimAudioArgs", () => { expect(trimNeeded.operation).toBe("trim"); }); - it("does not emit `file://` URLs in the pad-concat stdin (FFmpeg 8.x Windows compat)", () => { + it("does not emit `file://` URLs in the pad-concat script (FFmpeg 8.x Windows compat)", () => { // Regression pin for field-signal reports ts=1784169914 / 1784177061 / // ts=1784177375 (win32/x64, CLI 0.7.59, ffmpeg 8.1.1-full_build). The // concat demuxer's file open on Windows in FFmpeg 8.x rejects @@ -128,14 +139,33 @@ describe("buildPadTrimAudioArgs", () => { expect(winPlan.operation).toBe("pad"); const concatStep = winPlan.steps.find((s) => s.kind === "pad-concat"); expect(concatStep).toBeDefined(); - expect(concatStep!.stdin).toBeDefined(); - expect(concatStep!.stdin).not.toContain("file://"); - expect(concatStep!.stdin).not.toContain("file:\\\\"); + expect(concatStep!.concatListContent).toBeDefined(); + expect(concatStep!.concatListContent).not.toContain("file://"); + expect(concatStep!.concatListContent).not.toContain("file:\\\\"); // Bare Windows paths appear as-is in the concat directives. - expect(concatStep!.stdin).toContain( + expect(concatStep!.concatListContent).toContain( "file 'C:\\Users\\alice\\AppData\\Local\\Temp\\hf-render-abc\\audio.aac'", ); }); + + it("materializes the pad-concat script to a real file (not `pipe:0`)", () => { + // Regression pin for the Linux CI failure that surfaced when the + // fix originally dropped `file://` while still feeding the concat + // script via `pipe:0`. FFmpeg's URL joiner resolves bare absolute + // paths against the base `pipe:` URL, producing `pipe:/tmp/foo.aac` + // which the demuxer then tries to open as a pipe. Materializing to + // a real file makes the demuxer treat absolute paths as absolute. + const plan = buildPadTrimAudioPlan("/tmp/in.aac", "/tmp/out.aac", 4.0, 5.0); + const concatStep = plan.steps.find((s) => s.kind === "pad-concat"); + expect(concatStep).toBeDefined(); + // The concat script must NOT be piped in via stdin. + expect(concatStep!.args).not.toContain("pipe:0"); + // The `-i` arg points at the materialized concat list file. + const iIdx = concatStep!.args.indexOf("-i"); + expect(concatStep!.args[iIdx + 1]).toBe(concatStep!.concatListPath); + // The concat list file is cleaned up alongside the silence tail. + expect(plan.cleanupPaths).toContain(concatStep!.concatListPath!); + }); }); describe("padOrTrimAudioToVideoFrameCount", () => { diff --git a/packages/producer/src/services/render/audioPadTrim.ts b/packages/producer/src/services/render/audioPadTrim.ts index 81830a9a4..e576a9bee 100644 --- a/packages/producer/src/services/render/audioPadTrim.ts +++ b/packages/producer/src/services/render/audioPadTrim.ts @@ -19,11 +19,10 @@ */ import { spawn } from "node:child_process"; -import { rmSync } from "node:fs"; +import { rmSync, writeFileSync } from "node:fs"; import { extractAudioMetadata, formatFfmpegError, - getFfmpegBinary, getFfprobeBinary, runFfmpeg, type AudioMetadata, @@ -69,10 +68,7 @@ export interface PadTrimAudioInput { */ probeVideoFrameInfo?: (videoPath: string) => Promise; probeAudioInfo?: (audioPath: string) => Promise; - runFfmpeg?: ( - args: string[], - options?: { stdin?: string }, - ) => Promise<{ success: boolean; error?: string }>; + runFfmpeg?: (args: string[]) => Promise<{ success: boolean; error?: string }>; } export type PadTrimOperation = "pad" | "trim" | "copy"; @@ -95,7 +91,16 @@ export type PadTrimAudioStepKind = "copy" | "trim" | "pad-silence" | "pad-concat export interface PadTrimAudioStep { kind: PadTrimAudioStepKind; args: string[]; - stdin?: string; + /** + * Concat-demuxer script materialization. When both fields are set, the + * runner writes `concatListContent` to `concatListPath` synchronously + * before spawning ffmpeg, and the step's `args` reference that path via + * `-i concatListPath`. Feeding the concat script through a real file + * (instead of `pipe:0`) is what makes bare-path directives work on both + * Linux and Windows — see `concatFileLine` for the platform history. + */ + concatListPath?: string; + concatListContent?: string; } export interface PadTrimAudioPlan { @@ -137,6 +142,7 @@ export function buildPadTrimAudioPlan( if (delta > 0) { const padDur = formatSeconds(delta); const silencePath = `${outputPath}.pad-silence.aac`; + const concatListPath = `${outputPath}.concat-list.txt`; return { operation: "pad", steps: [ @@ -164,19 +170,18 @@ export function buildPadTrimAudioPlan( "concat", "-safe", "0", - "-protocol_whitelist", - "file,pipe,crypto,data", "-i", - "pipe:0", + concatListPath, "-c:a", "copy", "-y", outputPath, ], - stdin: `${concatFileLine(audioPath)}\n${concatFileLine(silencePath)}\n`, + concatListPath, + concatListContent: `${concatFileLine(audioPath)}\n${concatFileLine(silencePath)}\n`, }, ], - cleanupPaths: [silencePath], + cleanupPaths: [silencePath, concatListPath], }; } // Trim. `-t` truncates AAC without re-encoding because AAC frames are @@ -230,16 +235,26 @@ function channelLayoutForChannels(channels: number | undefined): string { } function concatFileLine(path: string): string { - // Bare paths in concat directives — NOT `file://` URLs. FFmpeg 8.x on - // Windows fails to open URL-form paths from the concat demuxer with - // "Impossible to open file:///C:/…" (its file protocol strips the - // `file:` prefix leaving `///C:/…`, which Windows path parsing then - // rejects). Field-signal reports (ts=1784169914 / 1784177061 / - // 1784177375, all win32/x64 CLI 0.7.59; the last isolated the module's - // arg shape vs a working manual `apad=whole_dur` command). Bare paths - // also match the convention already used by the sibling concat - // scripts in `assemble.ts` and `chunkEncoder.ts`. The single-quote - // escaping (`'\''`) is the concat demuxer's own escape rule. + // Bare paths in concat directives — NOT `file://` URLs. Two failure + // modes on the round trip to a working shape: + // 1. `file:///C:/…` — FFmpeg 8.x on Windows fails to open URL-form + // paths from the concat demuxer with "Impossible to open + // file:///C:/…" (its `file:` protocol strips the scheme leaving + // `///C:/…`, which Windows path parsing then rejects). Field- + // signal reports ts=1784169914 / 1784177061 / 1784177375 (all + // win32/x64 CLI 0.7.59; the last isolated the module's arg shape + // vs a working manual `apad=whole_dur` command). + // 2. Bare `/tmp/…` when the concat script was fed via `pipe:0` — + // FFmpeg's URL joiner resolves absolute POSIX paths against the + // base `pipe:` URL, producing `pipe:/tmp/…` which the demuxer + // then tries to open as a pipe. Broke Linux CI (regression shard + // + producer integration) once the `file://` prefix was dropped. + // Fix: emit bare paths AND materialize the concat script into a real + // file (see `concatListPath`/`concatListContent` on the pad-concat + // step), matching sibling `assemble.ts`'s concat convention. A real + // file's directory becomes the base URL, and absolute paths in the + // script resolve as-is on both platforms. The single-quote escaping + // (`'\''`) is the concat demuxer's own escape rule. return `file '${path.replace(/'/g, "'\\''")}'`; } @@ -308,7 +323,14 @@ export async function padOrTrimAudioToVideoFrameCount( try { for (const step of plan.steps) { - const ffmpegResult = await runner(step.args, { stdin: step.stdin }); + // Materialize the concat-demuxer script to a real file when the step + // needs one. Doing this here (instead of piping via `pipe:0` in the + // runner) is what makes the demuxer's URL resolution treat + // absolute paths as absolute — see `concatFileLine` for context. + if (step.concatListPath !== undefined && step.concatListContent !== undefined) { + writeFileSync(step.concatListPath, step.concatListContent, "utf-8"); + } + const ffmpegResult = await runner(step.args); if (!ffmpegResult.success) { return { success: false, @@ -435,12 +457,7 @@ async function defaultProbeAudioInfo(audioPath: string): Promise }; } -async function defaultRunFfmpeg( - args: string[], - options?: { stdin?: string }, -): Promise<{ success: boolean; error?: string }> { - if (options?.stdin !== undefined) return runFfmpegWithStdin(args, options.stdin); - +async function defaultRunFfmpeg(args: string[]): Promise<{ success: boolean; error?: string }> { const result = await runFfmpeg(args); if (result.success) return { success: true }; return { @@ -449,40 +466,6 @@ async function defaultRunFfmpeg( }; } -async function runFfmpegWithStdin( - args: string[], - stdin: string, -): Promise<{ success: boolean; error?: string }> { - return new Promise((resolve) => { - const proc = spawn(getFfmpegBinary(), args); - let stderr = ""; - - proc.stderr.on("data", (data: Buffer) => { - stderr += data.toString(); - }); - - proc.on("error", (err) => { - resolve({ - success: false, - error: `[audioPadTrim] ${err instanceof Error ? err.message : String(err)}`, - }); - }); - - proc.on("close", (code) => { - if (code === 0) { - resolve({ success: true }); - return; - } - resolve({ - success: false, - error: `[audioPadTrim] ${formatFfmpegError(code, stderr)}`, - }); - }); - - proc.stdin.end(stdin); - }); -} - // ── ffprobe JSON runner (shared between fast/slow video probe paths) ───── function runFfprobeJson(args: string[]): Promise {