Skip to content

Commit 5383e48

Browse files
miguel-heygenmiguel-heygen
authored
fix(producer): gif renders survive rgb frames among transparent ones (#4610)
* fix(producer): gif renders survive rgb frames among transparent ones * test(producer): run the gif mixed-frame encode test in the ffmpeg lane * fix(producer): gif encode test passes the argv, reachability and comment checks * fix(producer): transparent gifs clear every frame so an opaque frame never lingers * fix(producer): only gifs that turn translucent after an opaque frame get whole cleared frames * fix(core): plain text keeps its own gif control block; gif encode failures share one shape
1 parent e7a0b6b commit 5383e48

9 files changed

Lines changed: 451 additions & 58 deletions

File tree

‎packages/core/src/index.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,12 @@ export {
184184
export { RUNTIME_BOOTSTRAP_ATTR, stripEmbeddedRuntimeScripts } from "./compiler/htmlDocument";
185185
export { queryByAttr } from "./utils/cssSelector";
186186
export { decodeUrlPathVariants } from "./utils/urlPath";
187-
export { parseAnimatedGifMetadata, type AnimatedGifMetadata } from "./media/gif";
187+
export {
188+
clearGifFramesBeforeNext,
189+
gifClearsAfterLeavingFrameInPlace,
190+
parseAnimatedGifMetadata,
191+
type AnimatedGifMetadata,
192+
} from "./media/gif";
188193
export {
189194
HF_COLOR_GRADING_ATTR,
190195
HF_COLOR_GRADING_ADJUST_KEYS,

‎packages/core/src/media/gif.test.ts‎

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { describe, expect, it } from "vitest";
2-
import { parseAnimatedGifMetadata } from "./gif";
2+
import {
3+
clearGifFramesBeforeNext,
4+
gifClearsAfterLeavingFrameInPlace,
5+
parseAnimatedGifMetadata,
6+
} from "./gif";
37

48
function u16(value: number): number[] {
59
return [value & 0xff, (value >> 8) & 0xff];
@@ -90,3 +94,68 @@ describe("parseAnimatedGifMetadata", () => {
9094
expect(parseAnimatedGifMetadata(Uint8Array.from([0x89, 0x50, 0x4e, 0x47]))).toBeNull();
9195
});
9296
});
97+
98+
// A frame whose graphic control block has the given disposal and optional transparent index,
99+
// optionally carrying its own two-entry colour table.
100+
function controlledFrame(
101+
disposal: number,
102+
transparentIndex?: number,
103+
localTable = false,
104+
): number[] {
105+
const bytes = frame(10);
106+
bytes[3] = (disposal << 2) | (transparentIndex === undefined ? 0 : 1);
107+
bytes[6] = transparentIndex ?? 0;
108+
if (localTable) {
109+
bytes[17] = 0b1000_0000;
110+
bytes.splice(18, 0, 0, 0, 0, 255, 255, 255);
111+
}
112+
return bytes;
113+
}
114+
115+
function controls(bytes: Uint8Array): number[][] {
116+
return [...bytes.keys()]
117+
.filter((i) => bytes[i] === 0x21 && bytes[i + 1] === 0xf9 && bytes[i + 2] === 0x04)
118+
.map((i) => [bytes[i + 3]!, bytes[i + 6]!]);
119+
}
120+
121+
describe("gifClearsAfterLeavingFrameInPlace", () => {
122+
it("is true only when a frame left in place is followed by one cleared to background", () => {
123+
const leftInPlace = controlledFrame(1);
124+
const cleared = controlledFrame(2, 1);
125+
expect(gifClearsAfterLeavingFrameInPlace(gif([...leftInPlace, ...cleared]))).toBe(true);
126+
expect(gifClearsAfterLeavingFrameInPlace(gif([...cleared, ...leftInPlace]))).toBe(false);
127+
expect(gifClearsAfterLeavingFrameInPlace(gif([...leftInPlace, ...leftInPlace]))).toBe(false);
128+
expect(gifClearsAfterLeavingFrameInPlace(gif([...cleared, ...cleared]))).toBe(false);
129+
expect(gifClearsAfterLeavingFrameInPlace(Uint8Array.from(ascii("not a gif")))).toBeNull();
130+
});
131+
132+
it("does not give an image the control block of the plain text before it", () => {
133+
const plainText = [0x21, 0x01, 12, ...new Array<number>(12).fill(0), 1, 65, 0];
134+
const textLeftInPlace = [...controlledFrame(1).slice(0, 8), ...plainText];
135+
const imageWithoutControl = controlledFrame(2, 1).slice(8);
136+
const bytes = gif([...textLeftInPlace, ...imageWithoutControl, ...controlledFrame(2, 1)]);
137+
expect(gifClearsAfterLeavingFrameInPlace(bytes)).toBe(false);
138+
});
139+
});
140+
141+
describe("clearGifFramesBeforeNext", () => {
142+
it("clears every frame and names the shared table's transparent index where missing", () => {
143+
const bytes = gif([
144+
...controlledFrame(2, 1, true), // own table: its index means nothing in the shared one
145+
...controlledFrame(1),
146+
...controlledFrame(2, 0),
147+
]);
148+
149+
expect(clearGifFramesBeforeNext(bytes)).toBe(true);
150+
151+
expect(controls(bytes)).toEqual([
152+
[0b0000_1001, 1],
153+
[0b0000_1001, 0],
154+
[0b0000_1001, 0],
155+
]);
156+
});
157+
158+
it("refuses bytes that are not a GIF", () => {
159+
expect(clearGifFramesBeforeNext(Uint8Array.from(ascii("not a gif at all")))).toBe(false);
160+
});
161+
});

‎packages/core/src/media/gif.ts‎

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,17 @@ function parseApplicationExtension(
7676
return null;
7777
}
7878

79-
export function parseAnimatedGifMetadata(bytes: Uint8Array): AnimatedGifMetadata | null {
79+
const PLAIN_TEXT_LABEL = 0x01;
80+
81+
export interface GifFrameLayout {
82+
controlPackedOffset: number | null;
83+
localColorTable: boolean;
84+
}
85+
86+
export function parseAnimatedGifMetadata(
87+
bytes: Uint8Array,
88+
onFrame?: (frame: GifFrameLayout) => void,
89+
): AnimatedGifMetadata | null {
8090
if (bytes.length < 13) return null;
8191
const signature = readAscii(bytes, 0, 6);
8292
if (signature !== "GIF87a" && signature !== "GIF89a") return null;
@@ -92,6 +102,7 @@ export function parseAnimatedGifMetadata(bytes: Uint8Array): AnimatedGifMetadata
92102
}
93103

94104
let frameCount = 0;
105+
let controlPackedOffset: number | null = null;
95106
const delaysCentiseconds: number[] = [];
96107
let loopCount: number | null = null;
97108

@@ -113,6 +124,7 @@ export function parseAnimatedGifMetadata(bytes: Uint8Array): AnimatedGifMetadata
113124
const delay = readU16LE(bytes, pos + 2);
114125
if (delay == null) return null;
115126
delaysCentiseconds.push(normalizeDelayCentiseconds(delay));
127+
controlPackedOffset = pos + 1;
116128
pos += 1 + blockSize;
117129
if (bytes[pos] !== 0) return null;
118130
pos += 1;
@@ -127,6 +139,7 @@ export function parseAnimatedGifMetadata(bytes: Uint8Array): AnimatedGifMetadata
127139
continue;
128140
}
129141

142+
if (label === PLAIN_TEXT_LABEL) controlPackedOffset = null;
130143
const next = skipSubBlocks(bytes, pos);
131144
if (next == null) return null;
132145
pos = next;
@@ -136,8 +149,11 @@ export function parseAnimatedGifMetadata(bytes: Uint8Array): AnimatedGifMetadata
136149
if (introducer === 0x2c) {
137150
if (pos + 9 > bytes.length) return null;
138151
const imagePacked = bytes[pos + 8] ?? 0;
152+
const localColorTable = (imagePacked & 0b1000_0000) !== 0;
153+
onFrame?.({ controlPackedOffset, localColorTable });
154+
controlPackedOffset = null;
139155
pos += 9;
140-
if ((imagePacked & 0b1000_0000) !== 0) {
156+
if (localColorTable) {
141157
pos += colorTableByteLength(imagePacked);
142158
}
143159
if (pos >= bytes.length) return null;
@@ -164,3 +180,56 @@ export function parseAnimatedGifMetadata(bytes: Uint8Array): AnimatedGifMetadata
164180
animated: frameCount > 1,
165181
};
166182
}
183+
184+
const DISPOSAL_BITS = 0b0001_1100;
185+
const LEAVE_IN_PLACE = 1 << 2;
186+
const DISPOSE_TO_BACKGROUND = 2 << 2;
187+
const HAS_TRANSPARENT_INDEX = 0b0000_0001;
188+
const TRANSPARENT_INDEX_AFTER_PACKED = 3;
189+
190+
function frameLayouts(bytes: Uint8Array): GifFrameLayout[] | null {
191+
const frames: GifFrameLayout[] = [];
192+
return parseAnimatedGifMetadata(bytes, (frame) => frames.push(frame)) === null ? null : frames;
193+
}
194+
195+
function disposalOf(bytes: Uint8Array, frame: GifFrameLayout): number | null {
196+
return frame.controlPackedOffset === null
197+
? null
198+
: (bytes[frame.controlPackedOffset] ?? 0) & DISPOSAL_BITS;
199+
}
200+
201+
export function gifClearsAfterLeavingFrameInPlace(bytes: Uint8Array): boolean | null {
202+
const frames = frameLayouts(bytes);
203+
if (frames === null) return null;
204+
let sawOpaque = false;
205+
for (const frame of frames) {
206+
const disposal = disposalOf(bytes, frame);
207+
if (disposal === LEAVE_IN_PLACE) sawOpaque = true;
208+
else if (disposal === DISPOSE_TO_BACKGROUND && sawOpaque) return true;
209+
}
210+
return false;
211+
}
212+
213+
export function clearGifFramesBeforeNext(bytes: Uint8Array): boolean {
214+
const frames = frameLayouts(bytes);
215+
if (frames === null) return false;
216+
const controls = frames.flatMap((f) =>
217+
f.controlPackedOffset === null ? [] : [f.controlPackedOffset],
218+
);
219+
const shared = frames.flatMap((f) =>
220+
f.controlPackedOffset === null || f.localColorTable ? [] : [f.controlPackedOffset],
221+
);
222+
const hasIndex = (at: number) => ((bytes[at] ?? 0) & HAS_TRANSPARENT_INDEX) !== 0;
223+
const source = shared.find(hasIndex);
224+
for (const at of controls) {
225+
bytes[at] = ((bytes[at] ?? 0) & ~DISPOSAL_BITS) | DISPOSE_TO_BACKGROUND;
226+
}
227+
if (source === undefined) return true;
228+
for (const at of shared) {
229+
if (hasIndex(at)) continue;
230+
bytes[at] = (bytes[at] ?? 0) | HAS_TRANSPARENT_INDEX;
231+
bytes[at + TRANSPARENT_INDEX_AFTER_PACKED] =
232+
bytes[source + TRANSPARENT_INDEX_AFTER_PACKED] ?? 0;
233+
}
234+
return true;
235+
}

‎packages/producer/scripts/test-classification.mjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ const INTEGRATION_TEST_FILES = new Set([
3030
"src/services/htmlCompiler.naturalDuration.test.ts",
3131
"src/services/render/hlsRender.integration.test.ts",
3232
"src/services/render/stages/compileStage.mediaType.test.ts",
33+
"src/services/render/stages/gifEncodeArgs.test.ts",
3334
"src/utils/audioRegression.test.ts",
3435
"src/utils/streamDurationParity.test.ts",
3536
]);

‎packages/producer/src/services/render/stages/encodeStage.test.ts‎

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
1+
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
22
import { tmpdir } from "node:os";
33
import { join } from "node:path";
44
import { afterEach, describe, expect, it, mock } from "bun:test";
@@ -131,6 +131,8 @@ describe("gif encode args", () => {
131131
"-y",
132132
"-framerate",
133133
"15",
134+
"-reinit_filter",
135+
"0",
134136
"-i",
135137
"/tmp/hf/captured-frames/frame_%06d.jpg",
136138
"-vf",
@@ -144,6 +146,8 @@ describe("gif encode args", () => {
144146
"-y",
145147
"-framerate",
146148
"15",
149+
"-reinit_filter",
150+
"0",
147151
"-i",
148152
"/tmp/hf/captured-frames/frame_%06d.jpg",
149153
"-i",
@@ -402,20 +406,31 @@ describe("runEncodeStage config plumbing", () => {
402406
);
403407
});
404408

405-
it("encodes alpha GIFs from PNG frames with explicit transparency filters", async () => {
409+
async function encodeAlphaGif(paths: { framesDir: string; root: string }, outputPath: string) {
406410
const { runEncodeStage } = await import("./encodeStage.js");
407-
const paths = createFramesDir("png");
408-
409-
await runEncodeStage(
411+
return runEncodeStage(
410412
makeInput({
411413
framesDir: paths.framesDir,
412-
outputPath: join(paths.root, "out.gif"),
414+
outputPath,
413415
videoOnlyPath: join(paths.root, "video-only.mp4"),
414416
isGif: true,
415417
needsAlpha: true,
416418
captureImageFormat: "png",
417419
}),
418420
);
421+
}
422+
423+
it("encodes alpha GIFs from PNG frames with explicit transparency filters", async () => {
424+
const paths = createFramesDir("png");
425+
// One 1x1 frame whose graphic control block says "leave in place" (disposal 1).
426+
const oneFrameGif = Buffer.from(
427+
"47494638396101000100800000000000ffffff21f90405000000002c00000000010001000002024401003b",
428+
"hex",
429+
);
430+
const outputPath = join(paths.root, "out.gif");
431+
writeFileSync(outputPath, oneFrameGif);
432+
433+
await encodeAlphaGif(paths, outputPath);
419434

420435
expect(runFfmpegMock).toHaveBeenCalledTimes(2);
421436
expect(runFfmpegMock.mock.calls[0]?.[0]).toContain(join(paths.framesDir, "frame_%06d.png"));
@@ -426,6 +441,40 @@ describe("runEncodeStage config plumbing", () => {
426441
expect(runFfmpegMock.mock.calls[1]?.[0]).toContain(
427442
"fps=30 [x]; [x][1:v] paletteuse=dither=sierra2_4a:alpha_threshold=128",
428443
);
444+
// No frame turns translucent after an opaque one, so the plain encode is left untouched.
445+
expect(readFileSync(outputPath).equals(oneFrameGif)).toBe(true);
446+
});
447+
448+
it("re-encodes whole frames and clears them when a translucent frame follows an opaque one", async () => {
449+
const paths = createFramesDir("png");
450+
const image = "2c0000000001000100000202440100";
451+
// Frame 1 is left in place (opaque); frame 2 clears to background with transparent index 0.
452+
const staleProne = Buffer.from(
453+
`47494638396101000100800000000000ffffff21f9040400000000${image}21f9040900000000${image}3b`,
454+
"hex",
455+
);
456+
const outputPath = join(paths.root, "out.gif");
457+
writeFileSync(outputPath, staleProne);
458+
459+
await encodeAlphaGif(paths, outputPath);
460+
461+
expect(runFfmpegMock).toHaveBeenCalledTimes(3);
462+
expect(runFfmpegMock.mock.calls[1]?.[0]).not.toContain("-gifflags");
463+
expect(runFfmpegMock.mock.calls[2]?.[0]).toContain("-gifflags");
464+
const out = readFileSync(outputPath);
465+
const packed = [...out.keys()]
466+
.filter((i) => out[i] === 0x21 && out[i + 1] === 0xf9 && out[i + 2] === 0x04)
467+
.map((i) => out[i + 3]);
468+
expect(packed).toEqual([0b0000_1001, 0b0000_1001]);
469+
});
470+
471+
it("fails the encode when the GIF it wrote cannot be read back", async () => {
472+
const paths = createFramesDir("png");
473+
const outputPath = join(paths.root, "out.gif");
474+
writeFileSync(outputPath, "not a gif");
475+
476+
await expect(encodeAlphaGif(paths, outputPath)).rejects.toThrow("could not be parsed");
477+
expect(runFfmpegMock).toHaveBeenCalledTimes(3);
429478
});
430479

431480
it("keeps opaque GIF encoding on JPEG frames without alpha-only filters", async () => {
@@ -442,6 +491,7 @@ describe("runEncodeStage config plumbing", () => {
442491
}),
443492
);
444493

494+
expect(runFfmpegMock.mock.calls[1]?.[0]).not.toContain("-gifflags");
445495
expect(runFfmpegMock.mock.calls[0]?.[0]).toContain(join(paths.framesDir, "frame_%06d.jpg"));
446496
expect(runFfmpegMock.mock.calls[0]?.[0]).not.toContain("reserve_transparent");
447497
expect(runFfmpegMock.mock.calls[1]?.[0]).not.toContain("alpha_threshold");

0 commit comments

Comments
 (0)