Merge pull request #684 from TheodorKleynhans/feat/cli-fps-fraction-syntax

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

Author: jrusso1020

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,

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,

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<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,

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,

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,

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",