fix(player): address #298 review — tighter drift, dynamic proxies, ownership event

[miguel-heygen]

Follow-up to PR #298 addressing @jrusso1020's review. Each item below maps to a point in his comment.

Significant

1. Drift threshold 150 ms → 50 ms

mirrorParentMediaTime was too loose for lip-synced talking-head content. ITU-R BT.1359 puts A/V perceptibility at ±45 ms; 150 ms sat well inside the "unacceptable" zone. Dropped to 50 ms, extracted as a static constant for clarity.

Verified live on factory-series-c-video (agent-browser): steady-state offset under parent ownership sampled five times over 400 ms = [35.7, 33.5, 31.2, 27.2, 36.9] ms — below the perceptibility floor. Before this PR the same measurement could drift up to 150 ms before correction.

2. Dynamic sub-composition media proxies

Under parent ownership, a sub-composition that attaches a new <audio data-start> mid-playback was correctly silenced in the iframe (sticky outputMuted) but had no parent-frame counterpart to play → silent hole in the audio track.

Added a MutationObserver on the iframe body watching for audio[data-start] / video[data-start] additions. New elements are adopted through the same _adoptIframeMedia helper the initial scan uses, and if parent ownership is already active the new proxy gets its currentTime mirrored and play() called immediately (gated on !this._paused). Observer disconnects on iframe reload + component disconnect.

3. bridgeMuted sticky in syncRuntimeMedia

The asymmetry James flagged: outputMuted was sticky per-tick, bridgeMuted was one-shot via onSetMuted. A sub-composition activating after a user mute would briefly play at author volume before the next bridge message.

syncRuntimeMedia now accepts userMuted and the per-clip loop uses a single combined shouldMute gate. One invariant, two inputs.

4. Reset _audioOwner on iframe reload

The latch never cleared. On composition switch the player would stay in parent ownership against a fresh runtime that hadn't received set-media-output-muted and whose autoplay-blocked latch was clean — a brief double-audio window until the next NotAllowedError re-promoted (idempotently).

_onIframeLoad now resets _audioOwner = "runtime", pauses any parent proxies, and disconnects the old MutationObserver before a fresh one attaches to the new document. If the player had been in parent ownership, a corresponding audioownershipchange event fires with reason: "iframe-reload".

Worth addressing

5. Promotion → observable event + reason

Promotion was invisible. Added CustomEvent("audioownershipchange", { detail: { owner, reason } }) fired on every owner transition. reason is either "autoplay-blocked" (promote → parent) or "iframe-reload" (reset → runtime). Gives host apps an SLO-ready signal for "% of sessions in parent ownership" without exposing internal state.

Verified live: dispatching a synthetic media-autoplay-blocked in the live studio produced { owner: "parent", reason: "autoplay-blocked" } on the web component exactly once.

6. Parent proxy play() rejection → playbackerror event

Previously swallowed silently. Now re-emitted as CustomEvent("playbackerror", { detail: { source: "parent-proxy", error } }) so embedding apps can recover or fall back.

7. Mobile verification on real hardware

Tested with a tunnel in a real iOS device.

Test gaps (from review)

• userMuted stickiness (mirror of the existing outputMuted test).
• OR invariant between outputMuted and userMuted — explicit test that setting one false while the other is true keeps el.muted === true.
• Contract pin: syncRuntimeMedia fires onAutoplayBlocked on every rejection (no internal dedupe) — so a future refactor can't quietly move the latch and break the caller's posting logic.
• Caller-side latch pattern: a 5-rejection simulation with the init.ts-style wrapper posts exactly once.
• audioownershipchange dispatch on promotion + once per transition (no duplicate on idempotent re-promote).
• Mid-playback promotion: _paused = false at flip time fires _playParentMedia immediately.
• playbackerror surface on parent proxy rejection with the right source tag.

Minor

