Skip to content

[Security] Honor num_frames in Molmo2 default video sampling - #56207

Open
jperezdealgaba wants to merge 1 commit into
vllm-project:mainfrom
jperezdealgaba:fix/molmo2-honor-num-frames
Open

jperezdealgaba wants to merge 1 commit into
vllm-project:mainfrom
jperezdealgaba:fix/molmo2-honor-num-frames

Conversation

@jperezdealgaba

Copy link
Copy Markdown
Collaborator

Summary

The Molmo2 video loader's default sampling branch (frame_sample_mode unset) returned every source frame and ignored VideoTargetMetadata.num_frames. A client can select that CPU backend through request media_io_kwargs on an ordinary chat video request, so a small MP4 could expand into gigabytes of RGB even when num_frames was 1. This change uniformly samples at most num_frames when that cap is positive, and still returns all frames when the cap is unset (num_frames <= 0).

Changes

  • vllm/multimodal/video.py: honor a positive num_frames in Molmo2VideoBackend.compute_frames_index_to_sample when frame_sample_mode is omitted.
  • tests/multimodal/test_video.py: unit and loader tests for capped, uniform, and unlimited default sampling.
  • tests/multimodal/media/test_video.py: VideoMediaIO path with video_backend=molmo2 and num_frames=1.

Codepath coverage

  • Chat / messages / responses via MediaConnector.fetch_video / fetch_video_asyncVideoMediaIO.load_bytes / load_file / non-jpeg load_base64 → Molmo2 loader: yes.
  • Direct Molmo2VideoBackend.load_bytes (same compute_frames_index_to_sample): yes.
  • Decoder backends that call loader_cls.compute_frames_index_to_sample (OpenCV/TorchCodec/etc.): yes.
  • video/jpeg comma-split path: N/A (does not use the Molmo2 sampler).
  • GPU request-level backend stripping: N/A (molmo2 is a CPU backend).

Duplicate-work check

Searched open and merged PRs for Molmo2VideoBackend, compute_frames_index_to_sample, frame_sample_mode, and video_backend / num_frames. #51969 clamps request-level num_frames so clients cannot raise the engine ceiling; it does not make Molmo2's default branch read that ceiling. Other open video PRs cover GLM sampling, empty indices, or explicit frame-index selection. No open or merged PR closes this sink.

Tests

  • test_molmo2_default_mode_honors_positive_num_frames
  • test_molmo2_default_mode_uniformly_caps_to_num_frames
  • test_molmo2_default_mode_keeps_all_frames_when_unlimited
  • test_video_loader_frames_sampling[molmo2-default-honors-num_frames]
  • test_molmo2_backend_honors_num_frames

Commands: .venv/bin/pre-commit run --files <changed files> (passed after ruff format); .venv/bin/python -m pytest on the tests above (7 passed).

AI assistance

This PR was developed with AI assistance.

Made with Cursor

Co-authored-by: Cursor Agent
Signed-off-by: Juan Pérez de Algaba <jperezde@redhat.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added the multi-modality Related to multi-modality (#4194) label Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

multi-modality Related to multi-modality (#4194)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant