diff --git a/packages/engine/src/config.ts b/packages/engine/src/config.ts index c2f9bc130..c2738ee94 100644 --- a/packages/engine/src/config.ts +++ b/packages/engine/src/config.ts @@ -76,7 +76,21 @@ export interface EngineConfig { // ── Media ──────────────────────────────────────────────────────────── audioGain: number; + /** + * Hard upper bound on entries kept in the video frame data URI cache. + * Acts as a sanity cap; the byte budget below normally fires first on + * high-resolution renders. At 1080p with ~6 MB per JPEG frame the default + * 256 entries fit inside ~1.5 GB. At 4K the byte budget evicts long + * before this cap is reached. + */ frameDataUriCacheLimit: number; + /** + * Memory budget for the cache, in megabytes. Eviction kicks in once the + * sum of cached data-URI string lengths exceeds this. Sized so a worker + * stays comfortably under a few GB even at 4K (where each PNG frame is + * ~25 MB and the base64 data URI is ~33 MB). + */ + frameDataUriCacheBytesLimitMb: number; // ── Timeouts ───────────────────────────────────────────────────────── playerReadyTimeout: number; @@ -149,6 +163,7 @@ export const DEFAULT_CONFIG: EngineConfig = { audioGain: 1, frameDataUriCacheLimit: 256, + frameDataUriCacheBytesLimitMb: 1500, playerReadyTimeout: 45_000, renderReadyTimeout: 15_000, @@ -246,6 +261,13 @@ export function resolveConfig(overrides?: Partial): EngineConfig { 32, envNum("PRODUCER_FRAME_DATA_URI_CACHE_LIMIT", DEFAULT_CONFIG.frameDataUriCacheLimit), ), + frameDataUriCacheBytesLimitMb: Math.max( + 64, + envNum( + "PRODUCER_FRAME_DATA_URI_CACHE_BYTES_MB", + DEFAULT_CONFIG.frameDataUriCacheBytesLimitMb, + ), + ), playerReadyTimeout: envNum( "PRODUCER_PLAYER_READY_TIMEOUT_MS", diff --git a/packages/engine/src/services/videoFrameInjector.test.ts b/packages/engine/src/services/videoFrameInjector.test.ts new file mode 100644 index 000000000..28c813641 --- /dev/null +++ b/packages/engine/src/services/videoFrameInjector.test.ts @@ -0,0 +1,145 @@ +// @vitest-environment node +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { __testing } from "./videoFrameInjector.js"; +import { DEFAULT_CONFIG } from "../config.js"; + +const { createFrameSourceCache } = __testing; + +const SHARED_STATS = { evictions: 0, oversizedRejections: 0 }; + +describe("frame source cache eviction", () => { + let dir: string; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "hf-frame-cache-test-")); + }); + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + // Each PNG is base64-encoded into the data URI, so the cached string is + // ~4/3 the file size plus a small `data:image/png;base64,` prefix. Build + // distinct files so eviction has predictable victims. + function writeFrame(name: string, sizeBytes: number): string { + const filePath = join(dir, name); + writeFileSync(filePath, Buffer.alloc(sizeBytes, 0)); + return filePath; + } + + it("evicts oldest entry when entry count exceeds limit", async () => { + const cache = createFrameSourceCache(2, Number.MAX_SAFE_INTEGER); + const a = writeFrame("a.png", 16); + const b = writeFrame("b.png", 16); + const c = writeFrame("c.png", 16); + + await cache.get(a); + await cache.get(b); + expect(cache.stats().entries).toBe(2); + + await cache.get(c); + expect(cache.stats().entries).toBe(2); + expect(cache.stats().evictions).toBe(1); + + // Verify the *oldest* entry (a) was the victim — the LRU contract. + // A later get(a) is a miss-then-insert, which would also evict whichever + // entry is now oldest. We instrument the eviction counter to detect it. + const evictionsBefore = cache.stats().evictions; + await cache.get(a); + expect(cache.stats().evictions).toBe(evictionsBefore + 1); + // After re-inserting `a`, `b` is the next oldest. `c` is now newest. + // Touch `b` (move-to-front) → next eviction would be `c`, not `b`. + }); + + it("evicts oldest entry when byte budget is exceeded", async () => { + // 1 KB raw frame → ~1.4 KB base64 + ~22-byte data URI prefix. Pick a + // budget that comfortably fits two URIs but not three, so the third + // get() forces eviction even though the entry-count cap (100) is far + // from the limit. + const cache = createFrameSourceCache(100, 4 * 1024); + const a = writeFrame("a.png", 1024); + const b = writeFrame("b.png", 1024); + const c = writeFrame("c.png", 1024); + + await cache.get(a); + await cache.get(b); + expect(cache.stats().entries).toBe(2); + + await cache.get(c); + const afterC = cache.stats(); + // The byte budget is the contract — the cache MUST stay under it after + // an insert that would otherwise overflow. Entry count is incidental. + expect(afterC.bytes).toBeLessThanOrEqual(4 * 1024); + expect(afterC.entries).toBeLessThan(3); + }); + + it("returns the served URL untouched when frameSrcResolver yields one", async () => { + let served: string | null = "/served/frame.png"; + const cache = createFrameSourceCache(4, 64 * 1024, () => served); + const file = writeFrame("a.png", 256); + + expect(await cache.get(file)).toBe("/served/frame.png"); + // Cache stays empty because the resolver short-circuits the read. + expect(cache.stats()).toMatchObject({ entries: 0, bytes: 0 }); + + served = null; + const dataUri = await cache.get(file); + expect(dataUri.startsWith("data:image/png;base64,")).toBe(true); + expect(cache.stats().entries).toBe(1); + }); + + it("treats a re-read as a cache hit (no second file read)", async () => { + const cache = createFrameSourceCache(2, Number.MAX_SAFE_INTEGER); + const a = writeFrame("a.png", 64); + + const first = await cache.get(a); + const second = await cache.get(a); + expect(second).toBe(first); + expect(cache.stats().entries).toBe(1); + }); + + it("skips caching an entry that alone exceeds the byte budget (no self-eviction)", async () => { + // 64 KB raw → ~88 KB base64 + prefix. Budget of 32 KB rejects this entry. + // The contract: caller still gets the data URI; cache stays empty so + // future inserts aren't blocked by the rejected entry's bookkeeping. + const cache = createFrameSourceCache(100, 32 * 1024); + const big = writeFrame("big.png", 64 * 1024); + + const dataUri = await cache.get(big); + expect(dataUri.startsWith("data:image/png;base64,")).toBe(true); + expect(cache.stats().entries).toBe(0); + expect(cache.stats().bytes).toBe(0); + expect(cache.stats().oversizedRejections).toBe(1); + expect(cache.stats().evictions).toBe(0); + + // A subsequent normal-sized entry must cache cleanly — the rejection + // path didn't pollute internal state. + const small = writeFrame("small.png", 1024); + await cache.get(small); + expect(cache.stats().entries).toBe(1); + }); + + it("at the production default (1500 MB), 1080p frames stay cached", async () => { + // Regression for the post-PR-#662 default: previously the cache held up + // to 256 entries × ~8 MB ≈ 2 GB at 1080p. The new byte-budget default of + // 1500 MB caps it tighter (~187 entries at 1080p ≈ 6s @ 30fps). This + // test pins the math so a future tweak to the default is visible. + const oneEightyP_jpegSize = 8 * 1024 * 1024; // ~8 MB JPEG (data URI) + const defaultBytesLimit = DEFAULT_CONFIG.frameDataUriCacheBytesLimitMb * 1024 * 1024; + const expectedMaxEntries = Math.floor(defaultBytesLimit / oneEightyP_jpegSize); + expect(expectedMaxEntries).toBeGreaterThanOrEqual(180); + expect(expectedMaxEntries).toBeLessThanOrEqual(200); + // At 30fps that's at least 6 seconds of look-ahead. Sequential access is + // strictly cheaper, so the cache helps any seek-back ≤ 6s. + expect(expectedMaxEntries / 30).toBeGreaterThanOrEqual(6); + }); + + // Suppress unused-import warning when the SHARED_STATS sentinel is dropped. + it("stats() exposes counters used by telemetry", async () => { + const cache = createFrameSourceCache(1, Number.MAX_SAFE_INTEGER); + expect(cache.stats()).toMatchObject({ ...SHARED_STATS, entries: 0, bytes: 0 }); + }); +}); diff --git a/packages/engine/src/services/videoFrameInjector.ts b/packages/engine/src/services/videoFrameInjector.ts index befbbd19c..b928decf9 100644 --- a/packages/engine/src/services/videoFrameInjector.ts +++ b/packages/engine/src/services/videoFrameInjector.ts @@ -15,28 +15,92 @@ import { type BeforeCaptureHook } from "./frameCapture.js"; import { DEFAULT_CONFIG, type EngineConfig } from "../config.js"; export interface VideoFrameInjectorOptions extends Partial< - Pick + Pick > { frameSrcResolver?: (framePath: string) => string | null; } +interface FrameSourceCacheStats { + entries: number; + bytes: number; + /** Total entries evicted since cache creation. A high count vs a small + * composition signals the byte budget is too tight (cache thrash). */ + evictions: number; + /** Total inserts rejected because the entry alone exceeds bytesLimit. + * Non-zero means a single frame is bigger than the configured budget — + * raise `frameDataUriCacheBytesLimitMb` if it recurs in production. */ + oversizedRejections: number; +} + +interface FrameSourceCache { + get: (framePath: string) => Promise; + /** Exposed for tests + telemetry; reflects current cache occupancy. */ + stats: () => FrameSourceCacheStats; +} + +/** + * Two-bound LRU keyed by frame path. Either bound triggers eviction of the + * oldest entry — entry count protects against pathological many-tiny-frames + * cases, and the byte budget keeps memory bounded when the per-frame data + * URI grows (4K PNG frames are ~33 MB once base64-encoded). + * + * If a single entry's data URI exceeds `bytesLimit`, we skip caching it + * (returning the URI directly to the caller). Without this guard, the + * post-insert eviction loop would drop the entry we just inserted and the + * cache would degrade into a CPU hot path — every subsequent `get()` would + * re-read from disk and re-base64 the same frame. The lost cache hit costs + * one re-read per access; pretending to cache and immediately evicting + * costs one re-read per access *plus* the futile insert/evict bookkeeping. + */ function createFrameSourceCache( - cacheLimit: number, + entryLimit: number, + bytesLimit: number, frameSrcResolver?: (framePath: string) => string | null, -) { +): FrameSourceCache { const cache = new Map(); + const sizes = new Map(); const inFlight = new Map>(); + let totalBytes = 0; + let evictions = 0; + let oversizedRejections = 0; + + function evictOldest(): void { + const oldestKey = cache.keys().next().value; + if (!oldestKey) return; + const size = sizes.get(oldestKey) ?? 0; + cache.delete(oldestKey); + sizes.delete(oldestKey); + totalBytes = Math.max(0, totalBytes - size); + evictions++; + } function remember(framePath: string, dataUri: string): string { - if (cache.has(framePath)) { - cache.delete(framePath); - } - cache.set(framePath, dataUri); - if (cache.size > cacheLimit) { - const oldestKey = cache.keys().next().value; - if (oldestKey) { - cache.delete(oldestKey); + // Skip caching entries that alone exceed the byte budget. Caching them + // would trigger immediate self-eviction on insert and pollute LRU order + // by displacing the previous entry's slot. + if (dataUri.length > bytesLimit) { + oversizedRejections++; + // Drop any stale prior version so the caller sees consistent state. + if (cache.has(framePath)) { + const prev = sizes.get(framePath) ?? 0; + cache.delete(framePath); + sizes.delete(framePath); + totalBytes = Math.max(0, totalBytes - prev); } + return dataUri; + } + if (cache.has(framePath)) { + const prev = sizes.get(framePath) ?? 0; + cache.delete(framePath); + sizes.delete(framePath); + totalBytes = Math.max(0, totalBytes - prev); + } + const size = dataUri.length; + cache.set(framePath, dataUri); + sizes.set(framePath, size); + totalBytes += size; + while ((cache.size > entryLimit || totalBytes > bytesLimit) && cache.size > 0) { + evictOldest(); } return dataUri; } @@ -70,9 +134,19 @@ function createFrameSourceCache( return pending; } - return { get }; + return { + get, + stats: () => ({ + entries: cache.size, + bytes: totalBytes, + evictions, + oversizedRejections, + }), + }; } +export const __testing = { createFrameSourceCache }; + /** * Creates a BeforeCaptureHook that injects pre-extracted video frames * into the page, replacing native