Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions packages/cli/src/commands/benchmark.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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({
Expand Down Expand Up @@ -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,
Expand Down
22 changes: 11 additions & 11 deletions packages/cli/src/commands/render.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
63 changes: 52 additions & 11 deletions packages/cli/src/commands/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/server/studioServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
27 changes: 25 additions & 2 deletions packages/cli/src/utils/dockerRunArgs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 10 additions & 2 deletions packages/cli/src/utils/dockerRunArgs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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). */
Expand All @@ -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;
Expand Down Expand Up @@ -54,7 +62,7 @@ export function buildDockerRunArgs(input: DockerRunArgsInput): string[] {
"--output",
`/output/${outputFilename}`,
"--fps",
String(options.fps),
fpsToFfmpegArg(options.fps),
"--quality",
options.quality,
"--format",
Expand Down
Loading
Loading