feat(cli): accept ffmpeg-style rational fps (NTSC, PAL, slow-mo)

Replaces the rigid `--fps 24|30|60` whitelist with a numeric range and adds support for ffmpeg-style fractional framerates so NTSC stays exact end-to-end. - `--fps 30` keeps working (integer fps) - `--fps 30000/1001` now means exact NTSC 29.97 (not the lossy decimal) - `--fps 24000/1001`, `--fps 60000/1001`, `--fps 25/50/120/240` all work - Decimals like `--fps 29.97` are rejected with a friendly error pointing the user at the rational form, since `29.97` and `30000/1001` round to different framerates inside ffmpeg Carries an `Fps = { num: number; den: number }` rational end-to-end: RenderConfig, EncoderOptions, StreamingEncoderOptions, CaptureOptions, DockerRenderOptions, Studio API request body, regression-harness meta.json. The `-r` and `-framerate` ffmpeg args emit the rational form verbatim (`30000/1001`) so no decimal round-trip happens at the encoder boundary. Frame-interval math uses `1000 * den / num` ms (33.366… for NTSC, 33.333… for integer 30). Helpers live in @hyperframes/core: - `parseFps(input: string | number): FpsParseResult` — discriminated parser used by both the CLI and the Studio API route - `fpsToFfmpegArg(fps: Fps): string` — emits "30" or "30000/1001" - `fpsToNumber(fps: Fps): number` — for arithmetic (telemetry, frame count, frame-index → time) Studio API wire format accepts polymorphic `fps: number | string`: - number → integer fps (`30`) - string → rational (`"30000/1001"`) Decimals are rejected; matches the same rule as the CLI. Existing meta.json fixtures with integer `"fps": 30` continue to load unchanged — the regression-harness validator now normalizes both number and string inputs through `parseFps`.
2026-09-03 12:54:29 +00:00 · 2026-05-09 00:09:15 +02:00
parent b58e447305
commit 5dcc89c930
27 changed files with 725 additions and 91 deletions
@@ -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,
@@ -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,
@@ -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<FpsParseResult, { ok: true }>["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,
@@ -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,
@@ -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,
@@ -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",