fix(engine): hold finite video slots past EOF - #3962
Merged
Merged
Conversation
Allow explicit finite non-looping video slots to use the existing held-final-frame extraction path when data-media-start is at or beyond source duration. Warn during project lint for local static slots that will use this behavior. Co-authored-by: miguel.sierra <229591595+miguel-heygen@users.noreply.github.com>
miguel-heygen
approved these changes
Sep 15, 2026
miga-heygen
approved these changes
Sep 15, 2026
miga-heygen
left a comment
Contributor
There was a problem hiding this comment.
Reviewed head 59041d4 in an isolated worktree. Engine videoFrameExtractor.test.ts 158/159 here (the one failure, "unindexed negative-base MPEG-TS", fails identically on the base commit under this host's static ffprobe, so it is environmental); lint project.test.ts 37/37.
Strengths
videoFrameExtractor.ts:1018-1021— one predicate,canHoldFinalFramePastEof, decides eligibility at all three sites (:1126,:1208-1209,:1755), so loop and open-ended slots cannot drift apart between the planner and the guards.:1126-1137— the new branch sits after thesourceRemaining > 0held-tail branch and only reaches whensourceRemainingis 0, so every window withmediaStart < playableDurationtakes exactly the code it took before. Disabling the branch fails the new unit test and the decoded-frame integration test; nothing else moves.:1174-1178—Math.max(sourceRemaining, window.durationSeconds)is what makes the async resolver safe for a negativesourceRemaining; the emitted window still uses the probedextractionMediaStart: finalFrameTimestampandfinalFrameOnly: true, so the held frame is the real last decoded frame and the cache/dedupe identity (:1860"final-frame"transform,:2086key) is the same one the just-inside-EOF path already uses. Sharing that entry is correct: both slots want the same frame.- Guard mutations: dropping
!video.loopfails "rejects a looping explicit slot that starts at source EOF"; droppingNumber.isFinite(timelineDuration)fails both pre-existing "rejects a media start at … EOF" tests (theirvideo()fixture is open-ended), which also confirms the open-ended slot still errors as before. Flipping the lint predicate to<=fails themediaStart === durationcase; changing severity toerrorfails both warning tests.git diff --numstatshows the two test files are additions only (+70/0, +104/0).
Important (test coverage; the code is right by reading, so not blocking)
videoFrameExtractor.ts:1755— the SDR-in-HDR-timeline preflight guard now lets held-past-EOF slots through, but reverting it to the oldmediaStart >= playableDurationleaves the whole suite green. The integration test is all-SDR, so this branch (:1732hdrInfo.hasHdr && dominantTransfer) never runs. Without a mixed SDR/HDR fixture the render-side fix is unpinned on exactly the path where the old guard used to reject.project.test.ts:654-666— the "silently skips open, looping, remote, variable, missing, and unprobeable slots" case mocks ffprobe to fail for every file, so all six videos are dropped by the failed probe, not by their exclusion. Removing theloopexclusion athevcPreviewLint.ts:184keeps the test green; the same holds for the open-slot anddata-var-srcexclusions since they share the fixture. Give that test a succeeding probe (duration 2) and assert[]; then each exclusion is what suppresses the warning. Remote and missing are excluded before the probe, so they are covered as written.
Nits
hevcPreviewLint.ts:238-290— every local video is now probed twice percheck(HEVC codec, then duration). One-show_entries stream=codec_name,duration:format=durationcall could feed both rules.hevcPreviewLint.ts:184-190— slots authored withdata-endbut nodata-durationare skipped. Consistent withdata-durationbeing canonical, just noting the warning is best-effort there.
Verdict: APPROVE
Reasoning: Render now matches the runtime's hold for finite non-looping slots past EOF through the existing final-frame path, loop and open-ended slots still error exactly as before, and every planner change fails a named test when reverted; the two gaps are fixtures that do not discriminate, not behaviour.
— Miga
valeriangalliat
added a commit
to ArcadeHQ/hyperframes-next
that referenced
this pull request
Sep 15, 2026
valeriangalliat
added a commit
to ArcadeHQ/hyperframes-next
that referenced
this pull request
Sep 15, 2026
valeriangalliat
added a commit
to ArcadeHQ/hyperframes-next
that referenced
this pull request
Sep 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3958.
Summary
data-media-startat or beyond playable EOF through the existing held-final-frame extraction path.video_media_start_at_or_past_eofproject-lint finding for probeable local slots, and additive extraction-window and decoded-frame coverage.Validation
bun run --cwd packages/engine typecheckbun run --cwd packages/lint typecheckbunx vitest run src/services/videoFrameExtractor.test.ts -t 'resolveVideoExtractionDuration|renders the same final decoded frame just inside and past EOF'— 30 passedbunx vitest run src/project.test.ts— 37 passedbunx oxlintandbunx oxfmt --checkon the five changed filesbunx fallow audit --base origin/main --fail-on-issues— no new issues in the changed filesThe wider engine test file has seven pre-existing VFR integration failures on this host because FFmpeg 4.4 does not support its existing
-fps_modeoption; the new CFR held-frame integration case passes on the same host.Evidence
checkandrendercommands could not run: host Node isv20.20.2, whilehyperframes@0.8.40requires Node>=22. Exact check transcript and render transcript are retained.checkpasses with the expected warning; current-sourcerenderproduces a 6.0-second video after extracting one frame.Commit-pinned references
The main-branch rejection is at the early planner guard and the HDR preflight guard.
The changed head code is held-tail eligibility and planner routing, HDR preflight parity, extraction-window tests, decoded-frame integration test, project-lint aggregation, warning invocation, warning rule, and warning tests.