fix(core): avoid live volume probe during render - #2816
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| <head> | ||
| <meta charset="utf-8" /> | ||
| <title>Static volume with future nested set</title> | ||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/gsap/3.12.2/gsap.min.js"></script> |
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed exact head 477defc7e7397684c2def4459b3981f62a06816d. No blocking findings.
The render/probe marker is injected before the runtime on every producer-served HTML page, the runtime-side guard disables only the live timeline sampling, and the helper defaults preserve Studio/preview discovery. Producer-side automation discovery remains in its separate probe pass and continues to bake keyframes before capture. The nested future-tl.set golden exercises the reported non-zero distributed cold-start failure shape. All CI, including the full regression matrix and Windows render/tests, is green.
The CodeQL CDN observation is confined to the deterministic regression fixture and the repository security checks are green; it does not alter the production path.
— Magi
Verdict: APPROVE
Reasoning: The change removes the render-time timeline mutation at the correct boundary while preserving preview and producer-side audio automation behavior.
james-russo-rames-d-jusso
left a comment
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at 477defc7e7397684c2def4459b3981f62a06816d.
Clean small fix. The root cause reads correctly — probeAndCacheElementVolume() sampled the captured GSAP timeline at 60Hz across the media window and restored only the numeric playhead. That's fine for preview (single-composition, main timeline), but distributed cold-start chunks that begin at non-zero times were vulnerable: seeking the captured timeline forward materializes nested-composition scripted state (like a future tl.set(".future-state", { visibility: "hidden" }) at 9.65s), and restoring the playhead numerically doesn't reverse the state mutation the seek triggered.
The gate
Three files, one contract:
-
fileServer.ts:685— newRENDER_CAPTURE_MODE_SHIM = "globalThis.__HF_RENDER_CAPTURE_MODE = true;"injected viapreHeadScriptsat line 721-725, so it lands BEFORE the HF runtime script. ThecreateFileServertest atfileServer.test.ts:319-345locks in the ordering:expect(html.indexOf(RENDER_CAPTURE_MODE_SHIM)).toBeLessThan(html.indexOf(runtimeScript)). Correct — a global mutated after runtime execution wouldn't gate initialization. -
init.ts:1837-1840—initSandboxRuntimeModular()reads the global and passesallowLiveTimelineSeek: !window.__HF_RENDER_CAPTURE_MODE. Preview pages don't inject the shim →__HF_RENDER_CAPTURE_MODEisundefined→!undefined = true→ probe runs as before. Render pages inject →true→!true = false→ probe short-circuits. Backward-compatible. -
mediaVolumeEnvelope.ts:135-158—VolumeProbeOptions.allowLiveTimelineSeek?: boolean. Early return at line 156 uses strict=== false, soundefined(default when called without options) falls through to the original probe path. Existing callers unchanged.
The unit test does not seek or cache when live timeline probing is disabled at mediaVolumeEnvelope.test.ts:6-28 covers the exact contract: no seeks recorded, no cache entries added, media volume unchanged from author-declared value.
Regression fixture
static-volume-future-set/src/compositions/child.html sets up the reproducer: nested composition with timeline.set(".future-state", { visibility: "hidden" }, 9.65). Static-volume audio (data-volume="1") on the parent triggers probeAndCacheElementVolume on the child's captured timeline; the probe seeks to sample volume automation, materializing the future visibility: hidden prematurely. With chunkSize: 120 at 24fps = 5-second chunks, a chunk starting at t=5 (second chunk) would seek forward through 9.65 during the probe and lose the "MUST BE VISIBLE" panel.
maxAudioLagWindows: 120 (fixture-wide) is unusually permissive — this fixture uses silence.wav where correlation is meaningless, so widening the window past strict audio parity is the right call. maxFrameFailures: 0 keeps the visual assertion strict, which is what actually tests the fix.
The producer-side isolated discovery is untouched
Per your PR body: "producer-side isolated audio automation discovery/baking" still works. That's the isolated producer probe page (separate flow) — not touched by this PR, so scripted volume-automation coverage for baked audio is preserved. Verified by grep: probeAndCacheElementVolume is only called at init.ts:1837, and the producer's audio discovery uses its own separate probe path in packages/producer/src/services/audioExtractor.ts (unrelated code).
Small note
- GitHub Advanced Security flagged the CDN-loaded GSAP
<script>instatic-volume-future-set/src/index.html— expected for the test fixture. Not a security concern for test infrastructure. Every other distributed fixture that needs GSAP does the same.
Nothing blocking. LGTM.
4 participants
Summary
tl.setdistributed goldenFixes #2809.
Root cause
Runtime media initialization called
probeAndCacheElementVolume()for everyaudio/video element, including static
data-volumemedia. The helper sampledthe captured GSAP timeline at 60 Hz across the media window and restored only
the numeric playhead. On a non-zero distributed cold start, future nested
zero-duration state could remain materialized before its authored time.
Producer pages do not need this live runtime probe: scripted volume automation
is already discovered in an isolated producer probe page and baked into audio
before frame capture.
Validation
Real GCP (
hyperframes-dev/us-east1)Owner-isolated smoke using 120-frame chunks:
f6d639c79226dd7ef93369c5f8b1e296895e8049ca8234d887615c6ff1965b31MUST BE VISIBLEpanel in the non-zero-start chunkdestroyed and verified absent
The cross-Chrome PSNR against the local Chrome 152 baseline was 34.78 dB. The
smoke was intentionally invoked with an over-strict 40 dB wrapper threshold;
both workflows and all exact parity/visibility checks succeeded.