diff --git a/packages/engine/src/services/chunkEncoder.test.ts b/packages/engine/src/services/chunkEncoder.test.ts index cc6325333..80491e966 100644 --- a/packages/engine/src/services/chunkEncoder.test.ts +++ b/packages/engine/src/services/chunkEncoder.test.ts @@ -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(); diff --git a/packages/engine/src/services/chunkEncoder.ts b/packages/engine/src/services/chunkEncoder.ts index e373520a7..709d89ac4 100644 --- a/packages/engine/src/services/chunkEncoder.ts +++ b/packages/engine/src/services/chunkEncoder.ts @@ -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; diff --git a/packages/producer/src/services/distributed/assemble.ts b/packages/producer/src/services/distributed/assemble.ts index 2f98ccef9..5d19316a8 100644 --- a/packages/producer/src/services/distributed/assemble.ts +++ b/packages/producer/src/services/distributed/assemble.ts @@ -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 }, ); diff --git a/packages/producer/src/services/render/audioPadTrim.integration.test.ts b/packages/producer/src/services/render/audioPadTrim.integration.test.ts new file mode 100644 index 000000000..137dbdcd1 --- /dev/null +++ b/packages/producer/src/services/render/audioPadTrim.integration.test.ts @@ -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); + }); +}); diff --git a/packages/producer/src/services/render/audioPadTrim.test.ts b/packages/producer/src/services/render/audioPadTrim.test.ts index ee1931b5b..e6b6d2716 100644 --- a/packages/producer/src/services/render/audioPadTrim.test.ts +++ b/packages/producer/src/services/render/audioPadTrim.test.ts @@ -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 () => { diff --git a/packages/producer/src/services/render/audioPadTrim.ts b/packages/producer/src/services/render/audioPadTrim.ts index f7a40cf23..79128de68 100644 --- a/packages/producer/src/services/render/audioPadTrim.ts +++ b/packages/producer/src/services/render/audioPadTrim.ts @@ -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 = {}, ): 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 { diff --git a/packages/producer/src/services/render/stages/assembleStage.test.ts b/packages/producer/src/services/render/stages/assembleStage.test.ts index a6d392bac..ae0696202 100644 --- a/packages/producer/src/services/render/stages/assembleStage.test.ts +++ b/packages/producer/src/services/render/stages/assembleStage.test.ts @@ -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 }, ); }); diff --git a/packages/producer/src/services/render/stages/assembleStage.ts b/packages/producer/src/services/render/stages/assembleStage.ts index 6a2adf483..39dc431ca 100644 --- a/packages/producer/src/services/render/stages/assembleStage.ts +++ b/packages/producer/src/services/render/stages/assembleStage.ts @@ -78,8 +78,7 @@ export async function runAssembleStage(input: AssembleStageInput): Promise