fix(distributed): apply -r <fps> to single-chunk pass-through path

The v0.6.39 fix added -r <fps> to the multi-chunk concat ffmpeg
invocation but didn't reach the single-chunk pass-through path,
which is taken when totalFrames * fpsDen / fpsNum fits in one
chunk. Result: 1-chunk renders shipped with fractional
r_frame_rate (e.g. 359/12) while multi-chunk renders shipped
with exact 30/1.

Single-chunk path now goes through the same -r <fps> + -c copy
ffmpeg invocation as the concat path, ensuring uniform exact
r_frame_rate metadata across all chunk-count configurations.

Adds a regression test exercising the 1-chunk path and asserting
r_frame_rate === "<fpsNum>/<fpsDen>" exact.
This commit is contained in:
James
2026-05-24 03:15:56 +00:00
parent 0845dc9e56
commit 49281a5c17
2 changed files with 98 additions and 31 deletions
@@ -237,6 +237,49 @@ describe("assemble()", () => {
TIMEOUT_MS,
);
it(
"single-chunk render stamps exact r_frame_rate on the output container",
async () => {
if (!hasFfmpeg) {
console.warn(
"[assemble.test] skipping single-chunk r_frame_rate test — ffmpeg not available on this host",
);
return;
}
// Reproducer for the single-chunk pass-through regression: when
// `chunkPaths.length === 1`, assemble must still stamp an exact
// `r_frame_rate` matching the planDir's rational (here 30/1), not
// a PTS-derived fraction like `359/12`. Multi-chunk renders go
// through the concat demuxer; single-chunk renders skip it and
// need the `-r <fps>` flag on a direct remux step.
const chunks: ChunkSliceJson[] = [{ index: 0, startFrame: 0, endFrame: 10 }];
const planDir = buildPlanDir("mp4", chunks, 10, false);
const chunkPath = join(planDir, "chunk-0.mp4");
makeMp4Chunk(chunkPath, 10);
const outputPath = join(planDir, "output-single-chunk.mp4");
const result = await assemble(planDir, [chunkPath], null, outputPath);
expect(result.outputPath).toBe(outputPath);
expect(existsSync(outputPath)).toBe(true);
expect(result.framesEncoded).toBe(10);
const videoStream = probeStream(outputPath, "v:0");
expect(videoStream).toBeDefined();
expect(videoStream?.codec_name).toBe("h264");
// The exact-rational assertion — the regression hole that this
// test closes. Before the single-chunk -r fix, this came back as
// a PTS-derived fraction (e.g. `359/12`) on 1-chunk renders.
expect(videoStream?.r_frame_rate).toBe("30/1");
const expectedDuration = 10 / 30;
const probedDuration = Number(videoStream?.duration ?? 0);
expect(Math.abs(probedDuration - expectedDuration)).toBeLessThan(0.001);
},
TIMEOUT_MS,
);
it(
"muxes audio with frame-count-derived duration when audio.aac is present",
async () => {
@@ -131,42 +131,66 @@ export async function assemble(
mkdirSync(workDir, { recursive: true });
try {
// Concat list file — one `file '<path>'` per chunk, in order. ffmpeg's
// concat demuxer escapes single quotes via `'\''`; we replicate that
// here so chunk paths containing quotes don't break the parser.
const concatListPath = join(workDir, "concat-list.txt");
const concatBody = chunkPaths.map((path) => `file '${path.replace(/'/g, "'\\''")}'`).join("\n");
writeFileSync(concatListPath, `${concatBody}\n`, "utf-8");
const concatOutputPath = join(workDir, `concat.${plan.dimensions.format}`);
const fpsArg = fpsToFfmpegArg({
num: plan.dimensions.fpsNum,
den: plan.dimensions.fpsDen,
});
// Set the exact input framerate so the concat demuxer doesn't
// PTS-average a fractional rational like `360000/12001` instead
// of `30/1` into the output container metadata. `-c copy` is
// retained; no re-encode.
const concatArgs = [
"-r",
fpsArg,
"-f",
"concat",
"-safe",
"0",
"-i",
concatListPath,
"-c",
"copy",
"-y",
concatOutputPath,
];
const concatResult = await runFfmpeg(concatArgs, { signal: abortSignal });
if (!concatResult.success) {
throw new Error(
`[assemble] ffmpeg concat-copy failed (exit ${concatResult.exitCode}): ` +
`${concatResult.stderr.slice(-400)}`,
);
// Single-chunk renders bypass the concat demuxer entirely. ffmpeg's
// concat demuxer with a one-entry list re-runs as a straight remux of
// the single source, and in that path the input-side `-r` flag does
// not consistently override the source's PTS-derived r_frame_rate
// (observed: `359/12` carrying through to the output container while
// the equivalent multi-chunk path produces `30/1` exact). Running a
// direct `-c copy` remux with `-r <fps>` as an output flag gives the
// muxer the authoritative rate to stamp into the container without
// touching the encoded stream. Multi-chunk renders continue through
// the concat demuxer where the existing `-r` input flag works.
if (chunkPaths.length === 1) {
const remuxArgs = ["-i", chunkPaths[0]!, "-c", "copy", "-r", fpsArg, "-y", concatOutputPath];
const remuxResult = await runFfmpeg(remuxArgs, { signal: abortSignal });
if (!remuxResult.success) {
throw new Error(
`[assemble] ffmpeg single-chunk remux failed (exit ${remuxResult.exitCode}): ` +
`${remuxResult.stderr.slice(-400)}`,
);
}
} else {
// Concat list file — one `file '<path>'` per chunk, in order. ffmpeg's
// concat demuxer escapes single quotes via `'\''`; we replicate that
// here so chunk paths containing quotes don't break the parser.
const concatListPath = join(workDir, "concat-list.txt");
const concatBody = chunkPaths
.map((path) => `file '${path.replace(/'/g, "'\\''")}'`)
.join("\n");
writeFileSync(concatListPath, `${concatBody}\n`, "utf-8");
// Set the exact input framerate so the concat demuxer doesn't
// PTS-average a fractional rational like `360000/12001` instead
// of `30/1` into the output container metadata. `-c copy` is
// retained; no re-encode.
const concatArgs = [
"-r",
fpsArg,
"-f",
"concat",
"-safe",
"0",
"-i",
concatListPath,
"-c",
"copy",
"-y",
concatOutputPath,
];
const concatResult = await runFfmpeg(concatArgs, { signal: abortSignal });
if (!concatResult.success) {
throw new Error(
`[assemble] ffmpeg concat-copy failed (exit ${concatResult.exitCode}): ` +
`${concatResult.stderr.slice(-400)}`,
);
}
}
// ── 3. Audio: pad-or-trim then mux ────────────────────────────────────