perf(producer): skip wasted Chrome media work + injector page.evaluates during render

[jrusso1020]

What

Two small render-side improvements for video-heavy compositions:

1. packages/core/src/runtime/media.ts — gate the per-tick el.currentTime = relTime set + the el.load() drift-recovery retry on the absence of a <img id="__render_frame_<id>__"> sibling (i.e., we're in render mode + this video's visual is bypassed by frame injection + its audio is mixed by ffmpeg from source files).

2. packages/engine/src/services/videoFrameInjector.ts — probe window.__hfReseekGpu and window.__hf.colorGrading.redraw once at the first injector call; cache the booleans; skip the per-frame page.evaluate round-trips when neither capability is registered.

Why

media.ts

During render the runtime calls el.currentTime = relTime on every active video per sync tick. For frame-injected videos that's pure waste:

• The visual comes from the <img id="__render_frame_<id>__"> sibling injected by the producer's videoFrameInjector — the <video> element is visibility: hidden.
• Audio is mixed by ffmpeg from the source files in runAudioStage (separate stage) — it never goes through the in-browser audio pipeline during render.

So every per-tick seek just kicks Chrome's media pipeline (buffering checks, range fetches, decoder state changes) for no visible or audible benefit. On a 30 × 32 MB synth comp, that's ~2,400 wasted seeks per render — and the cost wasn't on the JS critical path, so it didn't show up in avgBeforeCapture directly. It bled into the BeginFrame compositor's per-frame screenshot time.

Preview is unaffected: the injection sibling only exists during render. In preview hasInjectionSibling is always false → existing seek path runs unchanged.

videoFrameInjector.ts

The injector hook ran __hfReseekGpu and redrawRuntimeColorGrading via page.evaluate on every render frame. For comps that don't register either capability (the common case — anything without WebGL/WebGPU video sub-comps or a color-grading layer), each was a no-op page-side function preceded by a ~CDP-round-trip-worth of overhead. Probing once and caching false eliminates that for the rest of the render.

How was this validated

Stress shape: synth-30-heavy — 30 × 32 MB MP4 / 3 s each, sequenced end-to-end over a 90 s timeline (data-composition-id root + per-video <video id="vid-NN" data-start data-duration data-track-index>). Host: 8-core / 30 GB Linux.

