-
Notifications
You must be signed in to change notification settings - Fork 3.5k
feat(producer): renderStretch to re-time short compositions across longer scenes #2676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import { describe, it, expect } from "vitest"; | ||
| import { outputFrameToTimelineSeconds } from "./core.types.js"; | ||
|
|
||
| describe("outputFrameToTimelineSeconds", () => { | ||
| const fps = { num: 30, den: 1 }; | ||
|
|
||
| it("no-op when renderStretch is omitted (defaults to 1)", () => { | ||
| for (const i of [0, 1, 29, 143]) { | ||
| expect(outputFrameToTimelineSeconds(i, fps)).toBe((i * fps.den) / fps.num); | ||
| } | ||
| }); | ||
|
|
||
| it("renderStretch=1 is byte-identical to the raw frame time", () => { | ||
| expect(outputFrameToTimelineSeconds(143, fps, 1)).toBe(143 / 30); | ||
| }); | ||
|
|
||
| it("stretches a 1s comp across a 4.8s output (renderStretch = intrinsic/target)", () => { | ||
| const rs = 1 / 4.8; | ||
| expect(outputFrameToTimelineSeconds(0, fps, rs)).toBe(0); | ||
| // last of 144 output frames lands just under intrinsic 1.0s — never past it | ||
| const last = outputFrameToTimelineSeconds(143, fps, rs); | ||
| expect(last).toBeGreaterThan(0.98); | ||
| expect(last).toBeLessThan(1.0); | ||
| }); | ||
|
|
||
| it("honors an exact rational fps (NTSC 30000/1001)", () => { | ||
| const ntsc = { num: 30000, den: 1001 }; | ||
| expect(outputFrameToTimelineSeconds(30, ntsc, 1)).toBe((30 * 1001) / 30000); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,6 +114,7 @@ export interface CaptureOptions { | |
| * timelines can outrun their declared duration). Consumers that derive | ||
| * frame indices meant to be drained by the producer (drawElement | ||
| * self-verification) MUST prefer this over `__hf.duration`. | ||
| * When renderStretch != 1 this is the OUTPUT (stretched) duration, not intrinsic. | ||
| */ | ||
| compositionDurationSeconds?: number; | ||
| /** | ||
|
|
@@ -123,6 +124,7 @@ export interface CaptureOptions { | |
| * rational form verbatim — see `fpsToFfmpegArg`. | ||
| */ | ||
| fps: Fps; | ||
| renderStretch?: number; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 Concern — field name and semantic have drifted. Post-PR, |
||
| format?: "jpeg" | "png"; | ||
| quality?: number; | ||
| deviceScaleFactor?: number; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -187,6 +187,18 @@ export function validateDistributedRenderConfig( | |
| } | ||
| } | ||
|
|
||
| if ( | ||
| config.renderStretch !== undefined && | ||
| (typeof config.renderStretch !== "number" || | ||
| !Number.isFinite(config.renderStretch) || | ||
| config.renderStretch <= 0) | ||
| ) { | ||
| throw new InvalidConfigError( | ||
| "config.renderStretch", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ Question — any upper bound needed on |
||
| `must be a positive number; got ${String(config.renderStretch)}`, | ||
| ); | ||
| } | ||
|
|
||
| if (config.runtimeCap !== undefined && !ALLOWED_RUNTIME_CAPS.includes(config.runtimeCap)) { | ||
| throw new InvalidConfigError( | ||
| "config.runtimeCap", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Nit — zero unit tests for
outputFrameToTimelineSeconds, the heart of the change. The PR body notesplanHash + renderRequest unit suites: pass— those exercise the plan-hash byte-identity claim and request-validation guards, but not the seek arithmetic itself. This helper is called from 8+ capture sites and its correctness underrenderStretch != 1is what the runtime verification will empirically confirm. A handful of unit tests would lock the math in place BEFORE the runtime pass finds a subtle IEEE-754 issue at rational fps ({num: 30000, den: 1001}×renderStretch = 0.2083, etc.): boundary frame (i=0), last output frame lands at ≈ intrinsic, no-op case (renderStretch=1returns exactly(i * den) / num). Cheap insurance, fits the PR's adversarial-self-review posture. — Rames D Jusso