Skip to content

fix(player): preserve parent media contracts - #3047

Open
cwhy wants to merge 1 commit into
heygen-com:mainfrom
cwhy:cy-cd/parent-proxy-media-contract
Open

fix(player): preserve parent media contracts#3047
cwhy wants to merge 1 commit into
heygen-com:mainfrom
cwhy:cy-cd/parent-proxy-media-contract

Conversation

@cwhy

@cwhy cwhy commented Aug 5, 2026

Copy link
Copy Markdown

Summary

  • preserve authored data-volume, player volume, and live runtime/GSAP volume envelopes when the parent fallback owns media
  • honor data-media-start / data-playback-start and authored data-playback-rate in parent proxy seek, scrub, mirror, and playback
  • keep URL-driven audio-src proxies compatible with the existing global volume/rate behavior

Problem

The parent autoplay fallback cloned media source and timeline bounds, but discarded per-track gain and source offsets. A background track authored at data-volume="0.12" could play at full gain, and a trimmed track could start from source time zero. Preview audio therefore diverged from the rendered output.

The iframe runtime already exposes the authoritative effective gain on each live media element, including GSAP envelopes and player volume. The proxy now mirrors that value directly, while keeping zero volume authoritative during the cross-frame update window.

Validation

  • bunx oxlint packages/player/src/parent-media.ts packages/player/src/parent-media.test.ts
  • bun run --filter @hyperframes/player typecheck
  • env NODE_OPTIONS=--no-experimental-webstorage bun run --filter @hyperframes/player test — 334/334
  • bun run --filter @hyperframes/player build
  • headless Chrome integration against a real generated 95.72s composition using player 0.7.94 with core 0.6.110: seven media proxies, narration gain 1.0, music gain 0.12, music source offset 36.947s, and volume 1.0 / 0.5 / 0 / 0.75 transitions all verified

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: fix(player): preserve parent media contracts @ 6479617

Design

The core insight is right: the iframe runtime already computes the effective gain (authored volume * global volume * GSAP envelope), so the parent proxy should copy source.volume directly rather than recomputing from static data-volume. Same for playback rate (authored * global) and media offset (data-media-start / data-playback-start).

The zero-volume bifurcation is well-motivated. When updateVolume(0) is called, the proxy silences immediately (can't wait for the iframe's next state tick). When updateVolume(non-zero) is called, the proxy keeps its current gain until the iframe runtime publishes the new effective value on source.volume — this prevents the "flash of old audio" between sending the volume change to the iframe and the iframe processing it.

The test at lines 55-119 walks through the full zero/non-zero/GSAP-envelope lifecycle and is thorough enough to lock in this invariant.

Findings

No blocking issues.

1. _sourceTime uses instantaneous rate, not integral (non-blocking)

private _sourceTime(m: ProxyEntry, relTime: number): number {
  return (m.mediaStart ?? 0) + relTime * (m.authorPlaybackRate ?? 1);
}

If data-playback-rate changes mid-clip (GSAP animation), this computes mediaStart + relTime * currentRate rather than the integral of rate over time. The position would jump on rate change — e.g., if rate was 1 for 5s then switches to 2, the next tick at relTime=5.01 computes mediaStart + 5.01 * 2 = mediaStart + 10.02 instead of the correct mediaStart + 5 + 0.01 * 2 = mediaStart + 5.02.

The drift detection corrects this within 2 samples (~160ms), and this is a fallback path (only active when iframe audio fails autoplay). Acceptable, but worth documenting if dynamic data-playback-rate animation becomes a supported pattern.

2. updatePlaybackRate vs _syncEntryPlaybackRate source asymmetry (non-blocking)

updatePlaybackRate applies (authorPlaybackRate ?? 1) * rate using the passed-in rate. _syncEntryPlaybackRate uses this._getPlaybackRate(). If the player's internal state hasn't been updated before calling updatePlaybackRate, the next mirrorTime would snap back to the old rate. This works in practice (the player updates state before calling), but unlike volume (which stores _userVolume explicitly), playback rate relies on an implicit contract with the caller.

Not a bug — just an asymmetry with the volume path. If you ever see drift in playback rate during transitions, this is the seam to check.

What's clean

  • data-playback-start takes precedence over data-media-start via readMediaStart, matching the iframe runtime
  • readAuthorPlaybackRate falls back to source.defaultPlaybackRate (a standard DOM property), not a magic default
  • clampVolume and normalizePlaybackRate guard against NaN/Infinity without swallowing them silently
  • _refreshEntryContract re-reads all four properties (start, duration, mediaStart, authorVolume, authorPlaybackRate) from the source every tick, so live edits propagate without re-adoption
  • URL-driven proxies correctly use authorVolume=1 / authorPlaybackRate=1 defaults and apply global volume/rate directly
  • Test coverage is thorough: gain deferral with GSAP envelope simulation, media offset with both attribute aliases, playback rate composition, and URL-driven proxy behavior

Verdict

Ship it. The volume mirroring design (defer to runtime's effective gain, except zero which is immediate) is the right call — it avoids the impossible problem of reconstructing GSAP envelopes from static attributes.

@miguel-heygen

Copy link
Copy Markdown
Collaborator

@cwhy can you sign the commit pls?

@cwhy
cwhy force-pushed the cy-cd/parent-proxy-media-contract branch from 6479617 to 3592575 Compare August 5, 2026 06:02
Signed-off-by: Chen Yu <chenyu.nus@gmail.com>
@cwhy
cwhy force-pushed the cy-cd/parent-proxy-media-contract branch from 3592575 to 062227c Compare August 5, 2026 06:11
@cwhy

cwhy commented Aug 5, 2026

Copy link
Copy Markdown
Author

Thanks. The amended head commit 062227c is now signed. @miguel-heygen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants