diff --git a/lib/ads/gif/encode.ts b/lib/ads/gif/encode.ts index 1de2f31..4060c9e 100644 --- a/lib/ads/gif/encode.ts +++ b/lib/ads/gif/encode.ts @@ -16,6 +16,7 @@ import { execFile } from "node:child_process"; import { promisify } from "node:util"; import path from "node:path"; import { GIF_FPS } from "../video/profiles"; +import { FRAME_PATTERN } from "../video/render"; const run = promisify(execFile); @@ -71,7 +72,12 @@ export async function encodeGif(input: { ffmpegPath?: string; }): Promise { const ffmpeg = input.ffmpegPath ?? "ffmpeg"; - const pattern = path.join(input.frameDir, "frame-%04d.png"); + // The shared pattern, not a second copy of it. These were written down twice + // and drifted: the capturer emits f-0000.png and this module asked ffmpeg for + // frame-0000.png, so every banner encode failed with "could find no file" + // while the frame count check passed — it counts .png files and never looks + // at their names. + const pattern = path.join(input.frameDir, FRAME_PATTERN); const palette = path.join(input.frameDir, "palette.png"); await run(ffmpeg, paletteArgs(pattern, palette), { maxBuffer: 16 * 1024 * 1024 }); diff --git a/lib/ads/gif/render.ts b/lib/ads/gif/render.ts index 3072568..2930412 100644 --- a/lib/ads/gif/render.ts +++ b/lib/ads/gif/render.ts @@ -14,7 +14,7 @@ import { mkdir, readdir, stat } from "node:fs/promises"; import path from "node:path"; import { GIF_FRAMES, videoProfile, type GifProfileId } from "../video/profiles"; import type { VideoDesignSnapshot } from "../video/snapshot"; -import type { FrameCapturer } from "../video/render"; +import { FRAME_PATTERN, type FrameCapturer } from "../video/render"; import { GIF_UNITS, gifDocument } from "./compose"; import { encodeGif, validateGif } from "./encode"; @@ -74,10 +74,19 @@ export async function renderAnimatedBanners(args: { // The compositor is the only thing that decides how many frames exist. If // the capture disagrees, the timeline did not run and encoding whatever // landed would ship a banner that is silently short. - const captured = (await readdir(framesDir)).filter((f) => f.endsWith(".png")); - if (captured.length !== GIF_FRAMES) { + // + // Checked by NAME, not just by count. Counting alone passed while the + // capturer wrote f-0000.png and the encoder asked ffmpeg for + // frame-0000.png, so the mismatch reached production as an ffmpeg error + // instead of a clear one here. + const wanted = Array.from({ length: GIF_FRAMES }, (_, i) => + FRAME_PATTERN.replace("%04d", String(i).padStart(4, "0")), + ); + const present = new Set(await readdir(framesDir)); + const missing = wanted.filter((f) => !present.has(f)); + if (missing.length > 0) { throw new Error( - `${unit.id}: captured ${captured.length} frames, expected ${GIF_FRAMES}`, + `${unit.id}: ${missing.length} of ${GIF_FRAMES} frames missing, first is ${missing[0]}`, ); } diff --git a/lib/ads/video/render.ts b/lib/ads/video/render.ts index 58a52fd..4d77402 100644 --- a/lib/ads/video/render.ts +++ b/lib/ads/video/render.ts @@ -388,11 +388,12 @@ export async function renderPreroll(args: { }); } } catch (err) { - problems.push({ - check: "animated banners", - expected: "three units", - actual: (err as Error).message, - }); + // Deliberately NOT pushed onto `problems`: everything in there fails the + // revision, and this comment previously claimed a failed banner would not + // — while the code did the opposite and failed whole renders whose video + // was fine. The banners are optional profiles; their absence is visible + // in the asset list, and the reason belongs in the log. + console.warn(`[render] animated banners skipped: ${(err as Error).message}`); } } diff --git a/tests/ads-gif-pipeline.test.ts b/tests/ads-gif-pipeline.test.ts index 21aced7..3224fc1 100644 --- a/tests/ads-gif-pipeline.test.ts +++ b/tests/ads-gif-pipeline.test.ts @@ -8,6 +8,7 @@ import { GIF_UNITS, gifDocument, gifFrameState, gifTimeline, GIF_TIMELINE } from import { GIF_PALETTE_COLORS, gifArgs, paletteArgs, validateGif } from "@/lib/ads/gif/encode"; import { renderAnimatedBanners } from "@/lib/ads/gif/render"; import { GIF_FPS, GIF_FRAMES, videoProfile } from "@/lib/ads/video/profiles"; +import { FRAME_PATTERN } from "@/lib/ads/video/render"; import type { VideoDesignSnapshot } from "@/lib/ads/video/snapshot"; const run = promisify(execFile); @@ -189,7 +190,10 @@ describe("end to end through real ffmpeg", () => { const svgPath = path.join(a.outDir, `f-${i}.svg`); await writeFile(svgPath, svg, "utf8"); await run("ffmpeg", ["-y", "-v", "error", "-i", svgPath, - path.join(a.outDir, `frame-${String(i).padStart(4, "0")}.png`)]); + // Named exactly as the real capturer names them, derived from the + // shared pattern. Writing "frame-%04d.png" here is what let the + // encoder's divergent literal pass CI and fail in production. + path.join(a.outDir, FRAME_PATTERN.replace("%04d", String(i).padStart(4, "0")))]); await rm(svgPath); } };