Skip to content

Commit fcfd330

Browse files
jrusso1020claude
andcommitted
fix(producer): include webm in plan-time needsAlpha + strengthen alpha smoke
PR review feedback from Miguel and Vai on #951 caught a real bug: `plan.ts`'s `needsAlpha` disjunction excluded `"webm"`, so the plan stage froze `forceScreenshot: false` into the `LockedRenderConfig` even though distributed webm uses `yuva420p`. Every chunk worker captured opaque RGB via BeginFrame (which doesn't preserve alpha on Linux headless-shell), and libvpx-vp9 encoded uniformly-opaque alpha that the encoder then dropped — producing un-keyable webm. Two changes: 1. **plan.ts**: include `"webm"` in `needsAlpha`. Matches the in-process renderer's logic at `renderOrchestrator.ts:1469` (`const needsAlpha = isWebm || isMov || isPngSequence`); the two sites must stay in sync since the distributed pipeline's PSNR regression compares against the in-process baseline. 2. **Smoke test (yuva420p describe)**: source frames now use a real alpha gradient (`geq=a='X*255/W'` on top of `testsrc2`) instead of `testsrc2 + format=rgba` which was uniformly opaque. The decode- pix_fmt assertion is dropped (ffprobe reports `yuv420p` for VP9-with-alpha because the alpha lives in a Matroska `BlockAdditional` sidecar) and replaced with two stronger checks: - `TAG:ALPHA_MODE=1` is present on the stream — proves the encoder was actually configured for alpha - alpha plane variance after `-c:v libvpx-vp9 -i ... -pix_fmt rgba -vf extractplanes=a,signalstats` — proves the alpha sub-stream round-trips through concat-copy with spatially-varying content, not uniform/dropped alpha - decode-test gate is now exit-code-only (was `exitCode || stderr` which would flake on chatty ffmpeg `-v error` builds emitting non-fatal DTS/container notes) These checks would have caught the `needsAlpha` bug before review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e867da6 commit fcfd330

2 files changed

Lines changed: 83 additions & 26 deletions

File tree

packages/producer/src/services/distributed/plan.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -653,10 +653,17 @@ export async function plan(
653653
// move the contents over once the staged work completes.
654654
const finalCompiledDir = join(planDir, "compiled");
655655

656-
// mov + png-sequence carry alpha — flip force-screenshot so compileStage
657-
// takes the alpha-aware capture path (BeginFrame doesn't preserve alpha
658-
// on Linux headless-shell).
659-
const needsAlpha = config.format === "png-sequence" || config.format === "mov";
656+
// webm + mov + png-sequence carry alpha — flip force-screenshot so
657+
// compileStage takes the alpha-aware capture path (BeginFrame doesn't
658+
// preserve alpha on Linux headless-shell). Must match the in-process
659+
// renderer's needsAlpha logic in `renderOrchestrator.ts` so chunked
660+
// webm output preserves the same alpha plane the in-process baseline
661+
// does. Omitting webm here silently freezes `forceScreenshot: false`
662+
// into the planDir and every chunk worker captures opaque RGB — the
663+
// libvpx-vp9 alpha sub-stream then encodes either uniform alpha or
664+
// gets downgraded by the encoder, producing un-keyable webm output.
665+
const needsAlpha =
666+
config.format === "png-sequence" || config.format === "mov" || config.format === "webm";
660667

661668
// ── Compile ──
662669
const compileResult = await runCompileStage({

packages/producer/tests/distributed/_smoke/webm-concat-copy.test.ts

Lines changed: 72 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -330,9 +330,13 @@ describe("webm VP9 concat-copy smoke (yuva420p alpha)", () => {
330330
alphaConcatListPath = join(alphaRoot, "concat-list.txt");
331331
alphaOutputPath = join(alphaRoot, "output.webm");
332332

333-
// For alpha frames, generate RGBA PNGs with a transparent background
334-
// and an animated solid rect. `color` lavfi source supports alpha
335-
// via `c=<color>@<alpha>` syntax.
333+
// For alpha frames, generate RGBA PNGs with spatially-varying alpha
334+
// so the encoder can't drop the alpha plane as uniform/redundant.
335+
// `testsrc2 + format=rgba` (the prior shape) produced uniformly-
336+
// opaque alpha and the libvpx-vp9 encoder silently downgraded the
337+
// output to yuv420p — masking any bug in the alpha pipeline. Here
338+
// `geq=a='X*255/W'` writes a horizontal alpha gradient on top of
339+
// the testsrc2 RGB so the alpha track has real per-pixel content.
336340
const frameGen = runFfmpegSync([
337341
"-hide_banner",
338342
"-y",
@@ -341,7 +345,7 @@ describe("webm VP9 concat-copy smoke (yuva420p alpha)", () => {
341345
"-i",
342346
`testsrc2=s=${WIDTH}x${HEIGHT}:r=${FPS}:d=${TOTAL_FRAMES / FPS}`,
343347
"-vf",
344-
"format=rgba",
348+
"format=rgba,geq=r='r(X,Y)':g='g(X,Y)':b='b(X,Y)':a='X*255/W'",
345349
"-frames:v",
346350
String(TOTAL_FRAMES),
347351
join(alphaFramesDir, "frame_%04d.png"),
@@ -439,15 +443,6 @@ describe("webm VP9 concat-copy smoke (yuva420p alpha)", () => {
439443
});
440444

441445
it("decodes alpha-track WebM cleanly without seam errors", () => {
442-
// The decode-test is the load-bearing assertion. The actual pixel
443-
// format reported by ffprobe depends on the source's alpha
444-
// distribution — an RGBA source whose alpha plane is uniformly
445-
// opaque can be encoded as yuva420p but ffprobe may report it as
446-
// yuv420p if libvpx-vp9 drops the alpha sub-stream as redundant. The
447-
// distributed renderer's real compositions (which have meaningful
448-
// alpha) get the alpha branch end-to-end; this smoke test only
449-
// proves that the closed-GOP encoder args + alpha-pixel-format flag
450-
// combination doesn't break concat-copy.
451446
const decodeResult = runFfmpegSync([
452447
"-hide_banner",
453448
"-v",
@@ -458,7 +453,12 @@ describe("webm VP9 concat-copy smoke (yuva420p alpha)", () => {
458453
"null",
459454
"-",
460455
]);
461-
if (decodeResult.exitCode !== 0 || decodeResult.stderr.length > 0) {
456+
// Gate only on exit code — `-v error` ffmpeg builds can emit
457+
// non-fatal stderr (DTS warnings, container-quirk notes) and we
458+
// don't want the test to flake on chatty stderr in a future
459+
// libavformat upgrade. Surface stderr in the failure message for
460+
// forensic context.
461+
if (decodeResult.exitCode !== 0) {
462462
throw new Error(
463463
`[alpha smoke decode-test] failed (exit ${decodeResult.exitCode}). ` +
464464
`Failure fingerprint: ${decodeResult.stderr.slice(-1000) || "(no stderr)"}`,
@@ -470,17 +470,67 @@ describe("webm VP9 concat-copy smoke (yuva420p alpha)", () => {
470470
"error",
471471
"-select_streams",
472472
"v:0",
473-
"-show_entries",
474-
"stream=codec_name,pix_fmt",
475-
"-of",
476-
"default=noprint_wrappers=1",
473+
"-show_streams",
477474
alphaOutputPath,
478475
]);
479476
expect(probeResult.exitCode).toBe(0);
480477
expect(probeResult.stdout).toMatch(/codec_name=vp9/);
481-
// Accept either yuv420p or yuva420p — see the comment above the
482-
// decode-test for why libvpx-vp9 may downgrade uniformly-opaque
483-
// alpha to yuv420p.
484-
expect(probeResult.stdout).toMatch(/pix_fmt=yuva?420p/);
478+
// libvpx-vp9 stores the alpha plane as a Matroska `BlockAdditional`
479+
// sidecar, NOT in the main stream's `pix_fmt` — so `ffprobe` always
480+
// reports `pix_fmt=yuv420p` for VP9-with-alpha. The right signal that
481+
// alpha encoding was enabled is the stream-level `TAG:ALPHA_MODE=1`
482+
// tag the encoder writes when `-metadata:s:v:0 alpha_mode=1` is set
483+
// on a yuva420p input.
484+
expect(probeResult.stdout).toMatch(/ALPHA_MODE=1/);
485+
});
486+
487+
it("alpha plane round-trips through concat-copy with spatially-varying content", () => {
488+
// Decode the concat-copied WebM via the libvpx-vp9 decoder forced to
489+
// RGBA, then extract the alpha plane and check it has real spatial
490+
// variance — catches the failure mode where the encoder accepted
491+
// yuva420p input but dropped the alpha sub-stream silently
492+
// (uniform alpha would mask any plan-time bug like the `needsAlpha`
493+
// hole that hid this PR's bug before review caught it). The
494+
// gradient source produces YMIN ≈ 0 / YMAX ≈ 255 on the alpha
495+
// plane; uniform alpha would give YMIN == YMAX. Spread > 100 is a
496+
// generous floor that catches the bad case cleanly.
497+
//
498+
// `-c:v libvpx-vp9` before `-i` is the load-bearing piece: ffmpeg's
499+
// default VP9 decoder path strips the BlockAdditional alpha track
500+
// when decoding to non-rgba pixel formats; forcing the libvpx-vp9
501+
// decoder + `-pix_fmt rgba` is how we get the alpha plane back.
502+
const statsResult = runFfmpegSync([
503+
"-hide_banner",
504+
"-v",
505+
"error",
506+
"-c:v",
507+
"libvpx-vp9",
508+
"-i",
509+
alphaOutputPath,
510+
"-pix_fmt",
511+
"rgba",
512+
"-vf",
513+
"extractplanes=a,signalstats,metadata=mode=print:file=-",
514+
"-f",
515+
"null",
516+
"-",
517+
]);
518+
if (statsResult.exitCode !== 0) {
519+
throw new Error(
520+
`[alpha smoke signalstats] failed (exit ${statsResult.exitCode}): ` +
521+
`${statsResult.stderr.slice(-500)}`,
522+
);
523+
}
524+
const yminMatch = statsResult.stdout.match(/lavfi\.signalstats\.YMIN=(\d+)/);
525+
const ymaxMatch = statsResult.stdout.match(/lavfi\.signalstats\.YMAX=(\d+)/);
526+
if (!yminMatch || !ymaxMatch) {
527+
throw new Error(
528+
`[alpha smoke signalstats] could not parse YMIN/YMAX from output: ` +
529+
`${statsResult.stdout.slice(0, 500)}`,
530+
);
531+
}
532+
const ymin = Number.parseInt(yminMatch[1], 10);
533+
const ymax = Number.parseInt(ymaxMatch[1], 10);
534+
expect(ymax - ymin).toBeGreaterThan(100);
485535
});
486536
});

0 commit comments

Comments
 (0)