Repository navigation
fix(core): retain complete preview pictures during native seeks - #7378
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
Edit accuracy: accurate 267 (base branch 267), smooth 95 of thoseThe gate passes. Quarantined, measured but not gated (8)
Unstable (2)
|
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Review at 9d5b242: looks good to merge on its own. CI is green (89 pass, 11 skipped).
Paused seeks hold every visible native video as a snapshot and keep timeline, captions and scenes at the old time. Everything commits together once each video is readyState ≥ 2, not seeking, and within 0.02 s. That tolerance matches syncRuntimeMedia's forceSync threshold, so it can't get stuck on a mismatch. Rapid seeks coalesce. Play, renderSeek, onStopMedia and teardown all cancel.
Non-blocking:
previewSeek.ts:8,136: at the 5 s cap it commits the mismatched picture anyway after a 5 s freeze. That's a fine fallback, but long next to the 1 s image-hold cap.- holdVideoFrame inserts the snapshot as the video's previous sibling with copied styles, which can briefly shift
video + x/:first-childselectors. Low risk. colorGrading.tsdrawableFrame returning null during the hold freezes graded layers too. That's coherent.
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 2 important · 0 nits · head 9d5b242c
Adds to @miguel-heygen's review, which already covers the 5 s cap fallback and the sibling-selector shift. These are the gaps it doesn't mention.
- Merge order (UX): on its own, this PR never commits an intermediate picture while the user is dragging.
finish()re-runs for the newest destination and doesn't publish, so the picture stays frozen for the whole drag (the 0.700 s still in #7379's Before capture). Ship it together with #7379, or port #7379's "publish the intermediate if it's ready" branch into this PR.
Scope
Read previewSeek.ts and the init.ts changes end to end. CI is green on this head. Local core runtime suite: 2881/2881 passed. Findings 1–2 were confirmed with scratch vitest repros (not committed).
— HyperFrames Team Review Agent (Genesis)
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Correction to my earlier note: I no longer think this is ready to merge alone. ready() in previewSeek.ts:75-85 treats an element as done only if it is disconnected, el.error != null, or readyState ≥ 2 at the target. A video whose sources never load (for example, a missing <source> with no src) sits at NETWORK_NO_SOURCE with error === null and readyState 0. So every paused seek waits for the 5 s cap, where main lands immediately. It should also count networkState === NETWORK_NO_SOURCE (and perhaps NETWORK_EMPTY) as done, plus add a test.
9d5b242 to
4ffd675
Compare
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Re-review at 4ffd675: looks good to merge. Each claimed fix is confirmed by a test that fails when the change is reverted:
- failed sources settle (
isUnplayablecovers NO_SOURCE, and the capture-phase error listener catches<source>errors) - newer clip selections supersede stuck work
- complete intermediate pictures publish
- image waits keep the hold
CI is green (89 pass, 11 skipped).
Non-blocking:
- A newer seek within the same clips still waits for the stuck run's cap.
- No test covers
canplay. - The cap still commits a mismatched picture after 5 s.
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>
The 1a9bb89 port of heygen-com#7402 and heygen-com#7378 brought in releaseMedia(el, flag) with window.__HF_RELEASE_PASSED_MEDIA (heygen-com#6929, needs the heygen-com#6785 buffered guard) and registerSeekCompletion(landed, "image-decode") (the SeekHold union from heygen-com#6596). Both upstream commits stay deferred, so typecheck:runtime failed in the Windows render verification workflow. The fork's releaseMedia already frees every clip, so the call keeps its behaviour. Add tsconfig.runtime.json to the pre-commit typecheck, which only covered tsconfig.json (it excludes src/runtime), and pin it in the hook test. Co-Authored-By: Claude Sonnet <noreply@anthropic.com>
* fix(skills): await the player seek in seam-gate heygen-com#7378 made __player.seek() asynchronous (it resolves once the preview picture is ready). seam-gate.mjs seeked and read in the same tick, so every sample measured t=0: on a correct film each seam failed with exit-visible op 0.00 and exit-moving 0 px/s, and probe found no movers. Make the page-side seek/sample/scan helpers async and evaluate them with awaitPromise, and note the false-failure signature in the reference doc. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(skills): await the player seek in changelog-video spot checks Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
What
A paused seek across stacked video cuts keeps the previous complete video and HTML picture visible until all destination layers have decoded. This prevents captions over black and a transparent person layer appearing without its background.
Why
The composition clock and clip visibility previously advanced before independent native media seeks finished. Moving across cuts could also release and reopen sources, making those incomplete pictures visible.
Related work
First of two smaller PRs replacing the combined scope in #7346. The follow-up branch
fix/full-quality-webcodecs-scrubbingtargets this branch and adds the optional decoded-frame provider and Studio integration. This PR is independently usable and does not install WebCodecs or activate the provider. Merge this PR first; then retarget/rebase the follow-up onto main.How
Test plan
This addresses native DOM/graded-video seek presentation. The optional provider and graphics-consumer compatibility belong to the follow-up. No merge, release, Canary bundle change or private project upload is part of this PR.
Before
Original seek behavior on the same generated test-pattern and alpha-circle composition: HTML reaches 6.367 seconds while media layers are blank. This reference capture is from the original baseline used for the investigation; the unit regression above was separately reproduced against current main.
After
Fresh capture from this PR on current main: the requested timeline is at 6.367 seconds while video and HTML both retain the complete 0.467-second picture during decoding.
All capture media is synthetic. No user footage, images or voice are included.