Skip to content

fix(video): stop enlarging when both max dimensions are set - #18

Merged
rohanpoudel2 merged 1 commit into
mainfrom
fix/video-resize-enlarges
Aug 5, 2026
Merged

fix(video): stop enlarging when both max dimensions are set#18
rohanpoudel2 merged 1 commit into
mainfrom
fix/video-resize-enlarges

Conversation

@rohanpoudel2

Copy link
Copy Markdown
Owner

The bug

imgvidcompress video clip.mp4 --max-width 4000 --max-height 4000   # source is 320x240

produced a 4000x3000 file. A 12x upscale, from two options the README documents as "Shrink to fit, never enlarge" — and from a function whose own docstring reads "Scale to fit inside the requested box without upscaling."

Cause

force_original_aspect_ratio=decrease fits the frame inside the box, but happily scales up to reach it. The single-dimension branches already guarded against that:

if (w !== undefined) return `scale=w=min(iw\,${w}):h=-2`;   // clamped
return `scale=w=-2:h=min(ih\,${h})`;                         // clamped

The both-dimensions branch took the box literally, with no clamp.

Separately, ResizeOptions.withoutEnlargement was honoured by the image path (src/codecs/image.ts:89) but never read anywhere in src/codecs/video.ts — silently ignored for video. It is now respected, defaulting to true to match images, so false is the way to opt into upscaling.

Scope

This is the CLI and library, not just the MCP server where it surfaced. Anyone passing both --max-width and --max-height to bound video output has been getting upscaled files.

Verified through the CLI

Case Before After
320x240, box 4000x4000 4000x3000 320x240
1920x1080, box 640x640 640x360 640x360
320x240, --max-width 4000 only 320x240 320x240

Tests

Two unit tests on the filter string, plus an end-to-end test that encodes a real file and reads the dimensions back with ffprobe. The existing coverage asserted only that the filter contained force_divisible_by=2 and never checked an actual output, which is why this survived.

Found while adversarially testing the MCP server (#16) against ffprobe rather than trusting the tool's own report.

--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.
@rohanpoudel2
rohanpoudel2 merged commit 8c004df into main Aug 5, 2026
8 checks passed
@rohanpoudel2 rohanpoudel2 mentioned this pull request Aug 5, 2026
rohanpoudel2 added a commit that referenced this pull request Aug 5, 2026
Ships the video resize fix (#18): --max-width with --max-height no longer
enlarges a source smaller than the box. Patch, since it is a bug fix — the
documented behaviour never changed, only the code now matches it.

Two packaging gaps closed while releasing:

- MIGRATION.md is in "files". The published README links to it and it was
  never in the tarball, so the link was dead for anyone reading from
  node_modules.
- prepublishOnly runs lint and format:check as well. A prettier failure
  previously only surfaced in CI, after the push.

mcp/ now requires ^2.0.3 rather than ^2.0.2. Semver would have resolved to
2.0.3 anyway, but the constraint is a correctness statement: the MCP
schema tells agents maxWidth/maxHeight "Never enlarges", and that is only
true from 2.0.3 on.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant