From 0b1fc990afc72811a906a83de8aff4e62ea05799 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Thu, 24 Sep 2026 19:52:37 +0000 Subject: [PATCH] State the audio duration instead of inferring it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The narrated master shipped with no audio stream at all, and before that with 2.3 seconds of audio against five seconds of picture. Both came from the same wrong assumption about -shortest: in ffmpeg 4.x it keys off INPUT durations, so pairing it with apad — whose output is infinite by design — does not trim the padded stream back to the video. It either left the original speech length alone or dropped the stream. The output duration is now stated outright with -t. That is not a workaround for -shortest; it is the thing actually being asserted, which is that picture and sound are the same length. Verified against real ffmpeg rather than by reading the argument list. The pipeline test now encodes a 2.3s tone over 150 frames and asserts the muxed result has an audio stream whose duration is within one AAC frame of five seconds. Both previous failures — the short track and the missing track — pass an arguments-only check, which is why neither was caught before production. Co-Authored-By: Claude Opus 5 (1M context) --- lib/ads/video/encode.ts | 12 +++++- tests/ads-video-compose-validate.test.ts | 8 +++- tests/ads-video-pipeline.test.ts | 48 ++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 3 deletions(-) diff --git a/lib/ads/video/encode.ts b/lib/ads/video/encode.ts index 4aaf348b..0fb3732b 100644 --- a/lib/ads/video/encode.ts +++ b/lib/ads/video/encode.ts @@ -14,6 +14,7 @@ import { PREROLL_FPS, PREROLL_FRAMES, videoProfile, + PREROLL_MS, type VideoProfileId, } from "./profiles"; @@ -107,8 +108,17 @@ export function mp4Args(o: Mp4EncodeOptions): string[] { // audio stream that ended early. Validation requires the two durations to // match within one AAC frame and rejected every narrated render, and a // player handed a short track is entitled to stop at its end. + // apad extends the audio with silence; -t then fixes the output at exactly + // the ad's length so picture and sound agree to the millisecond. + // + // -shortest was the obvious pairing and does not work here: in ffmpeg 4.x + // it keys off INPUT durations, so an infinitely padded filter output does + // not extend it and the encode either kept the original 2.3s of speech or, + // once padded, dropped the stream entirely. An explicit duration is not a + // workaround — it is the thing actually being asserted. args.push("-af", "apad"); - args.push("-c:a", "aac", "-profile:a", "aac_low", "-ar", String(AAC_SAMPLE_RATE), "-b:a", "128k", "-ac", "2", "-shortest"); + args.push("-t", String(PREROLL_MS / 1000)); + args.push("-c:a", "aac", "-profile:a", "aac_low", "-ar", String(AAC_SAMPLE_RATE), "-b:a", "128k", "-ac", "2"); } else { args.push("-an"); } diff --git a/tests/ads-video-compose-validate.test.ts b/tests/ads-video-compose-validate.test.ts index 800efd76..08d9f9ab 100644 --- a/tests/ads-video-compose-validate.test.ts +++ b/tests/ads-video-compose-validate.test.ts @@ -200,8 +200,12 @@ describe("encoder arguments carry the media contract", () => { videoKbps: 4000, }).join(" "); expect(a).toContain("-af apad"); - // apad alone runs forever; -shortest is what trims it back to the video. - expect(a).toContain("-shortest"); + // apad alone runs forever, and -shortest does NOT trim it: in ffmpeg 4.x + // that flag keys off input durations, so the padded stream either kept the + // original 2.3s of speech or vanished entirely. The output duration is + // stated outright instead. + expect(a).toContain("-t 5"); + expect(a).not.toContain("-shortest"); }); it("stream-copies into HLS rather than re-encoding", () => { diff --git a/tests/ads-video-pipeline.test.ts b/tests/ads-video-pipeline.test.ts index c6b37623..f6ab83f0 100644 --- a/tests/ads-video-pipeline.test.ts +++ b/tests/ads-video-pipeline.test.ts @@ -13,6 +13,7 @@ import { mkdtemp, rm, readFile, readdir } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { FFMPEG_BIN, FFPROBE_BIN, run } from "@/lib/ads/video/encode"; +import { mp4Args } from "@/lib/ads/video/encode"; import { renderPreroll, narrationVtt, type FrameCapturer } from "@/lib/ads/video/render"; import { probeMedia, evaluateProbe, validateMediaPlaylist } from "@/lib/ads/video/validate"; import { PREROLL_FRAMES, videoProfile } from "@/lib/ads/video/profiles"; @@ -222,4 +223,51 @@ describe("a snapshot renders to validated media", () => { const vtt = narrationVtt(" Try NicheDB\n today. "); expect(vtt).toBe("WEBVTT\n\n00:00:00.000 --> 00:00:05.000\nTry NicheDB today.\n"); }); + it("gives a narrated encode an audio track exactly as long as the picture", async () => { + // Every narration is shorter than its ad: a five-second read is about two + // and a half seconds of speech. This asserts the padding against real + // ffmpeg, because the obvious spelling of it does not work — `-shortest` + // keys off input durations in ffmpeg 4.x, so a padded filter output either + // left the short track alone or dropped the stream entirely, and both + // shipped past unit tests that only inspected the argument list. + const dir = await mkdtemp(path.join(tmpdir(), "narrated-")); + try { + // A 2.3s tone stands in for the voiceover. + const audio = path.join(dir, "narration.mp3"); + await run(FFMPEG_BIN, ["-y", "-v", "error", "-f", "lavfi", "-i", + "sine=frequency=440:duration=2.3", audio], 60_000); + + // 150 frames of flat colour: the picture is not what is under test. + for (let i = 0; i < 150; i++) { + await run(FFMPEG_BIN, ["-y", "-v", "error", "-f", "lavfi", "-i", + "color=c=#12161f:s=320x180:d=1", "-frames:v", "1", + path.join(dir, `f-${String(i).padStart(4, "0")}.png`)], 60_000); + } + + const out = path.join(dir, "narrated.mp4"); + await run(FFMPEG_BIN, mp4Args({ + framePattern: path.join(dir, "f-%04d.png"), + audioPath: audio, + outPath: out, + profile: "mp4_480p", + videoKbps: 800, + }), 180_000); + + const probed = await run(FFPROBE_BIN, ["-v", "error", "-show_entries", + "stream=codec_type,duration", "-of", "json", out], 60_000); + const streams = JSON.parse(String(probed)).streams as { codec_type: string; duration?: string }[]; + + const audioStream = streams.find((st) => st.codec_type === "audio"); + // The track must exist at all: a silent "narrated" ad is the bug that + // reached production. + expect(audioStream, "no audio stream in a narrated encode").toBeTruthy(); + + const seconds = Number(audioStream!.duration); + // Within one AAC frame of five seconds, which is what validation demands. + expect(Math.abs(seconds - 5)).toBeLessThan(0.05); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }, 240_000); + });