mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 12:54:29 +00:00
Merge pull request #2525 from heygen-com/via/audiopad-ffmpeg8-compat
fix(producer): audioPadTrim FFmpeg-8.x-compatible apad invocation
This commit is contained in:
@@ -44,12 +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");
|
||||
expect(plan.steps[1]!.stdin).toContain("file 'file:///tmp/in.aac'");
|
||||
expect(plan.steps[1]!.stdin).toContain("file 'file:///tmp/out.aac.pad-silence.aac'");
|
||||
expect(plan.cleanupPaths).toEqual(["/tmp/out.aac.pad-silence.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 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) =>
|
||||
@@ -106,6 +122,50 @@ describe("buildPadTrimAudioArgs", () => {
|
||||
const trimNeeded = buildPadTrimAudioArgs("/tmp/a.aac", "/tmp/o.aac", 5.002, 5.0);
|
||||
expect(trimNeeded.operation).toBe("trim");
|
||||
});
|
||||
|
||||
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
|
||||
// `file:///C:/…` URLs with "Impossible to open …". The concat script
|
||||
// MUST use bare paths. Match sibling `assemble.ts` /
|
||||
// `chunkEncoder.ts` conventions.
|
||||
const winPlan = buildPadTrimAudioPlan(
|
||||
"C:\\Users\\alice\\AppData\\Local\\Temp\\hf-render-abc\\audio.aac",
|
||||
"C:\\Users\\alice\\AppData\\Local\\Temp\\hf-render-abc\\audio-padded.aac",
|
||||
4.0,
|
||||
5.0,
|
||||
);
|
||||
expect(winPlan.operation).toBe("pad");
|
||||
const concatStep = winPlan.steps.find((s) => s.kind === "pad-concat");
|
||||
expect(concatStep).toBeDefined();
|
||||
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!.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", () => {
|
||||
|
||||
@@ -19,12 +19,10 @@
|
||||
*/
|
||||
|
||||
import { spawn } from "node:child_process";
|
||||
import { rmSync } from "node:fs";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { rmSync, writeFileSync } from "node:fs";
|
||||
import {
|
||||
extractAudioMetadata,
|
||||
formatFfmpegError,
|
||||
getFfmpegBinary,
|
||||
getFfprobeBinary,
|
||||
runFfmpeg,
|
||||
type AudioMetadata,
|
||||
@@ -70,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";
|
||||
@@ -96,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 {
|
||||
@@ -138,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: [
|
||||
@@ -165,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
|
||||
@@ -231,8 +235,27 @@ function channelLayoutForChannels(channels: number | undefined): string {
|
||||
}
|
||||
|
||||
function concatFileLine(path: string): string {
|
||||
const normalized = pathToFileURL(path).href;
|
||||
return `file '${normalized.replace(/'/g, "'\\''")}'`;
|
||||
// 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, "'\\''")}'`;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -300,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,
|
||||
@@ -427,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 {
|
||||
@@ -441,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> {
|
||||
|
||||
Reference in New Issue
Block a user