fix(producer): normalize padded audio on sample timeline

This commit is contained in:
Miguel Ángel
2026-07-25 21:14:18 +00:00
parent 63bc525ca9
commit 4b116b9880
8 changed files with 115 additions and 181 deletions
@@ -373,29 +373,6 @@ describe("encodeFramesChunkedConcat ffmpegEncodeTimeout", () => {
});
describe("muxVideoWithAudio audio codec handling", () => {
it("caps copied audio to the encoded video duration", async () => {
const { spawn, calls } = createSpawnSpy();
vi.resetModules();
vi.doMock("child_process", () => ({ spawn }));
const { muxVideoWithAudio } = await import("./chunkEncoder.js");
const muxPromise = muxVideoWithAudio(
"/tmp/video-only.mp4",
"/tmp/audio.duration-normalized.m4a",
"/tmp/output.mp4",
undefined,
{ audioCodec: "aac", durationSeconds: 16.066667 },
{ num: 30, den: 1 },
);
await flushMuxCodecResolution();
expect(calls[0]!.args).toContain("-shortest");
expect(calls[0]!.args).toContain("-t");
expect(calls[0]!.args).toContain("16.066667");
emitClose(calls[0]!.proc, 0);
await expect(muxPromise).resolves.toMatchObject({ success: true });
});
it("copies HyperFrames AAC sidecars into MP4 instead of re-encoding", async () => {
const { spawn, calls } = createSpawnSpy();
vi.resetModules();
@@ -80,7 +80,6 @@ export interface MuxVideoWithAudioOptions extends Partial<
/** Preserve a priming edit list known to have been created by AAC re-encoding. */
preserveAudioPrimingEditList?: boolean;
/** Hard cap copied audio to the already-encoded video's exact duration. */
durationSeconds?: number;
}
async function shouldCopyAacSidecar(
@@ -708,16 +707,6 @@ export async function muxVideoWithAudio(
// output container metadata. `-c:v copy` is retained; no re-encode.
args.push("-r", fpsToFfmpegArg(fps));
}
if (config?.durationSeconds !== undefined) {
// Stream-copying a normalized AAC sidecar can preserve a longer packet
// timeline than its container/edit-list duration. `-t` alone limits the
// output timestamp window but does not prevent the copied audio stream's
// tail from extending the MP4 timeline. When an explicit video-derived
// duration is supplied, also stop at the shortest stream so the final
// mux boundary is deterministic.
args.push("-shortest");
args.push("-t", String(config.durationSeconds));
}
args.push("-y", outputPath);
const processTimeout = config?.ffmpegProcessTimeout ?? DEFAULT_CONFIG.ffmpegProcessTimeout;
@@ -292,7 +292,6 @@ export async function assemble(
let normalizedAudio: {
path: string;
preserveAudioPrimingEditList: boolean;
durationSeconds: number;
} | null = null;
if (audioPath !== null && existsSync(audioPath)) {
const paddedAudioPath = join(workDir, "audio-padded.m4a");
@@ -307,8 +306,7 @@ export async function assemble(
}
normalizedAudio = {
path: paddedAudioPath,
preserveAudioPrimingEditList: padTrimResult.operation === "trim",
durationSeconds: padTrimResult.targetDurationSeconds,
preserveAudioPrimingEditList: padTrimResult.operation !== "copy",
};
log.info("[assemble] audio normalized for mux", {
operation: padTrimResult.operation,
@@ -332,7 +330,6 @@ export async function assemble(
{
audioCodec: "aac",
preserveAudioPrimingEditList: normalizedAudio.preserveAudioPrimingEditList,
durationSeconds: normalizedAudio.durationSeconds,
},
{ num: plan.dimensions.fpsNum, den: plan.dimensions.fpsDen },
);
@@ -0,0 +1,81 @@
import { execFileSync, spawnSync } from "node:child_process";
import { mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { padOrTrimAudioToVideoFrameCount } from "./audioPadTrim.js";
const dirs: string[] = [];
afterEach(() => dirs.splice(0).forEach((dir) => rmSync(dir, { recursive: true, force: true })));
const hasFfmpeg = (() => {
try {
execFileSync("ffmpeg", ["-version"], { stdio: "ignore" });
execFileSync("ffprobe", ["-version"], { stdio: "ignore" });
return true;
} catch {
return false;
}
})();
describe.skipIf(!hasFfmpeg)("audio pad real-media packet contract", () => {
it("normalizes a tiny raw-ADTS pad without an oversized terminal packet", async () => {
const dir = mkdtempSync(join(tmpdir(), "hf-pad-"));
dirs.push(dir);
const input = join(dir, "input.aac");
const output = join(dir, "normalized.m4a");
execFileSync("ffmpeg", [
"-hide_banner",
"-loglevel",
"error",
"-f",
"lavfi",
"-i",
"sine=frequency=440:duration=16.04",
"-c:a",
"aac",
"-b:a",
"192k",
"-f",
"adts",
input,
]);
const result = await padOrTrimAudioToVideoFrameCount({
videoPath: join(dir, "video.mp4"),
audioPath: input,
outputPath: output,
probeVideoFrameInfo: async () => ({ frameCount: 482, fpsNum: 30, fpsDen: 1 }),
probeAudioInfo: async () => ({ durationSeconds: 16.04 }),
runFfmpeg: async (args) => {
const p = spawnSync("ffmpeg", ["-hide_banner", "-loglevel", "error", ...args]);
return { success: p.status === 0, error: p.stderr?.toString() };
},
});
expect(result.success).toBe(true);
const probe: {
streams?: Array<{ duration?: string }>;
packets?: Array<{ duration_time?: string }>;
} = JSON.parse(
execFileSync(
"ffprobe",
[
"-v",
"error",
"-show_entries",
"stream=duration",
"-show_entries",
"packet=duration_time",
"-of",
"json",
output,
],
{ encoding: "utf8" },
),
);
const duration = Number(probe.streams?.[0]?.duration);
const packets = probe.packets ?? [];
expect(duration).toBeGreaterThan(16.0);
expect(duration).toBeLessThan(16.1);
expect(Number(packets.at(-1)?.duration_time)).toBeLessThan(0.1);
});
});
@@ -24,61 +24,27 @@ import {
} from "./audioPadTrim.js";
describe("buildPadTrimAudioArgs", () => {
it("emits a concat-copy pad plan when audio is shorter than target", () => {
it("emits a decode/filter/re-encode pad plan when audio is shorter than target", () => {
const plan = buildPadTrimAudioPlan("/tmp/in.aac", "/tmp/out.aac", 4.0, 5.0, {
sampleRate: 48000,
channels: 2,
});
expect(plan.operation).toBe("pad");
expect(plan.steps).toHaveLength(2);
const silenceArgs = plan.steps[0]!.args;
expect(plan.steps[0]!.kind).toBe("pad-silence");
expect(silenceArgs).not.toContain("/tmp/in.aac");
expect(silenceArgs[silenceArgs.indexOf("-i") + 1]).toBe(
"anullsrc=channel_layout=stereo:sample_rate=48000",
);
expect(silenceArgs[silenceArgs.indexOf("-t") + 1]).toBe("1.000000");
expect(silenceArgs[silenceArgs.indexOf("-c:a") + 1]).toBe("aac");
const concatArgs = plan.steps[1]!.args;
expect(plan.steps[1]!.kind).toBe("pad-concat");
expect(concatArgs).toContain("concat");
// 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 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://");
expect(plan.steps).toHaveLength(1);
const args = plan.steps[0]!.args;
expect(args[args.indexOf("-i") + 1]).toBe("/tmp/in.aac");
expect(args[args.indexOf("-af") + 1]).toBe("apad=whole_dur=5.000000");
expect(args[args.indexOf("-t") + 1]).toBe("5.000000");
expect(args[args.indexOf("-c:a") + 1]).toBe("aac");
// 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) =>
step.args.includes("/tmp/in.aac") && step.args[step.args.indexOf("-c:a") + 1] === "aac",
);
expect(reencodedSourceStep).toBeUndefined();
expect(plan.cleanupPaths).toEqual([]);
});
it("keeps the legacy args helper on the first pad materialization step", () => {
const { args, operation } = buildPadTrimAudioArgs("/tmp/in.aac", "/tmp/out.aac", 4.0, 5.0);
expect(operation).toBe("pad");
expect(args).not.toContain("/tmp/in.aac");
expect(args[args.indexOf("-t") + 1]).toBe("1.000000");
expect(args).toContain("/tmp/in.aac");
expect(args[args.indexOf("-t") + 1]).toBe("5.000000");
});
it("filter-trims and re-encodes AAC packet padding beyond the target", () => {
@@ -129,7 +95,7 @@ describe("buildPadTrimAudioArgs", () => {
expect(trimNeeded.operation).toBe("trim");
});
it("does not emit `file://` URLs in the pad-concat script (FFmpeg 8.x Windows compat)", () => {
it("uses apad for Windows-safe duration normalization", () => {
// 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
@@ -143,34 +109,9 @@ describe("buildPadTrimAudioArgs", () => {
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!);
const args = winPlan.steps[0]!.args;
expect(args).toContain("-af");
expect(args[args.indexOf("-af") + 1]).toBe("apad=whole_dur=5.000000");
});
});
@@ -232,11 +173,10 @@ describe("padOrTrimAudioToVideoFrameCount", () => {
expect(result.operation).toBe("pad");
expect(result.targetDurationSeconds).toBe(6);
expect(result.sourceDurationSeconds).toBe(5.5);
expect(captured.args).toHaveLength(2);
expect(captured.args).toHaveLength(1);
const tIdx = captured.args[0]!.indexOf("-t");
expect(captured.args[0]![tIdx + 1]).toBe("0.500000");
expect(captured.args[0]).not.toContain("/tmp/a.aac");
expect(captured.args[1]![captured.args[1]!.indexOf("-c:a") + 1]).toBe("copy");
expect(captured.args[0]![tIdx + 1]).toBe("6.000000");
expect(captured.args[0]![captured.args[0]!.indexOf("-c:a") + 1]).toBe("aac");
});
it("trims a video of N=120 frames at 30/1 fps with longer audio", async () => {
@@ -277,7 +217,7 @@ describe("padOrTrimAudioToVideoFrameCount", () => {
expect(result.operation).toBe("pad");
expect(result.targetDurationSeconds).toBeCloseTo((120 * 1001) / 30000, 9);
const tIdx = captured.args[0]!.indexOf("-t");
expect(captured.args[0]![tIdx + 1]).toMatch(/^0\.004\d+$/);
expect(captured.args[0]![tIdx + 1]).toBe("4.004000");
});
it("propagates video probe failure as success=false", async () => {
@@ -14,13 +14,13 @@
* "audio cuts off early" or "video shows a frozen final frame" bugs.
*
* The fix: post-pad/trim audio to *exactly* `frameCount / fps` seconds at
* assemble time. Pad by concat-copying a generated silence tail. For trim,
* decode and filter to the exact target before re-encoding into an M4A
* container; packet-copying AAC can only cut on packet boundaries.
* assemble time. Both branches decode/filter and re-encode AAC. Concatenating
* ADTS packets with `-c:a copy` is unsafe because concat timestamp estimation
* can stretch the terminal packet in the final MP4.
*/
import { spawn } from "node:child_process";
import { rmSync, writeFileSync } from "node:fs";
import { rmSync } from "node:fs";
import {
extractAudioMetadata,
formatFfmpegError,
@@ -90,21 +90,11 @@ export interface PadTrimAudioResult {
error?: string;
}
export type PadTrimAudioStepKind = "copy" | "trim" | "pad-silence" | "pad-concat";
export type PadTrimAudioStepKind = "copy" | "trim" | "normalize";
export interface PadTrimAudioStep {
kind: PadTrimAudioStepKind;
args: 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 {
@@ -132,7 +122,6 @@ export function buildPadTrimAudioPlan(
outputPath: string,
sourceDurationSeconds: number,
targetDurationSeconds: number,
audioInfo: Pick<AudioProbeInfo, "sampleRate" | "channels"> = {},
): PadTrimAudioPlan {
const delta = targetDurationSeconds - sourceDurationSeconds;
const targetSec = formatSeconds(targetDurationSeconds);
@@ -144,48 +133,28 @@ 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: [
{
kind: "pad-silence",
kind: "normalize",
args: [
"-f",
"lavfi",
"-i",
`anullsrc=channel_layout=${channelLayoutForChannels(audioInfo.channels)}:sample_rate=${sampleRateForFilter(audioInfo.sampleRate)}`,
audioPath,
"-af",
`apad=whole_dur=${targetSec}`,
"-t",
padDur,
targetSec,
"-c:a",
"aac",
"-b:a",
"192k",
"-y",
silencePath,
],
},
{
kind: "pad-concat",
args: [
"-f",
"concat",
"-safe",
"0",
"-i",
concatListPath,
"-c:a",
"copy",
"-y",
outputPath,
],
concatListPath,
concatListContent: `${concatFileLine(audioPath)}\n${concatFileLine(silencePath)}\n`,
},
],
cleanupPaths: [silencePath, concatListPath],
cleanupPaths: [],
};
}
// Packet-copy trimming snaps to AAC frame boundaries (typically 1024
@@ -245,20 +214,7 @@ function formatSeconds(sec: number): string {
return sec.toFixed(6);
}
function sampleRateForFilter(sampleRate: number | undefined): number {
return sampleRate !== undefined && Number.isFinite(sampleRate) && sampleRate > 0
? Math.round(sampleRate)
: 48000;
}
function channelLayoutForChannels(channels: number | undefined): string {
if (channels === 1) return "mono";
if (channels === 6) return "5.1";
if (channels === 8) return "7.1";
return "stereo";
}
function concatFileLine(path: string): string {
/*
// 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
@@ -279,8 +235,7 @@ function concatFileLine(path: string): string {
// 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, "'\\''")}'`;
}
*/
/**
* Pad or trim `audio.aac` so its exact duration matches `frameCount / fps`
@@ -344,7 +299,6 @@ export async function padOrTrimAudioToVideoFrameCount(
input.outputPath,
audioInfo.durationSeconds,
targetDurationSeconds,
audioInfo,
);
try {
@@ -353,9 +307,6 @@ export async function padOrTrimAudioToVideoFrameCount(
// 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 {
@@ -69,7 +69,7 @@ describe("runAssembleStage audio duration parity", () => {
"/tmp/audio.duration-normalized.m4a",
"/tmp/output.mp4",
undefined,
{ audioCodec: "aac", preserveAudioPrimingEditList: true, durationSeconds: 1 },
{ audioCodec: "aac", preserveAudioPrimingEditList: true },
{ num: 30, den: 1 },
);
});
@@ -78,8 +78,7 @@ export async function runAssembleStage(input: AssembleStageInput): Promise<Assem
abortSignal,
{
audioCodec: "aac",
preserveAudioPrimingEditList: normalizeResult.operation === "trim",
durationSeconds: normalizeResult.targetDurationSeconds,
preserveAudioPrimingEditList: normalizeResult.operation !== "copy",
},
job.config.fps,
);