ext/hyperframes
2
0
Fork 0
mirror of https://github.com/heygen-com/hyperframes.git synced 2026-09-03 04:38:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(producer): materialize audioPadTrim concat script to real file

Browse Source 
The prior fix (dc410ca) dropped `pathToFileURL` from the pad-concat step
to make FFmpeg 8.x on Windows stop rejecting `file:///C:/…` URLs — but
kept feeding the concat script via `pipe:0` stdin. That combination
broke Linux CI: FFmpeg's concat demuxer resolves bare paths in the
script against the base URL of the script's own source, and when the
script is fed via `pipe:0` the base URL is `pipe:`. Absolute POSIX
paths (`/tmp/foo.aac`) then join to `pipe:/tmp/foo.aac`, which the
demuxer tries to open as a pipe and fails with:

    [concat @ 0x…] Impossible to open 'pipe:/tmp/…/audio.aac'
    pipe:0: End of file

Manually reproduced with `ffmpeg-static@7.0.2` on this repo's binary.

Fix: write the concat script to a real temp file (`<outputPath>.concat-
list.txt`) and pass `-i concatListPath` — matching the sibling concat
in `distributed/assemble.ts:180-186` exactly. A real file's directory
becomes the base URL, so absolute paths in the script resolve as-is on
both Linux and Windows. The `file://` scheme prefix stays out of the
script (Windows FFmpeg 8.x fix preserved) and no `pipe:` prefix gets
prepended (Linux regression fixed). Cleanup path list now covers both
the silence tail and the concat list script.

Also drops the now-unused `runFfmpegWithStdin` helper — no consumer
needs stdin plumbing anymore.

Regression pins in `audioPadTrim.test.ts`:
  - `does not emit file:// URLs …` — Windows arg-shape pin (unchanged
    intent, moved from `stdin` to `concatListContent` field).
  - `materializes the pad-concat script to a real file …` — new pin
    that asserts `-i` is not `pipe:0` and points at the concat list
    path, so the Linux failure mode can't regress.

CI failures fixed:
  - CI / Producer: integration tests (assemble.test.ts pad case)
  - regression / regression-shards shard-1 (style-3-prod field-signal
    end-to-end render exercising the assemble pad path)

Co-Authored-By: Claude <noreply@anthropic.com>

— Via
This commit is contained in:
Via
2026-07-16 06:54:07 +00:00
co-authored by Claude
parent dc410ca990
commit 40f4cfe92f
2 changed files with 87 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/producer/src/services/render/audioPadTrim.test.ts
+41 -11
View File
@@ -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", () => {
packages/producer/src/services/render/audioPadTrim.ts
+46 -63
View File
@@ -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<ProbeVideoFrameInfo>;
probeAudioInfo?: (audioPath: string) => Promise<AudioProbeInfo>;
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<AudioProbeInfo>
};
}
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<T>(args: string[]): Promise<T> {