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

[TheodorKleynhans]

What

Replace the rigid --fps 24|30|60 whitelist with a numeric range and add support for ffmpeg-style fractional framerates so NTSC stays exact end-to-end.

• --fps 30 keeps working (integer fps, no behaviour change)
• --fps 30000/1001 now means exact NTSC 29.97 (not the lossy decimal)
• --fps 24000/1001, --fps 60000/1001, --fps 25, --fps 50, --fps 120, --fps 240 all work
• Decimals like --fps 29.97 are rejected with an error pointing at the rational form (since 29.97 and 30000/1001 round to different framerates inside ffmpeg)
• Validation widened to a sane numeric range (1-240)

Why

HyperFrames currently treats --fps N as exact integer N. Frame intervals are 1000 / fps ms and ffmpeg gets -framerate <int> / -r <int>. That means there is no way to render content destined for an NTSC delivery target — 30 is exactly 30.000 fps, but every NTSC pipeline downstream is at 29.97 (30000/1001), and the 0.1% drift accumulates into A/V sync errors over long renders. The integer-only assumption also blocks PAL (25/50) and high-frame-rate slow-mo workflows (120/240) that are otherwise in HF's wheelhouse.

ffmpeg natively accepts -framerate 30000/1001, so the right fix is to carry an exact rational through the pipeline rather than collapse to a decimal.

How

Introduces Fps = { num: number; den: number } as the canonical internal representation, carried end-to-end through RenderConfig, EncoderOptions, StreamingEncoderOptions, CaptureOptions, DockerRenderOptions, the Studio API request body, and the regression-harness meta.json validator. The encoders emit the rational form verbatim to ffmpeg (no decimal round-trip).

New helpers in @hyperframes/core:

• parseFps(input: string | number): FpsParseResult — discriminated parser shared by the CLI and Studio API. Returns { ok: true, value: Fps } or { ok: false, error: string }.
• fpsToFfmpegArg(fps: Fps): string — emits "30" for integer fps, "30000/1001" for rational.
• fpsToNumber(fps: Fps): number — for arithmetic (telemetry, frame-count → time conversions).

Frame-interval math uses 1000 * den / num ms (33.333… for integer 30, 33.366… for NTSC 30000/1001).

Studio API wire format accepts polymorphic fps: number | string:

• number → integer fps (30)
• string → rational ("30000/1001")
• Decimal numbers (29.97) and ambiguous strings ("29.97") rejected with the same rule as the CLI.

Backward compatibility: existing meta.json fixtures with integer "fps": 30 continue to load unchanged — the regression-harness validator and Studio API both normalize the wire-format number | string into the structured Fps via parseFps before the rest of the pipeline sees it.

Decisions worth flagging

1. Whitelist → range. The old VALID_FPS = {24, 30, 60} Set is gone; replaced with 1 ≤ fpsToNumber(fps) ≤ 240 plus den > 0. Repo style for quality and format is small finite whitelists, but those are categorical; fps is continuous and a range is more honest.
2. Reject decimal strings. "29.97" is not 30000/1001 — it's 2997/100, which ffmpeg honours, and the resulting drift compounds over a long render. Rather than silently round, the parser tells the caller to be explicit.
3. Vestigial EngineConfig.fps left as 24 | 30 | 60. That field in engine/src/config.ts is a default never read by the orchestrator (which carries RenderConfig.fps directly). Widening it would have churned config.test.ts for zero behavioural gain. Worth a follow-up cleanup but out of scope here.

Test plan

• Unit tests added/updated

  • packages/core/src/parseFps.test.ts (new) — 27 cases covering integer, rational, decimal-rejection, division-by-zero, whitespace, range bounds
  • packages/core/src/studio-api/routes/render.test.ts — 4 new cases for the number | string wire format
  • packages/cli/src/utils/dockerRunArgs.test.ts — 2 new cases for rational round-trip through Docker arg serialization
  • packages/engine/src/services/chunkEncoder.test.ts — rational -framerate/-r emission
  • packages/engine/src/services/streamingEncoder.test.ts — same, plus 4 wire-format cases

• Existing unit tests pass

  • @hyperframes/core 788/788
  • @hyperframes/engine 553/553
  • hyperframes (CLI) 294/294

• Producer regression-harness — green on CI: regression-shards (fast / hdr / render-compat / styles-a through styles-g, ~12-23 min each), plus regression, preview-regression, player-perf, and the targeted Perf: fps / Perf: drift / Perf: parity / Perf: scrub jobs.

• Manual end-to-end render with --fps 30000/1001 and ffprobe verification of r_frame_rate=30000/1001

  Local Windows + Node 24 build of this branch, 2-second box-translate composition at 1280×720:

  invocation	r_frame_rate	avg_frame_rate	nb_frames	duration
  --fps 30	30/1	30/1	60	2.000000
  --fps 30000/1001	30000/1001	30000/1001	60	2.002000
  --fps 24000/1001	24000/1001	24000/1001	48	2.002000

  ffmpeg muxer preserves the exact rational — no decimal round-trip, no integer fallback. Frame counts and 2.002s duration confirm the rational was honoured by both the capture clock and the encoder.

[jrusso1020]

Verdict

Approve. Clean, well-scoped feature — the Fps = { num, den } rational is threaded consistently from CLI flag through orchestrator, encoders, and the docker-arg builder, and the manual ffprobe verification (r_frame_rate=30000/1001, nb_frames=60, duration=2.002) confirms the rational survives the full pipeline. Test coverage is broad and the rejection of decimal strings is the right call given FFmpeg's 29.97 ≠ 30000/1001 behaviour.

A handful of small things below — all non-blocking.

Key Concerns

• packages/cli/src/commands/benchmark.ts:168 — JSON wire-format change for results[].fps. The benchmark JSON now writes fps as a string ("30" or "30000/1001") where it was previously a number (30). The inline comment explains why (round-trippability through parseFps), but this is a non-obvious shape change for anything reading tests/perf/benchmark-results.json outside the repo. Meanwhile RenderPerfSummary.fps in renderOrchestrator.ts:4039 stays a decimal number — so the two benchmark surfaces now disagree on whether fps is a string or a number. Worth either calling out as a contract change in the PR description or aligning the two (e.g. both decimal numbers, with a separate fpsExact string field). Not blocking — the CI snapshot/perf jobs the PR description lists are presumably already updated.

• packages/engine/src/services/frameCapture.ts:597 — quantizeTimeToFrame is called with fpsToNumber(options.fps). The decimal NTSC value is 29.97003…, so the quantization on the capture side uses a slightly different number than the rational the encoder receives. For 2-second tests it's invisible (PR's ffprobe output proves it), but if quantizeTimeToFrame is ever the source of a frame-boundary subtle drift, this is the obvious place to look. Consider plumbing the rational down into quantizeTimeToFrame in a follow-up so capture-side and encoder-side use the same exact value.

Test Coverage

• parseFps has 27 cases covering integer / rational / decimal-rejection / division-by-zero / range / whitespace — looks complete.
• Wire-format round-trip is tested at the right seams: studio-api route (render.test.ts), docker arg builder (dockerRunArgs.test.ts), and both encoders (chunkEncoder.test.ts, streamingEncoder.test.ts).
• The manual ffprobe table in the PR description is the right kind of end-to-end check for this class of change — exactly what I'd want.

Minor gaps (non-blocking):

• parseFpsWithDefault is exported but has no direct test. It's covered indirectly through the studio route tests for the undefined and empty-string fallbacks, but a direct unit test would lock in the contract.
• No test for packages/producer/src/server.ts:parseRenderOptions (the producer HTTP server's fps parsing). The studio-api route is well-tested but the producer route isn't.
• No coverage for the regression-harness validateMetadata rejection path when meta.json renderConfig.fps is malformed (the success path is implicitly covered by every regression fixture).

Nits / Future

• parseFps inconsistent error reasons for negative framerates. parseFps("-5/1") returns { reason: "non-positive" } but parseFps("5/-1") returns { reason: "invalid-fraction" }. Both are user-supplied negative framerates — having them resolve to the same non-positive reason would make the error UX more consistent.

• EngineConfig.fps still typed 24 | 30 | 60 (packages/engine/src/config.ts:16). PR description flags this as deliberately out of scope — agreed it'd churn config.test.ts for zero behavioural gain. Worth a follow-up issue/PR so the type story is fully consistent.

• parseFps allows leading +? Number("+30") is 30 but the regex /^-?\d+$/ doesn't match it, so "+30" falls through to not-a-number. That's fine, but inconsistent with the leading-- handling. Probably not worth a test, just noting for completeness.

• fpsToFfmpegArg({ num: 60, den: 2 }) emits "60/2" verbatim rather than simplifying to "30". PR description correctly justifies this for round-trippability; ffmpeg accepts it. No action needed — just making sure that's the intended trade-off.

• EncoderOptions and StreamingEncoderOptions now require the structured Fps shape. Any out-of-tree consumer that builds EncoderOptions literals (custom encoders, internal tooling) will fail to compile until they migrate. Acceptable cost given the rational is the whole point.