fix(producer): inline base64 frames in injector to unblock video-heavy renders (#1630)

* fix(producer): inline base64 frames in injector to unblock video-heavy renders

The URL-served frame path (PR #596) hands each injected `<img>` a fileServer URL
instead of a base64 data URI, on the theory that shipping a short URL through
`page.evaluate` beats shipping a multi-MB base64 string per frame. That holds
when the fileServer is otherwise idle.

But on video-heavy compositions, the same fileServer also serves every
`<video>.src`. The runtime's drift-recovery branch (`runtime/media.ts:294-302`)
issues `el.load()` on the underlying `<video>` during seeks, kicking off
full-file downloads that occupy the fileServer's single Node event loop (it
uses `readFileSync` and offers no `Accept-Ranges`). The injector's
`<img>.decode()` then queues behind those video fetches and is never serviced
before puppeteer's protocol timeout fires, surfacing as
`Runtime.callFunctionOn timed out` in `capture_streaming`.

Reproducer (30 × 32 MB videos / 90 s comp / 8-core / 30 GB host):

  baseline (broken corpus)            537 s   render fails
  baseline (corpus-fixed)             428 s   render fails
  this fix (drop frameSrcResolver)    121 s   render succeeds, 69 MB MP4

Control corpus (30 × 1.6 MB / 60 s) shows no regression: 137 s with this
change vs ~135 s on \`main\`. The \`createCompiledFrameSrcResolver\` builder and
the \`frameSrcResolver\` option stay in the codebase, just unused for now —
re-enabling them behind a proper gate ("only use URL-served frames when the
page has zero fileServer-bound \`<video>.src\` traffic") is a follow-up. The
cache memory ceiling (\`frameDataUriCacheBytesLimitMb\`, default 1500 MB above
8 GB hosts) already bounds the cost of base64 inlining.

— Jerrai

* refactor(producer): drop unused frameSrcResolver builder import in render orchestrator

Followup to the previous commit. The void-call and the
`createCompiledFrameSrcResolver` import in `renderOrchestrator.ts` were left
behind as a no-op breadcrumb for the future gating PR. Code review (PR #1630)
correctly flagged this as dead code — the builder is a pure factory with no
side effects, so calling it and discarding the result is just wasted CPU.
Remove both and explain in the in-source comment where the builder still
lives, so the gating PR knows where to re-import from.

— Jerrai
This commit is contained in:
James Russo
2026-06-22 09:53:04 -07:00
committed by GitHub
parent a97f80433f
commit 4be81b4fb4
@@ -79,12 +79,7 @@ import {
VIRTUAL_TIME_SHIM, VIRTUAL_TIME_SHIM,
} from "./fileServer.js"; } from "./fileServer.js";
import { defaultLogger, type ProducerLogger } from "../logger.js"; import { defaultLogger, type ProducerLogger } from "../logger.js";
import { import { createMemorySampler, type MemorySampler, updateJobStatus } from "./render/shared.js";
createCompiledFrameSrcResolver,
createMemorySampler,
type MemorySampler,
updateJobStatus,
} from "./render/shared.js";
import { buildRenderErrorDetails, cleanupRenderResources, safeCleanup } from "./render/cleanup.js"; import { buildRenderErrorDetails, cleanupRenderResources, safeCleanup } from "./render/cleanup.js";
import { normalizeErrorMessage } from "../utils/errorMessage.js"; import { normalizeErrorMessage } from "../utils/errorMessage.js";
import { formatCaptureFrameName } from "../utils/paths.js"; import { formatCaptureFrameName } from "../utils/paths.js";
@@ -1210,12 +1205,40 @@ export async function executeRenderJob(
videoMetadataHints, videoMetadataHints,
skipReadinessVideoIds: videoReadinessSkipIds, skipReadinessVideoIds: videoReadinessSkipIds,
}); });
const frameSrcResolver = createCompiledFrameSrcResolver(compiledDir); // The URL-served frame path (PR #596) hands each injected `<img>` a
// fileServer URL instead of a base64 data URI, on the theory that
// shipping a short URL through `page.evaluate` beats shipping a
// multi-MB base64 string per frame. That holds when the fileServer
// is otherwise idle — but on video-heavy compositions, the same
// fileServer also serves every `<video>.src`. The runtime's
// drift-recovery branch (`runtime/media.ts:294-302`) issues
// `el.load()` on the underlying `<video>` during seeks, kicking off
// full-file downloads that occupy the fileServer's single Node
// event loop (it uses `readFileSync` and offers no `Accept-Ranges`).
// The injector's `<img>.decode()` then queues behind those video
// fetches and is never serviced before puppeteer's protocol timeout
// fires (`Runtime.callFunctionOn timed out`).
//
// Repro: synth 30 × 32 MB videos / 90 s comp on an 8-core / 30 GB
// host = 537 s wall (broken corpus) / 428 s (corpus-fixed), every
// render fails. Disabling the resolver (force base64-inline) gives
// 1:59 (119 s) wall and a clean MP4 on the same comp, with no
// regression on the 30 × 1.6 MB control corpus (137 s vs 135 s
// baseline).
//
// Until this is properly gated (e.g. only enable URL-served when the
// page has zero fileServer-bound `<video>.src` traffic), the inline
// path is the safe default. The cache memory ceiling
// (`frameDataUriCacheBytesLimitMb`, default 1500 MB above 8 GB
// hosts) already bounds the cost. `createCompiledFrameSrcResolver`
// and the `frameSrcResolver` option remain in their respective
// modules (`packages/producer/src/services/render/shared.ts`,
// `packages/engine/src/services/videoFrameInjector.ts`); the gating
// PR will re-import the builder here.
const createRenderVideoFrameInjector = (): BeforeCaptureHook | null => const createRenderVideoFrameInjector = (): BeforeCaptureHook | null =>
createVideoFrameInjector(frameLookup, { createVideoFrameInjector(frameLookup, {
frameDataUriCacheLimit: cfg.frameDataUriCacheLimit, frameDataUriCacheLimit: cfg.frameDataUriCacheLimit,
frameDataUriCacheBytesLimitMb: cfg.frameDataUriCacheBytesLimitMb, frameDataUriCacheBytesLimitMb: cfg.frameDataUriCacheBytesLimitMb,
frameSrcResolver,
}); });
let captureCalibration: let captureCalibration: