From a9da888e1882c84547243a9a037cf7f2259daefb Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 21 Apr 2026 10:42:10 -0700 Subject: [PATCH] perf(producer): cache transfer-converted hdr image buffers per render job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Static HDR image layers whose source transfer differs from the render's effective transfer (PQ↔HLG) were re-running `Buffer.from` + `convertTransfer` on every composited frame, even though the converted buffer is identical for the entire job. Added `HdrImageTransferCache` — a per-render-job bounded LRU keyed by `(imageId, targetTransfer)` that converts once and reuses on every subsequent frame, while leaving same-transfer requests as a zero-copy passthrough. Wired into `renderOrchestrator.ts` via `HdrCompositeContext.hdrImageTransferCache`, instantiated once per job and consumed by `blitHdrImageLayer` on both the main composite path and the transition path. Covered by `hdrImageTransferCache.test.ts` (hit/miss, distinct keys per image and per target transfer, LRU eviction + promotion, `maxEntries=0` passthrough, source-buffer immutability for cached entries, invalid options). Made-with: Cursor --- .../services/hdrImageTransferCache.test.ts | 229 ++++++++++++++++++ .../src/services/hdrImageTransferCache.ts | 143 +++++++++++ .../src/services/renderOrchestrator.ts | 32 ++- 3 files changed, 397 insertions(+), 7 deletions(-) create mode 100644 packages/producer/src/services/hdrImageTransferCache.test.ts create mode 100644 packages/producer/src/services/hdrImageTransferCache.ts diff --git a/packages/producer/src/services/hdrImageTransferCache.test.ts b/packages/producer/src/services/hdrImageTransferCache.test.ts new file mode 100644 index 000000000..63a6420e7 --- /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 000000000..687a76fe8 --- /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 36911bd1c..65e8adc10 100644 --- a/packages/producer/src/services/renderOrchestrator.ts +++ b/packages/producer/src/services/renderOrchestrator.ts @@ -99,6 +99,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. @@ -557,6 +562,7 @@ function blitHdrImageLayer( canvas: Buffer, el: ElementStackingInfo, hdrImageBuffers: Map, + hdrImageTransferCache: HdrImageTransferCache, width: number, height: number, log?: ProducerLogger, @@ -569,13 +575,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 +642,7 @@ interface HdrCompositeContext { effectiveHdr: { transfer: HdrTransfer }; nativeHdrImageIds: Set; hdrImageBuffers: Map; + hdrImageTransferCache: HdrImageTransferCache; hdrFrameDirs: Map; hdrVideoStartTimes: Map; imageTransfers: Map; @@ -689,6 +696,7 @@ async function compositeHdrFrame( effectiveHdr, nativeHdrImageIds, hdrImageBuffers, + hdrImageTransferCache, hdrFrameDirs, hdrVideoStartTimes, imageTransfers, @@ -736,6 +744,7 @@ async function compositeHdrFrame( canvas, layer.element, hdrImageBuffers, + hdrImageTransferCache, width, height, log, @@ -1920,6 +1929,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 +1946,7 @@ export async function executeRenderJob( effectiveHdr, nativeHdrImageIds, hdrImageBuffers, + hdrImageTransferCache, hdrFrameDirs, hdrVideoStartTimes, imageTransfers, @@ -2049,6 +2066,7 @@ export async function executeRenderJob( sceneBuf as Buffer, el, hdrImageBuffers, + hdrImageTransferCache, width, height, log,