feat: make creator media edits render-safe

[miguel-heygen]

What

• Makes trim/splice/reorder/crossfade, punch/zoom/reframe/camera moves, crop/mask handoffs, and placed-audio editing discoverable through the HyperFrames router and creator skills.
• Makes constant data-playback-rate render-safe for final video/audio, preview duration surfaces, and the final compileForRender duration-injection path, including native parseFloat parity and known zero spans at source EOF.
• Adds 14 copyable creator-editing recipes with timeline/source math, matching audio markup, ownership, and limits.
• Corrects Remotion media mappings and regenerates the skills manifest.

Why

Creator capabilities were real but under-routed, and final rendering previously ignored constant playback rate. Review also found divergent playback-start parsing, natural preview windows that did not scale with authored media rate, and temporal recipe examples whose sound did not follow picture edits.

How

• Core owns temporal source trim/splice; keyframes own visual wrapper motion, crop, masks, directional/iris reveals, split-screen, and polygon handoffs.
• Final readers share strict finite playback-start -> media-start -> 0 parsing.
• Shared core readers derive media start and normalized rate once; preview callers and producer compilation use the same remaining-source/rate primitive. Known zero stays zero through cache and compiler injection, while unknown probe duration stays null/Infinity and explicit data-duration remains authoritative.
• WebAudio keeps the composition clock global while source playback/seek/bounds honor authored element rate; source node rate is authored × global.
• Hard cut, trim, split, duplicate, and reorder recipes carry matching audio source-range markup.
• The real FFmpeg-backed natural-duration compiler regression is explicitly classified as integration, keeping the host-agnostic producer unit lane independent of FFmpeg installation.
• Final video/audio parsers share an explicit-window boundary: compiled or authored zero/non-positive windows are inactive and dropped before extraction/probe/prep, while absent timing remains natural, positive out-of-range sources still diagnose, and video held tails remain intact.
• Speed ramps and arbitrary mid-source freeze remain explicitly unsupported.

Test plan

• Unit tests added/updated

• Manual A/V proof performed

• Documentation updated

• focused compiler/cache: real compileForRender 4/4 (30 assertions), core compiler/cache 126/126; prior compiler/runtime/WebAudio aggregate 306/306

• parsed creator capability and recipe A/V contract: 13/13

• runtime CI: 45 files / 927 tests

• core: 118 files / 2,368 tests

• engine: 63 files / 1,555 passed, 3 skipped

• CLI: 187 files / 2,686 passed, 3 skipped; producer unit passed

• producer classification: unit Bun 40 / Vitest 39; integration Bun 10 / Vitest 7; unit passes without FFmpeg and integration explicitly runs the compiler regression 4/4

• downstream zero-window engine regression: 6 new real video/audio cases plus nonzero EOF/held-tail preservation; focused 8/8, full engine 1,561 passed / 3 skipped

• real duration-less 10s MP4 at 2x: compiled and rendered to 5.000s with red/green/blue/yellow/magenta source segments; output SHA 159956e6658a2044253d97e7281f760685f5d250185a13142dc7aa08059b0d90

• explicit timecoded 2x A/V proof: 2.000s output, red/green/blue/yellow aligned with 442.5/660/880/1100 Hz; output SHA ac69400261decc18bf6ef7c88a9a51f178345910d8d33aa8680f2371a0058d97

• full pre-commit lint, format, manifest, fallow, typecheck, and hooks passed

[jrusso1020]

Reviewed at f763e6158f6 (full files, not just the diff). The retiming math is right in the places it is easy to get wrong, and the parity test is the strongest part of the PR. One finding I would want addressed before this lands, two nits, and a note on why I have not stamped.

1. The two halves read data-playback-start differently, and they disagree on the empty string

This is the one that matters, because the disagreement is between exactly the two readers that have to agree for audio and video to stay in sync.

// audioMixer.ts:475  — truthiness on the raw string
mediaStart: playbackStartAttr ? parseFloat(playbackStartAttr)
          : mediaStartAttr    ? parseFloat(mediaStartAttr)
          : 0,

// videoFrameExtractor.ts:547 — ?? on the raw string, which keeps ""
const mediaStartAttr =
  el.getAttribute("data-playback-start") ?? el.getAttribute("data-media-start");

getAttribute returns "" — not null — for a valueless attribute, so ?? does not fall through where the truthiness chain does. I ran both expressions against the same inputs rather than reasoning about it:

attributes	audio mediaStart	video mediaStart
data-media-start="5" (no playback-start)	5	5
data-playback-start="" data-media-start="5"	5	0
data-playback-start="0" data-media-start="5"	0	0
data-playback-start="2" data-media-start="5"	2	2
data-playback-start=" " data-media-start="5"	NaN	NaN

One divergent row, and its consequence is the full trim offset as A/V desync: the audio track starts 5s into the source while the frames start at 0. On a PR whose subject is render-safety for creator edits, that is the failure mode the change exists to prevent.

Two things keep me from calling it a blocker, and I want to be fair about both.

First, you did not introduce this inconsistency — it is inherited. The runtime already disagrees with itself on "": timeline.ts:19-23's parseNum explicitly returns null for "" (so its ?? chain falls through), while clipTree.ts:53-57's parseNum does not, and Number("") === 0, so it yields 0; init.ts:751 coalesces raw strings and also lands on 0. Your audio side matches timeline.ts and your video side matches clipTree.ts. Both have precedent.

