Skip to content

Commit f2b62a6

Browse files
miguel-heygenmiguel-heygen
committed
fix(core): preview seeks land on the composition's own frame rate
1 parent 36602a8 commit f2b62a6

6 files changed

Lines changed: 138 additions & 135 deletions

File tree

‎packages/core/src/core.types.ts‎

Lines changed: 2 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -2,139 +2,8 @@
22

33
export type ExecutionMode = "planning" | "design" | "execution" | null;
44

5-
// ── Frame rate ──────────────────────────────────────────────────────────────
6-
7-
/**
8-
* Frame rate as an exact rational. Carrying `{num, den}` end-to-end (rather
9-
* than collapsing to `29.97`) lets us pass NTSC / drop-frame rates straight
10-
* through to FFmpeg via `-r 30000/1001` without any decimal round-trip.
11-
*
12-
* Integer fps is represented with `den: 1` (e.g. `{ num: 30, den: 1 }`).
13-
*
14-
* Use {@link fpsToNumber} when arithmetic forces a decimal (e.g. `setTimeout`
15-
* intervals) and {@link fpsToFfmpegArg} when emitting FFmpeg `-r` /
16-
* `-framerate` strings.
17-
*/
18-
export interface Fps {
19-
num: number;
20-
den: number;
21-
}
22-
23-
export type FpsInput = number | Fps;
24-
25-
export function toFps(input: FpsInput): Fps {
26-
if (typeof input === "number") {
27-
return { num: input, den: 1 };
28-
}
29-
return input;
30-
}
31-
32-
/**
33-
* Decimal value of an {@link Fps} rational. Used at sites that need a
34-
* `number` for arithmetic (frame-index → time, frame intervals, telemetry
35-
* payloads) where the small precision loss of the decimal is acceptable.
36-
*/
37-
export function fpsToNumber(fps: Fps): number {
38-
return fps.num / fps.den;
39-
}
40-
41-
/**
42-
* FFmpeg-style fps argument. Returns `"30"` for integer fps and `"30000/1001"`
43-
* for rationals — both forms are accepted verbatim by FFmpeg's `-r` and
44-
* `-framerate` flags. We keep integer fps as a bare integer so existing
45-
* snapshot tests / log output don't churn for the common case.
46-
*/
47-
export function fpsToFfmpegArg(fps: Fps): string {
48-
return fps.den === 1 ? String(fps.num) : `${fps.num}/${fps.den}`;
49-
}
50-
51-
/**
52-
* Discriminated parse result for {@link parseFps}. Lets the CLI / route
53-
* validation own its own error UX without losing the structured failure
54-
* reason.
55-
*/
56-
export type FpsParseResult =
57-
| { ok: true; value: Fps }
58-
| {
59-
ok: false;
60-
reason:
61-
| "empty"
62-
| "not-a-number"
63-
| "non-positive"
64-
| "out-of-range"
65-
| "invalid-fraction"
66-
| "ambiguous-decimal";
67-
};
68-
69-
/**
70-
* Parse a user-supplied fps spec into an {@link Fps} rational.
71-
*
72-
* Accepted forms:
73-
* - integer string `"30"` → `{ num: 30, den: 1 }`
74-
* - integer number `30` → `{ num: 30, den: 1 }`
75-
* - rational string `"30000/1001"` → `{ num: 30000, den: 1001 }` (exact NTSC)
76-
*
77-
* Rejected:
78-
* - empty / non-numeric input
79-
* - decimals like `"29.97"` — callers must spell rationals with `/` so the
80-
* exact denominator is unambiguous (FFmpeg treats `29.97` as a slightly
81-
* different framerate than `30000/1001`).
82-
* - division by zero, negative or zero numerator
83-
* - decimal value outside `[1, 240]` — defensive bounds for "human" fps
84-
* ranges (24, 25, 30, 50, 60, 120, 240, plus the NTSC trio).
85-
*/
86-
export function parseFps(input: string | number): FpsParseResult {
87-
if (typeof input === "number") {
88-
if (!Number.isFinite(input)) return { ok: false, reason: "not-a-number" };
89-
if (!Number.isInteger(input)) return { ok: false, reason: "ambiguous-decimal" };
90-
if (input <= 0) return { ok: false, reason: "non-positive" };
91-
if (input > 240) return { ok: false, reason: "out-of-range" };
92-
return { ok: true, value: { num: input, den: 1 } };
93-
}
94-
const raw = input.trim();
95-
if (raw === "") return { ok: false, reason: "empty" };
96-
97-
if (raw.includes("/")) {
98-
const parts = raw.split("/");
99-
if (parts.length !== 2) return { ok: false, reason: "invalid-fraction" };
100-
const num = Number(parts[0]);
101-
const den = Number(parts[1]);
102-
if (!Number.isFinite(num) || !Number.isFinite(den)) {
103-
return { ok: false, reason: "not-a-number" };
104-
}
105-
if (!Number.isInteger(num) || !Number.isInteger(den)) {
106-
return { ok: false, reason: "invalid-fraction" };
107-
}
108-
if (den <= 0) return { ok: false, reason: "invalid-fraction" };
109-
if (num <= 0) return { ok: false, reason: "non-positive" };
110-
const decimal = num / den;
111-
if (decimal < 1 || decimal > 240) return { ok: false, reason: "out-of-range" };
112-
return { ok: true, value: { num, den } };
113-
}
114-
115-
// Integer-only path — reject `"29.97"` so users are explicit about the
116-
// exact rational they want.
117-
if (!/^-?\d+$/.test(raw)) {
118-
// Allow caller to differentiate "29.97" from "abc" if they want; both
119-
// are user errors but the message can be friendlier for decimals.
120-
if (/^-?\d*\.\d+$/.test(raw)) return { ok: false, reason: "ambiguous-decimal" };
121-
return { ok: false, reason: "not-a-number" };
122-
}
123-
const n = Number(raw);
124-
if (n <= 0) return { ok: false, reason: "non-positive" };
125-
if (n > 240) return { ok: false, reason: "out-of-range" };
126-
return { ok: true, value: { num: n, den: 1 } };
127-
}
128-
129-
/**
130-
* Convenience wrapper around {@link parseFps} for callsites that want the
131-
* default-30-fps fallback when input is `undefined`. Does NOT swallow parse
132-
* errors — those still surface via the discriminated result.
133-
*/
134-
export function parseFpsWithDefault(input: string | number | undefined): FpsParseResult {
135-
if (input === undefined || input === "") return { ok: true, value: { num: 30, den: 1 } };
136-
return parseFps(input);
137-
}
5+
export type { Fps, FpsInput, FpsParseResult } from "./fps";
6+
export { toFps, fpsToNumber, fpsToFfmpegArg, parseFps, parseFpsWithDefault } from "./fps";
1387

