mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(producer): localize remote <img> sources + await image readiness (#1197)
* fix(producer): localize remote <img> sources + await image readiness Producer's frame-capture has `pollVideosReady` (waits readyState >= 2 for every <video>) but no equivalent for <img>. Combined with htmlCompiler's `collectExternalAssets` explicitly skipping http(s) URLs (line 805-806), agent-pipeline-generated compositions (astral / daphne / hyperion multi-v2 outputs with raw S3 <img src>) reach Chrome with a network dependency that races the readiness gate AND can be evicted mid-render. Either path produces blank-frame flicker. Reproduction (02_kobe agent output, 42s render @ 30fps): scene_02's remote S3 background-image painted from t=7.0s, vanished at t=10.5s (frame size 139KB vs 700-940KB neighbors), back at t=11.0s. GSAP timeline said opacity:1 throughout — Chrome simply didn't have the pixels. Two-layer fix: 1. **Producer** — `localizeRemoteImageSources` in `htmlCompiler.ts` mirrors the existing `localizeRemoteMediaSources` (video/audio) + `localizeRemoteFontFaces` pattern, reusing `downloadAndRewriteUrls` and the `_remote_media/` subdir. Wired into `compileForRender` between the media and font localize steps. Once the file is local, Chrome's image cache is bounded by disk reads, not S3 latency. 2. **Engine** — `pollImagesReady` + `decodeAllImages` helpers in `frameCapture.ts` parallel to `pollVideosReady`. Waits for every `<img>` (skipping data: URIs) to have `complete && naturalWidth > 0`, then forces GPU upload via `img.decode()`. Called from both the classic-xvfb path and the BeginFrame path after their respective video readiness checks. Defense-in-depth — Layer 1 closes the symptom for current+future agent-pipeline outputs; Layer 2 protects any future code path that leaves a remote URL in place. Tests: 7 new cases in `htmlCompiler.test.ts` covering happy-path rewrite, 404 fallback, dedup of duplicate URLs, non-HTTP and data: URI passthrough, both quote styles, and the agent-pipeline shape where `src` is not the first attribute. All pass alongside the existing 56 htmlCompiler tests. * fix(producer): scope remote-img regex to real src; correct stale comments Review follow-ups on the remote-<img> localization fix: - Tighten REMOTE_IMG_TAG_RE with a (?<![\w-]) lookbehind so it matches a real `src` attribute only. The previous `\bsrc` also matched `data-src` (and `data-*-src`) lazy-loader placeholders, which would download/rewrite a URL the render never paints. Added a regression test; `srcset` stays excluded by the `\s*=` requirement. - Fix comments that claimed frameCapture has "no pollImagesReady analog" — this PR adds exactly that, so the docstrings were self-contradictory. Reframed localization as the primary fix and pollImagesReady as the defense-in-depth layer, and documented the <img src>-only scope (srcset / <picture> / SVG <image> / CSS background-image are follow-ups). Verified locally end-to-end on the 02_kobe repro: all 4 remote S3 <img> URLs localize to _remote_media/, the render completes, and the frame at t~10.5s that was a 139KB blank in the broken render now paints the trophy background in every native-fps frame. htmlCompiler.test.ts 64 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(engine): pollImagesReady broken-image escape + skip decode on in-flight Addresses two real bugs Magi caught in review on hf#1197: 1. pollImagesReady would spin the full pageReadyTimeout (45s default) for any <img> that settled with an error — Chrome marks 404 / decode failure / CORS rejection with (complete=true, naturalWidth=0), and the previous predicate `complete && naturalWidth > 0` returned false for those, so the poll ran to timeout. This is the HTMLImageElement equivalent of pollVideosReady's `ve.error` early-exit. Add a `complete && naturalWidth === 0` branch that treats settled-with- error as done — waiting won't make it load. Particularly relevant because localizeRemoteImageSources falls back to the original URL on download failure; that failed URL is now hit by a 45s stall instead of the broken-image marker rendering immediately. 2. decodeAllImages called img.decode() on every image, including those still in flight after pollImagesReady timed out. Per the WHATWG spec, decode() on a loading image awaits the fetch — never resolving until the network completes or puppeteer's evaluate timeout fires and throws an uncaught error that aborts the render. Pre-filter to only call decode() on images that successfully loaded. Test coverage: new frameCapture-pollImagesReady.test.ts with 8 cases covering empty docs, all-loaded, broken (complete + naturalWidth=0), data: URI, empty src, in-flight → resolves, in-flight → timeout, and the mixed batch. The broken-image test explicitly asserts elapsed < 500ms on a 1000ms timeout — guards against the regression Magi flagged. * docs(engine): clarify decodeAllImages prevents init race, not eviction Vai correctly noted that decode() forces initial GPU upload but does not prevent Chrome from evicting decoded pixels mid-render. The producer-side localizeRemoteImageSources is what bounds the eviction risk (local file-server paging vs S3 re-fetch). Comment updated to reflect that split of responsibilities. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
2be41937a9
commit
72c461d86a
@@ -550,6 +550,89 @@ async function pollVideosReady(
|
||||
return check();
|
||||
}
|
||||
|
||||
// Wait for every `<img>` with a non-`data:` src to have settled — either
|
||||
// successfully loaded (`complete && naturalWidth > 0`) or failed with a
|
||||
// broken-image marker (`complete && naturalWidth === 0`, the HTMLImageElement
|
||||
// equivalent of HTMLMediaElement.error). htmlCompiler localises remote `<img>`
|
||||
// URLs to the local file server before this point, so in practice this polls
|
||||
// for the local fetch to land — but the guard is a defensive net so that any
|
||||
// future composition path that leaves a remote URL in place won't capture
|
||||
// frames before the pixels arrive. Mirrors `pollVideosReady` for parity with
|
||||
// the video-side readiness contract (videos exit-early on `ve.error`; images
|
||||
// exit-early on `complete && naturalWidth === 0`).
|
||||
/** @internal exported for unit testing only */
|
||||
export async function pollImagesReady(
|
||||
page: Page,
|
||||
timeoutMs: number,
|
||||
intervalMs: number = 100,
|
||||
): Promise<boolean> {
|
||||
const check = async (): Promise<boolean> => {
|
||||
return Boolean(
|
||||
await page.evaluate(() => {
|
||||
const imgs = Array.from(document.querySelectorAll("img"));
|
||||
return (
|
||||
imgs.length === 0 ||
|
||||
imgs.every((img) => {
|
||||
const ie = img as HTMLImageElement;
|
||||
const src = ie.getAttribute("src") || "";
|
||||
if (!src || src.startsWith("data:")) return true;
|
||||
// A `complete` image with zero naturalWidth has settled with an
|
||||
// error (404 / decode failure / CORS rejection / blocked). Treat
|
||||
// as done — waiting won't make it load — and let the render
|
||||
// continue with the broken-image marker visible. Mirrors how
|
||||
// pollVideosReady treats `ve.error`.
|
||||
if (ie.complete && ie.naturalWidth === 0) return true;
|
||||
if (ie.complete && ie.naturalWidth > 0) return true;
|
||||
return false;
|
||||
})
|
||||
);
|
||||
}),
|
||||
);
|
||||
};
|
||||
const deadline = Date.now() + timeoutMs;
|
||||
while (Date.now() < deadline) {
|
||||
if (await check()) return true;
|
||||
await new Promise((resolve) => setTimeout(resolve, intervalMs));
|
||||
}
|
||||
return check();
|
||||
}
|
||||
|
||||
// Force every successfully-loaded `<img>` to be GPU-uploaded before the first
|
||||
// frame capture. `naturalWidth > 0` means the bitmap has been decoded into
|
||||
// CPU memory, but compositor-side GPU upload can still happen lazily on first
|
||||
// paint. Calling `img.decode()` returns a Promise that resolves once the image
|
||||
// is ready for synchronous painting — eliminating the small first-frame race
|
||||
// between "image is technically loaded" and "the rasterized texture is on the
|
||||
// GPU and ready to composite".
|
||||
//
|
||||
// Note this is purely an init-time guard; it doesn't prevent Chrome from
|
||||
// evicting decoded pixels mid-render. The producer-side `localizeRemoteImageSources`
|
||||
// is what bounds the eviction risk (a re-fetch hits the local file server's
|
||||
// disk-backed paging, not S3 over the network).
|
||||
//
|
||||
// Critical: `decode()` on an in-flight image waits for the fetch to resolve.
|
||||
// If `pollImagesReady` timed out with some images still loading (`!complete`),
|
||||
// calling `decode()` on them would block here until the network finally
|
||||
// completes — or until puppeteer's evaluate timeout fires and throws an
|
||||
// uncaught error that aborts the render. Skip in-flight and broken images;
|
||||
// only force GPU upload for images that successfully loaded.
|
||||
async function decodeAllImages(page: Page): Promise<void> {
|
||||
await page.evaluate(async () => {
|
||||
const imgs = Array.from(document.querySelectorAll("img"));
|
||||
await Promise.all(
|
||||
imgs.map((img) => {
|
||||
const ie = img as HTMLImageElement;
|
||||
if (typeof ie.decode !== "function") return Promise.resolve();
|
||||
// Skip still-loading images (in-flight decode() would hang) and
|
||||
// broken images (decode() rejects, but pre-filtering is clearer
|
||||
// than relying on the .catch).
|
||||
if (!ie.complete || ie.naturalWidth === 0) return Promise.resolve();
|
||||
return ie.decode().catch(() => undefined);
|
||||
}),
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
async function applyVideoMetadataHints(
|
||||
page: Page,
|
||||
hints: readonly CaptureVideoMetadataHint[] | undefined,
|
||||
@@ -707,6 +790,26 @@ export async function initializeSession(session: CaptureSession): Promise<void>
|
||||
);
|
||||
}
|
||||
|
||||
const imagesReady = await pollImagesReady(page, pageReadyTimeout);
|
||||
if (!imagesReady) {
|
||||
const failedImages = await page.evaluate(() => {
|
||||
return Array.from(document.querySelectorAll("img"))
|
||||
.filter((img) => {
|
||||
const ie = img as HTMLImageElement;
|
||||
const src = ie.getAttribute("src") || "";
|
||||
if (!src || src.startsWith("data:")) return false;
|
||||
return !(ie.complete && ie.naturalWidth > 0);
|
||||
})
|
||||
.map((img) => (img as HTMLImageElement).src || img.getAttribute("src") || "(no src)")
|
||||
.join(", ");
|
||||
});
|
||||
console.warn(
|
||||
`[FrameCapture] Some image elements did not load within ${pageReadyTimeout}ms: ${failedImages}. ` +
|
||||
`Continuing render — affected images may appear blank/missing in early frames.`,
|
||||
);
|
||||
}
|
||||
await decodeAllImages(page);
|
||||
|
||||
await page.evaluate(`document.fonts?.ready`);
|
||||
await waitForOptionalTailwindReady(page, pageReadyTimeout);
|
||||
|
||||
@@ -812,6 +915,28 @@ export async function initializeSession(session: CaptureSession): Promise<void>
|
||||
);
|
||||
}
|
||||
|
||||
// Image readiness — parity with pollVideosReady. Defense against remote
|
||||
// <img> URLs that bypass the htmlCompiler localize step.
|
||||
const bfImagesReady = await pollImagesReady(page, pageReadyTimeout);
|
||||
if (!bfImagesReady) {
|
||||
const failedImages = await page.evaluate(() => {
|
||||
return Array.from(document.querySelectorAll("img"))
|
||||
.filter((img) => {
|
||||
const ie = img as HTMLImageElement;
|
||||
const src = ie.getAttribute("src") || "";
|
||||
if (!src || src.startsWith("data:")) return false;
|
||||
return !(ie.complete && ie.naturalWidth > 0);
|
||||
})
|
||||
.map((img) => (img as HTMLImageElement).src || img.getAttribute("src") || "(no src)")
|
||||
.join(", ");
|
||||
});
|
||||
console.warn(
|
||||
`[FrameCapture] Some image elements did not load within ${pageReadyTimeout}ms: ${failedImages}. ` +
|
||||
`Continuing render — affected images may appear blank/missing in early frames.`,
|
||||
);
|
||||
}
|
||||
await decodeAllImages(page);
|
||||
|
||||
// Font check (no rAF dependency — uses fonts.ready API directly)
|
||||
await page.evaluate(`document.fonts?.ready`);
|
||||
await waitForOptionalTailwindReady(page, pageReadyTimeout);
|
||||
|
||||
Reference in New Issue
Block a user