Second, no tooling can emit an empty value — every writer stamps String(number) (compositionInsertion.ts:209, sourceMutation.ts:389-392, timelineEditingHelpers.ts), so this is reachable only from hand- or agent-authored HTML. That is a first-class authoring path here, and data-playback-start="" is a natural way for a generator to spell "unset", but it is not something the editor produces.

What makes it worth fixing here anyway is that this PR is where the consequence becomes A/V desync. Before it, both engine halves read only data-media-start through the same truthiness expression, so they could not disagree. Adding a second attribute name with different empty-string handling on each side is what turns an inherited cosmetic inconsistency into a sync bug.

The fix is also the PR's own thesis applied one level down — make the engine agree with the runtime by construction:

const mediaStart =
  parseNum(el.getAttribute("data-playback-start")) ??
  parseNum(el.getAttribute("data-media-start")) ??
  0;

using timeline.ts's parseNum semantics (reject null and "") in both files. Parse first, then coalesce on parse failure; coalescing raw strings is what admits "" as a value. That also fixes the " " row for free, which currently yields NaN on both sides and flows into -ss NaN — resolveVideoExtractionWindow's range guard cannot catch it, because every comparison against NaN is false.

Worth noting the new reader is also the one piece of this change with no test: the added cases cover data-playback-rate parsing on both sides, and nothing exercises data-playback-start at all.

2. Nit — the duplicated predicate in resolveVideoExtractionWindow is now load-bearing and unmarked

const resolvedDuration =
  Number.isFinite(requestedTimelineDuration) && requestedTimelineDuration > 0
    ? requestedTimelineDuration
    : resolveSegmentDuration(requestedTimelineDuration, video.mediaStart, playableDuration) /
      playbackRate;

resolveSegmentDuration opens with that same Number.isFinite(requested) && requested > 0 test and returns requested unchanged, so the inline branch is a deliberate duplicate: it exists so / playbackRate applies only to the source-derived fallback, an authored timeline duration already being in timeline units. That is correct, and it is also why the obvious cleanup is wrong — collapsing it to resolveSegmentDuration(...) / playbackRate would divide an explicit authored duration by the rate. One comment saying so would keep the next reader from tidying it into a bug. (I confirmed the rate-1 path is arithmetically unchanged from main, so this restructure carries no blast radius for existing compositions.)

3. Nit — the degenerate-duration fallback is no longer rate-aware

const effectiveDuration =
  (metadata.durationSeconds - element.mediaStart) / normalizePlaybackRate(element.playbackRate ?? 1);
element.end =
  element.start + (effectiveDuration > 0 ? effectiveDuration : metadata.durationSeconds);

The primary term is converted to timeline seconds; the metadata.durationSeconds fallback is left in source seconds. Only reachable when mediaStart >= durationSeconds, and the fallback was already there — but the two branches now carry different units, so a retimed clip in that state gets an end playbackRate× off. metadata.durationSeconds / rate would keep them consistent.

Checked and fine — recorded so nobody re-checks

• The atempo chain is correct at both clamp bounds. normalizePlaybackRate bounds input to [0.1, 5] and maps non-finite/non-positive to 1, which is what makes the Number.NaN sentinel for a missing attribute work and what bounds the two while loops. Worked the arithmetic at the extremes: 0.1 → [0.5, 0.5, 0.5, 0.8] and 5 → [2, 2, 1.25], both exact, and atempo is pitch-preserving as the docstring claims.
• The filter merge avoids a real footgun. preparedAudioOutputArgs folds atempo into the existing channel filter string and emits a single -af, rather than appending a second -af — which ffmpeg would silently resolve by dropping the first, taking the stereo normalization with it.
• Moving -ss/-t ahead of -i in extractAudioFromVideo is an optimization, not an accuracy regression. Input-side seek is fast, and -accurate_seek is on by default, so the start stays sample-accurate rather than snapping to a packet boundary. The input -t duration * rate paired with the output -t duration when rate ≠ 1 is right in both directions — checked 2× (read 2D source, compress to D) and 0.5× (read 0.5D, stretch to D).
• The held-tail branch gets the unit conversion right, which is the easiest line in the diff to get wrong: compositionStart: video.start + extractionOffset / playbackRate (timeline seconds) alongside mediaStart: video.mediaStart + extractionOffset (source seconds).
• getFrameIndexAtTime's unit chain is consistent — loopDuration divided into timeline seconds for the modulo, and the index taken from localTime * rate * fps.
• Automation × retiming is covered, which was my first guess at a gap: keeps automation on authored timeline time after constant retiming pins that a volume lane authored in timeline time stays in timeline time through a 2× retime.
• The parity test is the right construction for the claim it makes. Four colour bands crossed with four tones (440/660/880/1100 Hz), sampled at 0.25/0.75/1.25/1.75s, asserting both the dominant colour and the detected frequency at each point, plus output duration in [1.95, 2.05]. A test asserting only duration, or only frames, could not tell "both streams retimed" from "one retimed" or from "both retimed but mutually offset" — this one fails on any of those. Worth keeping as the template for the rest of the stack.

Why I have not approved

Nothing above is a merge blocker on its own, but Tests on windows-latest is still pending at this head and it is one of the eight required contexts on main — I do not stamp over a required check that has not reported. Non-required Perf: parity is also still running. The other seven required contexts are green, including Render on windows-latest and regression.

Once Windows reports and §1 is settled either way, ping me and I will re-check — this repo requires approval of the last push, so an approval posted now would not survive the fix commit anyway.

Note: /code-review max can't be invoked from my side, so this is that lens applied by hand rather than a lighter review silently substituted.

— Rames Jusso