diff --git a/packages/cli/src/commands/benchmark.ts b/packages/cli/src/commands/benchmark.ts index c18f714c8..d98648033 100644 --- a/packages/cli/src/commands/benchmark.ts +++ b/packages/cli/src/commands/benchmark.ts @@ -14,10 +14,11 @@ import { c } from "../ui/colors.js"; import { formatBytes, formatDuration, errorBox } from "../ui/format.js"; import * as clack from "@clack/prompts"; import { withMeta } from "../utils/updateCheck.js"; +import { fpsToFfmpegArg } from "@hyperframes/core"; interface BenchmarkConfig { label: string; - fps: 24 | 30 | 60; + fps: import("@hyperframes/core").Fps; quality: "draft" | "standard" | "high"; workers: number; } @@ -35,12 +36,14 @@ interface ConfigResult { avgSize: number | null; } +const FPS_30: import("@hyperframes/core").Fps = { num: 30, den: 1 }; +const FPS_60: import("@hyperframes/core").Fps = { num: 60, den: 1 }; const DEFAULT_CONFIGS: BenchmarkConfig[] = [ - { label: "30fps \u00B7 draft \u00B7 2w", fps: 30, quality: "draft", workers: 2 }, - { label: "30fps \u00B7 standard \u00B7 2w", fps: 30, quality: "standard", workers: 2 }, - { label: "30fps \u00B7 high \u00B7 2w", fps: 30, quality: "high", workers: 2 }, - { label: "30fps \u00B7 standard \u00B7 4w", fps: 30, quality: "standard", workers: 4 }, - { label: "60fps \u00B7 standard \u00B7 4w", fps: 60, quality: "standard", workers: 4 }, + { label: "30fps \u00B7 draft \u00B7 2w", fps: FPS_30, quality: "draft", workers: 2 }, + { label: "30fps \u00B7 standard \u00B7 2w", fps: FPS_30, quality: "standard", workers: 2 }, + { label: "30fps \u00B7 high \u00B7 2w", fps: FPS_30, quality: "high", workers: 2 }, + { label: "30fps \u00B7 standard \u00B7 4w", fps: FPS_30, quality: "standard", workers: 4 }, + { label: "60fps \u00B7 standard \u00B7 4w", fps: FPS_60, quality: "standard", workers: 4 }, ]; export default defineCommand({ @@ -162,7 +165,10 @@ export default defineCommand({ withMeta({ results: results.map((r) => ({ config: r.config.label, - fps: r.config.fps, + // Emit fps in the on-the-wire form so the JSON payload is + // round-trippable through `parseFps` (e.g. "30000/1001" stays + // exact; integer fps stays integer). + fps: fpsToFfmpegArg(r.config.fps), quality: r.config.quality, workers: r.config.workers, avgTimeMs: r.avgTime, diff --git a/packages/cli/src/commands/render.test.ts b/packages/cli/src/commands/render.test.ts index 28a5fc7ac..57cc05425 100644 --- a/packages/cli/src/commands/render.test.ts +++ b/packages/cli/src/commands/render.test.ts @@ -68,7 +68,7 @@ describe("renderLocal browser GPU config", () => { setEnv("PRODUCER_BROWSER_GPU_MODE", "hardware"); await renderLocal("/tmp/project", "/tmp/out.mp4", { - fps: 30, + fps: { num: 30, den: 1 }, quality: "standard", format: "mp4", gpu: false, @@ -86,7 +86,7 @@ describe("renderLocal browser GPU config", () => { it("forwards browserGpuMode='auto' into producer config (probe-then-choose)", async () => { await renderLocal("/tmp/project", "/tmp/out.mp4", { - fps: 30, + fps: { num: 30, den: 1 }, quality: "standard", format: "mp4", gpu: false, @@ -104,7 +104,7 @@ describe("renderLocal browser GPU config", () => { it("passes an explicit hardware override for default local browser GPU", async () => { await renderLocal("/tmp/project", "/tmp/out.mp4", { - fps: 30, + fps: { num: 30, den: 1 }, quality: "standard", format: "mp4", gpu: false, @@ -137,7 +137,7 @@ describe("renderLocal browser GPU config", () => { it("forwards parsed --variables payload to createRenderJob", async () => { await renderLocal("/tmp/project", "/tmp/out.mp4", { - fps: 30, + fps: { num: 30, den: 1 }, quality: "standard", format: "mp4", gpu: false, @@ -152,7 +152,7 @@ describe("renderLocal browser GPU config", () => { it("forwards format: png-sequence through to createRenderJob", async () => { await renderLocal("/tmp/project", "/tmp/frames", { - fps: 30, + fps: { num: 30, den: 1 }, quality: "standard", format: "png-sequence", gpu: false, @@ -166,7 +166,7 @@ describe("renderLocal browser GPU config", () => { it("omits variables from createRenderJob when not provided", async () => { await renderLocal("/tmp/project", "/tmp/out.mp4", { - fps: 30, + fps: { num: 30, den: 1 }, quality: "standard", format: "mp4", gpu: false, @@ -180,7 +180,7 @@ describe("renderLocal browser GPU config", () => { it("forwards entryFile to createRenderJob when --composition is set", async () => { await renderLocal("/tmp/project", "/tmp/out.mp4", { - fps: 30, + fps: { num: 30, den: 1 }, quality: "standard", format: "mp4", gpu: false, @@ -195,7 +195,7 @@ describe("renderLocal browser GPU config", () => { it("omits entryFile from createRenderJob when --composition is not set", async () => { await renderLocal("/tmp/project", "/tmp/out.mp4", { - fps: 30, + fps: { num: 30, den: 1 }, quality: "standard", format: "mp4", gpu: false, @@ -209,7 +209,7 @@ describe("renderLocal browser GPU config", () => { it("forwards outputResolution to createRenderJob when --resolution is set", async () => { await renderLocal("/tmp/project", "/tmp/out.mp4", { - fps: 30, + fps: { num: 30, den: 1 }, quality: "standard", format: "mp4", gpu: false, @@ -224,7 +224,7 @@ describe("renderLocal browser GPU config", () => { it("omits outputResolution from createRenderJob by default", async () => { await renderLocal("/tmp/project", "/tmp/out.mp4", { - fps: 30, + fps: { num: 30, den: 1 }, quality: "standard", format: "mp4", gpu: false, @@ -245,7 +245,7 @@ describe("renderLocal browser GPU config", () => { }); await renderLocal("/tmp/project", "/tmp/out.mp4", { - fps: 30, + fps: { num: 30, den: 1 }, quality: "standard", format: "mp4", gpu: false, diff --git a/packages/cli/src/commands/render.ts b/packages/cli/src/commands/render.ts index 56536dc8e..a8144c168 100644 --- a/packages/cli/src/commands/render.ts +++ b/packages/cli/src/commands/render.ts @@ -51,12 +51,43 @@ import { validateVariables, formatVariableValidationIssue, normalizeResolutionFlag, + parseFps, + fpsToNumber, + fpsToFfmpegArg, type VariableValidationIssue, type CanvasResolution, + type Fps, + type FpsParseResult, } from "@hyperframes/core"; -const VALID_FPS = new Set([24, 30, 60]); const VALID_QUALITY = new Set(["draft", "standard", "high"]); + +/** + * Map a {@link FpsParseResult} failure reason to a human-friendly + * error-box message. The empty / undefined / default-fallthrough case + * shouldn't be reachable from the CLI flag (citty supplies a default of + * "30") but the branch exists so this helper can be reused by other + * fps-accepting CLI surfaces in the future. + */ +function formatFpsParseError( + input: string, + reason: Exclude["reason"], +): string { + switch (reason) { + case "empty": + return "Frame rate must not be empty."; + case "not-a-number": + return `Got "${input}". Frame rate must be an integer (e.g. 30) or a rational (e.g. 30000/1001 for NTSC).`; + case "non-positive": + return `Got "${input}". Frame rate must be greater than zero.`; + case "out-of-range": + return `Got "${input}". Frame rate must be in the range 1–240.`; + case "invalid-fraction": + return `Got "${input}". Rational frame rates must be two positive integers separated by '/' (e.g. 30000/1001).`; + case "ambiguous-decimal": + return `Got "${input}". Decimal frame rates are ambiguous — use the exact rational form instead (e.g. 30000/1001 for 29.97).`; + } +} const VALID_FORMAT = new Set(["mp4", "webm", "mov", "png-sequence"]); // `png-sequence` writes a directory of frames rather than a single muxed file, // so its "extension" is empty — the auto-output path becomes a directory name. @@ -95,7 +126,10 @@ export default defineCommand({ fps: { type: "string", alias: "f", - description: "Frame rate: 24, 30, 60", + description: + "Frame rate. Accepts integer (24, 25, 30, 50, 60, 120, 240) or " + + "ffmpeg-style rational (30000/1001 for NTSC 29.97, 24000/1001 for " + + "23.976, 60000/1001 for 59.94). Range 1-240.", default: "30", }, quality: { @@ -194,12 +228,17 @@ export default defineCommand({ const project = resolveProject(args.dir); // ── Validate fps ─────────────────────────────────────────────────────── - const fpsRaw = parseInt(args.fps ?? "30", 10); - if (!VALID_FPS.has(fpsRaw)) { - errorBox("Invalid fps", `Got "${args.fps ?? "30"}". Must be 24, 30, or 60.`); + // Accept either integer (`30`) or ffmpeg-style rational (`30000/1001`). + // The whitelist-based validator was replaced with a sane numeric range so + // legitimate framerates (NTSC trio, PAL, 120/240 slow-mo) work without + // CLI gymnastics. The exact rational survives end-to-end into FFmpeg's + // `-r` / `-framerate` flags via `fpsToFfmpegArg`. + const fpsParse = parseFps(args.fps ?? "30"); + if (!fpsParse.ok) { + errorBox("Invalid fps", formatFpsParseError(args.fps ?? "30", fpsParse.reason)); process.exit(1); } - const fps = fpsRaw as 24 | 30 | 60; + const fps: Fps = fpsParse.value; // ── Validate quality ─────────────────────────────────────────────────── const qualityRaw = args.quality ?? "standard"; @@ -354,7 +393,9 @@ export default defineCommand({ console.log( c.accent("\u25C6") + " Rendering " + c.accent(nameLabel) + c.dim(" \u2192 " + outputPath), ); - console.log(c.dim(" " + fps + "fps \u00B7 " + quality + " \u00B7 " + workerLabel)); + console.log( + c.dim(" " + fpsToFfmpegArg(fps) + "fps \u00B7 " + quality + " \u00B7 " + workerLabel), + ); if (outputResolution) { // Don't claim "supersampled" — when the composition is already at the // target dimensions, the DPR resolves to 1 and no supersampling @@ -521,7 +562,7 @@ export default defineCommand({ }); interface RenderOptions { - fps: 24 | 30 | 60; + fps: Fps; quality: "draft" | "standard" | "high"; format: "mp4" | "webm" | "mov" | "png-sequence"; workers?: number; @@ -865,7 +906,7 @@ async function renderDocker( // Track metrics (no job object available from Docker — use a minimal stub) trackRenderComplete({ durationMs: elapsed, - fps: options.fps, + fps: fpsToNumber(options.fps), quality: options.quality, workers: options.workers, docker: true, @@ -964,7 +1005,7 @@ function handleRenderError( ): never { const message = error instanceof Error ? error.message : String(error); trackRenderError({ - fps: options.fps, + fps: fpsToNumber(options.fps), quality: options.quality, docker, workers: options.workers, @@ -1001,7 +1042,7 @@ function trackRenderMetrics( trackRenderComplete({ durationMs: elapsedMs, - fps: options.fps, + fps: fpsToNumber(options.fps), quality: options.quality, workers: options.workers ?? perf?.workers, docker, diff --git a/packages/cli/src/server/studioServer.ts b/packages/cli/src/server/studioServer.ts index 354be4f6b..b13b701ec 100644 --- a/packages/cli/src/server/studioServer.ts +++ b/packages/cli/src/server/studioServer.ts @@ -213,7 +213,9 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { } const job = createRenderJob({ - fps: opts.fps as 24 | 30 | 60, + // opts.fps is already an Fps rational — see vite-config-studio + // adapter for the same convention. + fps: opts.fps, quality: opts.quality as "draft" | "standard" | "high", format: opts.format, outputResolution: opts.outputResolution, diff --git a/packages/cli/src/utils/dockerRunArgs.test.ts b/packages/cli/src/utils/dockerRunArgs.test.ts index 981bc24c4..8b8e09e9d 100644 --- a/packages/cli/src/utils/dockerRunArgs.test.ts +++ b/packages/cli/src/utils/dockerRunArgs.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest"; import { buildDockerRunArgs, type DockerRenderOptions } from "./dockerRunArgs.js"; const BASE: DockerRenderOptions = { - fps: 30, + fps: { num: 30, den: 1 }, quality: "standard", format: "mp4", gpu: false, @@ -151,7 +151,7 @@ describe("buildDockerRunArgs", () => { const args = buildDockerRunArgs({ ...FIXED_INPUT, options: { - fps: 60, + fps: { num: 60, den: 1 }, quality: "high", format: "webm", workers: 8, @@ -240,6 +240,29 @@ describe("buildDockerRunArgs", () => { expect(args).not.toContain("--composition"); }); + it("forwards rational --fps verbatim (NTSC 30000/1001)", () => { + // Regression for the fps fraction-syntax feature: the rational form must + // survive the host → container hop as a single `30000/1001` argument so + // the in-container CLI re-parses it as exact NTSC, not 29.97 decimal. + const args = buildDockerRunArgs({ + ...FIXED_INPUT, + options: { ...BASE, fps: { num: 30000, den: 1001 } }, + }); + const fpsIdx = args.indexOf("--fps"); + expect(fpsIdx).toBeGreaterThanOrEqual(0); + expect(args[fpsIdx + 1]).toBe("30000/1001"); + }); + + it("forwards integer --fps as a bare integer string", () => { + const args = buildDockerRunArgs({ + ...FIXED_INPUT, + options: { ...BASE, fps: { num: 60, den: 1 } }, + }); + const fpsIdx = args.indexOf("--fps"); + expect(fpsIdx).toBeGreaterThanOrEqual(0); + expect(args[fpsIdx + 1]).toBe("60"); + }); + it("forwards --resolution to the container when outputResolution is set", () => { const args = buildDockerRunArgs({ ...FIXED_INPUT, diff --git a/packages/cli/src/utils/dockerRunArgs.ts b/packages/cli/src/utils/dockerRunArgs.ts index 8630d3bd4..986a75034 100644 --- a/packages/cli/src/utils/dockerRunArgs.ts +++ b/packages/cli/src/utils/dockerRunArgs.ts @@ -7,6 +7,8 @@ * a test in `dockerRunArgs.test.ts` — that combination is what catches * silent-drop regressions like the one that lost `--hdr` historically. */ +import { fpsToFfmpegArg, type Fps } from "@hyperframes/core"; + export interface DockerRunArgsInput { imageTag: string; /** Absolute host path to the project directory (mounted read-only at /project). */ @@ -19,7 +21,13 @@ export interface DockerRunArgsInput { } export interface DockerRenderOptions { - fps: 24 | 30 | 60; + /** + * Frame rate as an exact rational; see `Fps` in @hyperframes/core. The + * docker-run arg builder serializes this back to a `--fps` string + * (`"30"` or `"30000/1001"`) which the in-container CLI re-parses with + * `parseFps`, so the rational survives the host → container hop. + */ + fps: Fps; quality: "draft" | "standard" | "high"; format: "mp4" | "webm" | "mov" | "png-sequence"; workers?: number; @@ -54,7 +62,7 @@ export function buildDockerRunArgs(input: DockerRunArgsInput): string[] { "--output", `/output/${outputFilename}`, "--fps", - String(options.fps), + fpsToFfmpegArg(options.fps), "--quality", options.quality, "--format", diff --git a/packages/core/src/core.types.ts b/packages/core/src/core.types.ts index c3d069d31..a6552998b 100644 --- a/packages/core/src/core.types.ts +++ b/packages/core/src/core.types.ts @@ -2,6 +2,131 @@ export type ExecutionMode = "planning" | "design" | "execution" | null; +// ── Frame rate ────────────────────────────────────────────────────────────── + +/** + * Frame rate as an exact rational. Carrying `{num, den}` end-to-end (rather + * than collapsing to `29.97`) lets us pass NTSC / drop-frame rates straight + * through to FFmpeg via `-r 30000/1001` without any decimal round-trip. + * + * Integer fps is represented with `den: 1` (e.g. `{ num: 30, den: 1 }`). + * + * Use {@link fpsToNumber} when arithmetic forces a decimal (e.g. `setTimeout` + * intervals) and {@link fpsToFfmpegArg} when emitting FFmpeg `-r` / + * `-framerate` strings. + */ +export interface Fps { + num: number; + den: number; +} + +/** + * Decimal value of an {@link Fps} rational. Used at sites that need a + * `number` for arithmetic (frame-index → time, frame intervals, telemetry + * payloads) where the small precision loss of the decimal is acceptable. + */ +export function fpsToNumber(fps: Fps): number { + return fps.num / fps.den; +} + +/** + * FFmpeg-style fps argument. Returns `"30"` for integer fps and `"30000/1001"` + * for rationals — both forms are accepted verbatim by FFmpeg's `-r` and + * `-framerate` flags. We keep integer fps as a bare integer so existing + * snapshot tests / log output don't churn for the common case. + */ +export function fpsToFfmpegArg(fps: Fps): string { + return fps.den === 1 ? String(fps.num) : `${fps.num}/${fps.den}`; +} + +/** + * Discriminated parse result for {@link parseFps}. Lets the CLI / route + * validation own its own error UX without losing the structured failure + * reason. + */ +export type FpsParseResult = + | { ok: true; value: Fps } + | { + ok: false; + reason: + | "empty" + | "not-a-number" + | "non-positive" + | "out-of-range" + | "invalid-fraction" + | "ambiguous-decimal"; + }; + +/** + * Parse a user-supplied fps spec into an {@link Fps} rational. + * + * Accepted forms: + * - integer string `"30"` → `{ num: 30, den: 1 }` + * - integer number `30` → `{ num: 30, den: 1 }` + * - rational string `"30000/1001"` → `{ num: 30000, den: 1001 }` (exact NTSC) + * + * Rejected: + * - empty / non-numeric input + * - decimals like `"29.97"` — callers must spell rationals with `/` so the + * exact denominator is unambiguous (FFmpeg treats `29.97` as a slightly + * different framerate than `30000/1001`). + * - division by zero, negative or zero numerator + * - decimal value outside `[1, 240]` — defensive bounds for "human" fps + * ranges (24, 25, 30, 50, 60, 120, 240, plus the NTSC trio). + */ +export function parseFps(input: string | number): FpsParseResult { + if (typeof input === "number") { + if (!Number.isFinite(input)) return { ok: false, reason: "not-a-number" }; + if (!Number.isInteger(input)) return { ok: false, reason: "ambiguous-decimal" }; + if (input <= 0) return { ok: false, reason: "non-positive" }; + if (input > 240) return { ok: false, reason: "out-of-range" }; + return { ok: true, value: { num: input, den: 1 } }; + } + const raw = input.trim(); + if (raw === "") return { ok: false, reason: "empty" }; + + if (raw.includes("/")) { + const parts = raw.split("/"); + if (parts.length !== 2) return { ok: false, reason: "invalid-fraction" }; + const num = Number(parts[0]); + const den = Number(parts[1]); + if (!Number.isFinite(num) || !Number.isFinite(den)) { + return { ok: false, reason: "not-a-number" }; + } + if (!Number.isInteger(num) || !Number.isInteger(den)) { + return { ok: false, reason: "invalid-fraction" }; + } + if (den <= 0) return { ok: false, reason: "invalid-fraction" }; + if (num <= 0) return { ok: false, reason: "non-positive" }; + const decimal = num / den; + if (decimal < 1 || decimal > 240) return { ok: false, reason: "out-of-range" }; + return { ok: true, value: { num, den } }; + } + + // Integer-only path — reject `"29.97"` so users are explicit about the + // exact rational they want. + if (!/^-?\d+$/.test(raw)) { + // Allow caller to differentiate "29.97" from "abc" if they want; both + // are user errors but the message can be friendlier for decimals. + if (/^-?\d*\.\d+$/.test(raw)) return { ok: false, reason: "ambiguous-decimal" }; + return { ok: false, reason: "not-a-number" }; + } + const n = Number(raw); + if (n <= 0) return { ok: false, reason: "non-positive" }; + if (n > 240) return { ok: false, reason: "out-of-range" }; + return { ok: true, value: { num: n, den: 1 } }; +} + +/** + * Convenience wrapper around {@link parseFps} for callsites that want the + * default-30-fps fallback when input is `undefined`. Does NOT swallow parse + * errors — those still surface via the discriminated result. + */ +export function parseFpsWithDefault(input: string | number | undefined): FpsParseResult { + if (input === undefined || input === "") return { ok: true, value: { num: 30, den: 1 } }; + return parseFps(input); +} + /** Video orientation / aspect ratio. */ export type Orientation = "16:9" | "9:16"; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index f0706eaff..3f4f58447 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -11,6 +11,8 @@ export type { TimelineElementType, MediaElementType, CanvasResolution, + Fps, + FpsParseResult, MediaFile, CompositionAPI, PlayerAPI, @@ -38,6 +40,10 @@ export { CANVAS_DIMENSIONS, VALID_CANVAS_RESOLUTIONS, normalizeResolutionFlag, + parseFps, + parseFpsWithDefault, + fpsToNumber, + fpsToFfmpegArg, TIMELINE_COLORS, DEFAULT_DURATIONS, COMPOSITION_VARIABLE_TYPES, diff --git a/packages/core/src/parseFps.test.ts b/packages/core/src/parseFps.test.ts new file mode 100644 index 000000000..2a4f8705d --- /dev/null +++ b/packages/core/src/parseFps.test.ts @@ -0,0 +1,158 @@ +import { describe, expect, it } from "vitest"; +import { parseFps, fpsToNumber, fpsToFfmpegArg } from "./core.types"; + +describe("parseFps — integer forms", () => { + it("parses integer string '30' as { num: 30, den: 1 }", () => { + const result = parseFps("30"); + expect(result.ok).toBe(true); + if (result.ok) expect(result.value).toEqual({ num: 30, den: 1 }); + }); + + it("parses integer number 30 as { num: 30, den: 1 }", () => { + const result = parseFps(30); + expect(result.ok).toBe(true); + if (result.ok) expect(result.value).toEqual({ num: 30, den: 1 }); + }); + + it("parses '24' / '60' / '120' / '240' as integer fps with den=1", () => { + for (const n of [24, 60, 120, 240]) { + const result = parseFps(String(n)); + expect(result.ok).toBe(true); + if (result.ok) expect(result.value).toEqual({ num: n, den: 1 }); + } + }); + + it("trims surrounding whitespace on integer strings", () => { + const result = parseFps(" 30 "); + expect(result.ok).toBe(true); + if (result.ok) expect(result.value).toEqual({ num: 30, den: 1 }); + }); +}); + +describe("parseFps — rational forms", () => { + it("parses '30000/1001' as exact NTSC", () => { + const result = parseFps("30000/1001"); + expect(result.ok).toBe(true); + if (result.ok) expect(result.value).toEqual({ num: 30000, den: 1001 }); + }); + + it("parses '24000/1001' as 23.976", () => { + const result = parseFps("24000/1001"); + expect(result.ok).toBe(true); + if (result.ok) expect(result.value).toEqual({ num: 24000, den: 1001 }); + }); + + it("parses '60000/1001' as 59.94", () => { + const result = parseFps("60000/1001"); + expect(result.ok).toBe(true); + if (result.ok) expect(result.value).toEqual({ num: 60000, den: 1001 }); + }); + + it("parses '60/2' as { num: 60, den: 2 } (no auto-simplification)", () => { + // Preserving the literal numerator/denominator keeps `fpsToFfmpegArg` + // round-trippable — if the user typed the rational form, we forward it + // verbatim to ffmpeg. + const result = parseFps("60/2"); + expect(result.ok).toBe(true); + if (result.ok) expect(result.value).toEqual({ num: 60, den: 2 }); + }); +}); + +describe("parseFps — rejected inputs", () => { + it("rejects 'abc'", () => { + const result = parseFps("abc"); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe("not-a-number"); + }); + + it("rejects '30/0' (zero denominator)", () => { + const result = parseFps("30/0"); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe("invalid-fraction"); + }); + + it("rejects '30/-1' (negative denominator)", () => { + const result = parseFps("30/-1"); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe("invalid-fraction"); + }); + + it("rejects '0' (non-positive)", () => { + const result = parseFps("0"); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe("non-positive"); + }); + + it("rejects '241' (out of range)", () => { + const result = parseFps("241"); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe("out-of-range"); + }); + + it("rejects '29.97' (ambiguous decimal — must spell rational)", () => { + // The whole point of taking rationals is precision; accepting the + // decimal form silently would invert that guarantee. + const result = parseFps("29.97"); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe("ambiguous-decimal"); + }); + + it("rejects empty string", () => { + const result = parseFps(""); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe("empty"); + }); + + it("rejects '30000/1001/extra' (too many slashes)", () => { + const result = parseFps("30000/1001/extra"); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe("invalid-fraction"); + }); + + it("rejects rational with decimal numerator '30.5/1'", () => { + const result = parseFps("30.5/1"); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe("invalid-fraction"); + }); + + it("rejects rational > 240 by decimal value", () => { + // 1000/3 ≈ 333.33 — even though numerator and denominator are integers, + // the decimal value is out of the supported range. + const result = parseFps("1000/3"); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe("out-of-range"); + }); +}); + +describe("fpsToNumber + fpsToFfmpegArg + frame interval math", () => { + it("integer fps 30/1 → 33.333…ms frame interval", () => { + const fps = { num: 30, den: 1 }; + const intervalMs = (1000 * fps.den) / fps.num; + expect(intervalMs).toBeCloseTo(33.333, 3); + }); + + it("NTSC 30000/1001 → 33.366…ms frame interval", () => { + const fps = { num: 30000, den: 1001 }; + const intervalMs = (1000 * fps.den) / fps.num; + // 1001/30 = 33.36666…ms — the canonical NTSC interval. Differs from + // the integer-30 interval by ~0.033 ms (1 ULP at our scales). + expect(intervalMs).toBeCloseTo(33.36666, 3); + expect(intervalMs).not.toBeCloseTo(33.333, 3); + }); + + it("fpsToNumber collapses rational to decimal", () => { + expect(fpsToNumber({ num: 30, den: 1 })).toBe(30); + expect(fpsToNumber({ num: 30000, den: 1001 })).toBeCloseTo(29.97003, 5); + }); + + it("fpsToFfmpegArg emits bare integer for den=1", () => { + expect(fpsToFfmpegArg({ num: 30, den: 1 })).toBe("30"); + expect(fpsToFfmpegArg({ num: 60, den: 1 })).toBe("60"); + }); + + it("fpsToFfmpegArg emits 'num/den' for rationals", () => { + expect(fpsToFfmpegArg({ num: 30000, den: 1001 })).toBe("30000/1001"); + expect(fpsToFfmpegArg({ num: 24000, den: 1001 })).toBe("24000/1001"); + expect(fpsToFfmpegArg({ num: 60000, den: 1001 })).toBe("60000/1001"); + }); +}); diff --git a/packages/core/src/studio-api/routes/render.test.ts b/packages/core/src/studio-api/routes/render.test.ts index bb702cf92..784333410 100644 --- a/packages/core/src/studio-api/routes/render.test.ts +++ b/packages/core/src/studio-api/routes/render.test.ts @@ -115,3 +115,71 @@ describe("POST /projects/:id/render — outputResolution forwarding", () => { } }); }); + +describe("POST /projects/:id/render — fps wire format", () => { + // The fps fraction-syntax feature accepts JSON `number` (integer fps) and + // JSON `string` (ffmpeg-style rational) on the wire, normalizing both to + // the structured Fps form before invoking the adapter. + it("forwards integer fps as { num, den: 1 }", async () => { + const spy = vi.fn(); + const { app, cleanup } = buildApp(spy); + try { + await app.request("http://localhost/projects/demo/render", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ fps: 60, quality: "standard", format: "mp4" }), + }); + expect(spy.mock.calls[0][0].fps).toEqual({ num: 60, den: 1 }); + } finally { + cleanup(); + } + }); + + it("parses '30000/1001' string body as exact NTSC", async () => { + const spy = vi.fn(); + const { app, cleanup } = buildApp(spy); + try { + await app.request("http://localhost/projects/demo/render", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ fps: "30000/1001", quality: "standard", format: "mp4" }), + }); + expect(spy.mock.calls[0][0].fps).toEqual({ num: 30000, den: 1001 }); + } finally { + cleanup(); + } + }); + + it("falls back to 30/1 for malformed fps values", async () => { + // Matches the lenient handling of `quality` and `resolution` in the same + // route — the producer surfaces a clearer downstream error if the value + // is genuinely unusable. + const spy = vi.fn(); + const { app, cleanup } = buildApp(spy); + try { + await app.request("http://localhost/projects/demo/render", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ fps: "abc", quality: "standard", format: "mp4" }), + }); + expect(spy.mock.calls[0][0].fps).toEqual({ num: 30, den: 1 }); + } finally { + cleanup(); + } + }); + + it("falls back to 30/1 when fps is omitted", async () => { + const spy = vi.fn(); + const { app, cleanup } = buildApp(spy); + try { + await app.request("http://localhost/projects/demo/render", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ quality: "standard", format: "mp4" }), + }); + expect(spy.mock.calls[0][0].fps).toEqual({ num: 30, den: 1 }); + } finally { + cleanup(); + } + }); +}); diff --git a/packages/core/src/studio-api/routes/render.ts b/packages/core/src/studio-api/routes/render.ts index 7e5eb041c..fe5dc14e5 100644 --- a/packages/core/src/studio-api/routes/render.ts +++ b/packages/core/src/studio-api/routes/render.ts @@ -3,7 +3,7 @@ import { streamSSE } from "hono/streaming"; import { existsSync, readFileSync, mkdirSync, unlinkSync, readdirSync, statSync } from "node:fs"; import { join } from "node:path"; import type { StudioApiAdapter, RenderJobState } from "../types.js"; -import { VALID_CANVAS_RESOLUTIONS, type CanvasResolution } from "../../core.types.js"; +import { VALID_CANVAS_RESOLUTIONS, parseFps, type CanvasResolution } from "../../core.types.js"; const VALID_RESOLUTIONS = new Set(VALID_CANVAS_RESOLUTIONS); @@ -50,7 +50,12 @@ export function registerRenderRoutes(api: Hono, adapter: StudioApiAdapter): void if (!project) return c.json({ error: "not found" }, 404); const body = (await c.req.json().catch(() => ({}))) as { - fps?: number; + // Polymorphic per design note in core.types.Fps: + // number → integer fps (e.g. 30) + // string → rational fps (e.g. "30000/1001" for NTSC 29.97) + // Decimals are rejected on purpose so the exact denominator stays + // unambiguous (29.97 ≠ 30000/1001 when ffmpeg consumes them). + fps?: number | string; quality?: string; format?: string; resolution?: string; @@ -58,7 +63,13 @@ export function registerRenderRoutes(api: Hono, adapter: StudioApiAdapter): void const VALID_FORMATS = new Set(["mp4", "webm", "mov"]); const FORMAT_EXT: Record = { mp4: ".mp4", webm: ".webm", mov: ".mov" }; const format = VALID_FORMATS.has(body.format ?? "") ? (body.format as string) : "mp4"; - const fps: 24 | 30 | 60 = body.fps === 24 || body.fps === 60 ? body.fps : 30; + + // Default to 30 fps when unset or unparseable. The route stays lenient on + // invalid fps values (matching the lenient handling of `resolution` and + // `quality` already in this file) — the producer surfaces a clearer error + // message if the caller really did mean to fail loudly. + const fpsParse = body.fps === undefined ? null : parseFps(body.fps); + const fps = fpsParse && fpsParse.ok ? fpsParse.value : { num: 30, den: 1 }; const quality = ["draft", "standard", "high"].includes(body.quality ?? "") ? (body.quality as string) : "standard"; diff --git a/packages/core/src/studio-api/types.ts b/packages/core/src/studio-api/types.ts index c37e8bda6..a80b3f58d 100644 --- a/packages/core/src/studio-api/types.ts +++ b/packages/core/src/studio-api/types.ts @@ -63,7 +63,14 @@ export interface StudioApiAdapter { project: ResolvedProject; outputPath: string; format: "mp4" | "webm" | "mov"; - fps: number; + /** + * Frame rate as an exact rational. The HTTP layer (POST + * `/projects/:id/render`) accepts either a JSON number (integer fps, + * `30`) or a JSON string (ffmpeg-style rational, `"30000/1001"`); the + * route normalizes both into `Fps` before invoking the adapter, so + * adapter implementations only ever see the rational form. + */ + fps: import("../core.types.js").Fps; quality: string; jobId: string; /** diff --git a/packages/engine/src/services/chunkEncoder.test.ts b/packages/engine/src/services/chunkEncoder.test.ts index 6a2fe93c5..6795a6478 100644 --- a/packages/engine/src/services/chunkEncoder.test.ts +++ b/packages/engine/src/services/chunkEncoder.test.ts @@ -79,7 +79,7 @@ describe("getEncoderPreset", () => { }); describe("buildEncoderArgs anti-banding", () => { - const baseOptions = { fps: 30, width: 1920, height: 1080 }; + const baseOptions = { fps: { num: 30, den: 1 }, width: 1920, height: 1080 }; it("adds aq-mode=3 x264-params for h264 CPU encoding", () => { const args = buildEncoderArgs( @@ -148,8 +148,35 @@ describe("buildEncoderArgs anti-banding", () => { }); }); +describe("buildEncoderArgs fps rational forwarding", () => { + // Regression for the fps fraction-syntax feature: rational fps must reach + // ffmpeg's `-r` flag verbatim (e.g. "30000/1001") so NTSC stays exact end- + // to-end rather than being rounded to 29.97 decimal at the encoder boundary. + it("emits integer -r for { num: 30, den: 1 }", () => { + const args = buildEncoderArgs( + { fps: { num: 30, den: 1 }, width: 1920, height: 1080, codec: "h264" }, + ["-framerate", "30", "-i", "frames/%04d.png"], + "out.mp4", + ); + const rIdx = args.indexOf("-r"); + expect(rIdx).toBeGreaterThan(-1); + expect(args[rIdx + 1]).toBe("30"); + }); + + it("emits rational -r for NTSC { num: 30000, den: 1001 }", () => { + const args = buildEncoderArgs( + { fps: { num: 30000, den: 1001 }, width: 1920, height: 1080, codec: "h264" }, + ["-framerate", "30000/1001", "-i", "frames/%04d.png"], + "out.mp4", + ); + const rIdx = args.indexOf("-r"); + expect(rIdx).toBeGreaterThan(-1); + expect(args[rIdx + 1]).toBe("30000/1001"); + }); +}); + describe("buildEncoderArgs GPU preset mapping", () => { - const baseOptions = { fps: 30, width: 1920, height: 1080 }; + const baseOptions = { fps: { num: 30, den: 1 }, width: 1920, height: 1080 }; const inputArgs = ["-framerate", "30", "-i", "frames/%04d.png"]; function presetArg(args: string[]): string | undefined { @@ -232,7 +259,7 @@ describe("buildEncoderArgs GPU preset mapping", () => { }); describe("buildEncoderArgs color space", () => { - const baseOptions = { fps: 30, width: 1920, height: 1080 }; + const baseOptions = { fps: { num: 30, den: 1 }, width: 1920, height: 1080 }; const inputArgs = ["-framerate", "30", "-i", "frames/%04d.png"]; it("adds bt709 color space metadata for h264 CPU encoding", () => { @@ -365,7 +392,7 @@ describe("getEncoderPreset HDR", () => { }); describe("buildEncoderArgs HDR color space", () => { - const baseOptions = { fps: 30, width: 1920, height: 1080 }; + const baseOptions = { fps: { num: 30, den: 1 }, width: 1920, height: 1080 }; const inputArgs = ["-framerate", "30", "-i", "frames/%04d.png"]; it("emits BT.2020 + arib-std-b67 tags for HDR HLG (h265 SW)", () => { diff --git a/packages/engine/src/services/chunkEncoder.ts b/packages/engine/src/services/chunkEncoder.ts index 6e9ea1b2d..67c718303 100644 --- a/packages/engine/src/services/chunkEncoder.ts +++ b/packages/engine/src/services/chunkEncoder.ts @@ -17,6 +17,7 @@ import { } from "../utils/gpuEncoder.js"; import { type HdrTransfer, getHdrEncoderColorParams } from "../utils/hdr.js"; import { formatFfmpegError, runFfmpeg } from "../utils/runFfmpeg.js"; +import { fpsToFfmpegArg } from "@hyperframes/core"; import type { EncoderOptions, EncodeResult, MuxResult } from "./chunkEncoder.types.js"; export type { EncoderOptions, EncodeResult, MuxResult } from "./chunkEncoder.types.js"; @@ -105,7 +106,7 @@ export function buildEncoderArgs( options = { ...options, hdr: undefined }; } - const args: string[] = [...inputArgs, "-r", String(fps)]; + const args: string[] = [...inputArgs, "-r", fpsToFfmpegArg(fps)]; const shouldUseGpu = useGpu && gpuEncoder !== null; if (codec === "h264" || codec === "h265") { @@ -309,7 +310,7 @@ export async function encodeFramesFromDir( } const inputPath = join(framesDir, framePattern); - const inputArgs = ["-framerate", String(options.fps), "-i", inputPath]; + const inputArgs = ["-framerate", fpsToFfmpegArg(options.fps), "-i", inputPath]; const args = buildEncoderArgs(options, inputArgs, outputPath, gpuEncoder); return new Promise((resolve) => { @@ -432,7 +433,7 @@ export async function encodeFramesChunkedConcat( const inputPath = join(framesDir, framePattern); const inputArgs = [ "-framerate", - String(options.fps), + fpsToFfmpegArg(options.fps), "-start_number", String(startNumber), "-i", diff --git a/packages/engine/src/services/chunkEncoder.types.ts b/packages/engine/src/services/chunkEncoder.types.ts index cabb0be67..228aad63c 100644 --- a/packages/engine/src/services/chunkEncoder.types.ts +++ b/packages/engine/src/services/chunkEncoder.types.ts @@ -1,7 +1,9 @@ import type { HdrTransfer } from "../utils/hdr.js"; +import type { Fps } from "@hyperframes/core"; export interface EncoderOptions { - fps: number; + /** Frame rate as an exact rational; see `Fps` in @hyperframes/core. */ + fps: Fps; width: number; height: number; codec?: "h264" | "h265" | "vp9" | "prores"; diff --git a/packages/engine/src/services/frameCapture.ts b/packages/engine/src/services/frameCapture.ts index 4b7598548..72c64edd6 100644 --- a/packages/engine/src/services/frameCapture.ts +++ b/packages/engine/src/services/frameCapture.ts @@ -10,7 +10,7 @@ import { type Browser, type Page, type Viewport, type ConsoleMessage } from "puppeteer-core"; import { existsSync, mkdirSync, writeFileSync } from "fs"; import { join } from "path"; -import { quantizeTimeToFrame } from "@hyperframes/core"; +import { quantizeTimeToFrame, fpsToNumber } from "@hyperframes/core"; // ── Extracted modules ─────────────────────────────────────────────────────── import { @@ -228,7 +228,10 @@ export async function createCaptureSession( }, captureMode, beginFrameTimeTicks: 0, - beginFrameIntervalMs: 1000 / Math.max(1, options.fps), + // Frame interval in ms: 1000 * den / num. For 30/1 → 33.333…, for + // 30000/1001 (NTSC) → 33.366…. JavaScript number precision is fine at + // these scales — no rounding required. + beginFrameIntervalMs: (1000 * options.fps.den) / Math.max(1, options.fps.num), beginFrameHasDamageCount: 0, beginFrameNoDamageCount: 0, config, @@ -591,7 +594,7 @@ async function prepareFrameForCapture( throw new Error("[FrameCapture] Session not initialized"); } - const quantizedTime = quantizeTimeToFrame(time, options.fps); + const quantizedTime = quantizeTimeToFrame(time, fpsToNumber(options.fps)); const seekStart = Date.now(); // Seek via the __hf protocol. The page's seek() implementation handles diff --git a/packages/engine/src/services/parallelCoordinator.ts b/packages/engine/src/services/parallelCoordinator.ts index 38c3472b6..bbee9a15e 100644 --- a/packages/engine/src/services/parallelCoordinator.ts +++ b/packages/engine/src/services/parallelCoordinator.ts @@ -185,7 +185,10 @@ async function executeWorkerTask( if (signal?.aborted) { throw new Error("Parallel worker cancelled"); } - const time = i / captureOptions.fps; + // captureOptions.fps is an Fps rational; collapse to decimal for the + // frame-index → time math. The 1-in-1001 ULP loss for NTSC is invisible + // at our scales (frame count tops out at single-digit thousands). + const time = (i * captureOptions.fps.den) / captureOptions.fps.num; if (onFrameBuffer) { // Streaming mode: capture to buffer and invoke callback diff --git a/packages/engine/src/services/screenshotService.test.ts b/packages/engine/src/services/screenshotService.test.ts index b0f9f36c1..30a711400 100644 --- a/packages/engine/src/services/screenshotService.test.ts +++ b/packages/engine/src/services/screenshotService.test.ts @@ -27,7 +27,7 @@ describe("pageScreenshotCapture supersample plumbing", () => { await pageScreenshotCapture(page, { width: 1920, height: 1080, - fps: 30, + fps: { num: 30, den: 1 }, format: "jpeg", quality: 80, }); @@ -45,7 +45,7 @@ describe("pageScreenshotCapture supersample plumbing", () => { await pageScreenshotCapture(page, { width: 1920, height: 1080, - fps: 30, + fps: { num: 30, den: 1 }, format: "jpeg", deviceScaleFactor: 1, }); @@ -61,7 +61,7 @@ describe("pageScreenshotCapture supersample plumbing", () => { await pageScreenshotCapture(page, { width: 1920, height: 1080, - fps: 30, + fps: { num: 30, den: 1 }, format: "jpeg", deviceScaleFactor: 2, }); @@ -81,7 +81,7 @@ describe("pageScreenshotCapture supersample plumbing", () => { await pageScreenshotCapture(page, { width: 1280, height: 720, - fps: 30, + fps: { num: 30, den: 1 }, format: "jpeg", deviceScaleFactor: 3, }); diff --git a/packages/engine/src/services/streamingEncoder.test.ts b/packages/engine/src/services/streamingEncoder.test.ts index 2d3f4f891..cfcb6774c 100644 --- a/packages/engine/src/services/streamingEncoder.test.ts +++ b/packages/engine/src/services/streamingEncoder.test.ts @@ -23,7 +23,7 @@ import { import { DEFAULT_HDR10_MASTERING } from "../utils/hdr.js"; const baseHdrPq: StreamingEncoderOptions = { - fps: 30, + fps: { num: 30, den: 1 }, width: 1920, height: 1080, codec: "h265", @@ -41,7 +41,7 @@ const baseHdrHlg: StreamingEncoderOptions = { }; const baseSdr: StreamingEncoderOptions = { - fps: 30, + fps: { num: 30, den: 1 }, width: 1920, height: 1080, codec: "h264", @@ -166,9 +166,52 @@ describe("buildStreamingArgs", () => { }); }); + describe("fps rational forwarding", () => { + // Regression for the fps fraction-syntax feature: both `-framerate` + // (input timestamping) and `-r` (output framerate) must carry the + // rational verbatim — collapsing to 29.97 decimal at this boundary + // would defeat the whole point of supporting NTSC end-to-end. + it("emits rational -framerate and -r for NTSC 30000/1001 (image2pipe)", () => { + const sdrNtsc: StreamingEncoderOptions = { + ...baseSdr, + fps: { num: 30000, den: 1001 }, + }; + const args = buildStreamingArgs(sdrNtsc, "/tmp/ntsc.mp4"); + const framerateIdx = args.indexOf("-framerate"); + expect(framerateIdx).toBeGreaterThan(-1); + expect(args[framerateIdx + 1]).toBe("30000/1001"); + + const rIdx = args.indexOf("-r"); + expect(rIdx).toBeGreaterThan(-1); + expect(args[rIdx + 1]).toBe("30000/1001"); + }); + + it("emits rational -framerate and -r for NTSC 30000/1001 (rawvideo HDR)", () => { + const hdrNtsc: StreamingEncoderOptions = { + ...baseHdrPq, + fps: { num: 30000, den: 1001 }, + }; + const args = buildStreamingArgs(hdrNtsc, "/tmp/ntsc-hdr.mp4"); + const framerateIdx = args.indexOf("-framerate"); + expect(framerateIdx).toBeGreaterThan(-1); + expect(args[framerateIdx + 1]).toBe("30000/1001"); + + const rIdx = args.indexOf("-r"); + expect(rIdx).toBeGreaterThan(-1); + expect(args[rIdx + 1]).toBe("30000/1001"); + }); + + it("emits bare integer -r for { num: 30, den: 1 }", () => { + const args = buildStreamingArgs(baseSdr, "/tmp/30.mp4"); + const rIdx = args.indexOf("-r"); + expect(rIdx).toBeGreaterThan(-1); + expect(args[rIdx + 1]).toBe("30"); + }); + }); + describe("GPU preset mapping", () => { const baseGpu: StreamingEncoderOptions = { - fps: 30, + fps: { num: 30, den: 1 }, width: 1920, height: 1080, codec: "h264", @@ -348,7 +391,7 @@ function createSpawnSpy(): { } const baseOptions: StreamingEncoderOptions = { - fps: 30, + fps: { num: 30, den: 1 }, width: 100, height: 100, codec: "h264", diff --git a/packages/engine/src/services/streamingEncoder.ts b/packages/engine/src/services/streamingEncoder.ts index 03c0d0735..de2d2c000 100644 --- a/packages/engine/src/services/streamingEncoder.ts +++ b/packages/engine/src/services/streamingEncoder.ts @@ -26,6 +26,7 @@ import { formatFfmpegError } from "../utils/runFfmpeg.js"; import { getHdrEncoderColorParams } from "../utils/hdr.js"; import { type EncoderOptions } from "./chunkEncoder.types.js"; import { DEFAULT_CONFIG, type EngineConfig } from "../config.js"; +import { fpsToFfmpegArg, type Fps } from "@hyperframes/core"; // Re-export EncoderOptions so callers can reference the type via this module. export type { EncoderOptions } from "./chunkEncoder.types.js"; @@ -99,7 +100,8 @@ export function createFrameReorderBuffer(startFrame: number, endFrame: number): // --------------------------------------------------------------------------- export interface StreamingEncoderOptions { - fps: number; + /** Frame rate as an exact rational; see `Fps` in @hyperframes/core. */ + fps: Fps; width: number; height: number; codec?: "h264" | "h265" | "vp9" | "prores"; @@ -169,7 +171,7 @@ export function buildStreamingArgs( "-s", `${options.width}x${options.height}`, "-framerate", - String(fps), + fpsToFfmpegArg(fps), ); if (inputColorTrc) { args.push( @@ -184,9 +186,18 @@ export function buildStreamingArgs( args.push("-i", "-"); } else { const inputCodec = imageFormat === "png" ? "png" : "mjpeg"; - args.push("-f", "image2pipe", "-vcodec", inputCodec, "-framerate", String(fps), "-i", "-"); + args.push( + "-f", + "image2pipe", + "-vcodec", + inputCodec, + "-framerate", + fpsToFfmpegArg(fps), + "-i", + "-", + ); } - args.push("-r", String(fps)); + args.push("-r", fpsToFfmpegArg(fps)); const shouldUseGpu = useGpu && gpuEncoder !== null; diff --git a/packages/engine/src/types.ts b/packages/engine/src/types.ts index 963bcaa7d..1fde81edc 100644 --- a/packages/engine/src/types.ts +++ b/packages/engine/src/types.ts @@ -4,6 +4,7 @@ * The engine's page contract. Any web page that wants to be rendered * as video must expose `window.__hf` implementing the HfProtocol interface. */ +import type { Fps } from "@hyperframes/core"; // ── Seek Protocol ────────────────────────────────────────────────────────────── @@ -81,7 +82,13 @@ export interface HfProtocol { export interface CaptureOptions { width: number; height: number; - fps: number; + /** + * Frame rate as an exact rational. Integer fps is `{ num: 30, den: 1 }`; + * NTSC is `{ num: 30000, den: 1001 }`. Captures are scheduled by the + * decimal interval (1000 * den / num ms) but FFmpeg arg builders emit the + * rational form verbatim — see `fpsToFfmpegArg`. + */ + fps: Fps; format?: "jpeg" | "png"; quality?: number; deviceScaleFactor?: number; diff --git a/packages/producer/src/benchmark.ts b/packages/producer/src/benchmark.ts index 68d843eff..e13259f45 100644 --- a/packages/producer/src/benchmark.ts +++ b/packages/producer/src/benchmark.ts @@ -36,6 +36,7 @@ import { executeRenderJob, type RenderPerfSummary, } from "./services/renderOrchestrator.js"; +import { parseFps } from "@hyperframes/core"; const scriptDir = dirname(fileURLToPath(import.meta.url)); const testsDir = resolve(scriptDir, "../tests"); @@ -44,7 +45,9 @@ const perfDir = resolve(testsDir, "perf"); interface TestMeta { name: string; tags?: string[]; - renderConfig: { fps: 24 | 30 | 60 }; + // Same on-disk shape as the regression harness — JSON `number` (integer + // fps) or JSON `string` ("30000/1001"). Normalized to Fps when loaded. + renderConfig: { fps: import("@hyperframes/core").Fps }; } interface BenchmarkRun { @@ -125,7 +128,25 @@ function discoverFixtures( if (only && entry !== only) continue; - const meta: TestMeta = JSON.parse(readFileSync(metaPath, "utf-8")); + const rawMeta = JSON.parse(readFileSync(metaPath, "utf-8")) as { + name: string; + tags?: string[]; + renderConfig: { fps: number | string }; + }; + // meta.json on disk uses a JSON `number` for legacy integer fps values + // and a JSON `string` for new ffmpeg-style rationals (e.g. "30000/1001"). + // Normalize to the Fps rational shape so downstream code only sees the + // structured form — same convention as the regression harness. + const fpsParse = parseFps(rawMeta.renderConfig.fps); + if (!fpsParse.ok) { + throw new Error( + `Benchmark fixture ${entry}: invalid renderConfig.fps ${JSON.stringify(rawMeta.renderConfig.fps)}`, + ); + } + const meta: TestMeta = { + ...rawMeta, + renderConfig: { ...rawMeta.renderConfig, fps: fpsParse.value }, + }; const fixtureTags = meta.tags ?? []; // Positive filter (--tags): if provided, fixture must match at least one. if (tags.length > 0 && !fixtureTags.some((t) => tags.includes(t))) continue; diff --git a/packages/producer/src/regression-harness.ts b/packages/producer/src/regression-harness.ts index a518fba18..69c4f1aa2 100644 --- a/packages/producer/src/regression-harness.ts +++ b/packages/producer/src/regression-harness.ts @@ -19,6 +19,7 @@ import { compileForRender } from "./services/htmlCompiler.js"; import { validateCompilation } from "./services/compilationTester.js"; import { extractMediaMetadata } from "./utils/ffprobe.js"; import { buildRmsEnvelope, compareAudioEnvelopes } from "./utils/audioRegression.js"; +import { parseFps, fpsToNumber } from "@hyperframes/core"; // ── Types ──────────────────────────────────────────────────────────────────── @@ -31,7 +32,13 @@ type TestMetadata = { minAudioCorrelation: number; maxAudioLagWindows: number; renderConfig: { - fps: 24 | 30 | 60; + /** + * Frame rate. Stored on disk as a JSON number (integer fps, e.g. `30`) + * for legacy meta.json files, or a JSON string (`"30000/1001"` for NTSC) + * for rationals. The metadata validator normalizes both into an `Fps` + * rational at load time so downstream code only sees the structured form. + */ + fps: import("@hyperframes/core").Fps; format?: "mp4" | "webm"; // Optional: defaults to "mp4" workers?: number; // Optional: auto-calculates if omitted /** Force HDR in the harness; omitted/false preserves historical SDR-only test behavior. */ @@ -154,9 +161,23 @@ function validateMetadata(meta: unknown): TestMetadata { throw new Error("meta.json: 'renderConfig' must be an object"); } const rc = m.renderConfig as Record; - if (![24, 30, 60].includes(rc.fps as number)) { - throw new Error("meta.json: 'renderConfig.fps' must be 24, 30, or 60"); + // Accept either a JSON number (integer fps, e.g. 30) or a JSON string + // (ffmpeg-style rational, e.g. "30000/1001"). Normalize both into the Fps + // rational shape and write it back onto the metadata object so all + // downstream callers can assume the structured form. + const fpsRaw = rc.fps; + const fpsParse = + typeof fpsRaw === "number" || typeof fpsRaw === "string" + ? parseFps(fpsRaw) + : ({ ok: false, reason: "not-a-number" } as const); + if (!fpsParse.ok) { + throw new Error( + `meta.json: 'renderConfig.fps' must be an integer (e.g. 30) or rational string (e.g. "30000/1001"); got ${JSON.stringify( + fpsRaw, + )}`, + ); } + rc.fps = fpsParse.value; if (rc.format !== undefined && rc.format !== "mp4" && rc.format !== "webm") { throw new Error("meta.json: 'renderConfig.format' must be 'mp4' or 'webm' (or omit for mp4)"); } @@ -452,13 +473,13 @@ function saveFailureDetails( renderedVideoPath, checkpoint.time, join(framesDir, `actual_${timeStr}s.png`), - suite.meta.renderConfig.fps, + fpsToNumber(suite.meta.renderConfig.fps), ); extractFrameAsImage( snapshotVideoPath, checkpoint.time, join(framesDir, `expected_${timeStr}s.png`), - suite.meta.renderConfig.fps, + fpsToNumber(suite.meta.renderConfig.fps), ); } catch { logPretty(` Warning: Could not extract frame at ${checkpoint.time}s`, "⚠️"); @@ -666,7 +687,7 @@ async function runTestSuite( renderedOutputPath, snapshotVideoPath, time, - suite.meta.renderConfig.fps, + fpsToNumber(suite.meta.renderConfig.fps), ); visualCheckpoints.push({ time, diff --git a/packages/producer/src/server.ts b/packages/producer/src/server.ts index 861141a7d..1addbd6ff 100644 --- a/packages/producer/src/server.ts +++ b/packages/producer/src/server.ts @@ -38,6 +38,7 @@ import { prepareHyperframeLintBody, runHyperframeLint } from "./services/hyperfr import { resolveRenderPaths } from "./utils/paths.js"; import { defaultLogger, type ProducerLogger } from "./logger.js"; import { Semaphore } from "./utils/semaphore.js"; +import { parseFps } from "@hyperframes/core"; // --------------------------------------------------------------------------- // Types @@ -68,7 +69,7 @@ export interface ServerOptions extends HandlerOptions { interface RenderInput { projectDir: string; outputPath?: string | null; - fps: 24 | 30 | 60; + fps: import("@hyperframes/core").Fps; quality: "draft" | "standard" | "high"; format?: "mp4" | "webm" | "mov"; workers?: number; @@ -83,7 +84,14 @@ interface PreparedRenderInput { } function parseRenderOptions(body: Record): Omit { - const fps = ([24, 30, 60].includes(body.fps as number) ? body.fps : 30) as 24 | 30 | 60; + // Accept either a JSON `number` (integer fps) or a JSON `string` (rational + // like "30000/1001"). Falls back to 30 fps on parse failure to preserve the + // forgiving behaviour the original whitelist had — the producer surfaces a + // clearer downstream error if the value is genuinely unusable. + const fpsRaw = body.fps; + const fpsParse = + typeof fpsRaw === "number" || typeof fpsRaw === "string" ? parseFps(fpsRaw) : null; + const fps = fpsParse && fpsParse.ok ? fpsParse.value : ({ num: 30, den: 1 } as const); const quality = ( ["draft", "standard", "high"].includes(body.quality as string) ? body.quality : "high" ) as "draft" | "standard" | "high"; diff --git a/packages/producer/src/services/renderOrchestrator.ts b/packages/producer/src/services/renderOrchestrator.ts index c7026c55d..d535eb2a9 100644 --- a/packages/producer/src/services/renderOrchestrator.ts +++ b/packages/producer/src/services/renderOrchestrator.ts @@ -29,7 +29,13 @@ import { symlinkSync, } from "fs"; import { parseHTML } from "linkedom"; -import { CANVAS_DIMENSIONS, type CanvasResolution } from "@hyperframes/core"; +import { + CANVAS_DIMENSIONS, + type CanvasResolution, + type Fps, + fpsToNumber, + fpsToFfmpegArg, +} from "@hyperframes/core"; import { type EngineConfig, resolveConfig, @@ -214,7 +220,17 @@ export type RenderStatus = | "cancelled"; export interface RenderConfig { - fps: 24 | 30 | 60; + /** + * Frame rate as an exact rational. Integer fps is `{ num: 30, den: 1 }`; + * NTSC is `{ num: 30000, den: 1001 }`. This shape lets the orchestrator + * pass the exact rational through to FFmpeg's `-r` / `-framerate` flags + * without a decimal round-trip — see `fpsToFfmpegArg` in @hyperframes/core. + * + * Use `fpsToNumber(config.fps)` at any site that needs a `number` for + * arithmetic (frame-index → time, telemetry, frame-interval ms). Decimal + * precision at our scales is more than sufficient. + */ + fps: Fps; quality: "draft" | "standard" | "high"; /** * Output container format. Defaults to `"mp4"`; existing renders are @@ -2345,7 +2361,7 @@ export async function executeRenderJob( perfStages.browserProbeMs = Date.now() - probeStart; job.duration = composition.duration; - job.totalFrames = Math.ceil(composition.duration * job.config.fps); + job.totalFrames = Math.ceil(composition.duration * fpsToNumber(job.config.fps)); const totalFrames = job.totalFrames; if (job.duration <= 0) { @@ -2487,7 +2503,14 @@ export async function executeRenderJob( extractionResult = await extractAllVideoFrames( composition.videos, projectDir, - { fps: job.config.fps, outputDir: join(compiledDir, "__hyperframes_video_frames") }, + // extractAllVideoFrames takes fps as a number (decimal). Frames sampled + // from a video at 29.97 vs 30 differ by ~1 frame in 1000 — not enough + // to break visual parity, and the encoder-side rational keeps the + // output framerate exact. + { + fps: fpsToNumber(job.config.fps), + outputDir: join(compiledDir, "__hyperframes_video_frames"), + }, abortSignal, { extractCacheDir: cfg.extractCacheDir }, compiledDir, @@ -2697,7 +2720,7 @@ export async function executeRenderJob( captureCalibration = await measureCaptureCostFromSession( calibrationSession, totalFrames, - job.config.fps, + fpsToNumber(job.config.fps), ); logCaptureCalibrationResult(captureCalibration, log); } catch (error) { @@ -2741,7 +2764,7 @@ export async function executeRenderJob( captureCalibration = await measureCaptureCostFromSession( calibrationSession, totalFrames, - job.config.fps, + fpsToNumber(job.config.fps), ); logCaptureCalibrationResult(captureCalibration, log); } catch (fallbackError) { @@ -2967,10 +2990,11 @@ export async function executeRenderJob( return map; }); + const fpsDecimal = fpsToNumber(job.config.fps); const transitionRanges: TransitionRange[] = transitionMeta.map((t) => ({ ...t, - startFrame: Math.floor(t.time * job.config.fps), - endFrame: Math.ceil((t.time + t.duration) * job.config.fps), + startFrame: Math.floor(t.time * fpsDecimal), + endFrame: Math.ceil((t.time + t.duration) * fpsDecimal), })); if (transitionRanges.length > 0) { @@ -3118,7 +3142,8 @@ export async function executeRenderJob( "-t", String(duration), "-r", - String(job.config.fps), + // Pass the rational form to FFmpeg so NTSC stays exact end-to-end. + fpsToFfmpegArg(job.config.fps), "-vf", `scale=${dims.width}:${dims.height}:force_original_aspect_ratio=increase,crop=${dims.width}:${dims.height}`, "-pix_fmt", @@ -3267,7 +3292,7 @@ export async function executeRenderJob( beforeCaptureHook, width, height, - fps: job.config.fps, + fps: fpsToNumber(job.config.fps), compositeTransfer, nativeHdrImageIds, hdrImageBuffers, @@ -3295,7 +3320,7 @@ export async function executeRenderJob( for (let i = 0; i < totalFrames; i++) { assertNotAborted(); - const time = i / job.config.fps; + const time = (i * job.config.fps.den) / job.config.fps.num; if (hdrPerf) hdrPerf.frames += 1; // Seek timeline @@ -3420,7 +3445,7 @@ export async function executeRenderJob( sceneBuf as Buffer, el, time, - job.config.fps, + fpsToNumber(job.config.fps), hdrVideoFrameSources, hdrVideoStartTimes, width, @@ -3733,7 +3758,7 @@ export async function executeRenderJob( for (let i = 0; i < totalFrames; i++) { assertNotAborted(); - const time = i / job.config.fps; + const time = (i * job.config.fps.den) / job.config.fps.num; const { buffer } = await captureFrameToBuffer(session, i, time); await reorderBuffer.waitForFrame(i); currentEncoder.writeFrame(buffer); @@ -3841,7 +3866,7 @@ export async function executeRenderJob( for (let i = 0; i < job.totalFrames; i++) { assertNotAborted(); - const time = i / job.config.fps; + const time = (i * job.config.fps.den) / job.config.fps.num; await captureFrame(session, i, time); job.framesRendered = i + 1; @@ -4011,7 +4036,11 @@ export async function executeRenderJob( const perfSummary: RenderPerfSummary = { renderId: job.id, totalElapsedMs: totalElapsed, - fps: job.config.fps, + // RenderPerfSummary surfaces fps as a decimal because it lands in JSON + // payloads (CLI telemetry, regression-harness reports) where a single + // number is friendlier than `{num,den}`. Callers needing the rational + // back can read `job.config.fps`. + fps: fpsToNumber(job.config.fps), quality: job.config.quality, workers: workerCount, chunkedEncode: enableChunkedEncode, diff --git a/packages/producer/src/transparency-test.ts b/packages/producer/src/transparency-test.ts index 4b2d8a4d1..0e0031461 100644 --- a/packages/producer/src/transparency-test.ts +++ b/packages/producer/src/transparency-test.ts @@ -31,7 +31,7 @@ const FIXTURE_SRC = join(FIXTURE_DIR, "src"); const WIDTH = 200; const HEIGHT = 200; -const FPS = 30; +const FPS: import("@hyperframes/core").Fps = { num: 30, den: 1 }; const TRANSPARENT_X = 10; // expected fully transparent const TRANSPARENT_Y = 10; const OPAQUE_X = 100; // inside the 50–150 red card diff --git a/packages/studio/vite.config.ts b/packages/studio/vite.config.ts index 13296ebab..4ceb8764d 100644 --- a/packages/studio/vite.config.ts +++ b/packages/studio/vite.config.ts @@ -74,7 +74,7 @@ function createViteAdapter(dataDir: string, server: ViteDevServer): StudioApiAda | null = null; let _producerModulePromise: Promise<{ createRenderJob: (config: { - fps: 24 | 30 | 60; + fps: import("@hyperframes/core").Fps; quality: "draft" | "standard" | "high"; format: string; outputResolution?: "landscape" | "portrait" | "landscape-4k" | "portrait-4k"; @@ -242,7 +242,10 @@ function createViteAdapter(dataDir: string, server: ViteDevServer): StudioApiAda } const { createRenderJob, executeRenderJob } = await getProducerModule(); const job = createRenderJob({ - fps: opts.fps as 24 | 30 | 60, + // opts.fps is already an Fps rational — the studio-api route + // normalized any wire-format `number | string` into the structured + // form before calling this adapter. + fps: opts.fps, quality: opts.quality as "draft" | "standard" | "high", format: opts.format, outputResolution: opts.outputResolution,