mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-04 07:19:52 +00:00
fix(producer): reject unknown codec strings + extract testable preset-override helper
Address @vanceingalls review on #850: 1. Unknown codec strings (typos like 'H265', future additions like 'av1') silently fell through to libx264 in resolveEncoderTriple. Add an explicit throw symmetric to the non-mp4-format branch already there. A JS caller building config from JSON who passes 'codec: "h266"' now gets a clear error at plan time instead of unflagged h264 output. 2. The preset.codec override in renderChunk had no fast unit coverage — only the heavyweight Docker fixture in #851 would catch a regression if someone refactored the spread (e.g. moved it into getEncoderPreset itself). Extract resolvePresetForLockedEncoder() and add 4 fast unit tests pinning the four encoder shapes (libx265/libx264/prores/png-seq). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This commit is contained in:
@@ -267,4 +267,24 @@ describe("plan() — codec knob", () => {
|
||||
expect(caught).toBeInstanceOf(Error);
|
||||
expect((caught as Error).message).toMatch(/codec.*only valid for format="mp4"/);
|
||||
});
|
||||
|
||||
it("rejects unknown codec strings for format=mp4 (no silent fall-through to h264)", async () => {
|
||||
const planDir = join(runRoot, "plan-codec-unknown");
|
||||
mkdirSync(planDir, { recursive: true });
|
||||
let caught: unknown;
|
||||
try {
|
||||
await plan(
|
||||
projectDir,
|
||||
// @ts-expect-error — runtime check is the test's purpose. Catches
|
||||
// typos ("H265") and future codec additions ("av1") that a JS
|
||||
// caller building config from JSON might pass.
|
||||
{ fps: 30, width: 320, height: 240, format: "mp4", codec: "h266" },
|
||||
planDir,
|
||||
);
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
expect(caught).toBeInstanceOf(Error);
|
||||
expect((caught as Error).message).toMatch(/codec must be "h264" or "h265"/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -482,6 +482,17 @@ function resolveEncoderTriple(config: DistributedRenderConfig): {
|
||||
} {
|
||||
if (config.format === "mp4") {
|
||||
const codec = config.codec ?? "h264";
|
||||
// Explicit unknown-codec throw rather than silent fall-through to h264.
|
||||
// A JS caller building config from JSON who passes `codec: "h266"` or
|
||||
// `codec: "H265"` (typo / wrong case) would otherwise produce h264
|
||||
// output with no signal. The non-mp4-format branch below already throws
|
||||
// for the symmetric "wrong combination" case — match that shape.
|
||||
if (codec !== "h264" && codec !== "h265") {
|
||||
throw new Error(
|
||||
`[plan] DistributedRenderConfig.codec must be "h264" or "h265" for format="mp4"; ` +
|
||||
`received ${JSON.stringify(codec)}. Omit codec to default to h264.`,
|
||||
);
|
||||
}
|
||||
if (codec === "h265") {
|
||||
return { encoder: "libx265-software", pixelFormat: "yuv420p", preset: "medium" };
|
||||
}
|
||||
|
||||
@@ -29,6 +29,7 @@ import {
|
||||
PLAN_HASH_MISMATCH,
|
||||
renderChunk,
|
||||
RenderChunkValidationError,
|
||||
resolvePresetForLockedEncoder,
|
||||
} from "./renderChunk.js";
|
||||
|
||||
// Tiny fixture: 5 frames at 30fps. Captures finish in a few seconds on the
|
||||
@@ -309,3 +310,41 @@ describe("renderChunk()", () => {
|
||||
TIMEOUT_MS,
|
||||
);
|
||||
});
|
||||
|
||||
describe("resolvePresetForLockedEncoder", () => {
|
||||
// Tiny fast tests for the codec-override helper. No Chrome, no ffmpeg —
|
||||
// exists so a refactor that moves the override (e.g. into
|
||||
// `getEncoderPreset` itself) gets caught here before the heavyweight
|
||||
// Docker fixture is even run.
|
||||
it("flips codec from h264 to h265 when encoder is libx265-software", () => {
|
||||
const base = { preset: "medium", quality: 18, codec: "h264" as const, pixelFormat: "yuv420p" };
|
||||
const out = resolvePresetForLockedEncoder(base, "libx265-software");
|
||||
expect(out.codec).toBe("h265");
|
||||
expect(out.preset).toBe("medium");
|
||||
expect(out.quality).toBe(18);
|
||||
expect(out.pixelFormat).toBe("yuv420p");
|
||||
});
|
||||
|
||||
it("leaves the preset unchanged for libx264-software", () => {
|
||||
const base = { preset: "medium", quality: 18, codec: "h264" as const, pixelFormat: "yuv420p" };
|
||||
const out = resolvePresetForLockedEncoder(base, "libx264-software");
|
||||
expect(out).toBe(base);
|
||||
});
|
||||
|
||||
it("leaves the preset unchanged for prores-software", () => {
|
||||
const base = {
|
||||
preset: "4444",
|
||||
quality: 18,
|
||||
codec: "prores" as const,
|
||||
pixelFormat: "yuva444p10le",
|
||||
};
|
||||
const out = resolvePresetForLockedEncoder(base, "prores-software");
|
||||
expect(out).toBe(base);
|
||||
});
|
||||
|
||||
it("leaves the preset unchanged for png-sequence", () => {
|
||||
const base = { preset: "medium", quality: 18, codec: "h264" as const, pixelFormat: "yuv420p" };
|
||||
const out = resolvePresetForLockedEncoder(base, "png-sequence");
|
||||
expect(out).toBe(base);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -290,6 +290,27 @@ function hashChunkOutput(outputPath: string, kind: "file" | "frame-dir"): string
|
||||
return sha256Hex(lines.join("\0"));
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply the planDir's locked-encoder choice on top of an
|
||||
* `EncoderPreset` from `getEncoderPreset`. `getEncoderPreset` returns
|
||||
* h265 only on the HDR branch, but distributed mode is SDR-only — for
|
||||
* an `libx265-software` planDir we still need to flip the preset's
|
||||
* codec to h265 so `runEncodeStage` invokes libx265. Exported so a
|
||||
* unit test can pin the override independently of the heavyweight
|
||||
* Docker fixture: a refactor that moves the override (e.g. into
|
||||
* `getEncoderPreset` itself) shouldn't be able to silently regress
|
||||
* the contract without a fast-test signal.
|
||||
*/
|
||||
export function resolvePresetForLockedEncoder<P extends { codec: "h264" | "h265" | "vp9" | "prores" }>(
|
||||
basePreset: P,
|
||||
lockedEncoder: LockedRenderConfig["encoder"],
|
||||
): P {
|
||||
if (lockedEncoder === "libx265-software") {
|
||||
return { ...basePreset, codec: "h265" as const };
|
||||
}
|
||||
return basePreset;
|
||||
}
|
||||
|
||||
/**
|
||||
* Activity B: render a single chunk of the planDir. The `outputChunkPath`
|
||||
* argument is a file for mp4/mov outputs and a directory for png-sequence
|
||||
@@ -553,13 +574,7 @@ export async function renderChunk(
|
||||
? "mp4"
|
||||
: (plan.dimensions.format as "mp4" | "mov");
|
||||
const basePreset = getEncoderPreset(job.config.quality, presetFormat, undefined);
|
||||
// Override the preset's codec from the planDir's locked encoder so
|
||||
// h265 mp4 chunks call libx265 instead of getEncoderPreset's default
|
||||
// h264. `getEncoderPreset` only returns h265 in HDR paths today;
|
||||
// distributed mode is SDR-only, so the override here is the
|
||||
// canonical way for chunks to honor `DistributedRenderConfig.codec`.
|
||||
const preset: typeof basePreset =
|
||||
encoder.encoder === "libx265-software" ? { ...basePreset, codec: "h265" } : basePreset;
|
||||
const preset = resolvePresetForLockedEncoder(basePreset, encoder.encoder);
|
||||
const effectiveQuality = encoder.crf ?? preset.quality;
|
||||
const effectiveBitrate = encoder.crf != null ? undefined : encoder.bitrate;
|
||||
// For non-pngseq, encodeStage writes to `outputPath` when `isPngSequence`
|
||||
|
||||
Reference in New Issue
Block a user