Skip to content

fix(core): skip a video the browser can't decode instead of freezing the film - #7355

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/runtime-skip-undecodable
Nov 7, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
fix/runtime-skip-undecodable

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

What

A video the browser stopped loading after its header no longer freezes the whole film in preview. The runtime now skips such a clip, plays the rest, and logs one console.warn naming it.

A clip counts when, 5 s after it was first asked to play, it is still at HAVE_METADATA with networkState NETWORK_IDLE and has never had a frame of its current source. A clip that is still loading (NETWORK_LOADING) never skips, however long it takes. A clip whose play() the browser refused (NotAllowedError, waiting for a tap) never skips either: each refusal restarts its 5 s.

Why

On iOS Safari (iPhone 17 Simulator, iOS 26.5), a film with VP9 WebM clips never played: the playhead stopped at about 0.04 s and stayed. That Safari answers "" for video/webm; codecs="vp9", yet with a plain src it reads each WebM's header (480 px wide, no error), sets networkState to idle and never delivers a frame. The runtime only treated an element as unplayable on an error or NETWORK_NO_SOURCE, so it waited on those clips forever and held the whole film.

A typed <source> the browser rejects is already dropped by the browser (NETWORK_NO_SOURCE), so it needed no change. The gap is a source with no type: canPlayType("video/webm") answers "maybe" there, so the file type alone can't tell.

How

  • isUnplayable in packages/core/src/runtime/media.ts, the check every playable-or-not decision routes through (the transport's buffering hold, the play() guard, the iOS tap priming), adds the header case and warns once per element.
  • markPlayRequested records when an element was first asked to play; loadeddata marks it as having had a frame and emptied (a new source) resets it. A NotAllowedError from play() forgets the ask, so a clip waiting for a tap keeps being primed and retried as on main.
  • Render mode is untouched: the render path doesn't ask elements to play.

Test plan

  • Unit tests added in play() storm guard (unplayable elements), all on fake timers:
    • skips a clip whose fetch stopped at its header 5 s after it was asked to play: not at 4999 ms, yes at 5000 ms, one warning naming the clip.
    • keeps waiting on a clip still loading at its header 5 s after it was asked to play: 60 s, no skip, no warning.
    • keeps waiting on a clip at its header whose play() the browser refused: a NotAllowedError, then 5 s, no skip, no warning.
  • Fails without the fix: with isUnplayable reduced to main's rule, the header test fails; with the NETWORK_IDLE condition removed, the still-loading test fails; with the refusal reset removed, the refused-play test fails.
  • media.test.ts and mediaProxy.test.ts pass 3 runs in a row; the whole src/runtime suite (120 files, 2861 tests) passes on Linux; core tsc --noEmit clean.
  • iPhone 17 Simulator (iOS 26.5), the player perf bench's heavy-alpha-webm film, 3 plays at the first head: before, the playhead held at 0.03 to 0.07 s in every play; after, it reaches 10.0 of 10.0 s each time, with one skip warning per WebM clip. The later change only adds the refusal reset (the Simulator run allows autoplay, so no play() was refused) and removes a branch that never fired there; the Simulator run was not repeated.

The film still waits 5 s at each start where such a clip falls due (here at 0 s and about 3 s) before skipping it.

Size

Small on purpose: one shared check gains one case, the play() rejection handler resets it on a refusal, and three tests.

Size: a lone fix. A video the browser cannot decode freezes the whole film, and no open PR in the player can carry it.

@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 267 (base branch 267), smooth 104 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: REQUEST CHANGES · 1 blocker · 1 important · 0 nits · head 6cca23b9

The 5 s header rule can't tell an undecodable clip from one the browser simply refused to start, and the codec-map branch can't fire with any map the server produces. Details inline.

Scope

Read all 4 files, plus the callers of isUnplayable (media.ts sync and primeMediaForLaterStarts, init.ts isPlayableTrack), maybeProxyProactively, and the codec map producer (studio-server/src/helpers/mediaCodecMap.ts, mediaProxyPreview.ts). CI was still running at review time. The fails without the fix: line is present.

}
}

function stoppedAtMetadata(el: HTMLMediaElement): boolean {

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: a clip whose play() was rejected with NotAllowedError meets this test too. On iOS the element sits at HAVE_METADATA + NETWORK_IDLE until a play is allowed. The 50 ms retry never resets firstPlayAskedAt, and loadeddata never fires, so 5 s later the clip counts as unplayable. Nothing clears that until emptied. Where the player doesn't promote to parent proxies (shouldPromoteMediaAutoplayFallback is false for the slideshow player), this happens to an unmuted clip that wasn't primed. The user's next tap can't save it, because primeMediaForLaterStarts (L225) skips isUnplayable elements and the sync stops calling play(). On main, the next tap primes it and it plays.

Fix: forget the ask when the browser refuses it, e.g. firstPlayAskedAt.delete(el) in the play().catch when name === "NotAllowedError" (L865), or start the clock only on a play() that didn't reject. Please add a test: a rejected play at header, 5 s, and isUnplayable stays false.

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 in 12cfb9b: a NotAllowedError from play() now forgets the ask (the rejection handler deletes it), so a clip waiting for a tap restarts its 5 s on every refused retry and keeps being primed. Added keeps waiting on a clip at its header whose play() the browser refused, which fails with that reset removed.

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 12cfb9b

Comment thread packages/core/src/runtime/mediaProxy.ts Outdated
swapToProxy(el, entry, "proactive");
return;
}
if (entry?.representativeMime && canPlay === "") noteCodecRejected(el);

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: no real map reaches this line. scanProjectMedia only puts browserHostile facts in __HF_MEDIA_CODEC_MAP__ (mediaCodecMap.ts:438), and representativeMime is only non-null for hostile codecs (codecFactsFor). So any entry with a MIME the browser rejects takes the swapToProxy branch above and returns. The new test uses browserHostile: false with a VP9 MIME, which the server never emits. That makes the PR body's first case ("the codec map says so") and its fails-without-fix claim for that test hold only for a synthetic input. On a real iOS VP9 map entry, the clip is proxied, as it is on main. Either drop this branch and its test, or build the test fixture from codecFactsFor/scanProjectMedia output and show the input that reaches it.

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.

Agreed, and checked: scanProjectMedia keeps only browserHostile entries and codecFactsFor sets representativeMime only for hostile codecs, so that line could not run with a real map. Dropped the branch and its test in 12cfb9b; mediaProxy.ts and its test are back to main. The header rule alone is the fix.

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 12cfb9b

@miguel-heygen
miguel-heygen force-pushed the fix/runtime-skip-undecodable branch from 6cca23b to 12cfb9b Compare November 7, 2026 08:29

@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 · 2 resolved, 0 open, 0 new · 6cca23b9 → 12cfb9b6. A NotAllowedError now clears the ask, so the 5 s clock restarts on the next retry. The new refused-play test goes red without that reset. The codec-map branch and its synthetic test are gone, and maybeProxyProactively matches main again.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 7, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 7, 2026
@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 7, 2026
Merged via the queue into main with commit 9058fd1 Nov 7, 2026
174 of 176 checks passed
@miguel-heygen
miguel-heygen deleted the fix/runtime-skip-undecodable branch November 7, 2026 09:34
@miguel-heygen miguel-heygen mentioned this pull request Nov 7, 2026
2 of 4 tasks
SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 7, 2026
…-com#7336-heygen-com#7358

Adopt heygen-com#7304 (without CLI renderDiagnostic, needs heygen-com#6725), heygen-com#7331, heygen-com#7332, heygen-com#7334,
heygen-com#7337, heygen-com#7336, heygen-com#7339, heygen-com#7289, heygen-com#5594, heygen-com#6900, heygen-com#7322, heygen-com#7342, heygen-com#7343, heygen-com#7350 (onProgress
only), heygen-com#7352, heygen-com#7348, heygen-com#7344, heygen-com#7353, heygen-com#7356 and heygen-com#7355. Defer heygen-com#5708 (needs heygen-com#6911) and
heygen-com#7345 (READ_VERSION 4 vs 7); heygen-com#7307 not applicable. Re-read 14 moved pins, pin 6
new PRs (monitored 126). Make checkReportFile module-private and suppress the
cross-package closeTestChrome duplicate for fallow.

Co-authored-by: Miguel Ángel <[email protected]>
Co-authored-by: SophiaHuangCc <[email protected]>
Co-authored-by: Dante <[email protected]>
Co-authored-by: Muzzy5150 <[email protected]>
Co-authored-by: ump45nose <[email protected]>
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
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