• One-line comment on _promoteToParentProxy explaining the postMessage async race (the mute lands after ~one message-loop tick; the autoplay gate that triggered promotion keeps the iframe rejecting play() during that window, so the double-play bug doesn't reappear).

What's good (from the review)

Kept as-is — noted for posterity:

• muted vs volume framing (orthogonal channels).
• Probing reality via NotAllowedError instead of matchMedia('(pointer: coarse)') / UA sniffing.
• Two orthogonal mute channels.
• Backwards compat (new actions / messages safely ignored by either side).

Test results

• packages/core/src/runtime/media.test.ts — 42 tests pass (+4 new: userMuted sticky, OR invariant, fires-every-rejection, caller-latch dedupe)
• packages/core/src/runtime/bridge.test.ts — 15 tests pass
• packages/player/src/hyperframes-player.test.ts — 26 tests pass (+3 new: audioownershipchange dispatch, mid-playback promotion, playbackerror surface)
• Typecheck green on core + player
• tsup build green on core / player / cli
• Live factory-series-c-video repro via agent-browser: runtime ownership still zero volumechange thrash, zero PARENT.play() calls; parent ownership measures 27–37 ms steady-state drift, well inside the 50 ms threshold.

Test plan

• Unit tests (83 total across touched files)
• Typecheck clean
• Build clean
• Live studio repro on factory-series-c-video: runtime path unchanged, parent path drift tightened
• audioownershipchange event fires with correct detail on synthetic autoplay block
• Physical iOS / Android device verification (unchanged since fix(player): single-owner audio to prevent double voice in preview #298)

[miguel-heygen]

• fix(studio): iPhone Safari layout + touch-drag scrubber #308
• fix(player): address #298 review — tighter drift, dynamic proxies, ownership event #307 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[jrusso1020]

Thanks for the thorough response — the 50 ms threshold with the BT.1359 citation, the OR-combined mute invariant, and the ownership-reset on iframe reload all address the #298 points cleanly, and the four new media.test.ts cases (sticky userMuted, OR invariant, fires-every-rejection, caller-latch) are exactly the right pins.

A few concerns worth addressing before merge, flagged inline. The most important is the playbackerror dedup — under parent ownership with parent-also-blocked (the scenario the code comment explicitly anticipates), every paused→playing transition in the iframe message loop will fire a fresh event to the host. Same class of noise problem that mediaAutoplayBlockedPosted already solved on the runtime side.

Secondary: the MutationObserver handles additions but not removals, which means parent-frame proxies accumulate across sub-composition cycles until the next iframe reload. And the _adoptIframeMedia length-delta dance is fine today but one refactor away from a subtle bug.

One broader note, not worth blocking: audioownershipchange and playbackerror are now part of the player's public contract. Worth exporting AudioOwnershipChangeDetail / PlaybackErrorDetail types from @hyperframes/player so host apps can addEventListener<Typed> without casting e.detail as any. Can be a follow-up.

[jrusso1020]

Walked through 9afafc5. All four blockers resolved cleanly.

Prior concerns — resolved

1. playbackerror dedup. _playbackErrorPosted latch is the right shape — mirrors mediaAutoplayBlockedPosted, cleared in _onIframeLoad alongside the owner reset, so latch lifetime is correctly bound to the _audioOwner === "parent" session (which is the only thing that demotes back to "runtime" today). Both _playParentMedia and _adoptIframeMedia funnel through _reportPlaybackError, so there's one gate, not two. Regression test pins it.

2. _createParentMedia typed return. { el, start, duration } | null with the dedup-returns-null contract in the JSDoc. Caller drops the length-delta dance. Refactor-safe.

3. removedNodes handling. Symmetric _detachIframeMedia resolves via new URL(rawSrc, iframeEl.ownerDocument.baseURI).href (identical to adoption), pauses, clears src to free the decoder, splices out. Walks direct match plus nested querySelectorAll, mirroring the adoption walk.

4. Hysteresis / typed event details. Deferred with clear rationale; follow-ups noted. Fine.

Nits for a follow-up (non-blocking)

1. No regression test for _detachIframeMedia. The addition path has tests; the symmetric removal doesn't. Long-running sub-composition cycling is the exact case the detach exists for — a test that adds, removes, and asserts _parentMedia.length === 0 plus pause() called would lock in the symmetry.

2. The dedup test drives player.play() / player.pause() directly rather than simulating the iframe state-message loop the code comment describes as the real trigger (_onMessage → _playParentMedia). Close enough in practice, but a test that posts synthetic iframe messages would exercise the real path.

3. Processing order within a single MutationRecord. The loop does addedNodes then removedNodes. For the childList edge case where a record carries both (e.g. replaceChildren), you'd adopt then immediately detach. Almost certainly never fires for audio[data-start] / video[data-start] in practice, and the outcome is correct anyway — theoretical.

4. _createParentMedia → null dedup contract isn't directly tested. Call sites now branch on it; a one-line unit test would pin it.

Approve.