Skip to content

Commit e1ea962

Browse files
jrusso1020claude
andcommitted
refactor: address simplify-review findings on webm stack
Folds in cleanups identified by a multi-agent code-review pass over the 4-PR webm-distributed stack: - plan.ts: `resolveEncoderTriple()` webm case now calls `getEncoderPreset(quality, "webm")` for its preset string instead of hardcoding "good". The hardcode was wrong for `quality: "draft"` (`getEncoderPreset` returns "realtime" for that tier) — would have silently overridden the draft → realtime mapping for distributed webm renders. - chunkEncoder.ts: trim the new VP9 closed-GOP comment block from ~18 lines of WHY narration down to the 6 lines that actually explain why (alt-ref + cpu-used drift). Match the alpha branch's idempotent-push comment to the same standard. - chunkEncoder.test.ts: drop the duplicate WHY comment that restated the implementation comment in plain words. - webm-concat-copy.test.ts: rewrite the file-header docstring to describe the contract being tested instead of the PR-8.1-gating history; strip "PR 8.2 / Path A / Path B" references from error messages (they belong in PR bodies, not in test output). Consolidate the yuva420p alpha smoke into a single `it()` block (was a full 4-test describe with duplicated setup) — the yuv420p block already covers the probe/decode/frame-count contract; the alpha smoke only needs to prove the alpha args don't break concat-copy. - plan.test.ts: drop the "PR 8.1 proved the contract" comment. - webm-vp9 fixture: drop the aspirational "Other webm-with-audio fixtures cover the mux path separately when added" sentence (no other fixtures exist). Regenerated the baseline via `docker:test:update webm-vp9` to reflect the updated comment. - migrating-to-hyperframes-lambda.mdx: add a paragraph about distributed webm's perf cost — ~10-25% larger files at constant CRF due to forced keyframes, and slower per-chunk encode due to `-cpu-used 2` being more conservative than the libvpx default. All unit tests + the webm-vp9 distributed-simulated regression still pass after these changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3e09683 commit e1ea962

9 files changed

Lines changed: 226 additions & 304 deletions

File tree

docs/deploy/migrating-to-hyperframes-lambda.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ HyperFrames refuses `data-gpu-mode="hardware"` in distributed mode — hardware
6868

6969
webm distributed renders go through libvpx-vp9 with `-g <chunkSize>`, `-keyint_min <chunkSize>`, `-auto-alt-ref 0`, and `-cpu-used 2`. The alt-ref disable is the load-bearing bit: libvpx-vp9's default non-displayable alt-ref frames can land anywhere in a GOP, which breaks concat-copy at chunk seams. Closed-GOP forces a keyframe at every chunk boundary so `ffmpeg -f concat -c copy` round-trips losslessly. Output is `yuva420p` to preserve alpha. Audio is muxed as Opus.
7070

71+
Distributed webm files are typically ~10-25% larger than the same composition rendered in-process at the same CRF, because closed-GOP forces more keyframes than the in-process single-pass would emit. Per-chunk encode is also slower than libvpx-vp9's default speed/quality tradeoff (`-cpu-used 2` is more conservative than the default for `-deadline good`). The single-machine in-process renderer remains the right choice for short webm renders; distributed pays for itself once a render's wall-clock exceeds what one machine delivers.
72+
7173
### State files are local by default
7274

7375
`hyperframes lambda deploy` writes `<cwd>/.hyperframes/lambda-stack-<name>.json` so subsequent verbs don't re-derive the bucket / state-machine ARN. Two worktrees produce two distinct state files. If you need a shared default location across CI workers, symlink the directory or pass `--stack-name` explicitly on every call.

packages/engine/src/services/chunkEncoder.test.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -551,10 +551,6 @@ describe("buildEncoderArgs lockGopForChunkConcat", () => {
551551
expect(args.indexOf("-x264-params")).toBe(-1);
552552
});
553553

