Skip to content

Commit 90bf485

Browse files
committed
fix(distributed): reject cfr:true with h265 codec (per review)
The cfr re-encode pass hardcodes `-c:v libx264`. Pairing it with `codec: "h265"` would silently transcode the h265 chunks to h264. Detect the encoder discriminant in `meta/encoder.json` and throw a typed error parallel to the existing non-mp4 format guard, so callers surface the conflict instead of producing a wrong-codec deliverable. — Rames Jusso
1 parent 71d1889 commit 90bf485

2 files changed

Lines changed: 61 additions & 1 deletion

File tree

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ function buildPlanDir(
4646
chunks: ChunkSliceJson[],
4747
totalFrames: number,
4848
hasAudio: boolean,
49+
encoder: "libx264-software" | "libx265-software" = "libx264-software",
4950
): string {
5051
const planDir = mkdtempSync(join(runRoot, `plan-${format}-`));
5152
mkdirSync(join(planDir, "meta"), { recursive: true });
@@ -60,6 +61,10 @@ function buildPlanDir(
6061
"utf-8",
6162
);
6263
writeFileSync(join(planDir, "meta", "chunks.json"), JSON.stringify(chunks), "utf-8");
64+
// Minimal encoder.json — assemble reads this when cfr=true to detect h265
65+
// chunks (the cfr re-encode hardcodes libx264 and would silently transcode
66+
// h265). Tests default to libx264 to match the in-production default.
67+
writeFileSync(join(planDir, "meta", "encoder.json"), JSON.stringify({ encoder }), "utf-8");
6368
return planDir;
6469
}
6570

@@ -462,6 +467,39 @@ describe("assemble()", () => {
462467
TIMEOUT_MS,
463468
);
464469

470+
it(
471+
"cfr:true rejects h265 chunks with a clear error",
472+
async () => {
473+
if (!hasFfmpeg) {
474+
console.warn("[assemble.test] skipping cfr-h265 test — ffmpeg not available");
475+
return;
476+
}
477+
// The cfr re-encode hardcodes `-c:v libx264`; pairing it with h265
478+
// chunks would silently transcode them to h264. Assemble must throw
479+
// a typed error instead of producing a wrong-codec deliverable. We
480+
// stage a plan whose `meta/encoder.json` reports `libx265-software`
481+
// and chunks built with libx264 (the bytes don't matter — the guard
482+
// trips on the encoder discriminant before the re-encode runs).
483+
const chunks: ChunkSliceJson[] = [{ index: 0, startFrame: 0, endFrame: 5 }];
484+
const planDir = buildPlanDir("mp4", chunks, 5, false, "libx265-software");
485+
486+
const chunkPath = join(planDir, "chunk-0.mp4");
487+
makeMp4Chunk(chunkPath, 5);
488+
489+
let caught: unknown;
490+
try {
491+
await assemble(planDir, [chunkPath], null, join(planDir, "out.mp4"), { cfr: true });
492+
} catch (err) {
493+
caught = err;
494+
}
495+
expect(caught).toBeDefined();
496+
expect((caught as Error).message).toContain(
497+
`cfr=true is not yet supported with codec: "h265"`,
498+
);
499+
},
500+
TIMEOUT_MS,
501+
);
502+
465503
it(
466504
"merges png-sequence chunk directories with continuous global numbering",
467505
() => {

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,10 @@ export async function assemble(
221221
// opt-in re-encode with `-fps_mode cfr -r <fps>` lands the stream's
222222
// avg-frame-rate on the requested rational exactly. Restricted to
223223
// mp4 / libx264 — webm and mov go through their own stream-copy
224-
// paths that don't exhibit the same avg-frame-rate drift.
224+
// paths that don't exhibit the same avg-frame-rate drift, and h265
225+
// mp4 would silently transcode to h264 under the hardcoded
226+
// `-c:v libx264` re-encode (a typed throw is preferable to silent
227+
// codec loss).
225228
let postConcatPath = concatOutputPath;
226229
if (cfr) {
227230
if (plan.dimensions.format !== "mp4") {
@@ -231,6 +234,25 @@ export async function assemble(
231234
`already produce exact avg_frame_rate; cfr re-encode is not needed.`,
232235
);
233236
}
237+
// Read `meta/encoder.json` to detect the chunk encoder. The cfr
238+
// re-encode hardcodes `-c:v libx264`; pairing it with h265 chunks
239+
// would silently transcode them to h264. Throw a typed error so the
240+
// caller surfaces the conflict instead of producing a wrong-codec
241+
// deliverable.
242+
const encoderJsonPath = join(planDir, "meta", "encoder.json");
243+
if (!existsSync(encoderJsonPath)) {
244+
throw new Error(`[assemble] planDir missing meta/encoder.json: ${encoderJsonPath}`);
245+
}
246+
const encoderJson = JSON.parse(readFileSync(encoderJsonPath, "utf-8")) as {
247+
encoder?: string;
248+
};
249+
if (encoderJson.encoder === "libx265-software") {
250+
throw new Error(
251+
`[assemble] cfr=true is not yet supported with codec: "h265". The ` +
252+
`cfr re-encode pass uses libx264 and would silently transcode the ` +
253+
`h265 chunks. Either disable cfr or render with codec: "h264".`,
254+
);
255+
}
234256
const cfrOutputPath = join(workDir, `cfr.${plan.dimensions.format}`);
235257
const cfrArgs = [
236258
"-i",

0 commit comments

Comments
 (0)