fix(player): preserve parent media contracts - #3047
Conversation
miga-heygen
left a comment
There was a problem hiding this comment.
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-starttakes precedence overdata-media-startviareadMediaStart, matching the iframe runtimereadAuthorPlaybackRatefalls back tosource.defaultPlaybackRate(a standard DOM property), not a magic defaultclampVolumeandnormalizePlaybackRateguard against NaN/Infinity without swallowing them silently_refreshEntryContractre-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=1defaults 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.
|
@cwhy can you sign the commit pls? |
6479617 to
3592575
Compare
Signed-off-by: Chen Yu <chenyu.nus@gmail.com>
3592575 to
062227c
Compare
|
Thanks. The amended head commit 062227c is now signed. @miguel-heygen |
Summary
data-volume, player volume, and live runtime/GSAP volume envelopes when the parent fallback owns mediadata-media-start/data-playback-startand authoreddata-playback-ratein parent proxy seek, scrub, mirror, and playbackaudio-srcproxies compatible with the existing global volume/rate behaviorProblem
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.tsbun run --filter @hyperframes/player typecheckenv NODE_OPTIONS=--no-experimental-webstorage bun run --filter @hyperframes/player test— 334/334bun run --filter @hyperframes/player build