Skip to content

fix(core): Safari video picture lands on the cut again, undoing the hidden pre-roll - #7383

Merged
miguel-heygen merged 2 commits into
mainfrom
rsafari/picture-preroll
Nov 7, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
rsafari/picture-preroll

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What

Undoes #7265. On Safari the video's picture lands about 100 ms behind the cut (iPhone Simulator) and 30 to 50 ms behind (desktop Safari) with the one-second hidden pre-roll, and on the Simulator 3 to 9 of the 44 scored cues per run show no picture flash at all (0 or 1 without it). Without it the picture is on the cut or one frame early in 2 of 3 Simulator runs and 1 of 3 desktop Safari runs, and about 100 ms late in the others. Chrome is the same either way. Sound timing is not touched (identical before and after).

Why

#7265 was measured by the timeline-versus-frame display time inside the page. Measured on what the machine outputs (screen buffers and system audio with their own timestamps), the picture on the pre-roll build trails the timeline in every run on Safari. The pre-rolled video plays hidden for up to a second while steered at up to twice its speed, but in the Simulator's per-frame trace it advances at about 1.3x, not 2x, after a 150 to 400 ms freeze following play(), so it is 175 to 820 ms behind at the cut and the visible-phase steering (at most 10%) then closes it over one to two seconds.

Related work

Refs #7265. Not a full fix of the picture trailing a cut: the pre-#7265 timing is bimodal (about on time or about 100 ms late, run to run), and this does not change that; it removes a build that is late every run.

How

git revert of #7265, no conflicts: the learned start latency (capped at 0.1 s) returns, the hidden pre-roll and its steering go.

Numbers

Rig: screen buffers and system audio of the running browser, 321 s source of ten takes, a 3-frame magenta start and a 20 ms click at each of 50 cues, a green patch while the player's public time is inside a cue. Picture lag = magenta onset minus the playhead's green onset at each cue, takes 2 to 10 (44 cues per run); + is late. Run medians, runs interleaved, load average 5 to 15 (the Mac was shared).

engine this PR (undone) main (0.9.20)
iPhone Simulator (iOS 26.5) +87, -33, -17 ms +108, +108, +125 ms (and +108..+150 in 6 more runs)
desktop Safari (safaridriver) -17, +58, +50 ms +50, +29, +46 ms (and +17, +42, +50 in 3 more runs)
Chromium (headed) -17, -17, -17 ms 0, 0, 0 ms

Chromium differs by one video frame (40 ms) in the other direction, both within one frame. The Simulator runs of the previous release (0.9.19, before #7265 and the echo fix) read +108, -17, +83 ms, so the undone build is a return to that behaviour. The echo fix (#7309) is untouched: extra click onsets stay at 0 of 50 on both sides.

Test plan

fails without the fix: 11 tests in media.test.ts (start latency: does not start an upcoming video early before any start is measured, starts an upcoming video early by the start latency measured on earlier ones, caps the learned start delay at 100 ms and 5 more in that block, "starts nothing early while paused, and never a video"), media.preseek.test.ts (lands a video clip's in-point before the cut without playing or seeking it again at activation) and init.videoPreseek.test.ts fail against main's media.ts.

…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.
@miguel-heygen
miguel-heygen force-pushed the rsafari/picture-preroll branch from c6a14b6 to 48ebce3 Compare November 7, 2026 21:37
@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 267 (base branch 267), smooth 101 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (8)

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE · 0 blocker · 0 important · 0 nits · head 48ebce32

Scope
  • Diffed the change against a straight revert of #7265 (3e86def5). The content is the same, line for line, except for the comment lines trimmed at 48ebce32. From c6a14b6b to 48ebce32 only comments changed.
  • startedEarly, isRenderDriven, steerHidden and HIDDEN_GAIN: no leftover uses. startedEarly itself predates #7265 and stays.
  • Main changed media.ts after #7265 (#7355, #7368). The revert applies to those changes without conflict, and Typecheck and Test pass.
  • The player size budget goes back to its value before #7265, and CI passes with it.
  • "fails without the fix" names real tests that fail on main: the learned-latency and preseek tests, which import resetVideoStartLatency.
  • The Safari timing figures in the body come from the author's own runs. I didn't reproduce them.
  • CI at this head: CI, Comments, CodeQL, Player Safari audio, Player perf, PR captures and preview-regression pass. regression, Windows render and Player Safari perf were still running when I posted.
  • No prior reviews.

— genesis-v2

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE · 0 open, 0 new · 48ebce3 → 25c753d (only one comment line in media.ts changed; CI still running at this head)

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 7, 2026
Merged via the queue into main with commit 841e90f Nov 7, 2026
98 checks passed
@miguel-heygen
miguel-heygen deleted the rsafari/picture-preroll branch November 7, 2026 22:27
@miguel-heygen miguel-heygen mentioned this pull request Nov 7, 2026
3 of 5 tasks
SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 9, 2026
Port the adopted upstream fixes from f5bde95 to the current upstream tip:
heygen-com#7375 multipart cloud upload, heygen-com#7382 minParallelChunks, heygen-com#7349 HLG header SEI,
heygen-com#7369 skip unmounted compositions in lint/check, heygen-com#7381 filesystem-path media
in the cloud archive, heygen-com#7383 Safari cut-point pre-roll revert, heygen-com#7373 clock
steering between audio time updates, heygen-com#7384 injector test isolation, heygen-com#7378
preview seek retention, heygen-com#7390 virtual timer schedule per worker, heygen-com#7394 CDN
script failure naming, heygen-com#7395 menu row radius, heygen-com#7119 decode timeout caller
test, heygen-com#7397 authored tween order, heygen-com#7402 capture page clip loading, heygen-com#7412 muted
video audio track at cuts, heygen-com#6150 template root variable test, heygen-com#7442 held tail
seek once.

Deliberate local differences: heygen-com#7067's audio-start hold is not carried into
clock.ts, mountedSources comes from heygen-com#6827 as a helper only, cloud/_gen/types.ts
omits the image-request types the fork does not generate, and the lint
symlink test skips through symlinkOrSkip where the account cannot create links.

Co-Authored-By: Claude Sonnet <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant