From 5f66edd4498bcecd32ac5355f03c4ed3f5b291ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Fri, 17 Apr 2026 19:45:12 +0200 Subject: [PATCH 1/2] fix(player): address review feedback on single-owner audio (#298) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses James Russo's review on PR #298. - Drop drift threshold 150ms → 50ms. ITU-R BT.1359 puts A/V offset perceptibility at ±45ms; 150ms risks audible lip-sync drift on talking-head content. Steady-state offset under parent ownership on the factory-series-c-video repro measures 27–37ms. - Add MutationObserver for dynamic sub-composition media so late- attached audio[data-start] / video[data-start] elements get a parent proxy. Without it, parent ownership silences the new element in the iframe via sticky outputMuted but has no parent counterpart to play — silent hole. - Make bridgeMuted sticky in syncRuntimeMedia. Symmetric with mediaOutputMuted so a sub-composition activating after user mute doesn't briefly play at author volume before the next bridge message. Both flags feed a single shouldMute gate per tick. - Reset _audioOwner on iframe load. Latch previously never cleared, leaving stale parent-ownership against a fresh runtime after composition switch — a brief double-voice window until the next NotAllowedError re-promoted. - Dispatch audioownershipchange CustomEvent on every owner change, with reason: 'autoplay-blocked' | 'iframe-reload'. Gives host apps an SLO-ready signal for '% of sessions in parent ownership'. - Surface parent proxy play() rejection as a playbackerror event instead of swallowing silently. Covers the rare case where the parent also lacks activation. - Test gaps filled: userMuted stickiness, OR invariant between flags, contract pin (media.ts fires onAutoplayBlocked every rejection), caller-side latch pattern dedupe, audioownershipchange dispatch, mid-playback promotion path, playbackerror surface. - One-line comment in _promoteToParentProxy on the postMessage async race — defensible because the autoplay gate keeps rejecting until the mute lands. Not addressed here: - Manual verification on a physical iOS / Android device (review item #7). Simulated autoplay block via agent-browser + mobile emulation was done on the original PR; real-device validation is still pending before the next release. --- packages/core/src/runtime/init.ts | 1 + packages/core/src/runtime/media.test.ts | 117 +++++++++++ packages/core/src/runtime/media.ts | 20 +- .../player/src/hyperframes-player.test.ts | 54 +++++ packages/player/src/hyperframes-player.ts | 184 +++++++++++++++--- 5 files changed, 341 insertions(+), 35 deletions(-) diff --git a/packages/core/src/runtime/init.ts b/packages/core/src/runtime/init.ts index a4ba9552c..7a430a47d 100644 --- a/packages/core/src/runtime/init.ts +++ b/packages/core/src/runtime/init.ts @@ -1190,6 +1190,7 @@ export function initSandboxRuntimeModular(): void { playing: state.isPlaying, playbackRate: state.playbackRate, outputMuted: state.mediaOutputMuted, + userMuted: state.bridgeMuted, onAutoplayBlocked: () => { if (state.mediaAutoplayBlockedPosted) return; state.mediaAutoplayBlockedPosted = true; diff --git a/packages/core/src/runtime/media.test.ts b/packages/core/src/runtime/media.test.ts index f9d9a537f..951ca7969 100644 --- a/packages/core/src/runtime/media.test.ts +++ b/packages/core/src/runtime/media.test.ts @@ -458,4 +458,121 @@ describe("syncRuntimeMedia", () => { await Promise.resolve(); expect(onAutoplayBlocked).not.toHaveBeenCalled(); }); + + it("asserts muted=true every tick while userMuted is set", () => { + // Mirror of the `outputMuted` test — user preference must be sticky + // too. A sub-composition that activates after the user mutes should + // inherit the silence, not briefly play at author volume before the + // next bridge message lands. + const clip = createMockClip({ start: 0, end: 10, volume: 1 }); + Object.defineProperty(clip.el, "readyState", { value: 4, writable: true }); + Object.defineProperty(clip.el, "muted", { value: false, writable: true }); + syncRuntimeMedia({ + clips: [clip], + timeSeconds: 5, + playing: true, + playbackRate: 1, + userMuted: true, + }); + expect(clip.el.muted).toBe(true); + }); + + it("fires onAutoplayBlocked for every rejected play (caller owns the latch)", async () => { + // media.ts is intentionally memoryless — each NotAllowedError rejection + // invokes the callback. The init.ts caller wraps with + // `mediaAutoplayBlockedPosted` so the outbound message is posted at most + // once per session. This test pins down the contract (fires always) so + // a future refactor can't quietly add deduplication here and break the + // caller's latching logic. + const clip = createMockClip({ start: 0, end: 10 }); + Object.defineProperty(clip.el, "readyState", { value: 4, writable: true }); + const rejection = Object.assign(new Error("blocked"), { name: "NotAllowedError" }); + clip.el.play = vi.fn(() => Promise.reject(rejection)); + const onAutoplayBlocked = vi.fn(); + + // Simulate two ticks — between them `playRequested` clears so play() runs + // again and rejects again. + syncRuntimeMedia({ + clips: [clip], + timeSeconds: 5, + playing: true, + playbackRate: 1, + onAutoplayBlocked, + }); + await Promise.resolve(); + await Promise.resolve(); + syncRuntimeMedia({ + clips: [clip], + timeSeconds: 5.05, + playing: true, + playbackRate: 1, + onAutoplayBlocked, + }); + await Promise.resolve(); + await Promise.resolve(); + + // No latch inside media.ts — two rejections, two callback invocations. + // The caller's latch is what prevents a second outbound message. + expect(onAutoplayBlocked).toHaveBeenCalledTimes(2); + }); + + it("caller-side latch pattern posts once across many rejections", async () => { + // Mirrors what init.ts does: the onAutoplayBlocked wrapper checks and + // sets a boolean flag so the outbound post fires exactly once even if + // the raw callback fires many times. Regression guard for the latch + // wiring in the init.ts handler. + const clip = createMockClip({ start: 0, end: 10 }); + Object.defineProperty(clip.el, "readyState", { value: 4, writable: true }); + const rejection = Object.assign(new Error("blocked"), { name: "NotAllowedError" }); + clip.el.play = vi.fn(() => Promise.reject(rejection)); + + let posted = 0; + const state = { latched: false }; + const wrapped = () => { + if (state.latched) return; + state.latched = true; + posted += 1; + }; + + for (let i = 0; i < 5; i++) { + syncRuntimeMedia({ + clips: [clip], + timeSeconds: 5 + i * 0.05, + playing: true, + playbackRate: 1, + onAutoplayBlocked: wrapped, + }); + await Promise.resolve(); + await Promise.resolve(); + } + + expect(posted).toBe(1); + }); + + it("mutes when either outputMuted OR userMuted is true (OR invariant)", () => { + // Explicit validation of the combined-flag contract: setting one to + // false while the other is true must keep the element muted. + const clip = createMockClip({ start: 0, end: 10, volume: 1 }); + Object.defineProperty(clip.el, "readyState", { value: 4, writable: true }); + Object.defineProperty(clip.el, "muted", { value: false, writable: true }); + syncRuntimeMedia({ + clips: [clip], + timeSeconds: 5, + playing: true, + playbackRate: 1, + outputMuted: false, + userMuted: true, + }); + expect(clip.el.muted).toBe(true); + Object.defineProperty(clip.el, "muted", { value: false, writable: true }); + syncRuntimeMedia({ + clips: [clip], + timeSeconds: 5, + playing: true, + playbackRate: 1, + outputMuted: true, + userMuted: false, + }); + expect(clip.el.muted).toBe(true); + }); }); diff --git a/packages/core/src/runtime/media.ts b/packages/core/src/runtime/media.ts index b82b95b2a..f0f65d8d7 100644 --- a/packages/core/src/runtime/media.ts +++ b/packages/core/src/runtime/media.ts @@ -93,13 +93,18 @@ export function syncRuntimeMedia(params: { playing: boolean; playbackRate: number; /** - * When `true`, assert `el.muted = true` on every active media element on - * every tick. Sticky against newly-discovered media (sub-composition - * activation, dynamic DOM) so the parent-frame audio-owner invariant holds. - * `false` is a no-op — we don't un-mute, because other code paths - * (`