From 85607eede76ccaf02c588571d44ffee64d9090b4 Mon Sep 17 00:00:00 2001 From: Rohan Poudel Date: Wed, 5 Aug 2026 13:10:04 -0600 Subject: [PATCH] fix(video): stop enlarging when both max dimensions are set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --max-width 4000 --max-height 4000 on a 320x240 clip produced a 4000x3000 file: a 12x upscale from two options documented as "Shrink to fit, never enlarge", and from a function whose own docstring says "without upscaling". force_original_aspect_ratio=decrease fits the frame inside the box but will scale up to reach it. The single-dimension branches already guarded against this with min(iw,W) / min(ih,H); the both-dimensions branch did not, so the box was taken literally. ResizeOptions.withoutEnlargement existed and was honoured by the image path, but src/codecs/video.ts never read it — the flag was silently ignored for video. It is now respected, defaulting to true to match the image path, so passing false is the way to opt into upscaling. Affects the CLI and the library, not only the MCP server where it surfaced. Reproduced through the CLI before fixing: before: 320x240 --max-width 4000 --max-height 4000 -> 4000x3000 after: 320x240 --max-width 4000 --max-height 4000 -> 320x240 still: 1920x1080 into a 640x640 box -> 640x360 Covered by two unit tests on the filter string and one end-to-end test that encodes a real file and reads the dimensions back with ffprobe, since the earlier tests asserted only that the filter contained force_divisible_by=2 and never checked the output. --- src/codecs/video.ts | 18 ++++++++++-- test/video.test.ts | 67 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/src/codecs/video.ts b/src/codecs/video.ts index 6183208..58374d0 100644 --- a/src/codecs/video.ts +++ b/src/codecs/video.ts @@ -290,11 +290,23 @@ export function buildScaleFilter(resize?: ResizeOptions): string | null { const h = resize?.maxHeight; if (w === undefined && h === undefined) return null; + // Matches the image path's default; an explicit false opts into upscaling. + const noEnlarge = resize?.withoutEnlargement ?? true; + if (w !== undefined && h !== undefined) { - return `scale=w=${w}:h=${h}:force_original_aspect_ratio=decrease:force_divisible_by=2`; + // Bound the box by the source itself. `force_original_aspect_ratio=decrease` + // fits the frame *inside* the box but will scale up to reach it, so a + // 320x240 clip asked to fit 4000x4000 came out at 4000x3000 — a 12x + // enlargement from options documented as "never enlarge". The + // single-dimension branches below already clamped; this one did not. + const boundW = noEnlarge ? `min(iw\\,${w})` : `${w}`; + const boundH = noEnlarge ? `min(ih\\,${h})` : `${h}`; + return `scale=w=${boundW}:h=${boundH}:force_original_aspect_ratio=decrease:force_divisible_by=2`; + } + if (w !== undefined) { + return noEnlarge ? `scale=w=min(iw\\,${w}):h=-2` : `scale=w=${w}:h=-2`; } - if (w !== undefined) return `scale=w=min(iw\\,${w}):h=-2`; - return `scale=w=-2:h=min(ih\\,${h})`; + return noEnlarge ? `scale=w=-2:h=min(ih\\,${h})` : `scale=w=-2:h=${h}`; } /** Curated path: validates the speed knob and maps quality onto the codec's CRF. */ diff --git a/test/video.test.ts b/test/video.test.ts index 85a6ecc..c7e2908 100644 --- a/test/video.test.ts +++ b/test/video.test.ts @@ -10,17 +10,21 @@ import { toQuality, toPixels } from "../src/types/brand.js"; import { resolveFfmpeg, resetFfmpegCache } from "../src/codecs/ffmpeg.js"; import { CompressorError } from "../src/core/errors.js"; -/** Read a stream's codec name back out of the encoded file. */ -function probeCodec(file: string, stream: "v" | "a"): Promise { +/** Read arbitrary stream fields back out of the encoded file. */ +function probeStream( + file: string, + entries: string, + stream: "v" | "a" = "v", +): Promise { const args = [ "-v", "error", "-select_streams", `${stream}:0`, "-show_entries", - "stream=codec_name", + `stream=${entries}`, "-of", - "default=noprint_wrappers=1:nokey=1", + "csv=p=0", file, ]; @@ -33,6 +37,11 @@ function probeCodec(file: string, stream: "v" | "a"): Promise { }); } +/** Read a stream's codec name back out of the encoded file. */ +function probeCodec(file: string, stream: "v" | "a"): Promise { + return probeStream(file, "codec_name", stream); +} + describe("video argument construction", () => { it("escapes the comma inside a scale expression", () => { // An unescaped comma is read by ffmpeg as an option separator. @@ -51,6 +60,31 @@ describe("video argument construction", () => { expect(buildScaleFilter({})).toBeNull(); }); + it("clamps both dimensions to the source, so a small input is never enlarged", () => { + // `force_original_aspect_ratio=decrease` fits the frame inside the box but + // scales *up* to reach it, so the box has to be bounded by the source. + // Without the clamp a 320x240 clip given a 4000x4000 box encoded at + // 4000x3000 — from options documented as "never enlarge". + const filter = buildScaleFilter({ + maxWidth: toPixels(4000), + maxHeight: toPixels(4000), + }); + + expect(filter).toContain("min(iw\\,4000)"); + expect(filter).toContain("min(ih\\,4000)"); + }); + + it("allows upscaling only when withoutEnlargement is explicitly false", () => { + const filter = buildScaleFilter({ + maxWidth: toPixels(4000), + maxHeight: toPixels(4000), + withoutEnlargement: false, + }); + + expect(filter).not.toContain("min("); + expect(filter).toContain("w=4000"); + }); + it("adds faststart for MP4 so playback can begin before download finishes", () => { const args = buildVideoArgs({ inputPath: "in.mov", @@ -134,6 +168,31 @@ describe.skipIf(!(await hasFfmpeg()))("video encoding (requires ffmpeg)", () => expect(await probeCodec(join(dir, "mp4-out", "clip.mp4"), "v")).toBe("h264"); }, 120_000); + it("does not enlarge a small source given a large resize box", async () => { + // The fixture is 320x240; the box is far larger in both dimensions. This + // encoded at 4000x3000 before the clamp, so assert against the real file + // rather than the filter string. + const src = join(dir, "no-enlarge"); + await makeVideo(join(src, "clip.mp4")); + + const report = await compressVideos([src], { + outDir: join(dir, "no-enlarge-out"), + resize: { + maxWidth: toPixels(4000), + maxHeight: toPixels(4000), + withoutEnlargement: true, + }, + skipLarger: false, + }); + + expect(report.summary.failed).toBe(0); + const size = await probeStream( + join(dir, "no-enlarge-out", "clip.mp4"), + "width,height", + ); + expect(size).toBe("320,240"); + }, 120_000); + it("produces a playable WebM, which v1 could not", async () => { // v1 muxed H.264 into WebM; ffmpeg refuses, so the run always failed. const src = join(dir, "webm");