1398
/** Video orientation / aspect ratio. */
1409
export type Orientation = "16:9" | "9:16";

‎packages/core/src/fps.ts‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Exact rational, so NTSC rates reach FFmpeg as `30000/1001` with no decimal round trip.
2+
export interface Fps {
3+
num: number;
4+
den: number;
5+
}
6+
7+
export type FpsInput = number | Fps;
8+
9+
export function toFps(input: FpsInput): Fps {
10+
if (typeof input === "number") {
11+
return { num: input, den: 1 };
12+
}
13+
return input;
14+
}
15+
16+
export function fpsToNumber(fps: Fps): number {
17+
return fps.num / fps.den;
18+
}
19+
20+
export function fpsToFfmpegArg(fps: Fps): string {
21+
return fps.den === 1 ? String(fps.num) : `${fps.num}/${fps.den}`;
22+
}
23+
24+
export type FpsParseResult =
25+
| { ok: true; value: Fps }
26+
| {
27+
ok: false;
28+
reason:
29+
| "empty"
30+
| "not-a-number"
31+
| "non-positive"
32+
| "out-of-range"
33+
| "invalid-fraction"
34+
| "ambiguous-decimal";
35+
};
36+
37+
// Decimals like "29.97" are rejected: FFmpeg reads them as a different rate than 30000/1001.
38+
export function parseFps(input: string | number): FpsParseResult {
39+
if (typeof input === "number") {
40+
if (!Number.isFinite(input)) return { ok: false, reason: "not-a-number" };
41+
if (!Number.isInteger(input)) return { ok: false, reason: "ambiguous-decimal" };
42+
if (input <= 0) return { ok: false, reason: "non-positive" };
43+
if (input > 240) return { ok: false, reason: "out-of-range" };
44+
return { ok: true, value: { num: input, den: 1 } };
45+
}
46+
const raw = input.trim();
47+
if (raw === "") return { ok: false, reason: "empty" };
48+
49+
if (raw.includes("/")) {
50+
const parts = raw.split("/");
51+
if (parts.length !== 2) return { ok: false, reason: "invalid-fraction" };
52+
const num = Number(parts[0]);
53+
const den = Number(parts[1]);
54+
if (!Number.isFinite(num) || !Number.isFinite(den)) {
55+
return { ok: false, reason: "not-a-number" };
56+
}
57+
if (!Number.isInteger(num) || !Number.isInteger(den)) {
58+
return { ok: false, reason: "invalid-fraction" };
59+
}
60+
if (den <= 0) return { ok: false, reason: "invalid-fraction" };
61+
if (num <= 0) return { ok: false, reason: "non-positive" };
62+
const decimal = num / den;
63+
if (decimal < 1 || decimal > 240) return { ok: false, reason: "out-of-range" };
64+
return { ok: true, value: { num, den } };
65+
}
66+
67+
if (!/^-?\d+$/.test(raw)) {
68+
if (/^-?\d*\.\d+$/.test(raw)) return { ok: false, reason: "ambiguous-decimal" };
69+
return { ok: false, reason: "not-a-number" };
70+
}
71+
const n = Number(raw);
72+
if (n <= 0) return { ok: false, reason: "non-positive" };
73+
if (n > 240) return { ok: false, reason: "out-of-range" };
74+
return { ok: true, value: { num: n, den: 1 } };
75+
}
76+
77+
export function parseFpsWithDefault(input: string | number | undefined): FpsParseResult {
78+
if (input === undefined || input === "") return { ok: true, value: { num: 30, den: 1 } };
79+
return parseFps(input);
80+
}

