Repository navigation
fix(core): video starts a second early and catches up hidden, so the picture meets the cut - #7265
Conversation
There was a problem hiding this comment.
Requesting changes. Reviewed at c948b5fd.
I checked the fails without the fix: line. Swapping main's media.ts back in makes exactly the four video pre-roll cases fail (176 others pass). All 204 tests in the four touched files pass at the head.
Blocker: the most common clip gets no pre-roll, and now has no lead at all. For a pre-rolled non-loop video, relTime = clip.mediaStart - dueIn * rate. isActive then requires relTime >= 0. With mediaStart: 0 (any clip without data-media-start, the default), relTime is negative for the whole window where dueIn > 0. So the clip never becomes active early, and play() first fires on the cut tick. I probed videoClip({ start: 10, end: 20, mediaStart: 0 }) at the head, ticking 9 / 9.5 / 9.9 / 9.96 / 10. The play() call count was 0 0 0 0 1. On main, the learned lead (up to 0.1 s) still applied to these clips, because mediaSourceTimeAt clamps to mediaStart. So for default clips this PR is a regression from main, not half the lag. The "under a second in" test is the same thing in miniature: with mediaStart: 0.3, the lead shrinks to 0.3 s. The rig numbers come from clips that have source before their in-point, so they don't show this.
To fix it, give clips with little or no source before mediaStart a pre-roll that still runs to the first frame. For example, hold the first frame (seek to mediaStart) and play() it about the start latency early, which is what main did, or clamp the pre-roll target at 0 rather than dropping the clip. Then add a mediaStart: 0 case that asserts play() fires before clip.start.
Non-blocking
- The PR body still has the unfilled placeholder
Chrome: CHROME_NUMBERS.Please fill it in or remove it. - The "hidden" steering (up to 2x) depends on the visibility pass hiding the element before
clip.start. A one-line comment at thesteerMedia(..., startedEarly.has(el))call that names this assumption would help. - CI (CI, Player Safari perf/audio, regression) was still running at review time.
There was a problem hiding this comment.
Requesting changes. Reviewed at 95f984dc.
The earlier blocker is fixed. A mediaStart: 0 clip now gets play() 0.25 s before the cut. At this head all 207 tests in the touched files pass. With main's media.ts, 6 pre-roll cases fail, so the fails without the fix: line still holds.
Blocker: a fast-starting mediaStart: 0 clip reaches the cut more than 0.1 s ahead, and the lead hold then pauses it. For these clips the target is clamped at 0 until the cut. If the element starts quickly, it runs ahead while hidden, and the hidden steering's 0.5x floor can't hold it back enough. At the cut startedEarly is cleared, so offset < -VIDEO_LEAD_HOLD_SECONDS applies again and the element is paused. The picture freezes, and resuming pays the start latency again. That is the failure this PR exists to remove.
I simulated it at the head: videoClip({ start: 10, end: 20, mediaStart: 0 }), 60 Hz ticks, a 40 ms start latency, and the element advancing at its steered rate. Results:
- 9.983: currentTime 0.136, rate 0.5
- 10.000: currentTime 0.145,
pause()called
A 40 ms start is the low end of the WebKit range in the PR body. It is also typical for Chrome, and the pre-roll isn't WebKit-gated. So this likely hits Chrome on every from-file-start cut.
Suggested fixes, any one of these:
- Keep the lead-hold exemption for a pre-rolled element until it first comes within tolerance after the cut.
- Let the hidden steering slow below 0.5x, or pause on the first frame, while the target is clamped at 0.
- Shrink the minimum lead so the worst-case run-ahead stays under
VIDEO_LEAD_HOLD_SECONDS.
Please add a test that plays a mediaStart: 0 clip that starts quickly through the cut and asserts that pause() is not called.
Non-blocking
- The PR body still has
Chrome: CHROME_NUMBERS.(unfilled). - The
fails without the fix:line still says "the four" cases. It is now six.
Edit accuracy: accurate 267 (base branch 267), smooth 122 of thoseThe gate passes. Quarantined, measured but not gated (8)
Unstable (2)
|
There was a problem hiding this comment.
Requesting changes. Reviewed at e3fa1bb9.
The second blocker is fixed. A mediaStart: 0 clip that starts instantly or in 40 ms now plays through its cut without pause(). The new it.each covers this. All 209 tests in the touched files pass.
Blocker: default clips now get half of main's lead, so on Safari they likely trail the cut more than on main. VIDEO_MIN_LEAD_SECONDS = VIDEO_LEAD_HOLD_SECONDS / 2 gives a mediaStart: 0 clip a 50 ms lead. Main's learned lead is capped at 0.1 s. With the ~0.2 s mean WebKit start latency in the PR body, main sits at that cap. So the most common clip loses about 50 ms of lead compared with main. The body has no mediaStart: 0 measurement that shows otherwise.
Halving isn't needed to avoid the pause. With the hidden steering (rate = 1 - 5x, floored at 0.5x), an instant start over a lead L ends up about 0.2(1 - e^(-5L)) ahead at the cut. For L = 0.1 that is about 0.08 s, under the 0.1 s hold. I set VIDEO_MIN_LEAD_SECONDS = VIDEO_LEAD_HOLD_SECONDS locally. Your new plays a from-file-start clip ... through its cut without pausing it cases still pass for 0 s and 0.04 s. They assert one play() and no pause().
To clear this, either:
- Use the full
VIDEO_LEAD_HOLD_SECONDS(or anything below ~0.13 s) and keep theit.eachas the guard, or - Add a rig number for a
mediaStart: 0cut showing it is no worse than main.
Non-blocking
- The PR body still has
Chrome: CHROME_NUMBERS., unfilled.
|
On the lead for
So 50 ms is no worse than main on this film, and 0.1 s is about 250 ms worse. On WebKit a from-file-start clip starts almost at once, so it reaches the cut at or past the 0.1 s hold, the hold pauses it, and it pays its start latency again: that is the failure the hidden steering cannot undo in time, and it is why the lead is half the hold. The 0.2 s mean in the body is for clips seeked into the middle of a file; those do not use this lead. Chrome (Chrome 113, same film): main 25, 8; this head -1, 0, 6. The body has the table and the |
There was a problem hiding this comment.
Approving at af082998. No blockers.
The third finding is answered with measurements. With the 50 ms lead, file-start clips on WebKit measure -4 to 8 ms, against -7 and -5 on main. The 0.1 s lead I proposed measured 200 to 280 ms behind, because the hold still paused those clips at the cut. So the rig shows my estimate was wrong, and the half-hold lead is the right choice. Chrome is not slower. The CHROME_NUMBERS placeholder is gone.
At this head the four touched test files pass (209 tests). With main's media.ts, 6 video pre-roll cases fail, so the fails without the fix: line holds. The steerHidden extraction keeps the same behaviour. It drops the !audioOnly guard, but the only caller that passes hidden (isHiddenPreRoll) is already VIDEO-gated.
Non-blocking:
- The mid-file WebKit result is noisy. The final build measured 134, 102 and 272 ms, and the 272 is worse than main's ~185. The body states this honestly. A larger take count before claiming "about 45%" would help.
- This head removed the comments on
VIDEO_PREROLL_SECONDS,VIDEO_MIN_LEAD_SECONDS,isRenderDrivenand the hidden-steer call. The ones that explain why are worth keeping: why the lead is half the hold, and that hidden steering relies on the visibility pass hiding the clip before its start. - CI was still running at approval time.
…picture meets the cut
…st start is not paused
af08299 to
9c9632b
Compare
There was a problem hiding this comment.
Approving at 4261e8a2. No blockers.
The rebase did not change the code I approved. The packages/core patch against main at 9c9632bc is byte-identical to the patch I approved at af082998. All 209 tests in the four touched test files pass on the rebased tree.
The only new change is 4261e8a2, which raises the @hyperframes/player packed budget from 157145 to 158800 bytes (+1.6 KB, about 1%). That fits the runtime moving into the player bundle. CI passes on this head (CI, regression, Player perf, Safari perf/audio, Windows render, CodeQL), so the new budget holds.
My earlier non-blocking notes still apply: the mid-file WebKit numbers are noisy, and the explanatory comments were removed.
…view Adopts heygen-com#7299 (accented Latin letters draw in a bundled family at its bundled weight), heygen-com#7301 (installing Chrome on a busy Windows PC), heygen-com#7262 (cloud render logs a line a minute while a job holds its status), heygen-com#7265 (video starts a second early and catches up hidden) and heygen-com#7305 (check_report names the finding that stopped the check). Player packed budget takes upstream's +1655 from heygen-com#7265 since the player bundles core's media-steer. Defers heygen-com#7302; heygen-com#7261 and heygen-com#7263 do not apply. Watermark advances to 9e2510b. Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com> Co-authored-by: WaterrrForever <miao.yang@heygen.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…view to heygen-com#7265 Adopt heygen-com#7284 (Studio Storyboard tab and `doc: "storyboard"` Desktop handoff, adapted to the fork's desktopApp without heygen-com#6897) and heygen-com#7324 (drawElement gate: webgl_unavailable / beginframe_no_self_verify). PR axis heygen-com#7260-heygen-com#7265 carries earlier verdicts; re-pin heygen-com#6695 and heygen-com#6816, resolve heygen-com#7150 as superseded by heygen-com#5746. Watermarks: commit 18fc75d, PR heygen-com#7265, issue heygen-com#7223, monitored 99. Co-authored-by: WaterrrForever <miao.yang@heygen.com> Co-authored-by: Vance Ingalls <vance@heygen.com> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…idden pre-roll Reverts the one-second hidden video pre-roll (#7265): on Safari the pre-rolled video is still behind at the cut and trails it by 30 to 150 ms in every measured run, and some cut flashes never show.
…idden pre-roll (heygen-com#7383) * fix(core): Safari video picture lands on the cut again, undoing the hidden pre-roll Reverts the one-second hidden video pre-roll (heygen-com#7265): on Safari the pre-rolled video is still behind at the cut and trails it by 30 to 150 ms in every measured run, and some cut flashes never show. * fix(core): shorten a restored comment line to the 120-column limit
What
A video clip's picture now trails the cut by less in Safari: about 100 ms instead of about 185 ms on clips cut from the middle of a file, and unchanged (within 10 ms) on clips that play from the start of their file. Chrome is unchanged.
Why
On WebKit a muted video starts 0.04 to 0.4 s after
play()(30 starts measured, mean about 0.2 s) when it is seeked into the middle of a file. The runtime pre-started the next clip by a value learned from earlier starts and capped at 0.1 s, so the picture trailed the timeline at each cut and then caught up by rate steering over about a second. The latency varies by about 0.1 s per cut, so a learned or fixed lead only moves the error between early and late.How
data-media-start0, the default) starts on its first frame 50 ms early, which is half ofVIDEO_LEAD_HOLD_SECONDS: a clip that starts instantly must stay under the hold, or the hold pauses it at the cut and it restarts late.Numbers
Rig: system WebKit on a Mac (muted) and Chrome 113 headless on a Linux box. Picture lag at each cut = timeline time at a frame's display time minus the frame's own source-implied time, median over takes 2 to 10 (clips 2 to 6 on the second film), first 0.5 s of each take; + means behind. One play = one number.
The mid-file WebKit result is noisy: the final build alone measured 134, 102 and 272 ms, so the gain is about 45% of the lag, not all of it, and short of a +-45 ms bar. Chrome's mid-file set has two plays per side (the runner cut the batch).
What did not work, so it is not retried: raising the 0.1 s cap alone; a fixed lead; letting a pre-started clip skip the lead hold; steering on the last presented frame's time instead of
currentTime(306, 150 and -575 ms, with picture stalls); landing the pre-seek at the pre-roll position instead of the first frame (242, 258, 171 ms); a 0.25 s or 0.1 s lead on clips that start at the file's start (about 200 to 280 ms behind on WebKit, because the hold paused them at the cut).Tests
media.test.ts(including a from-file-start clip that starts instantly or in 40 ms and must reach and pass its cut withoutpause()) and hidden-steering cases inmediaSteer.test.ts; they fail against main'smedia.ts. The learned-lead cases they replace are deleted.fails without the fix: the
video pre-rollcases inmedia.test.tsfail against main'smedia.ts.