Repository navigation
fix(core): the playhead keeps moving at a cut while the incoming take has not started - #6768
Conversation
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 2 blockers · 0 important · 1 nit · head 26fe0f64
- blocker: the body has no
fails without the fix:line naming a test. The tests exist: with main'sclock.tsand this branch'sclock.test.ts, 4 tests fail and pass here. For example, addfails without the fix: clock.test.ts › keeps the playhead moving while a just-played element is frozen behind it, as WebKit leaves it(the seeking test and the late-start test fail on main too). - blocker: the required Typecheck check is red because of
webkit-stops.ts(inline). - nit: the non-required Comments check fails. It says
clock.tscomment share rose from 7.7% to 8.9% (+3 lines). Trim one of the two new comment pairs, or move the explanation into a name.
Scope
Read clock.ts, clock.test.ts and the playbench script in full, plus their callers refreshTransportClockSource/audioMayDrivePlayhead in init.ts and the audio steer/hard-sync tiers in media.ts. The clock change looks right to me: null falls back to the monotonic clock, which continues from the last audio read, so the playhead neither rewinds nor jumps. A starved element still holds, bounded by STARVED_AUDIO_HOLD_MS, and the loop-wrap escape is kept. Ran vitest run src/runtime/clock.test.ts: 50/50 pass. When I posted, Windows, regression, Safari perf and edit-accuracy CI were still running. No prior reviews.
Edit accuracy: accurate 271 (base branch 271), smooth 124 of thoseThe gate passes. Quarantined, measured but not gated (0) |
What
In every WebKit, the playhead stops about three times at each cut of a multi-take edit. This keeps the playhead moving. Problem 2 of Robin Ebers' report: https://gist.github.com/robinebers/d06818f0c2fae0f15e11b6decd1ae529
Why
At a cut the incoming take's
<audio>getsplay(), then media sync seeks it. WebKit leaves a just-played or just-seeked element'scurrentTimewhere it was for a while, andTransportClockheld the playhead whenever the element it follows was seeking or up to 0.5 s behind. So the picture stood still, and the hold was followed by a seek.Change
TransportClockno longer waits on the element it follows. While that element is seeking, the playhead runs on the monotonic clock. An element behind the playhead is steered or seeked by media sync, never waited for. The playhead follows the element once it is ahead. The one hold left is for an element starved of data (readyStatebelowHAVE_FUTURE_DATA), which the existing 3 s limit ininit.tsstill bounds.Also adds
packages/player/tests/playbench/webkit-stops.ts, a measure of "playhead stops over 50 ms per cut" that plays the 10-take or 3-take playbench edit in WebKit.fails without the fix: clock.test.ts › keeps the playhead moving while a just-played element is frozen behind it, as WebKit leaves it (the seeking and late-start tests fail on main too)
Measured
Playwright WebKit 27.2 on macOS, 10-take edit (53.08 s), 4 plays each, playhead read from the runtime on every frame:
3-take edit (24.8 s): main 3, 2, 3, 11 stops (two plays froze for 40 s), this PR 0, 0, 0, 0.
Chrome 156 on Linux, 10-take edit, 3 plays: median playhead hold per cut -5.8 ms on main and -3.3 ms here, cuts with a hold over 100 ms 0 [0-2] on both. Unchanged.
Not proven
This is Playwright's WebKit, not Safari itself, and not an iPhone. Safari's WebDriver needs "Allow remote automation" turned on, which needs an admin password. A trace from a real Safari and iPhone on the release would close it. The runtime suite (2645 tests) passes, including new clock tests for a frozen just-played element and a seeking element.