Skip to content

fix(core): Safari keeps playing edits' audio without seeks or rate changes - #6888

Merged
miguel-heygen merged 7 commits into
mainfrom
fix/webkit-audio-seek-loop
Nov 1, 2026
Merged

miguel-heygen merged 7 commits into
mainfrom
fix/webkit-audio-seek-loop

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

What

In WebKit, playing audio is no longer steered by rate and is seeked only when it holds more than 0.65 s behind the playhead. Every other browser keeps today's steering untouched; the player's audio proxies share the rule. The same PR adds the check that holds it: webkit-stops.ts --browser safari plays the 3-take and 10-take edits in the real Safari through safaridriver, and a macos-latest workflow runs both edits with --assert.

Why

WebKit reports a just-played or just-seeked <audio>'s start time for 150 to 400 ms before output begins. Any seek or rate change aimed at that lag lands the audio behind by the same amount again. #6854 cut the loop but real Safari still froze the sound 10 to 99% of the time. Remotion tolerates the lag instead of steering it (never a rate change, seeks at 0.65 s); this does the same.

Numbers

Real WebKit (macOS 26.5 system WebKit in a WKWebView), the 3-take edit (24.8 s), 5 plays each, playhead and every playing <audio> sampled each frame. Frozen = share of playing-audio time currentTime stood still for over 50 ms. Medians, with the range in brackets.