‎packages/core/src/runtime/init.test.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
SKIPPED_CLIP,
1010
} from "./timedClipHide";
1111
import { applyAuthoredTiming, STUDIO_PREVIEW_MARK_META } from "../studioPreviewMark";
12+
import { runtimeProtocolMetadata } from "./protocol";
1213
import { collectRuntimeTimelinePayload } from "./timeline";
1314
import { TYPEGPU_PRESENT_HEARTBEAT_MS } from "./adapters/typegpu";
1415
import { WebAudioTransport } from "./webAudioTransport";
@@ -1432,6 +1433,45 @@ describe("initSandboxRuntimeModular", () => {
14321433
});
14331434
});
14341435

1436+
describe("a preview seek lands on the composition's own frame grid", () => {
1437+
// Frame 7 on a 30 fps grid would be 7/30: earlier than frame 7 at 25 and 29.97 fps.
1438+
const rates = [
1439+
{ dataFps: "25", fps: 25 },
1440+
{ dataFps: "30000/1001", fps: 30_000 / 1_001 },
1441+
];
1442+
1443+
function previewTimeline(dataFps: string): RuntimeTimelineLike {
1444+
document.body.innerHTML = `<div data-composition-id="main" data-root="true" data-start="0" data-duration="20" data-fps="${dataFps}" data-width="1920" data-height="1080"></div>`;
1445+
const timeline = createMockTimeline(20);
1446+
window.__timelines = { main: timeline };
1447+
initSandboxRuntimeModular();
1448+
return timeline;
1449+
}
1450+
1451+
it.each(rates)("player.seek to frame 7 at $dataFps fps", ({ dataFps, fps }) => {
1452+
const timeline = previewTimeline(dataFps);
1453+
window.__player?.seek(7 / fps);
1454+
expect(timeline.time()).toBeCloseTo(7 / fps, 9);
1455+
});
1456+
1457+
it.each(rates)("a parent frame seek to frame 7 at $dataFps fps", ({ dataFps, fps }) => {
1458+
const timeline = previewTimeline(dataFps);
1459+
window.dispatchEvent(
1460+
new MessageEvent("message", {
1461+
source: window.parent,
1462+
data: {
1463+
source: "hf-parent",
1464+
type: "control",
1465+
action: "seek",
1466+
frame: 7,
1467+
...runtimeProtocolMetadata(fps),
1468+
},
1469+
}),
1470+
);
1471+
expect(timeline.time()).toBeCloseTo(7 / fps, 9);
1472+
});
1473+
});
1474+
14351475
it("uses live child timeline duration when a composition host has no authored duration", () => {
14361476
const root = document.createElement("div");
14371477
root.setAttribute("data-composition-id", "main");

‎packages/core/src/runtime/init.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ import type {
191191
SceneAnimation,
192192
} from "./types";
193193
import type { PlayerAPI } from "../core.types";
194+
import { parseFps } from "../fps";
194195
import { recordStartupError, SCENE_CALLBACK_ERROR_PREFIX, swallow } from "./diagnostics";
195196
import { readTimelineDurationSeconds } from "./timelineDuration";
196197
import {
@@ -289,6 +290,11 @@ function resolveExportRenderFps(): ExportRenderFpsResolution {
289290
};
290291
}
291292

293+
function compositionFps(): number | null {
294+
const declared = parseFps(findRootCompositionElement()?.getAttribute("data-fps") ?? "");
295+
return declared.ok ? declared.value.num / declared.value.den : null;
296+
}
297+
292298
function samePromiseSet(a: PromiseLike<unknown>[], b: PromiseLike<unknown>[] | null): boolean {
293299
return b !== null && a.length === b.length && a.every((p, i) => p === b[i]);
294300
}
@@ -538,7 +544,7 @@ export function initSandboxRuntimeModular(): void {
538544
// captured frame ever sees the group as a layout item.
539545
ensureAudioGroupInertStyle(document);
540546
const exportRenderFps = resolveExportRenderFps();
541-
state.canonicalFps = exportRenderFps.fps ?? state.canonicalFps;
547+
state.canonicalFps = exportRenderFps.fps ?? compositionFps() ?? state.canonicalFps;
542548
setRuntimeProtocolFps(state.canonicalFps);
543549
if (window.__HF_EXPORT_RENDER_SEEK_CONFIG) {
544550
const fpsDetail = JSON.stringify({

‎packages/core/src/runtime/protocol.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ describe("runtime protocol", () => {
1414
},
1515
);
1616

17+
it.each([24_000, 30_000, 60_000])("states %s/1001 fps exactly", (numerator) => {
18+
expect(runtimeProtocolFpsFromNumber(numerator / 1_001)).toEqual({
19+
numerator,
20+
denominator: 1_001,
21+
});
22+
});
23+
1724
it("accepts protocol v1 metadata and exposes its fps", () => {
1825
expect(inspectRuntimeProtocol(runtimeProtocolMetadata(60))).toMatchObject({
1926
status: "supported",

‎packages/core/src/runtime/protocol.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ function greatestCommonDivisor(a: number, b: number): number {
4444

4545
export function runtimeProtocolFpsFromNumber(value: number): RuntimeProtocolFps {
4646
const safe = Number.isFinite(value) && value > 0 ? value : 30;
47-
const denominator = Number.isInteger(safe) ? 1 : 1_000_000;
47+
const isNtscRate = Math.abs(safe * 1001 - Math.round(safe * 1001)) < 1e-6;
48+
const denominator = Number.isInteger(safe) ? 1 : isNtscRate ? 1001 : 1_000_000;
4849
const numerator = Math.round(safe * denominator);
4950
const divisor = greatestCommonDivisor(numerator, denominator);
5051
return { numerator: numerator / divisor, denominator: denominator / divisor };

0 commit comments

Comments
 (0)