N=3 baseline against stock origin/main (post-#1630), N=3 with-fix on the same machine, same corpus, fresh worker pool each run. Phase timings via [Render:trace] JSON; per-frame sub-breakdown via a one-line [CapturePerf] stderr emit (kept locally, not in this PR — dedupPerfs already carries the data, this branch surfaces it).

	Baseline N=3	With-fix N=3	Δ
wall mean	119.5 s ± 1.4 s	117.3 s ± 0.9 s	-2.2 s (-1.8%)
avg screenshot / frame	50.0 ms	49.0 ms	-2.0%
avg beforeCapture / frame	13.0 ms	12.1 ms	-7.0%
avg total / frame	66.0 ms	63.9 ms	-3.2%
output md5	5a22be64...	identical ×3	✓

The 1 ms screenshot drop is the load-bearing signal: it confirms the kicked Chrome media-pipeline work was bleeding into BeginFrame compositor time, even though it wasn't on the JS critical path. Per-frame budget improved 2.1 ms × 2700 / 3 workers ≈ 1.9 s of capture_disk savings, which matches the observed wall delta.

This stacks cleanly with #1630 (which removed the injector's fileServer contention). #1630 moved the injector's PNG fetches off the fileServer's hot path; this PR keeps Chrome's media pipeline quiet during render so the BeginFrame compositor runs unhindered.

Test plan

• Local-CLI render on synth-30-heavy × N=3 baseline + N=3 with-fix; wall, per-frame, md5 captured (above).
• Lint / format / typecheck via lefthook pre-commit (oxlint, oxfmt, fallow audit, tsc --noEmit across @hyperframes/core + @hyperframes/engine + @hyperframes/producer).
• Real-world video-heavy comp validation — would love a Magi / Miga eye on a HF-heygen-stripe-shape or a Rahino-shape comp to confirm there's no audible artifact on unmuted videos. The change shouldn't affect them — in render mode the audio path is ffmpeg, not the in-browser pipeline — but a sanity-check render is cheap.

Scope notes

• Not addressed in this PR: the user-facing request for an upfront-extract concurrency cap (Promise.all in extractAllVideoFrames is currently unbounded across all videos). Filing as a follow-up PR — different layer of the pipeline, different user surface (CLI flag), worth keeping separate for review.
• Edge case: in the calibration test-frame phase, the injection sibling may not yet exist when drift recovery first checks a video at the very start of its active window. The gate correctly defaults to "no sibling → run the seek" in that case, which is the existing behavior.

Authored by Jerrai (Rames team).

[jrusso1020]

/code-review verdict — leaning approve after dropping a no-op change I caught in my own review

Recall pass at xhigh effort: 4 finder angles + cross-file tracer + Phase 2 verification. The PR's core claim (skip el.currentTime on frame-injected videos in render mode) is verified safe by a comprehensive consumer-trace; one bundled change was a no-op and is now dropped.

Findings

1. (Dropped from the PR after self-review) videoFrameInjector.ts capability cache was dead code

The first revision of this PR bundled a "capability cache" in videoFrameInjector.ts — probe window.__hfReseekGpu and window.__hf.colorGrading.redraw once at first injector call; gate per-frame page.evaluate round-trips on the cached booleans.

I caught it in self-review:

• window.__hfReseekGpu is assigned unconditionally at packages/core/src/runtime/init.ts:2015 during initRuntime().
• window.__hf.colorGrading is assigned unconditionally at packages/core/src/runtime/colorGrading.ts:1211 from createColorGradingRuntime() (called unconditionally at init.ts:1802).

Both run synchronously during runtime init — before the producer's pollHfReady passes, before the first injector call. So both cached booleans are always true, and the per-frame gates never skip. Dead code on the standard runtime path.

Re-isolated measurement (media-only N=3 vs media-plus-cap-cache N=3) confirmed: removing the cap-cache doesn't change wall or md5. Dropped via amend + force-push. New HEAD: 7e5354801 (was f647c75ef). PR is now 1 file / 36 ins / 9 del — just the media.ts gate.

2. Cross-file trace of <video>.currentTime consumers during render — verified safe

The core claim — skipping el.currentTime for frame-injected videos during render breaks nothing — verified by tracing every potential consumer:

• Audio extraction (packages/producer/src/services/audioExtractor.ts): uses source files + playbackStart / duration metadata, not <video>.currentTime. ✓
• Runtime event listeners on <video> (media.ts:111-114): registered for playing / pause / error only — none key off currentTime advances. ✓
• GSAP timelines: do not animate currentTime as a property. ✓
• Chrome headless audio (browserManager.ts:146-160): no audio output device in headless mode → el.volume updates have no audible effect during render. ✓
• Parity tests (packages/producer/tests/parity/): compare pixel output, not seek state. ✓
• Volume envelope tracking: reads el.volume not currentTime. ✓

3. (Efficiency, follow-up) Per-tick document.getElementById for every active video

media.ts:317 calls document.getElementById('__render_frame_' + el.id + '__') per sync tick per video. For a 30-video / 90 s comp that's ~80,000 DOM queries. Each is O(1) but compounds. A per-element WeakMap<HTMLVideoElement, boolean> cache would replace the per-tick lookup with a O(1) map check after the first hit. Not a correctness bug; deferring to a follow-up.

4. (Tests, follow-up) No unit-test coverage of the new gate

media.test.ts exercises the drift-recovery paths but no test asserts that el.currentTime is NOT set when the gate fires. A future contributor could regress the optimization without a test failure. Recommend a small unit test mocking the __render_frame_<id>__ sibling + asserting el.currentTime is left untouched across the three sync conditions.

5. (Comment density) The 25-line WHY-comment is on the heavier side

It explains the implicit gate (sibling presence → render mode → audio handled by ffmpeg → seek is waste). Per repo CLAUDE.md "non-obvious WHY" exception, justified — but could be condensed to ~10 lines. Borderline; not blocking.

Refuted by Phase 2 verification

• Late-binding capability registration: refuted — both __hfReseekGpu and __hf.colorGrading.redraw are set synchronously during init (see Finding #1).
• Audio regression on unmuted videos: refuted by cross-file trace — render-mode audio is ffmpeg-mixed from source files.

Validation summary

• Local CLI, synth-30-heavy / 30 × 32 MB / 90 s: wall 119.5 s ± 1.4 s (baseline N=3) → ~118 s (with-fix). Per-frame avgScreenshot 50.0 ms → 49.0 ms (-2%). Output md5 identical to baseline ×3.
• Lint / format / typecheck / fallow / commitlint: all green via lefthook pre-commit.

Reviewers — Miguel + Magi: this is a small, defensive cleanup with a modest per-frame improvement. The bigger wall improvement was already in #1630; this is the follow-up that quiets Chrome's media pipeline during render. The dropped cap-cache was caught in my own review — please double-check my code-read on init.ts:2015 + colorGrading.ts:1211 to confirm those are truly unconditional.

— Jerrai

[miga-heygen]

Independent review — Miga

Examined the diff, the PR description, Rames's self-review, and cross-referenced the relevant source files on main and the PR branch. Here are my findings.

---

1. Render-mode detection logic (sibling <img> presence) — VERIFIED CORRECT

The gate at media.ts:305-306:

const skipForInjectedVideo =
  el.tagName === "VIDEO" && el.id && !!document.getElementById(`__render_frame_${el.id}__`);

Cross-referenced against the creation site in screenshotService.ts:471-475:

img = document.createElement("img");
img.classList.add("__render_frame__");
img.id = `__render_frame_${item.videoId}__`;
img.style.pointerEvents = "none";
video.parentNode?.insertBefore(img, video.nextSibling);

The ID convention matches exactly (__render_frame_${videoId}__). The <img> is only created during the render pipeline's injectVideoFramesBatch — never in preview. The el.tagName === "VIDEO" guard correctly excludes <audio> elements (the sync loop iterates document.querySelectorAll("video, audio") at media.ts:39). The el.id truthiness check prevents a spurious document.getElementById("__render_frame___") lookup on any id-less video.

Detection is sound. No false positives in preview; no false negatives in render.

2. Consumers of <video>.currentTime during render — VERIFIED SAFE

Rames's cross-file trace is correct. I independently confirmed:

• Audio extraction (audioExtractor.ts): uses source files + metadata, not <video>.currentTime.
• Runtime event listeners (media.ts:111-114): playing / pause / error — none read currentTime.
• Video-texture-compat (video-texture-compat.ts): reads the <img> sibling for GPU texture upload during render, not the <video> element's seek state. The render-mode __hfReseekGpu dispatches a seek event to re-render GPU compositions after frame injection, but that operates on the adapter layer (seek-dispatch.ts), not on <video>.currentTime.
• Volume envelope (media.ts): reads el.volume, not currentTime.
• No GSAP timeline animates currentTime as a target property.

Skipping currentTime sets is safe for the render path.

3. Capability registrations at init.ts:2015 + colorGrading.ts:1211 — VERIFIED UNCONDITIONAL

Rames asked reviewers to double-check this. Confirmed:

• window.__hfReseekGpu is assigned at init.ts:~2016 (inside initRuntime(), top-level assignment — no conditionals, no feature flags, no early returns before it).
• window.__hf.colorGrading is assigned at colorGrading.ts:~1211 inside createColorGradingRuntime(), which is called unconditionally at init.ts:1802. The assignment win.__hf.colorGrading = api has no conditional wrapping.

Both run synchronously during initRuntime(), before pollHfReady resolves, before the first injector call. The dropped cap-cache was indeed dead code — the cached booleans would always be true. Correct to remove.

4. Edge cases with sibling detection

Timing: The PR description notes the calibration test-frame edge case — at the very start of a video's active window, the injection sibling may not yet exist. The gate correctly defaults to "no sibling → run the seek," which is the existing behavior. Good.

DOM order: The <img> is inserted as video.nextSibling by screenshotService.ts:475, but the detection in media.ts uses document.getElementById, not sibling traversal. This is more robust — DOM reordering (e.g., a wrapper insertion by GSAP or a layout library) won't break the lookup. video-texture-compat.ts does the same: sibling check first for speed, then getElementById fallback (lines 27-40). The media gate skips the sibling shortcut and goes straight to the ID lookup, which is fine for correctness if slightly less optimal.

Multiple videos: Each video has a unique el.id (e.g., vid-01, vid-02), and each injected <img> gets __render_frame_${videoId}__. The getElementById lookup is per-video-ID, so multiple concurrent videos are handled correctly with no cross-contamination.

Videos without IDs: The el.id truthiness check short-circuits before the DOM lookup. Videos without an id attribute always take the existing seek path. In practice, all data-start videos in HyperFrames compositions have IDs (the producer requires them for frame injection), but the guard is defensive regardless.

5. Corroborating Rames's follow-up items

• Per-tick getElementById (Finding #3 in Rames's review): Agreed this is O(1) per call but ~80K calls on a 30-video comp. A WeakMap<HTMLVideoElement, boolean> cache would be cleaner. Not blocking — follow-up material.
• No unit test for the gate (Finding #4): Agreed. A test that mocks a __render_frame_<id>__ sibling and asserts el.currentTime is NOT set across the three sync conditions (hardSync, strictSync, forceSync) would lock this contract. Not blocking for this PR, but should land soon after.

6. Comment density

The 14-line comment (trimmed from 25 in the original revision) explains a non-obvious gate. Per repo convention for "non-obvious WHY," this is justified. I'd leave it as-is — someone reading media.ts in six months will thank it.

---

Summary

Single-file, 36 ins / 9 del, defensive and well-scoped. The render-mode detection is correct, the consumer trace checks out, the dropped cap-cache was dead code, and the edge cases are handled. CI looks green on all completed checks (regression shards still running at time of review).

Recommend approval once regression shards finish green. The two follow-up items (WeakMap cache + unit test) can land in a separate PR.

— Miga

[miguel-heygen]

Verified current head e5cc3f4f. Additive stamp after Rames/Miga's reviews: the PR is still the single-file render-mode media gate, and CI is green.

Specific checks:

• packages/core/src/runtime/media.ts:304-305 uses the exact __render_frame_${video.id}__ ID convention.
• packages/engine/src/services/screenshotService.ts:470-475 creates that image only in the frame-injection path and inserts it beside the video, so preview keeps the existing seek behavior.
• packages/core/src/runtime/media.ts:306-320 preserves the existing currentTime + el.load() drift-recovery path whenever the render-frame image is absent.
• Confirmed the dropped capability-cache concern remains out of the diff; the underlying registrations are unconditional at packages/core/src/runtime/init.ts:2015 and packages/core/src/runtime/colorGrading.ts:1211.

The WeakMap lookup cache and a small unit test for the gate are good follow-ups, not merge blockers for this narrowly scoped perf cleanup.

Verdict: APPROVE
Reasoning: The render-only detection is tied to the actual injected-frame contract, non-render media sync is unchanged, and all required checks pass; GitHub is blocked only on review approval.

— Magi