diff --git a/packages/producer/src/services/hdrImageTransferCache.test.ts b/packages/producer/src/services/hdrImageTransferCache.test.ts new file mode 100644 index 00000000..63a6420e --- /dev/null +++ b/packages/producer/src/services/hdrImageTransferCache.test.ts @@ -0,0 +1,229 @@ +import { describe, expect, test } from "bun:test"; +import { convertTransfer } from "@hyperframes/engine"; +import { createHdrImageTransferCache } from "./hdrImageTransferCache.ts"; + +/** + * Build a deterministic rgb48le buffer for `pixelCount` pixels. + * Each pixel is 3 channels × 2 bytes = 6 bytes. Values vary per pixel/channel + * so the LUT-based `convertTransfer` produces bytes that differ from the + * source. + */ +function makeSourceBuffer(pixelCount: number, seed = 0): Buffer { + const buf = Buffer.alloc(pixelCount * 6); + for (let i = 0; i < pixelCount; i++) { + const off = i * 6; + // Spread values across the 16-bit range so HLG↔PQ LUT lookups land on + // mid-curve entries that are guaranteed to differ from the input. + buf.writeUInt16LE((seed + i * 257) & 0xff_ff, off); + buf.writeUInt16LE((seed + i * 521 + 1) & 0xff_ff, off + 2); + buf.writeUInt16LE((seed + i * 1031 + 2) & 0xff_ff, off + 4); + } + return buf; +} + +function expectedConverted(source: Buffer, from: "hlg" | "pq", to: "hlg" | "pq"): Buffer { + const copy = Buffer.from(source); + convertTransfer(copy, from, to); + return copy; +} + +describe("hdrImageTransferCache", () => { + test("returns source buffer unchanged when sourceTransfer === targetTransfer", () => { + const cache = createHdrImageTransferCache(); + const source = makeSourceBuffer(4); + + const result = cache.getConverted("img1", "pq", "pq", source); + + expect(result).toBe(source); + expect(cache.size()).toBe(0); + }); + + test("first miss converts and caches", () => { + const cache = createHdrImageTransferCache(); + const source = makeSourceBuffer(4); + const expected = expectedConverted(source, "hlg", "pq"); + + const result = cache.getConverted("img1", "hlg", "pq", source); + + expect(result).not.toBe(source); + expect(Buffer.compare(result, expected)).toBe(0); + expect(cache.size()).toBe(1); + }); + + test("second hit returns cached buffer reference", () => { + const cache = createHdrImageTransferCache(); + const source = makeSourceBuffer(4); + + const first = cache.getConverted("img1", "hlg", "pq", source); + const second = cache.getConverted("img1", "hlg", "pq", source); + + expect(second).toBe(first); + expect(cache.size()).toBe(1); + }); + + test("does not re-run convertTransfer on cache hit", () => { + const cache = createHdrImageTransferCache(); + const source = makeSourceBuffer(4); + + const first = cache.getConverted("img1", "hlg", "pq", source); + const snapshot = Buffer.from(first); + // If a hit ran convertTransfer again on the cached buffer (PQ→PQ would + // be a no-op, but PQ→HLG would mutate), the bytes would change. + cache.getConverted("img1", "hlg", "pq", source); + + expect(Buffer.compare(first, snapshot)).toBe(0); + }); + + test("different target transfers for same imageId are cached independently", () => { + const cache = createHdrImageTransferCache(); + const source = makeSourceBuffer(4); + + const toPq = cache.getConverted("img1", "hlg", "pq", source); + const toHlg = cache.getConverted("img1", "pq", "hlg", source); + + expect(toPq).not.toBe(toHlg); + expect(Buffer.compare(toPq, expectedConverted(source, "hlg", "pq"))).toBe(0); + expect(Buffer.compare(toHlg, expectedConverted(source, "pq", "hlg"))).toBe(0); + expect(cache.size()).toBe(2); + }); + + test("different imageIds are cached independently", () => { + const cache = createHdrImageTransferCache(); + const a = makeSourceBuffer(4, 100); + const b = makeSourceBuffer(4, 200); + + const convA = cache.getConverted("a", "hlg", "pq", a); + const convB = cache.getConverted("b", "hlg", "pq", b); + + expect(convA).not.toBe(convB); + expect(Buffer.compare(convA, expectedConverted(a, "hlg", "pq"))).toBe(0); + expect(Buffer.compare(convB, expectedConverted(b, "hlg", "pq"))).toBe(0); + expect(cache.size()).toBe(2); + }); + + test("LRU evicts oldest entry when maxEntries exceeded", () => { + const cache = createHdrImageTransferCache({ maxEntries: 2 }); + const a = makeSourceBuffer(2, 1); + const b = makeSourceBuffer(2, 2); + const c = makeSourceBuffer(2, 3); + + const convA1 = cache.getConverted("a", "hlg", "pq", a); + cache.getConverted("b", "hlg", "pq", b); + cache.getConverted("c", "hlg", "pq", c); + + expect(cache.size()).toBe(2); + + const convA2 = cache.getConverted("a", "hlg", "pq", a); + expect(convA2).not.toBe(convA1); + expect(Buffer.compare(convA2, expectedConverted(a, "hlg", "pq"))).toBe(0); + }); + + test("access promotes entry to most-recently-used", () => { + const cache = createHdrImageTransferCache({ maxEntries: 2 }); + const a = makeSourceBuffer(2, 1); + const b = makeSourceBuffer(2, 2); + const c = makeSourceBuffer(2, 3); + + const convA1 = cache.getConverted("a", "hlg", "pq", a); + cache.getConverted("b", "hlg", "pq", b); + + const convA2 = cache.getConverted("a", "hlg", "pq", a); + expect(convA2).toBe(convA1); + + cache.getConverted("c", "hlg", "pq", c); + + const convA3 = cache.getConverted("a", "hlg", "pq", a); + expect(convA3).toBe(convA1); + + const convB2 = cache.getConverted("b", "hlg", "pq", b); + expect(Buffer.compare(convB2, expectedConverted(b, "hlg", "pq"))).toBe(0); + expect(cache.size()).toBe(2); + }); + + test("maxEntries: 0 disables caching but still returns correct converted bytes", () => { + const cache = createHdrImageTransferCache({ maxEntries: 0 }); + const source = makeSourceBuffer(4); + const expected = expectedConverted(source, "hlg", "pq"); + + const first = cache.getConverted("img1", "hlg", "pq", source); + const second = cache.getConverted("img1", "hlg", "pq", source); + + expect(first).not.toBe(second); + expect(Buffer.compare(first, expected)).toBe(0); + expect(Buffer.compare(second, expected)).toBe(0); + expect(cache.size()).toBe(0); + }); + + test("cached buffer is independent from the source buffer", () => { + const cache = createHdrImageTransferCache(); + const source = makeSourceBuffer(4); + const sourceSnapshot = Buffer.from(source); + + const cached = cache.getConverted("img1", "hlg", "pq", source); + source.fill(0); + + expect(cache.getConverted("img1", "hlg", "pq", source)).toBe(cached); + expect(Buffer.compare(cached, expectedConverted(sourceSnapshot, "hlg", "pq"))).toBe(0); + }); + + // Source-buffer-immutability guarantee (PR #384 review feedback): the cache + // MUST NOT mutate the source buffer the caller hands in, on any path. + // `convertTransfer` mutates in place, so the implementation has to clone + // before converting — these tests pin the invariant against future + // refactors that might forget the `Buffer.from(source)` defense. + + test("does not mutate the source buffer on a convert+cache miss", () => { + const cache = createHdrImageTransferCache(); + const source = makeSourceBuffer(4); + const sourceSnapshot = Buffer.from(source); + + cache.getConverted("img1", "hlg", "pq", source); + + expect(Buffer.compare(source, sourceSnapshot)).toBe(0); + }); + + test("does not mutate the source buffer on a convert+cache miss with maxEntries=0 passthrough", () => { + const cache = createHdrImageTransferCache({ maxEntries: 0 }); + const source = makeSourceBuffer(4); + const sourceSnapshot = Buffer.from(source); + + const result = cache.getConverted("img1", "hlg", "pq", source); + + expect(Buffer.compare(source, sourceSnapshot)).toBe(0); + expect(result).not.toBe(source); + expect(Buffer.compare(result, expectedConverted(sourceSnapshot, "hlg", "pq"))).toBe(0); + expect(cache.size()).toBe(0); + }); + + test("does not mutate the source buffer on a cache hit", () => { + const cache = createHdrImageTransferCache(); + const source = makeSourceBuffer(4); + const sourceSnapshot = Buffer.from(source); + + cache.getConverted("img1", "hlg", "pq", source); + cache.getConverted("img1", "hlg", "pq", source); + + expect(Buffer.compare(source, sourceSnapshot)).toBe(0); + }); + + test("rejects invalid maxEntries", () => { + expect(() => createHdrImageTransferCache({ maxEntries: -1 })).toThrow(); + expect(() => createHdrImageTransferCache({ maxEntries: 1.5 })).toThrow(); + expect(() => createHdrImageTransferCache({ maxEntries: Number.NaN })).toThrow(); + }); + + test("default maxEntries is large enough for typical compositions", () => { + const cache = createHdrImageTransferCache(); + const source = makeSourceBuffer(2); + + for (let i = 0; i < 16; i++) { + cache.getConverted(`img${i}`, "hlg", "pq", source); + } + expect(cache.size()).toBe(16); + + // The first inserted entry should still be present (no eviction yet). + const first = cache.getConverted("img0", "hlg", "pq", source); + expect(Buffer.compare(first, expectedConverted(source, "hlg", "pq"))).toBe(0); + expect(cache.size()).toBe(16); + }); +}); diff --git a/packages/producer/src/services/hdrImageTransferCache.ts b/packages/producer/src/services/hdrImageTransferCache.ts new file mode 100644 index 00000000..687a76fe --- /dev/null +++ b/packages/producer/src/services/hdrImageTransferCache.ts @@ -0,0 +1,143 @@ +import { type HdrTransfer, convertTransfer } from "@hyperframes/engine"; + +/** + * Cache of transfer-converted HDR image buffers keyed by + * `(imageId, targetTransfer)`. + * + * ## Why this exists + * + * Static HDR images are decoded once per render at setup time and stored as + * `rgb48le` buffers in `hdrImageBuffers`. When the encode target's transfer + * function (e.g. `pq`) differs from the image's source transfer (e.g. `hlg`), + * `blitHdrImageLayer` must run an LUT-based transfer conversion before + * blitting. `convertTransfer` mutates its input in-place, so the call site + * has historically allocated a fresh `Buffer.from(buf.data)` clone every + * frame to keep the original decode pristine for subsequent frames. + * + * For a 30 s, 60 fps, 1080p render with one HDR image, that's: + * + * - ~1800 × `Buffer.from(...)` allocations (~12 MB each → ~22 GB churn) + * - ~1800 × ~6 M LUT lookups in `convertTransfer` + * + * Both are pure functions of `(source bytes, sourceTransfer, targetTransfer)`, + * and within a single render job the source bytes for a given `imageId` are + * fixed. Caching the converted buffer per `(imageId, targetTransfer)` reduces + * the work to one allocation and one LUT pass per unique pair — independent + * of frame count. + * + * ## Lifetime + * + * Instances are constructed per render job and dropped on job exit (success + * or failure) by going out of scope. **Do not reuse a single cache across + * jobs** — `imageId` collisions could return stale converted bytes from a + * different source buffer. + * + * ## Bounds + * + * The cache is LRU-bounded by entry count (default 16). At 1080p each entry + * is ~12 MB, so the default cap is ~200 MB worst case. Compositions with + * more unique HDR images than `maxEntries` will evict older entries on a + * least-recently-used basis; cache misses just rebuild the converted buffer. + * + * ## Caller contract + * + * The buffer returned by `getConverted` is shared cache state and **MUST NOT + * be mutated** by the caller. All downstream HDR blit functions + * (`blitRgb48leAffine`, `blitRgb48leRegion`) read from it without writing, + * so this is naturally upheld today. + */ +export interface HdrImageTransferCache { + /** + * Return a buffer in `targetTransfer` for the given image. + * + * - When `sourceTransfer === targetTransfer`, returns `source` unchanged + * (no allocation, no caching). + * - On the first call for `(imageId, targetTransfer)`, clones `source`, + * converts in-place via {@link convertTransfer}, caches the result, and + * returns it. + * - On subsequent calls with the same `(imageId, targetTransfer)`, returns + * the cached buffer (and promotes it to most-recently-used). + * + * The returned buffer is read-only from the caller's perspective. + */ + getConverted( + imageId: string, + sourceTransfer: HdrTransfer, + targetTransfer: HdrTransfer, + source: Buffer, + ): Buffer; + + /** Number of currently cached entries. Diagnostic / test aid. */ + size(): number; +} + +export interface HdrImageTransferCacheOptions { + /** + * Maximum number of converted buffers to retain before evicting the + * least-recently-used entry. Defaults to 16. Must be a non-negative + * integer; `0` disables caching entirely (every call allocates fresh). + */ + maxEntries?: number; +} + +const DEFAULT_MAX_ENTRIES = 16; + +export function createHdrImageTransferCache( + options: HdrImageTransferCacheOptions = {}, +): HdrImageTransferCache { + const maxEntries = options.maxEntries ?? DEFAULT_MAX_ENTRIES; + if (!Number.isInteger(maxEntries) || maxEntries < 0) { + throw new Error( + `createHdrImageTransferCache: maxEntries must be a non-negative integer, got ${String(maxEntries)}`, + ); + } + + // Map iteration order is insertion order in JS, so promoting an entry to + // most-recently-used is just a `delete` + `set`. The first key in the + // iterator is therefore the LRU candidate. + const entries = new Map(); + + function makeKey(imageId: string, targetTransfer: HdrTransfer): string { + return `${imageId}|${targetTransfer}`; + } + + return { + getConverted(imageId, sourceTransfer, targetTransfer, source) { + if (sourceTransfer === targetTransfer) { + return source; + } + + if (maxEntries === 0) { + const fresh = Buffer.from(source); + convertTransfer(fresh, sourceTransfer, targetTransfer); + return fresh; + } + + const key = makeKey(imageId, targetTransfer); + const existing = entries.get(key); + if (existing) { + // Promote to MRU. + entries.delete(key); + entries.set(key, existing); + return existing; + } + + const converted = Buffer.from(source); + convertTransfer(converted, sourceTransfer, targetTransfer); + + if (entries.size >= maxEntries) { + // Evict LRU (first key in insertion-ordered iterator). + const lruKey = entries.keys().next().value; + if (lruKey !== undefined) { + entries.delete(lruKey); + } + } + entries.set(key, converted); + return converted; + }, + + size() { + return entries.size; + }, + }; +} diff --git a/packages/producer/src/services/renderOrchestrator.ts b/packages/producer/src/services/renderOrchestrator.ts index 36911bd1..6f48995c 100644 --- a/packages/producer/src/services/renderOrchestrator.ts +++ b/packages/producer/src/services/renderOrchestrator.ts @@ -18,7 +18,6 @@ import { mkdirSync, rmSync, readFileSync, - readdirSync, writeFileSync, copyFileSync, appendFileSync, @@ -99,6 +98,11 @@ import { } from "./htmlCompiler.js"; import { defaultLogger, type ProducerLogger } from "../logger.js"; import { isPathInside } from "../utils/paths.js"; +import { clearMaxFrameIndex, getMaxFrameIndex } from "./frameDirCache.js"; +import { + type HdrImageTransferCache, + createHdrImageTransferCache, +} from "./hdrImageTransferCache.js"; /** * Wrap a cleanup operation so it never throws, but logs any failure. @@ -117,39 +121,6 @@ async function safeCleanup( } } -/** - * Cache of the maximum 1-based frame index present in each pre-extracted frame - * directory (e.g. `frame_0001.png … frame_0150.png` → 150). The directory is - * read once on first access and the max is computed by parsing filenames. - * - * Used to bounds-check `videoFrameIndex` against the directory size before - * calling `existsSync` per frame, which avoids redundant filesystem syscalls - * when the requested time falls past the last extracted frame (e.g. a clip - * shorter than the composition's effective video range). - */ -const frameDirMaxIndexCache = new Map(); - -const FRAME_FILENAME_RE = /^frame_(\d+)\.png$/; - -function getMaxFrameIndex(frameDir: string): number { - const cached = frameDirMaxIndexCache.get(frameDir); - if (cached !== undefined) return cached; - let max = 0; - try { - for (const name of readdirSync(frameDir)) { - const m = FRAME_FILENAME_RE.exec(name); - if (!m) continue; - const n = Number(m[1]); - if (Number.isFinite(n) && n > max) max = n; - } - } catch { - // Directory missing or unreadable → max stays 0; downstream existsSync - // check will still produce the right "no frame" outcome. - } - frameDirMaxIndexCache.set(frameDir, max); - return max; -} - // Diagnostic helpers used by the HDR layered compositor when KEEP_TEMP=1 // is set. They are pure (capture no state), so we keep them at module scope // to avoid re-creating closures per frame and to make them callable from @@ -557,6 +528,7 @@ function blitHdrImageLayer( canvas: Buffer, el: ElementStackingInfo, hdrImageBuffers: Map, + hdrImageTransferCache: HdrImageTransferCache, width: number, height: number, log?: ProducerLogger, @@ -569,13 +541,13 @@ function blitHdrImageLayer( } try { - let hdrRgb = buf.data; - if (sourceTransfer && targetTransfer && sourceTransfer !== targetTransfer) { - // convertTransfer mutates in place; copy first so the cached decode stays - // pristine for subsequent frames. - hdrRgb = Buffer.from(buf.data); - convertTransfer(hdrRgb, sourceTransfer, targetTransfer); - } + // The cache returns `buf.data` unchanged when no conversion is needed, + // and otherwise returns a per-(imageId, targetTransfer) buffer that was + // converted exactly once and reused across every subsequent frame. + const hdrRgb = + sourceTransfer && targetTransfer + ? hdrImageTransferCache.getConverted(el.id, sourceTransfer, targetTransfer, buf.data) + : buf.data; const viewportMatrix = parseTransformMatrix(el.transform); @@ -636,6 +608,7 @@ interface HdrCompositeContext { effectiveHdr: { transfer: HdrTransfer }; nativeHdrImageIds: Set; hdrImageBuffers: Map; + hdrImageTransferCache: HdrImageTransferCache; hdrFrameDirs: Map; hdrVideoStartTimes: Map; imageTransfers: Map; @@ -689,6 +662,7 @@ async function compositeHdrFrame( effectiveHdr, nativeHdrImageIds, hdrImageBuffers, + hdrImageTransferCache, hdrFrameDirs, hdrVideoStartTimes, imageTransfers, @@ -736,6 +710,7 @@ async function compositeHdrFrame( canvas, layer.element, hdrImageBuffers, + hdrImageTransferCache, width, height, log, @@ -1618,7 +1593,7 @@ export async function executeRenderJob( let hdrEncoderClosed = false; let domSessionClosed = false; // Track HDR video frame directories at this scope so the outer finally - // can clear their entries from the module-scoped frameDirMaxIndexCache. + // can clear their entries from the shared frameDirCache module. // Without this, the cache leaks one entry per HDR video per render. const hdrFrameDirs = new Map(); try { @@ -1784,7 +1759,7 @@ export async function executeRenderJob( // ── Pre-extract all HDR video frames in a single FFmpeg pass ────── // hdrFrameDirs is declared above the try block so the outer finally - // can clear matching frameDirMaxIndexCache entries on any exit path. + // can clear matching frameDirCache entries on any exit path. for (const [videoId, srcPath] of hdrVideoSrcPaths) { const video = composition.videos.find((v) => v.id === videoId); if (!video) continue; @@ -1920,6 +1895,13 @@ export async function executeRenderJob( "Internal: HDR render path entered without effectiveHdr — this is a bug.", ); } + // Per-job LRU cache for transfer-converted HDR image buffers. Static HDR + // images that need PQ↔HLG conversion are converted exactly once per + // (imageId, targetTransfer) and then reused for every subsequent frame + // instead of paying a fresh `Buffer.from` + `convertTransfer` on every + // composite. The cache is local to this render job so concurrent renders + // do not share state. + const hdrImageTransferCache = createHdrImageTransferCache(); const hdrCompositeCtx: HdrCompositeContext = { log, domSession, @@ -1930,6 +1912,7 @@ export async function executeRenderJob( effectiveHdr, nativeHdrImageIds, hdrImageBuffers, + hdrImageTransferCache, hdrFrameDirs, hdrVideoStartTimes, imageTransfers, @@ -2049,6 +2032,7 @@ export async function executeRenderJob( sceneBuf as Buffer, el, hdrImageBuffers, + hdrImageTransferCache, width, height, log, @@ -2166,9 +2150,9 @@ export async function executeRenderJob( } // Drop the matching cache entry so we don't leak a stale // max-frame-index reading for a directory that no longer - // exists. Without this, the module-scoped cache grows + // exists. Without this, the shared cache grows // monotonically across renders. - frameDirMaxIndexCache.delete(frameDir); + clearMaxFrameIndex(frameDir); hdrFrameDirs.delete(videoId); } cleanedUpVideos.add(videoId); @@ -2226,13 +2210,13 @@ export async function executeRenderJob( }); }); } - // Drop frameDirMaxIndexCache entries for any HDR frame directories - // that survived the in-loop cleanup (early failures, KEEP_TEMP=1, - // videos still active when the render exits). The on-disk frames - // themselves are torn down with workDir; we just don't want the - // module-scoped cache to leak entries across renders. + // Drop frameDirCache entries for any HDR frame directories that + // survived the in-loop cleanup (early failures, KEEP_TEMP=1, videos + // still active when the render exits). The on-disk frames themselves + // are torn down with workDir; we just don't want the shared cache to + // leak entries across renders. for (const frameDir of hdrFrameDirs.values()) { - frameDirMaxIndexCache.delete(frameDir); + clearMaxFrameIndex(frameDir); } hdrFrameDirs.clear(); }