Skip to content

fix(cli): recapture snapshot composites after video injection - #4767

Open
Dante-dan wants to merge 2 commits into
heygen-com:mainfrom
Dante-dan:fix/4762-snapshot-video-readiness
Open

Dante-dan wants to merge 2 commits into
heygen-com:mainfrom
Dante-dan:fix/4762-snapshot-video-readiness

Conversation

@Dante-dan

Copy link
Copy Markdown

What

Recapture page-side VFX composites after decoded video frames have been injected into a snapshot.

Why

A snapshot seeks and waits for the VFX preview capture before injecting decoded FFmpeg frames. The injected overlay updates the DOM, but the VFX output canvas still holds its earlier capture of the native video. Under load that capture can contain frame zero.

Related work

Fixes #4762. The engine already runs a post-injection page-composite protocol; this brings snapshot capture into line with it.

How

After injecting frames and syncing visibility, run the existing prepare → paint → resolve protocol. A one-pixel screenshot makes the updated subtree's paint records available before VFX captures them. Pages without a page compositor skip the extra screenshot. This adds no sleep or native-video decode heuristic.

Test plan

  • bun run lint passed.

  • bun run format:check passed.

  • Existing snapshot and captureCompositionFrame suites: 72 tests passed.

  • bun run --filter @hyperframes/cli typecheck passed.

  • Repository pre-commit checks passed: core/studio/scripts typecheck, lint/format, tracked artifacts, and fallow audit (no newly introduced findings).

  • The initial whole-workspace typecheck failed on missing dependencies in the reused checkout (acorn, sharp, @puppeteer/browsers, dockview-react, @base-ui/react). Dependencies were then refreshed; the CLI typecheck above is the subsequent scoped check.

  • The issue's exact synthetic-bar reproduction was attempted, but local Chrome failed to launch (Code: null, empty stderr), so this does not claim a local under-load pixel reproduction.

  • Unit tests added/updated

  • Manual testing performed

  • Documentation updated (if applicable)

  • Comments follow CONTRIBUTING.md "Comments"

@vanceingalls

Copy link
Copy Markdown
Collaborator

@Dante-dan thanks, the approach looks right: re-running the existing prepare → paint → resolve protocol after injection, with no sleeps, fits how the engine does it. Two things before this merges:

  1. Verification. The PR says the repro couldn't run locally. Please run the issue's moving-bar repro under load, before and after. The bright columns should be at 800–1099 at --at 5, not 0–299. If Chrome still won't launch for you, say so and I'll run it on my side.
  2. A unit test. The added block has none. A test that the recapture runs only when __hf_page_composite_resolve exists, and runs after frame injection, would keep it from regressing.

Also a question: is the 1×1 JPEG screenshot what makes the updated subtree's paint records available? A short comment saying why it is needed would help the next reader.

@Dante-dan

Copy link
Copy Markdown
Author

Added the requested regression coverage: it exercises prepare → paint → resolve only when __hf_page_composite_resolve exists, and checks that the snapshot call is after decoded-frame injection and visibility synchronization and before the final PNG capture. The 1×1 JPEG is a paint barrier: it forces Chrome to publish the updated subtree paint records for resolve to consume. I added that explanation next to the screenshot. The two relevant test files pass (75 tests), along with CLI typecheck and changed-file lint/format checks.

I retried the original moving-bar fixture with --at 5 --no-end --no-browser-gpu after rebuilding a missing local dependency. Chrome still cannot launch here: Failed to launch the browser process: Code: null, with empty stderr. No screenshot was produced, so I cannot report before/after under-load results or verify the 800–1099 bright columns. Please run that pixel check on your side as offered.

@vanceingalls

Copy link
Copy Markdown
Collaborator

@Dante-dan thanks for the test and the comment. I ran the pixel check on my side, since Chrome won't launch for you. Each side was a real install and build, running its own built CLI: base = this PR's merge-base 9a27b9f93, head = 7ca980bf7 (the diff between them is snapshot.ts and snapshot.test.ts only). The fixture is the issue's moving-bar clip, snapshot . --no-browser-gpu --at 5 --no-end, judged by bright column range (800–1099 correct, 0–299 frozen at frame 0), base and head interleaved each round.

cell correct frozen at frame 0 load avg
base + capture canvas 0 of 10 10 of 10 55–70
head + capture canvas 10 of 10 0 of 10 57–76
head + plain div 10 of 10 0 of 10 55–90
base + plain div 5 of 5 0 of 5 75–94

The fix works: base froze every time, head was correct every time, and the plain-div control never froze on either side. Caveat: the machine was never idle (load 55–115 from other sessions), so every run was under load. I didn't test an idle machine.

From reading the diff, nothing blocks this. One nit, not a blocker: as Window & {…} is a type assertion, but snapshot.ts already uses the same pattern near line 463. It also runs once per frame, including frames with no video, but it returns early without a screenshot when the page has no compositor, so only compositor pages pay for it.

This branch has not been deployed

No deployments
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.

snapshot can capture a <video> inside a vfx capture canvas before its seeked frame decodes (shows frame 0; rate rises with load)

2 participants