From 1059106c7556c30c8771faa84843e11906bc2702 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Thu, 11 Jun 2026 00:04:56 -0700 Subject: [PATCH] fix: address review feedback from #1333 and #1335 --- docs/guides/rendering.mdx | 2 +- packages/cli/src/server/studioServer.ts | 2 +- packages/core/src/media/gif.test.ts | 9 +++ packages/core/src/media/gif.ts | 13 +++- .../src/services/animatedGifPrep.test.ts | 18 +++++ .../src/services/render/stages/encodeStage.ts | 71 ++++++++++--------- 6 files changed, 77 insertions(+), 38 deletions(-) diff --git a/docs/guides/rendering.mdx b/docs/guides/rendering.mdx index cf7a4c127..7cad224e0 100644 --- a/docs/guides/rendering.mdx +++ b/docs/guides/rendering.mdx @@ -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. diff --git a/packages/cli/src/server/studioServer.ts b/packages/cli/src/server/studioServer.ts index 1e38df09a..f275e8ddc 100644 --- a/packages/cli/src/server/studioServer.ts +++ b/packages/cli/src/server/studioServer.ts @@ -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 = - /]*?\bsrc\s*=\s*["'](https:\/\/[^"']+\.gif(?:[?#][^"']*)?)["'][^>]*>/gi; + /]*?\bsrc\s*=\s*["'](https?:\/\/[^"']+\.gif(?:[?#][^"']*)?)["'][^>]*>/gi; // ── Path resolution ───────────────────────────────────────────────────────── diff --git a/packages/core/src/media/gif.test.ts b/packages/core/src/media/gif.test.ts index ab76fe279..c29e3327d 100644 --- a/packages/core/src/media/gif.test.ts +++ b/packages/core/src/media/gif.test.ts @@ -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)); diff --git a/packages/core/src/media/gif.ts b/packages/core/src/media/gif.ts index 112bac2ae..036f242c7 100644 --- a/packages/core/src/media/gif.ts +++ b/packages/core/src/media/gif.ts @@ -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, diff --git a/packages/producer/src/services/animatedGifPrep.test.ts b/packages/producer/src/services/animatedGifPrep.test.ts index 543b90218..05bd9d42e 100644 --- a/packages/producer/src/services/animatedGifPrep.test.ts +++ b/packages/producer/src/services/animatedGifPrep.test.ts @@ -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(``, { + 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`); + }); }); diff --git a/packages/producer/src/services/render/stages/encodeStage.ts b/packages/producer/src/services/render/stages/encodeStage.ts index c4ef22b7f..dd666dcf5 100644 --- a/packages/producer/src/services/render/stages/encodeStage.ts +++ b/packages/producer/src/services/render/stages/encodeStage.ts @@ -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 { DEFAULT_CONFIG, @@ -145,44 +145,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) { - return { - success: false, - outputPath, - durationMs: Date.now() - startTime, - framesEncoded: 0, - fileSize: 0, - error: formatFfmpegError(paletteResult.exitCode, paletteResult.stderr), - }; - } + 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) { + 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(gifResult.exitCode, gifResult.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 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 {