Skip to content

Cap video stills inside ffmpeg instead of after the pipe - #413

Merged
lstein merged 2 commits into
masterfrom
lstein/fix/cap-video-frame-in-ffmpeg
Sep 29, 2026
Merged

lstein merged 2 commits into
masterfrom
lstein/fix/cap-video-frame-in-ffmpeg

Conversation

@lstein

@lstein lstein commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Closes #366.

Problem

_frame_command asked ffmpeg for a PNG at the source's native resolution, and the MAX_FRAME_EDGE cap was applied afterwards in Python. A 4K frame therefore crossed the pipe and was decoded at full size, about 11 MB each (more for 10-bit HEVC), times the number of concurrent loaders.

Fix

scale=iw*sar:ih,showinfo,scale=w=min(2048\,iw):h=min(2048\,ih):force_original_aspect_ratio=decrease
  • Square the pixels first, then cap. Capping in the same scale as the SAR fix keeps the storage aspect and re-squashes anamorphic sources, which was the trap described in Video frame extraction pipes the full-resolution PNG before capping it #366.
  • Fit a box rather than cap the width. The width-only filter in the issue leaves a portrait 4K phone video at 2048x3640. min(…, iw) means small sources are never upscaled. The 2048 is taken from MAX_FRAME_EDGE.
  • Source resolution via showinfo. VideoInfo.width/height were read from the decoded frame, so ffmpeg-side capping would have labelled every 4K video 2048-wide. showinfo logs the geometry after rotation and SAR correction but before the cap. _showinfo_size() parses it from the metadata-stripped report.
  • frame.thumbnail(...) stays as a backstop.

Hardening from adversarial review

  • ffmpeg builds without showinfo: a missing filter fails the whole graph, which would drop every video. _ffmpeg_has_showinfo() probes ffmpeg -filters once per binary and leaves the filter out if it's absent. A failed probe is not cached. Without it, the resolution falls back to the frame's own size when uncapped, or is left blank at the cap.
  • Filenames containing newlines: ffmpeg echoes the input path in its banner, so a newline in a filename could forge showinfo, duration or codec lines. The ladder now removes the exact path from the stderr bytes before any parsing. The review reproduced a forged 99999x77777 resolution, and the new test catches it.

Other attacks the review tried that behaved correctly: 90°, 180° and flip rotation; SAR 2, 1/100 and 1/64; needle-shaped 2x16384 sources; rgba64 and 10-bit HEVC; a mid-stream resolution change with accurate seek (the first showinfo line always matched the frame that was output, on both ffmpeg 7.0.2 and 6.1); \r progress lines; and consumers of width/height handling None.

Measurements

End-to-end through extract_video_frame, bundled ffmpeg 7.0.2, noisy synthetic clips:

source max pipe payload time
4K landscape 11,439,064 → 3,059,929 2.34s → 1.48s
4K portrait (rotated) 11,044,150 → 3,014,828 2.29s → 1.49s
4K 10-bit HEVC 13,018,617 → 2,278,311 2.42s → 1.00s
720x480 SAR 32:27 51,125 → 47,381 (853x480, aspect 1.777) unchanged

The stored stills and the reported resolutions are the same as before.

Notes

  • FRAME_SELECTION_GENERATION is not bumped. ffmpeg's scaler differs slightly from PIL's, which could in rare cases change which frame is picked. That doesn't seem worth making every album re-extract its stills.
  • An unrelated finding from the review: the bundled ffmpeg 7.0.2 segfaults on MPEG-TS h264 test files, even with -vf null. It predates this change and is left alone here.

Tests

New tests in test_video_probe.py cover:

  • anamorphic aspect, with and without the cap;
  • a portrait video capped on its long edge;
  • the payload arriving capped from ffmpeg itself;
  • no upscaling of small sources;
  • showinfo parsing, and tag-injection attempts;
  • a missing showinfo filter;
  • a probe that fails to run not being cached;
  • a newline in the filename not forging the resolution or duration.

The full backend suite passes (1207), and ruff is clean.

🤖 Generated with Claude Code

Frame extraction asked ffmpeg for a native-resolution PNG and shrank it in
Python, so a 4K frame crossed the pipe at ~11 MB (more for 10-bit HEVC).
The filtergraph now squares pixels first, then fits the frame inside a
MAX_FRAME_EDGE box, which keeps anamorphic sources at their display aspect
and caps portrait video on its height.

The source resolution can no longer be read off the capped frame, so a
showinfo filter between the two scales logs it. Builds without showinfo
leave it out (probed once per binary) and fall back to the frame's own
size when uncapped. The input path is removed from ffmpeg's stderr before
parsing so a filename containing newlines cannot forge report lines.

Closes #366

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lstein
lstein enabled auto-merge (squash) September 29, 2026 22:32
@lstein
lstein merged commit 4793ae8 into master Sep 29, 2026
10 checks passed
@lstein
lstein deleted the lstein/fix/cap-video-frame-in-ffmpeg branch September 29, 2026 22:46
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.

Video frame extraction pipes the full-resolution PNG before capping it

1 participant