fix: address review feedback from #1333 and #1335 (#1343)

Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
This commit is contained in:
Matt Van Horn
2026-06-12 16:18:18 -07:00
committed by GitHub
co-authored by Matt Van Horn
parent 1ea0ed55e3
commit 28e2ab9d5b
6 changed files with 78 additions and 39 deletions
+1 -1
View File
@@ -161,7 +161,7 @@ Use GIF when the output needs to autoplay inline in GitHub PRs, READMEs, issue r
npx hyperframes render --format gif --fps 15 --gif-loop 0 --output demo.gif
```
GIF output uses a two-pass FFmpeg palette encode (`palettegen` with diff statistics, then `paletteuse` with Sierra dithering) for better gradients and text edges than a single-pass conversion. GIFs are still much larger than MP4/WebM at the same dimensions, so prefer `--fps 15` and short compositions. Hyperframes caps GIF renders at 30fps.
GIF output uses a two-pass FFmpeg palette encode (`palettegen` with diff statistics, then `paletteuse` with Sierra dithering) for better gradients and text edges than a single-pass conversion. GIFs are still much larger than MP4/WebM at the same dimensions, so prefer short compositions. GIF renders are capped at 30fps; pass `--fps 15` for smaller files.
GIF does not carry audio and only has 1-bit transparency. For transparent overlays, use `--format webm`, `--format mov`, or `--format png-sequence` instead.
+1 -1
View File
@@ -32,7 +32,7 @@ import type { RenderJob } from "@hyperframes/producer";
const STUDIO_MANUAL_EDITS_PATH = ".hyperframes/studio-manual-edits.json";
const REMOTE_GIF_IMG_SRC_RE =
/<img\b[^>]*?\bsrc\s*=\s*["'](https:\/\/[^"']+\.gif(?:[?#][^"']*)?)["'][^>]*>/gi;
/<img\b[^>]*?\bsrc\s*=\s*["'](https?:\/\/[^"']+\.gif(?:[?#][^"']*)?)["'][^>]*>/gi;
// ── Path resolution ─────────────────────────────────────────────────────────
+9
View File
@@ -71,6 +71,15 @@ describe("parseAnimatedGifMetadata", () => {
expect(metadata?.durationSeconds).toBe(0.2);
});
it("clamps all-zero frame delays to the browser playback minimum", () => {
const frames = Array.from({ length: 10 }, () => frame(0)).flat();
const metadata = parseAnimatedGifMetadata(gif(frames, 0));
expect(metadata?.animated).toBe(true);
expect(metadata?.delaysCentiseconds).toEqual(Array.from({ length: 10 }, () => 10));
expect(metadata?.durationSeconds).toBe(1);
});
it("reads Netscape loop metadata", () => {
const metadata = parseAnimatedGifMetadata(gif([...frame(8), ...frame(8)], 0));
+10 -3
View File
@@ -9,6 +9,14 @@ export interface AnimatedGifMetadata {
animated: boolean;
}
const BROWSER_MIN_DELAY_CENTISECONDS = 10;
function normalizeDelayCentiseconds(delay: number): number {
// Chrome clamps GIF frame delays <= 1cs to 10cs (100ms); mirror browser playback timing.
if (delay <= 1) return BROWSER_MIN_DELAY_CENTISECONDS;
return Math.max(0, delay);
}
function readAscii(bytes: Uint8Array, start: number, length: number): string {
if (start + length > bytes.length) return "";
let value = "";
@@ -104,7 +112,7 @@ export function parseAnimatedGifMetadata(bytes: Uint8Array): AnimatedGifMetadata
if (blockSize !== 4 || pos + 6 > bytes.length) return null;
const delay = readU16LE(bytes, pos + 2);
if (delay == null) return null;
delaysCentiseconds.push(delay);
delaysCentiseconds.push(normalizeDelayCentiseconds(delay));
pos += 1 + blockSize;
if (bytes[pos] !== 0) return null;
pos += 1;
@@ -144,8 +152,7 @@ export function parseAnimatedGifMetadata(bytes: Uint8Array): AnimatedGifMetadata
return null;
}
const durationSeconds =
delaysCentiseconds.reduce((total, delay) => total + Math.max(0, delay), 0) / 100;
const durationSeconds = delaysCentiseconds.reduce((total, delay) => total + delay, 0) / 100;
return {
width,
@@ -208,4 +208,22 @@ describe("prepareAnimatedGifInputs", () => {
);
expect(result.preparedGifs[0]?.sourceSrc).toBe(sourceUrl);
});
it("propagates actionable transcode failure messages", async () => {
const projectDir = makeProject();
const sourcePath = join(projectDir, "broken.gif");
writeFileSync(sourcePath, gif([...frame(10), ...frame(10)], 0));
await expect(
prepareAnimatedGifInputs(`<img src="broken.gif" />`, {
projectDir,
downloadDir: projectDir,
transcode: async (request) => {
throw new Error(
`ffmpeg failed for ${request.inputPath}: Invalid data found when processing input`,
);
},
}),
).rejects.toThrow(`ffmpeg failed for ${sourcePath}: Invalid data found`);
});
});
@@ -28,7 +28,7 @@
* `success: false`.
*/
import { copyFileSync, existsSync, mkdirSync, readdirSync, statSync } from "node:fs";
import { copyFileSync, existsSync, mkdirSync, readdirSync, rmSync, statSync } from "node:fs";
import { dirname, join } from "node:path";
import {
encodeFramesChunkedConcat,
@@ -148,44 +148,49 @@ async function encodeGifFromDir(
fps: input.fps,
loop: input.loop,
};
const paletteResult = await runFfmpeg(buildGifPalettegenArgs(argsInput), {
signal: input.signal,
timeout: input.timeout,
});
if (!paletteResult.success) {
try {
const paletteResult = await runFfmpeg(buildGifPalettegenArgs(argsInput), {
signal: input.signal,
timeout: input.timeout,
});
if (!paletteResult.success) {
return {
success: false,
outputPath,
durationMs: Date.now() - startTime,
framesEncoded: 0,
fileSize: 0,
error: formatFfmpegError(paletteResult.exitCode, paletteResult.stderr),
};
}
const gifResult = await runFfmpeg(buildGifPaletteuseArgs(argsInput), {
signal: input.signal,
timeout: input.timeout,
});
if (!gifResult.success) {
return {
success: false,
outputPath,
durationMs: Date.now() - startTime,
framesEncoded: 0,
fileSize: 0,
error: formatFfmpegError(gifResult.exitCode, gifResult.stderr),
};
}
const fileSize = existsSync(outputPath) ? statSync(outputPath).size : 0;
return {
success: false,
success: true,
outputPath,
durationMs: Date.now() - startTime,
framesEncoded: 0,
fileSize: 0,
error: formatFfmpegError(paletteResult.exitCode, paletteResult.stderr),
framesEncoded: frameCount,
fileSize,
};
} finally {
// The GIF palette is a temp file; remove it after success or any encode failure.
rmSync(input.palettePath, { force: true });
}
const gifResult = await runFfmpeg(buildGifPaletteuseArgs(argsInput), {
signal: input.signal,
timeout: input.timeout,
});
if (!gifResult.success) {
return {
success: false,
outputPath,
durationMs: Date.now() - startTime,
framesEncoded: 0,
fileSize: 0,
error: formatFfmpegError(gifResult.exitCode, gifResult.stderr),
};
}
const fileSize = existsSync(outputPath) ? statSync(outputPath).size : 0;
return {
success: true,
outputPath,
durationMs: Date.now() - startTime,
framesEncoded: frameCount,
fileSize,
};
}
export async function runEncodeStage(input: EncodeStageInput): Promise<EncodeStageResult> {