Render the five-second pre-roll: compositor, encoder, validation (package B) - #273
Merged
ralyodio merged 2 commits intoSep 24, 2026
Merged
Conversation
…kage B) A design snapshot now produces the media a pre-roll actually needs: three MP4 renditions, an fMP4 HLS package, a poster, and captions plus an audible companion where there is narration to carry. Package B of the streaming-ads spec. Nothing selects or serves this yet — that is package D. Two properties the pipeline is built around. It is deterministic. The composition exposes window.__seek(frame) and every animated value is a pure function of that index; nothing reads a clock, and there is no CSS animation or rAF loop whose output would depend on how quickly Playwright got round to the screenshot. That is what lets a render be cached by a hash of its inputs without the hash being a lie. RENDERER_VERSION is part of that hash, so changing the look invalidates every cached encode rather than serving the old bytes forever. It is closed. Artwork reaches the compositor only as a data: URI the caller already fetched and hashed, advertiser copy is escaped before it enters a document a browser executes, and the capture context aborts every request that is not data:/blob:/about:. A renderer that fetched a URL for itself would be a request-forgery primitive running on our own network, and it would also make renders depend on what that URL served today. The snapshot stores artwork content hashes rather than URLs for the same reason: the same URL can serve different bytes, and a cache keyed on the URL would reuse a render of artwork the advertiser has replaced. Validation asks the decoded output, never the job that produced it. ffprobe runs with -count_frames, so "150 frames" means 150 frames decoded rather than whatever the muxer wrote in the header; 29.97 fails where 30 passes; audio gets exactly one AAC frame (21.333ms at 48kHz) of endpoint rounding and no more, and the ad is never padded to six seconds to land on a round segment size. The GOP is pinned to 60 frames with scene-change detection off so the HLS boundaries land on 0/2/4s regardless of artwork, and HLS is packaged by stream copy so the segments carry the media the advertiser approved. EXT-X-INDEPENDENT-SEGMENTS is deliberately never emitted: our segments open on a keyframe but are not independently decodable in the sense that tag asserts, and a player acts on the claim. Publishing a revision is a compare-and-swap against requested_revision. Two quick edits queue revisions 4 and 5; if 4 finishes second, a naive write would point the creative back at media for a design already replaced. Conditioned on requested_revision still being 4, the stale write matches no rows and is discarded. Render jobs dedupe on a hash of immutable inputs only, and the BullMQ job id is colon-free — bullmq parses a colon-bearing custom id as a structured key and rejects anything that is not exactly three parts, and an id derived from state the job itself resets would either stop deduping or collide with a finished job and drop the retry silently. The Redis connection parser moved out of lib/prober-queue into lib/redis-connection now that there are two queues; prober behaviour is unchanged. Frame capture is injected rather than imported so Playwright stays in the worker image, and so the pipeline can be driven in a test without Chromium. tests/ads-video-pipeline.test.ts does exactly that and then runs real ffmpeg: 150 decoded frames at 30fps in every rendition, each inside its byte budget, segments summing to five seconds, init map and ENDLIST present. The spec is explicit that a filename or a five-second timer is not evidence a pre-roll works, so the one test that could have been faked is the one that is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan48 finding(s) HIGH/CRITICAL: 2 | MEDIUM: 31 | LOW: 15
Snippets are redacted; ThreatCrush never prints matched credential material. |
tests/ads-video-pipeline.test.ts skips itself when ffmpeg/ffprobe are absent, which is right for a developer machine and wrong for CI: the one test that proves a pre-roll decodes to 150 frames at 30fps would quietly never run, and a green check would say nothing about it. Installing the binaries makes the check mean what it appears to mean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #272 (package A) — base is that branch, not master, so this diff shows only package B. Merge #272 first and this retargets cleanly.
A design snapshot now produces the media a pre-roll needs: three MP4 renditions, an fMP4 HLS package, a poster, and captions + an audible companion where there is narration. Nothing selects or serves it yet — that's package D.
Two properties the pipeline is built around
It is deterministic. The composition exposes
window.__seek(frame)and every animated value is a pure function of that index. Nothing reads a clock; there's no CSS animation or rAF loop whose output would depend on how quickly Playwright got round to the screenshot. That's what lets a render be cached by a hash of its inputs without the hash being a lie.RENDERER_VERSIONis part of that hash, so changing the look invalidates cached encodes instead of serving the old bytes forever. The snapshot stores artwork content hashes rather than URLs — the same URL can serve different bytes, and a URL-keyed cache would reuse a render of artwork the advertiser has since replaced.It is closed. Artwork reaches the compositor only as a
data:URI the caller already fetched and hashed; advertiser copy is escaped before entering a document a browser executes; the capture context aborts every request that isn'tdata:/blob:/about:. A renderer that fetched a URL for itself would be a request-forgery primitive running on our own network.Validation asks the decoded output, not the job
-count_frames30/1-sc_threshold 0EXT-X-INDEPENDENT-SEGMENTSThe ad is never padded to six seconds to land on a round segment size.
Correctness details worth a reviewer's eye
requested_revision. Two quick edits queue revisions 4 and 5; if 4 finishes second, a naive write points the creative back at media for a design already replaced. Conditioned onrequested_revisionstill being 4, the stale write matches no rows and is discarded.renderJobId()throws rather than letting one through.lib/prober-queueintolib/redis-connectionnow that there are two queues. Prober behaviour unchanged.Verification
tests/ads-video-pipeline.test.tsinjects a synthetic frame capturer (ffmpegtestsrc2, so frames genuinely differ — 150 identical frames would compress to nothing and let a broken bitrate ladder pass) and then runs real ffmpeg and ffprobe:EXT-X-MAP+EXT-X-ENDLIST, segments summing to 5000 ms720p-init.mp4and.m4ssegments actually on disk; multivariant playlist carries both resolutionsPassed locally in ~14 s. It skips if ffmpeg/ffprobe are absent — if this CI runner has no ffmpeg, that test will not have run here, and a green check alone shouldn't be read as it passing. It logs a warning rather than skipping silently.
Totals: 2520 passed / 1 failed repo-wide. The failure is the pre-existing
tests/contract/tracker-geo.test.ts(@ip-location-db/geolite2-city-mmdbnot installed), unrelated to this diff. Root and worker typechecks both clean — the worker tsconfig covers files roottscdoes not, and caught a real error here.Not done here
No selection, decision, serving, accounting, dashboard workflow or API endpoint (packages C–F). The worker consumer is wired into
worker/index.tsand ffmpeg is added to the worker Dockerfile, but nothing enqueues a render yet — that's package C.