Repository navigation
fix(player): iPhone Safari keeps every take's sound after the first seconds of an edit - #6724
Conversation
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 1 important · 0 nits · head a591248b
- blocker (thread gate): this is a fix PR, but the body has no
fails without the fix:line naming a test that fails onmain. Add one, e.g. the newmedia.test.ts/parent-media.test.tspriming cases. Those fail onmainonly becauseprimeMediaForLaterStarts/primeAlldon't exist there, so if a regression test can pin the refused-play behavior instead, name that one.
Scope
Read all 8 files end to end. I checked the play/pause listeners that the priming events reach (webAudioTransport.ts:478-486, init.ts:3634 wake, video-source.ts), and they net out neutral. No prior reviews. CI was still running at review time (an earlier PR captures run on this head failed; it is being re-run).
— Genesis
| // Play-then-pause each idle element from the tap's own call stack to allow later starts. | ||
| export function primeMediaForLaterStarts(els: Iterable<HTMLMediaElement>): void { | ||
| for (const el of els) { | ||
| if (!el.paused || playRequested.has(el) || isUnplayable(el)) continue; |
There was a problem hiding this comment.
important: this primes every idle clip on every platform, including muted <video>s. In Studio preview (defersMedia()), deferMediaUntilDue deliberately parks later clips at preload="none". A play() call starts their fetch, and the immediate pause() doesn't stop it. So the first Play in Desktop/Studio now downloads and opens a decoder for every take in the edit at once, which defeats the deferral and the release window. iOS only refuses unmuted starts, so at minimum skip el.muted elements. Consider also gating to the platform that needs this (touch WebKit) so desktop keeps the deferred loading.
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 2 blocker · 0 important · 1 nit · a591248b → 2baef6c8
The code is fixed: priming is iOS-only, skips muted clips, and the fails without the fix: line is there. Two red checks on this head are caused by the PR:
- Comments:
packages/core/src/runtime/mediaClock.tsgoes from 0/6 to 1/12 comment lines. Drop thestartsSoundOnlyInsideATapdoc line, or move it into the name. - PR captures: the two screenshots sit under
## Not in this PR.## Beforeand## Afterhave only text, so the check still fails on the current body. Move each image into its section.
nit, not gating: skipping el.muted also skips clips muted only because the player is muted. After a muted autoplay, a tap on unmute (not play) primes nothing, and later takes can still be refused. Skipping defaultMuted (authored muted) instead would cover that. This came from my own earlier suggestion.
— Genesis
Edit accuracy: accurate 271 (base branch 271), smooth 149 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
There was a problem hiding this comment.
Verdict: APPROVE — 3 resolved, 0 open, 0 new · 2baef6c8 → ec8b253c. All runs are green. Windows engine-cli first failed in browserManager.test.ts BeginFrame probe, the known flake, which this diff doesn't touch; it passed on rerun (attempt 2).
— Genesis
What
On iPhone Safari, an edit with one
<audio>per take went silent from the first take that started more than about 5 s after the tap, for the rest of the edit. This makes every take start with sound for the whole edit.Reported with traces by Robin Ebers (Attention Machine): https://gist.github.com/robinebers/d06818f0c2fae0f15e11b6decd1ae529 (problem 1).
Why it broke
iOS only lets an unmuted element start while the page still has user activation (about 5 s after the tap), unless that element was played inside a tap before. The runtime starts each take's
<audio>when its window opens, so late takes getNotAllowedError. The autoplay fallback (#6417) then mutes the composition and moves sound to parent-frameAudio()copies that were never played in a tap either, so those are refused too. Desktop Chrome lets any element play after one tap, so it never showed.Change
Inside the tap that starts playback, play-then-pause every idle timed media element once, in both places that own audio:
transport.play()primes the composition's clips (primeMediaForLaterStarts)play()primes the parent-frame copies (ParentMediaManager.primeAll)Only on iPhone and iPad (
startsSoundOnlyInsideATap, one detector inmediaClock.tsshared by both), and not clips authored muted (mutedattribute): those start freely, and Studio and desktop browsers keep their deferred loading untouched. Elements already sounding, already starting or without a usable source are skipped. It is one pass over the clips on play.Checked
iOS 26.5 Simulator (iPhone 17 Pro), real Safari, one tap on Play, edits shaped like the report's (3 takes 24.8 s, 10 takes 53.1 s; one muted
<video>and one<audio>per take on one MP4; fixture fromtests/playbench):play()callsfails without the fix:
init.test.ts"primes a clip that starts later inside the tap on iOS, so a late start is not refused" andmedia.test.ts"plays then pauses every idle unmuted take so iOS lets it start after the tap" (both fail on a build with priming switched off; run on the branch and again with it disabled).parent-media.test.tscovers the parent-frame copies, and a case in each file shows nothing is primed off iOS or for muted clips.Before
0.8.210 in the iOS 26.5 Simulator, 3-take edit after one tap: 480 refused plays, audio handed to the parent copies and muted.
After
This branch, same edit and tap: 0 refused plays, no ownership change, plays to the end.
Not in this PR
The picture stops at the start and at each cut (the playhead waits for a freshly started
<audio>, problem 2 in the report) is separate and not fixed here. It is still measurable on this branch.