Repository navigation
Conversation
Collect drawElement ground-truth frames on the existing disposable verification page. This prevents verifier seeks from caching hidden state in late-starting GSAP clips on the live capture page. Close the verification page on success and failure. Addresses heygen-com#5415. Prepared with Codex; regression tests and real 3,000-frame exports are documented in the pull request.
Verify that ground-truth capture never seeks or injects into the live page, closes its disposable page on success and failure, and allocates no page when disabled. Three symptom tests fail before the engine change and pass afterward. Prepared with Codex.
jrusso1020
left a comment
There was a problem hiding this comment.
Thanks for tracking this down. The root cause in #5415 is right:
- Verification seeked the live capture page out of order, so the
tl.set(wrap, {visibility:"hidden"}, 4.75)render recordedhiddenas the start value. - Rewinding to 0 then kept the title hidden for the whole render, and every sample fell after its window, so verification still passed.
- Moving verification onto its own page is the right fix, and it matches what static dedup already does for the same reason.
There's one blocker the separate page introduces, plus two should-fixes.
Blocker: a video can be captured blank after verification
session.onBeforeCaptureis the video injector closure. It remembers the last frame it injected for each video inlastInjectedFrameByVideo(videoFrameInjector.ts:189), and skips a video whose frame index hasn't changed (:223).- That memory is per session, not per page. Verification now runs the same hook against the throwaway page (
frameCapture.ts, verification loop), then closes the page, so the memory describes a page that no longer exists. - On the first real capture frame, a video whose frame index equals the one from the last verification sample is skipped. The capture page never received that frame, so the video area renders empty, with no warning.
- Example: a short looping background video whose frame index at the 95% sample matches t=0. A scratch test against this head reproduced it: 0 injections on the capture page where 1 was expected.
- Fix: key that memory by page (e.g. a
WeakMap<Page, Map>) or reset it after verification. Please add a regression test.
Should fix
- Deduped media ids aren't injected on the verification page. It doesn't install
installMediaRenderIdBridge, which the capture page does (frameCapture.ts:1440). A video with an id likeclip__hf2therefore never gets injected into the reference frames. A sample on it then fails verification and re-renders the whole job by screenshot, about 2× the wall time. Installing the bridge increateStaticVerificationPagefixes it. - If the second page fails to load, the whole render fails. A navigation or readiness timeout throws an error the orchestrator doesn't fall back on. Static dedup catches the same failure and carries on (
frameCapture.ts:3654-3663). This path should fall back to screenshots the same way.
Worth knowing: this adds one extra full page load per drawElement session, which means per worker, at init. Comps eligible for static dedup pay two. It's fine for the fix, but the PR doesn't measure it.
Nit: the deleted comment explaining "seek 0 first, then ascending" still applies on the new page; a trimmed version is worth keeping.
Tests: the new tests pass 4/4, and 3 of them fail against main's frameCapture.ts. The related frameCapture*, drawElementService* and videoFrameInjector* suites pass (323). Merges cleanly with main. I couldn't run a real #5415 repro here because this box has no GPU that supports drawElement.
— Rames
drawElement verification now runs the session's video frame injector on a throwaway page, but the injector remembered the last frame index per video for the whole session, so the first real capture frame could skip injecting a video the dead verification page had already received, leaving its area blank. The injector now keeps that memory per page. The verification page also installs the media render-id bridge the capture page has, so deduped media ids get injected into reference frames, and a verification page that fails to load keeps the session on its launch capture mode instead of failing the render or shipping drawElement unverified. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@sulmatajb I wrote up a fix for the review points so this can land. It's one signed commit on top of your head
Engine tests (314), Could you either:
I'll approve as soon as it's in. Thanks for finding the root cause of #5415! — Rames |
What
Preserve late-starting lower thirds during default drawElement capture. A 100-second composition with the unchanged
lt-side-ruleblock mounted at 0.3 seconds currently exports without its title; with this change, the title appears while drawElement capture and its verification remain enabled.Refs #5415. This addresses the missing graphic; the report's separate observation about the debug-artifact directory is outside this patch.
Why
captureDeVerificationFramesseeks the live capture page through late sample times and then rewinds it to zero. In this reproduction, the block's delayed visibility tween captures inheritedvisibility:hiddenduring those seeks. The live page retains that state, so the subsequent render loses the title. The eight verification samples are all after the 0.3–5.1 second title window and still pass.Disabling only
HF_DE_VERIFYrestores the title. Inspecting the real page at 2 seconds shows the inner#ltelement hidden before the change and visible afterward.Related work
8bca7c4417b59b47fca851183a8d47a4565bad0c(0.8.146). Issue discussion and overlapping PRs rechecked before submission.How
Capture drawElement ground-truth samples on
createStaticVerificationPage(session), then close that page infinally. Preserve the existing sample selection, video-frame injection, double screenshots, and verification results. The live capture page is used only for the existing read-only eligibility checks.Regression tests run the actual browser callbacks in separate mocked page contexts. They assert that verification never seeks or injects into the live page, that the isolated page supplies the expected frames, and that screenshot/injection failures propagate while closing it. Disabled verification allocates no page.
Test plan
The broader
bun run testcompleted with failures outside this change. After hydrating the exact LFS fixture and correcting the local tool environment, all ten failing files were rerun against both the untouched base and the PR: the same six tests still fail (two FFmpeg color/timestamp cases, one macOS watcher case, and three cancellation cases), and the same font-suite setup is blocked by the protected user home. The remaining 392 tests in those comparison files pass in each checkout. The isolated test environment also deniespsprocess inspection, which the cancellation ancestry check requires. Full logs and comparison results are retained; this is not a claim that the full repository suite is green.Reproduction: follow #5415's
init/add lt-side-rulesetup, mount the block atdata-start="0.3"for 4.8 seconds, render the full 100 seconds with default capture, and extract the frame at 2 seconds. For a screenshot reference usePRODUCER_FORCE_SCREENSHOT=true; for a diagnostic on the old implementation useHF_DE_VERIFY=0. The fix does not disable verification or force screenshot capture.Prepared with Codex. The contribution is limited to the engine function and its regression tests; diagnostic harnesses and generated media are not included in the source diff.