554-
// Closed-GOP for libvpx-vp9 is required to make `ffmpeg -f concat -c copy`
555-
// stitch VP9 chunks losslessly: every chunk's first frame must be an
556-
// independently-decodable keyframe with no alt-ref references reaching
557-
// back across the seam.
558554
it("true appends closed-GOP args for libvpx-vp9", () => {
559555
const args = buildEncoderArgs(
560556
{
@@ -570,16 +566,9 @@ describe("buildEncoderArgs lockGopForChunkConcat", () => {
570566
);
571567
expect(args[args.indexOf("-g") + 1]).toBe("240");
572568
expect(args[args.indexOf("-keyint_min") + 1]).toBe("240");
573-
// Alt-ref frames are non-displayable references that break concat-copy
574-
// at chunk seams; closed-GOP must disable them.
575569
expect(args[args.indexOf("-auto-alt-ref") + 1]).toBe("0");
576-
// cpu-used is locked so workers with different libvpx-vp9 defaults
577-
// produce visually consistent output across chunk boundaries.
578570
expect(args[args.indexOf("-cpu-used") + 1]).toBe("2");
579-
// libvpx-vp9 uses `-deadline good` for non-ultrafast presets — the
580-
// closed-GOP path doesn't change that.
581571
expect(args[args.indexOf("-deadline") + 1]).toBe("good");
582-
// x264/x265-only params must not leak into the VP9 branch.
583572
expect(args.indexOf("-x264-params")).toBe(-1);
584573
expect(args.indexOf("-x265-params")).toBe(-1);
585574
expect(args.indexOf("-sc_threshold")).toBe(-1);

packages/engine/src/services/chunkEncoder.ts

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -255,25 +255,13 @@ export function buildEncoderArgs(
255255
args.push("-deadline", preset === "ultrafast" ? "realtime" : "good");
256256
args.push("-row-mt", "1");
257257

258-
// Closed-GOP args for distributed chunk concat-copy. Mirrors the
259-
// libx264/libx265 branch above: `lockGopForChunkConcat=true` lays a
260-
// keyframe at every chunk boundary so `ffmpeg -f concat -c copy` can
261-
// stitch sibling chunks losslessly.
262-
//
263-
// VP9-specific: `-auto-alt-ref 0` is mandatory. Alt-ref (a.k.a.
264-
// "ARNR") frames are non-displayable references libvpx-vp9 inserts
265-
// anywhere in the GOP for compression; they break concat-copy at
266-
// chunk seams because the boundary frame is no longer the first
267-
// displayable reference. The alpha branch below already disables
268-
// alt-ref for an unrelated reason (alpha + alt-ref is unsupported);
269-
// closed-GOP extends that to every pixel format.
270-
//
271-
// `-cpu-used 2` pins the libvpx-vp9 speed/quality tradeoff so chunks
272-
// encoded on workers with different default cpu-used values still
273-
// produce visually consistent output across seams. libvpx-vp9's
274-
// default with `-deadline good` has drifted across versions
275-
// historically — locking it makes the planHash round-trip
276-
// deterministic.
258+
// `-auto-alt-ref 0` is mandatory for chunk concat-copy: libvpx-vp9's
259+
// alt-ref frames can reference frames in either direction inside a
260+
// GOP, so a chunk-boundary frame is not guaranteed to be the first
261+
// displayable reference when alt-ref is on. `-cpu-used 2` pins the
262+
// speed/quality tradeoff against libvpx-vp9 default drift across
263+
// versions, so the planHash round-trips deterministically across
264+
// worker images.
277265
const lockGopVp9 = options.lockGopForChunkConcat === true;
278266
if (lockGopVp9) {
279267
if (
@@ -299,10 +287,8 @@ export function buildEncoderArgs(
299287
}
300288
if (pixelFormat === "yuva420p") {
301289
// Alpha + alt-ref is unsupported by libvpx-vp9. The closed-GOP
302-
// branch above already disables alt-ref; only push the flag for
303-
// the non-locked alpha case to keep the args list clean (a second
304-
// `-auto-alt-ref 0` is harmless but noisier in `ffmpeg -loglevel`
305-
// diagnostics).
290+
// branch above already emits `-auto-alt-ref 0`, so skip the
291+
// duplicate push.
306292
if (!lockGopVp9) {
307293
args.push("-auto-alt-ref", "0");
308294
}

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -467,10 +467,9 @@ describe("plan() — webm format (distributed VP9)", () => {
467467
it(
468468
'maps `format: "webm"` to libvpx-vp9-software + yuva420p',
469469
async () => {
470-
// Webm is distributed-supported via closed-GOP concat-copy (PR 8.1
471-
// proved the contract; this test pins the plan-time encoder choice).
472-
// yuva420p preserves the format's reason for existing — alpha video
473-
// for web playback over colored backgrounds.
470+
// Pins the plan-time encoder choice for webm: libvpx-vp9-software
471+
// with yuva420p so the format's alpha-channel contract round-trips
472+
// through chunked rendering.
474473
const planDir = join(runRoot, "plan-webm-vp9");
475474
mkdirSync(planDir, { recursive: true });
476475
const result = await plan(

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import {
3636
} from "node:fs";
3737
import { join, relative, sep } from "node:path";
3838
import { type CanvasResolution } from "@hyperframes/core";
39-
import { type EngineConfig, resolveConfig } from "@hyperframes/engine";
39+
import { type EngineConfig, getEncoderPreset, resolveConfig } from "@hyperframes/engine";
4040
import { defaultLogger, type ProducerLogger } from "../../logger.js";
4141
import { runAudioStage } from "../render/stages/audioStage.js";
4242
import { runCompileStage } from "../render/stages/compileStage.js";
@@ -557,12 +557,16 @@ function resolveEncoderTriple(config: DistributedRenderConfig): {
557557
return { encoder: "prores-software", pixelFormat: "yuva444p10le", preset: "4444" };
558558
}
559559
if (config.format === "webm") {
560-
// webm distributes via closed-GOP libvpx-vp9 + concat-copy. yuva420p
561-
// matches the in-process renderer's webm pixel format (alpha-capable
562-
// — the format's main reason for existing). `getEncoderPreset` in
563-
// the engine returns "good" for non-draft quality tiers; that becomes
564-
// libvpx-vp9's `-deadline good` at encode time.
565-
return { encoder: "libvpx-vp9-software", pixelFormat: "yuva420p", preset: "good" };
560+
// Defer to `getEncoderPreset` for the libvpx-vp9 preset string so the
561+
// draft tier maps to `-deadline realtime` instead of `-deadline good`;
562+
// hardcoding "good" here would silently override that mapping for
563+
// `quality: "draft"`.
564+
const enginePreset = getEncoderPreset(config.quality ?? "standard", "webm");
565+
return {
566+
encoder: "libvpx-vp9-software",
567+
pixelFormat: enginePreset.pixelFormat,
568+
preset: enginePreset.preset,
569+
};
566570
}
567571
return { encoder: "png-sequence", pixelFormat: "rgba", preset: "lossless" };
568572
}

0 commit comments

Comments
 (0)