audio seeks frozen audio
main (with #6854) 6 (2 to 12) 5.5% (1.0 to 75.1)
this PR 2 (1 to 5) 2.3% (2.1 to 14.3)
Remotion 4.0 <Player> 6 (5 to 12) 6.9% (6.0 to 21.4)

Media served over a slow link in this run. With the media on the same machine (5 plays): main 6 seeks and 11.5% frozen, this PR 2 seeks and 3.2%, Remotion 3 seeks and 0%. What remains in this PR is WebKit's own start delay at each of the 3 audio starts (about 250 ms each), which Remotion does not show on the same file. Not fixed here.

10-take edit (53 s), media on the same machine, 5 plays: main 18 seeks (16 to 27) and 13.9% frozen, this PR 9 seeks and 4.5%. Remotion's page only builds the 3-take edit, so it has no 10-take row.

Chrome 156 (Playwright), same edit, 3 plays each: main and this PR both 2 audio seeks, 0% frozen, at most 1 playhead stop under 80 ms.

Before

One play of the 3-take edit in WebKit (the median of 5 by frozen time), drawn from the audio element's sampled time; red is the audio not advancing for more than 50 ms.

Before: main

After

After: this PR

Tests

  • fails without the fix: mediaSteer.test.ts > "never steers WebKit audio: it is seeked only once its lag holds past 0.65 s"; clock.safariAudio.test.ts > "plays through without seeking the audio it follows" (its 0.65 s bound, with a Safari user agent).
  • Core runtime suite passes (112 files); oxfmt, oxlint and core tsc clean.
  • The workflow's --assert fails a run above 15 audio seeks or 15% frozen.

Notes

In WebKit audio can now sit up to 0.65 s behind the picture where it used to be pulled back within 0.1 s. That is the trade Remotion makes. Other browsers are unchanged.

@miguel-heygen
miguel-heygen force-pushed the fix/webkit-audio-seek-loop branch from 835c49b to 3ec4752 Compare October 31, 2026 18:33
@miguel-heygen miguel-heygen changed the title test(player): measure audio seeks and frozen sound in real Safari test(player): play the WebKit audio edits in real Safari on a macOS runner Oct 31, 2026
@miguel-heygen miguel-heygen changed the title test(player): play the WebKit audio edits in real Safari on a macOS runner fix(core): Safari keeps playing edits' audio without seeks or rate changes Oct 31, 2026

@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: REQUEST CHANGES · 1 blocker · 1 important · 0 nits · head 65493212

  • blocker — PR body has no fails without the fix: line. The Tests section says mediaSteer.test.ts and media.test.ts fail on main; name the exact tests on that line (e.g. mediaSteer.test.ts › "never steers audio: …", media.test.ts › "leaves a playing audio lagging past the sync tolerance at its rate, unseeked").
Scope

Read the full diff: mediaSteer.ts, the markMediaRestarted removals in media.ts (no other callers left), unit tests, webkit-stops.ts safaridriver path and --assert, the new macOS workflow (actions SHA-pinned; brew install ffmpeg unpinned, same as the existing Safari perf job). CI at review time: CI, regression, Windows, Player Safari audio/perf still running.

Comment thread packages/core/src/runtime/mediaSteer.ts Outdated
if (settled && lag <= MEDIA_SYNC_TOLERANCE_SECONDS) catchingUp.delete(el);
const seekable = !catchingUp.has(el) || lag > MEDIA_HARD_SYNC_SECONDS;
if (audioOnly && settled && lag > AUDIO_STEER_LIMIT_SECONDS && seekable) {
if (audioOnly) {

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.

important — this loosens audio sync to 0.65 s in every browser, not just WebKit. In the PR's own Chrome numbers, main already had 0% frozen and 2 seeks, so Chrome users get up to 0.65 s audio/picture drift (≈16 frames — visible lip-sync slip on a voiceover track over avatar video) and nothing in return. clock.safariAudio.test.ts is loosened from 0.1 to 0.65 to match. Consider gating the 0.65 s threshold to WebKit, or keeping a tighter bound elsewhere. You already asked whether to add a lower bound, so this is the answer for non-WebKit browsers.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Gated: WebKit keeps 0.65 s, every other browser seeks audio past 0.1 s (no rate nudges for audio). Chrome 156, 3 plays: still 2 audio seeks, 0% frozen. Pinned by new tests in mediaSteer.test.ts. The PR body now has the fails-without-the-fix line.

@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 105 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 (0)

Unstable (2)

  • seqplainkeys-none-center-r0-nested-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true
  • seqplainkeys-none-center-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo false, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true

@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 · 65493212 → 7992aba1

2 resolved, 0 open, 0 new. The fails without the fix: line now names the tests. The 0.65 s audio-seek limit now applies only when isWebKit() is true, so Safari and iOS browsers (CriOS/EdgiOS included) get 0.65 s, while Chrome, Edge, Android and Firefox keep 0.1 s, covered by the new Chrome test. Player Safari audio/perf passed on the previous head. The non-required Comments ratchet fails on this head because of the new doc comment on isWebKit in mediaClock.ts; CI, CodeQL, Windows and regression were still running at review time.

@miguel-heygen
miguel-heygen force-pushed the fix/webkit-audio-seek-loop branch from 7992aba to 1c2531e Compare October 31, 2026 22:30

@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: REQUEST CHANGES · 1 blocker · 0 important · 0 nits · 7992aba1 → 1c2531ee

0 resolved, 1 open, 1 new: restoring main's steering outside WebKit is right, but the stricter isWebKit regex now leaves embedded WKWebViews out of the fix (inline).

Comment thread packages/core/src/runtime/mediaClock.ts Outdated
Comment on lines +15 to +16
/AppleWebKit.*(Safari|Version)\//.test(userAgent) &&
!/Chrom(e|ium)\/|Edg\/|Android/.test(userAgent)

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.

blocker — requiring Safari/ or Version/ drops every embedded WKWebView. The default WKWebView user agent is … AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148, with no Safari/ or Version/ (in-app browsers and native-app webviews on iOS and macOS). Those players take main's steering path again and keep the seek loop this PR fixes. The PR's own numbers come from a WKWebView, and the Safari CI job can't catch this because Playwright WebKit and real Safari both send Safari/. To keep jsdom out, exclude it by name:

Suggested change
/AppleWebKit.*(Safari|Version)\//.test(userAgent) &&
!/Chrom(e|ium)\/|Edg\/|Android/.test(userAgent)
/AppleWebKit/.test(userAgent) &&
!/Chrom(e|ium)\/|Edg\/|Android|jsdom/.test(userAgent)

Also add a test with a bare WKWebView user agent.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed: isWebKit() is now AppleWebKit and not Chrome/Chromium/Edg/Android, no Safari token needed. mediaSteer.test.ts has a case with a default iPhone WKWebView user agent (no Safari/, no Version/), which fails on the previous regex. The test DOMs (jsdom in core, happy-dom in player) say AppleWebKit too, so both vitest configs now pin a Chrome user agent instead of the detector learning about them.

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.

✅ Fixed in 8165392.

@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: REQUEST CHANGES · 0 resolved, 1 open, 0 new · 1c2531ee → 721a8e56

Only a constant was renamed; the isWebKit blocker (embedded WKWebViews excluded) is unchanged and has no reply.

@miguel-heygen
miguel-heygen force-pushed the fix/webkit-audio-seek-loop branch from 721a8e5 to 8165392 Compare November 1, 2026 00:22

@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: REQUEST CHANGES · 1 resolved, 0 open, 1 new · 721a8e56 → 81653923

✅ isWebKit now matches embedded WKWebViews, and a bare-WKWebView test covers it; the test setup pins a Chrome user agent, so jsdom no longer counts as WebKit.

  • blocker — the required check Studio and player captures fails on this head: the packages/player/vitest.config.ts change puts the PR under the capture rule, and the body has no ## Before/## After. Body-only fix: add a ## No visible change section (test-config only) and re-run the check. The code is fine; I'll approve once that check passes.

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

The capture check passes on 8165392: the body now has Before and After sections with images (run 41260338657, after the body edit). The failed entry in the rollup is the earlier run on the same head before the body was fixed.

@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 · 1 resolved, 0 open, 0 new · 81653923 (no code change)

✅ The body has ## Before/## After with images, and the fresh PR captures run 41260338657 passed on this head; CI, regression and Windows passed alongside it. Heads-up: the re-run of the stale run 41256465022 (attempt 2, 02:49 UTC) failed again because it replays the old event's body without Before/After. That leaves the newest Studio and player captures result red. Re-run 41260338657, or edit the body, to make the latest result green.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit 2c1a10f Nov 1, 2026
323 of 330 checks passed
@miguel-heygen
miguel-heygen deleted the fix/webkit-audio-seek-loop branch November 1, 2026 03:24
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