Skip to content

Commit 53d6a0e

Browse files
committed
perf(producer): cache transfer-converted hdr image buffers per render job
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
1 parent d3899b1 commit 53d6a0e

3 files changed

Lines changed: 407 additions & 51 deletions

File tree

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { convertTransfer } from "@hyperframes/engine";
3+
import { createHdrImageTransferCache } from "./hdrImageTransferCache.ts";
4+
5+
/**
6+
* Build a deterministic rgb48le buffer for `pixelCount` pixels.
7+
* Each pixel is 3 channels × 2 bytes = 6 bytes. Values vary per pixel/channel
8+
* so the LUT-based `convertTransfer` produces bytes that differ from the
9+
* source.
10+
*/
11+
function makeSourceBuffer(pixelCount: number, seed = 0): Buffer {
12+
const buf = Buffer.alloc(pixelCount * 6);
13+
for (let i = 0; i < pixelCount; i++) {
14+
const off = i * 6;
15+
// Spread values across the 16-bit range so HLG↔PQ LUT lookups land on
16+
// mid-curve entries that are guaranteed to differ from the input.
17+
buf.writeUInt16LE((seed + i * 257) & 0xff_ff, off);
18+
buf.writeUInt16LE((seed + i * 521 + 1) & 0xff_ff, off + 2);
19+
buf.writeUInt16LE((seed + i * 1031 + 2) & 0xff_ff, off + 4);
20+
}
21+
return buf;
22+
}
23+
24+
function expectedConverted(source: Buffer, from: "hlg" | "pq", to: "hlg" | "pq"): Buffer {
25+
const copy = Buffer.from(source);
26+
convertTransfer(copy, from, to);
27+
return copy;
28+
}
29+
30+
describe("hdrImageTransferCache", () => {
31+
test("returns source buffer unchanged when sourceTransfer === targetTransfer", () => {
32+
const cache = createHdrImageTransferCache();
33+
const source = makeSourceBuffer(4);
34+
35+
const result = cache.getConverted("img1", "pq", "pq", source);
36+
37+
expect(result).toBe(source);
38+
expect(cache.size()).toBe(0);
39+
});
40+
41+
test("first miss converts and caches", () => {
42+
const cache = createHdrImageTransferCache();
43+
const source = makeSourceBuffer(4);
44+
const expected = expectedConverted(source, "hlg", "pq");
45+
46+
const result = cache.getConverted("img1", "hlg", "pq", source);
47+
48+
expect(result).not.toBe(source);
49+
expect(Buffer.compare(result, expected)).toBe(0);
50+
expect(cache.size()).toBe(1);
51+
});
52+
53+
test("second hit returns cached buffer reference", () => {
54+
const cache = createHdrImageTransferCache();
55+
const source = makeSourceBuffer(4);
56+
57+
const first = cache.getConverted("img1", "hlg", "pq", source);
58+
const second = cache.getConverted("img1", "hlg", "pq", source);
59+
60+
expect(second).toBe(first);
61+
expect(cache.size()).toBe(1);
62+
});
63+
64+
test("does not re-run convertTransfer on cache hit", () => {
65+
const cache = createHdrImageTransferCache();
66+
const source = makeSourceBuffer(4);
67+
68+
const first = cache.getConverted("img1", "hlg", "pq", source);
69+
const snapshot = Buffer.from(first);
70+
// If a hit ran convertTransfer again on the cached buffer (PQ→PQ would
71+
// be a no-op, but PQ→HLG would mutate), the bytes would change.
72+
cache.getConverted("img1", "hlg", "pq", source);
73+
74+
expect(Buffer.compare(first, snapshot)).toBe(0);
75+
});
76+
77+
test("different target transfers for same imageId are cached independently", () => {
78+
const cache = createHdrImageTransferCache();
79+
const source = makeSourceBuffer(4);
80+
81+
const toPq = cache.getConverted("img1", "hlg", "pq", source);
82+
const toHlg = cache.getConverted("img1", "pq", "hlg", source);
83+
84+
expect(toPq).not.toBe(toHlg);
85+
expect(Buffer.compare(toPq, expectedConverted(source, "hlg", "pq"))).toBe(0);
86+
expect(Buffer.compare(toHlg, expectedConverted(source, "pq", "hlg"))).toBe(0);
87+
expect(cache.size()).toBe(2);
88+
});
89+
90+
test("different imageIds are cached independently", () => {
91+
const cache = createHdrImageTransferCache();
92+
const a = makeSourceBuffer(4, 100);
93+
const b = makeSourceBuffer(4, 200);
94+
95+
const convA = cache.getConverted("a", "hlg", "pq", a);
96+
const convB = cache.getConverted("b", "hlg", "pq", b);
97+
98+
expect(convA).not.toBe(convB);
99+
expect(Buffer.compare(convA, expectedConverted(a, "hlg", "pq"))).toBe(0);
100+
expect(Buffer.compare(convB, expectedConverted(b, "hlg", "pq"))).toBe(0);
101+
expect(cache.size()).toBe(2);
102+
});
103+
104+
test("LRU evicts oldest entry when maxEntries exceeded", () => {
105+
const cache = createHdrImageTransferCache({ maxEntries: 2 });
106+
const a = makeSourceBuffer(2, 1);
107+
const b = makeSourceBuffer(2, 2);
108+
const c = makeSourceBuffer(2, 3);
109+
110+
const convA1 = cache.getConverted("a", "hlg", "pq", a);
111+
cache.getConverted("b", "hlg", "pq", b);
112+
cache.getConverted("c", "hlg", "pq", c);
113+
114+
expect(cache.size()).toBe(2);
115+
116+
const convA2 = cache.getConverted("a", "hlg", "pq", a);
117+
expect(convA2).not.toBe(convA1);
118+
expect(Buffer.compare(convA2, expectedConverted(a, "hlg", "pq"))).toBe(0);
119+
});
120+
121+
test("access promotes entry to most-recently-used", () => {
122+
const cache = createHdrImageTransferCache({ maxEntries: 2 });
123+
const a = makeSourceBuffer(2, 1);
124+
const b = makeSourceBuffer(2, 2);
125+
const c = makeSourceBuffer(2, 3);
126+
127+
const convA1 = cache.getConverted("a", "hlg", "pq", a);
128+
cache.getConverted("b", "hlg", "pq", b);
129+
130+
const convA2 = cache.getConverted("a", "hlg", "pq", a);
131+
expect(convA2).toBe(convA1);
132+
133+
cache.getConverted("c", "hlg", "pq", c);
134+
135+
const convA3 = cache.getConverted("a", "hlg", "pq", a);
136+
expect(convA3).toBe(convA1);
137+
138+
const convB2 = cache.getConverted("b", "hlg", "pq", b);
139+
expect(Buffer.compare(convB2, expectedConverted(b, "hlg", "pq"))).toBe(0);
140+
expect(cache.size()).toBe(2);
141+
});
142+
143+
test("maxEntries: 0 disables caching but still returns correct converted bytes", () => {
144+
const cache = createHdrImageTransferCache({ maxEntries: 0 });
145+
const source = makeSourceBuffer(4);
146+
const expected = expectedConverted(source, "hlg", "pq");
147+
148+
const first = cache.getConverted("img1", "hlg", "pq", source);
149+
const second = cache.getConverted("img1", "hlg", "pq", source);
150+
151+
expect(first).not.toBe(second);
152+
expect(Buffer.compare(first, expected)).toBe(0);
153+
expect(Buffer.compare(second, expected)).toBe(0);
154+
expect(cache.size()).toBe(0);
155+
});
156+
157+
test("cached buffer is independent from the source buffer", () => {
158+
const cache = createHdrImageTransferCache();
159+
const source = makeSourceBuffer(4);
160+
const sourceSnapshot = Buffer.from(source);
161+
162+
const cached = cache.getConverted("img1", "hlg", "pq", source);
163+
source.fill(0);
164+
165+
expect(cache.getConverted("img1", "hlg", "pq", source)).toBe(cached);
166+
expect(Buffer.compare(cached, expectedConverted(sourceSnapshot, "hlg", "pq"))).toBe(0);
167+
});
168+
169+
// Source-buffer-immutability guarantee (PR #384 review feedback): the cache
170+
// MUST NOT mutate the source buffer the caller hands in, on any path.
171+
// `convertTransfer` mutates in place, so the implementation has to clone
172+
// before converting — these tests pin the invariant against future
173+
// refactors that might forget the `Buffer.from(source)` defense.
174+
175+
test("does not mutate the source buffer on a convert+cache miss", () => {
176+
const cache = createHdrImageTransferCache();
177+
const source = makeSourceBuffer(4);
178+
const sourceSnapshot = Buffer.from(source);
179+
180+
cache.getConverted("img1", "hlg", "pq", source);
181+
182+
expect(Buffer.compare(source, sourceSnapshot)).toBe(0);
183+
});
184+
185+
test("does not mutate the source buffer on a convert+cache miss with maxEntries=0 passthrough", () => {
186+
const cache = createHdrImageTransferCache({ maxEntries: 0 });
187+
const source = makeSourceBuffer(4);
188+
const sourceSnapshot = Buffer.from(source);
189+
190+
const result = cache.getConverted("img1", "hlg", "pq", source);
191+
192+
expect(Buffer.compare(source, sourceSnapshot)).toBe(0);
193+
expect(result).not.toBe(source);
194+
expect(Buffer.compare(result, expectedConverted(sourceSnapshot, "hlg", "pq"))).toBe(0);
195+
expect(cache.size()).toBe(0);
196+
});
197+
198+
test("does not mutate the source buffer on a cache hit", () => {
199+
const cache = createHdrImageTransferCache();
200+
const source = makeSourceBuffer(4);
201+
const sourceSnapshot = Buffer.from(source);
202+
203+
cache.getConverted("img1", "hlg", "pq", source);
204+
cache.getConverted("img1", "hlg", "pq", source);
205+
206+
expect(Buffer.compare(source, sourceSnapshot)).toBe(0);
207+
});
208+
209+
test("rejects invalid maxEntries", () => {
210+
expect(() => createHdrImageTransferCache({ maxEntries: -1 })).toThrow();
211+
expect(() => createHdrImageTransferCache({ maxEntries: 1.5 })).toThrow();
212+
expect(() => createHdrImageTransferCache({ maxEntries: Number.NaN })).toThrow();
213+
});
214+
215+
test("default maxEntries is large enough for typical compositions", () => {
216+
const cache = createHdrImageTransferCache();
217+
const source = makeSourceBuffer(2);
218+
219+
for (let i = 0; i < 16; i++) {
220+
cache.getConverted(`img${i}`, "hlg", "pq", source);
221+
}
222+
expect(cache.size()).toBe(16);
223+
224+
// The first inserted entry should still be present (no eviction yet).
225+
const first = cache.getConverted("img0", "hlg", "pq", source);
226+
expect(Buffer.compare(first, expectedConverted(source, "hlg", "pq"))).toBe(0);
227+
expect(cache.size()).toBe(16);
228+
});
229+
});
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
import { type HdrTransfer, convertTransfer } from "@hyperframes/engine";
2+
3+
/**
4+
* Cache of transfer-converted HDR image buffers keyed by
5+
* `(imageId, targetTransfer)`.
6+
*
7+
* ## Why this exists
8+
*
9+
* Static HDR images are decoded once per render at setup time and stored as
10+
* `rgb48le` buffers in `hdrImageBuffers`. When the encode target's transfer
11+
* function (e.g. `pq`) differs from the image's source transfer (e.g. `hlg`),
12+
* `blitHdrImageLayer` must run an LUT-based transfer conversion before
13+
* blitting. `convertTransfer` mutates its input in-place, so the call site
14+
* has historically allocated a fresh `Buffer.from(buf.data)` clone every
15+
* frame to keep the original decode pristine for subsequent frames.
16+
*
17+
* For a 30 s, 60 fps, 1080p render with one HDR image, that's:
18+
*
19+
* - ~1800 × `Buffer.from(...)` allocations (~12 MB each → ~22 GB churn)
20+
* - ~1800 × ~6 M LUT lookups in `convertTransfer`
21+
*
22+
* Both are pure functions of `(source bytes, sourceTransfer, targetTransfer)`,
23+
* and within a single render job the source bytes for a given `imageId` are
24+
* fixed. Caching the converted buffer per `(imageId, targetTransfer)` reduces
25+
* the work to one allocation and one LUT pass per unique pair — independent
26+
* of frame count.
27+
*
28+
* ## Lifetime
29+
*
30+
* Instances are constructed per render job and dropped on job exit (success
31+
* or failure) by going out of scope. **Do not reuse a single cache across
32+
* jobs** — `imageId` collisions could return stale converted bytes from a
33+
* different source buffer.
34+
*
35+
* ## Bounds
36+
*
37+
* The cache is LRU-bounded by entry count (default 16). At 1080p each entry
38+
* is ~12 MB, so the default cap is ~200 MB worst case. Compositions with
39+
* more unique HDR images than `maxEntries` will evict older entries on a
40+
* least-recently-used basis; cache misses just rebuild the converted buffer.
41+
*
42+
* ## Caller contract
43+
*
44+
* The buffer returned by `getConverted` is shared cache state and **MUST NOT
45+
* be mutated** by the caller. All downstream HDR blit functions
46+
* (`blitRgb48leAffine`, `blitRgb48leRegion`) read from it without writing,
47+
* so this is naturally upheld today.
48+
*/
49+
export interface HdrImageTransferCache {
50+
/**
51+
* Return a buffer in `targetTransfer` for the given image.
52+
*
53+
* - When `sourceTransfer === targetTransfer`, returns `source` unchanged
54+
* (no allocation, no caching).
55+
* - On the first call for `(imageId, targetTransfer)`, clones `source`,
56+
* converts in-place via {@link convertTransfer}, caches the result, and
57+
* returns it.
58+
* - On subsequent calls with the same `(imageId, targetTransfer)`, returns
59+
* the cached buffer (and promotes it to most-recently-used).
60+
*
61+
* The returned buffer is read-only from the caller's perspective.
62+
*/
63+
getConverted(
64+
imageId: string,
65+
sourceTransfer: HdrTransfer,
66+
targetTransfer: HdrTransfer,
67+
source: Buffer,
68+
): Buffer;
69+
70+
/** Number of currently cached entries. Diagnostic / test aid. */
71+
size(): number;
72+
}
73+
74+
export interface HdrImageTransferCacheOptions {
75+
/**
76+
* Maximum number of converted buffers to retain before evicting the
77+
* least-recently-used entry. Defaults to 16. Must be a non-negative
78+
* integer; `0` disables caching entirely (every call allocates fresh).
79+
*/
80+
maxEntries?: number;
81+
}
82+
83+
const DEFAULT_MAX_ENTRIES = 16;
84+
85+
export function createHdrImageTransferCache(
86+
options: HdrImageTransferCacheOptions = {},
87+
): HdrImageTransferCache {
88+
const maxEntries = options.maxEntries ?? DEFAULT_MAX_ENTRIES;
89+
if (!Number.isInteger(maxEntries) || maxEntries < 0) {
90+
throw new Error(
91+
`createHdrImageTransferCache: maxEntries must be a non-negative integer, got ${String(maxEntries)}`,
92+
);
93+
}
94+
95+
// Map iteration order is insertion order in JS, so promoting an entry to
96+
// most-recently-used is just a `delete` + `set`. The first key in the
97+
// iterator is therefore the LRU candidate.
98+
const entries = new Map<string, Buffer>();
99+
100+
function makeKey(imageId: string, targetTransfer: HdrTransfer): string {
101+
return `${imageId}|${targetTransfer}`;
102+
}
103+
104+
return {
105+
getConverted(imageId, sourceTransfer, targetTransfer, source) {
106+
if (sourceTransfer === targetTransfer) {
107+
return source;
108+
}
109+
110+
if (maxEntries === 0) {
111+
const fresh = Buffer.from(source);
112+
convertTransfer(fresh, sourceTransfer, targetTransfer);
113+
return fresh;
114+
}
115+
116+
const key = makeKey(imageId, targetTransfer);
117+
const existing = entries.get(key);
118+
if (existing) {
119+
// Promote to MRU.
120+
entries.delete(key);
121+
entries.set(key, existing);
122+
return existing;
123+
}
124+
125+
const converted = Buffer.from(source);
126+
convertTransfer(converted, sourceTransfer, targetTransfer);
127+
128+
if (entries.size >= maxEntries) {
129+
// Evict LRU (first key in insertion-ordered iterator).
130+
const lruKey = entries.keys().next().value;
131+
if (lruKey !== undefined) {
132+
entries.delete(lruKey);
133+
}
134+
}
135+
entries.set(key, converted);
136+
return converted;
137+
},
138+
139+
size() {
140+
return entries.size;
141+
},
142+
};
143+
}

0 commit comments

Comments
 (0)