Skip to content

[Security] Cap Qwen-VL video sampling knobs - #56729

Merged
Isotr0py merged 1 commit into
vllm-project:mainfrom
jperezdealgaba:fix/qwen-vl-sampling-caps
Sep 14, 2026
Merged

Isotr0py merged 1 commit into
vllm-project:mainfrom
jperezdealgaba:fix/qwen-vl-sampling-caps

Conversation

@jperezdealgaba

@jperezdealgaba jperezdealgaba commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Qwen2-VL and Qwen3-VL video samplers ignore num_frames and bound sampling on request-controlled max_frames and fps. A client can raise those knobs through media_io_kwargs and force the frontend to decode every source frame. This applies the same class-level ceilings already used by the GLMGA sampler (_MAX_FRAMES, _MAX_FPS), so a request may lower the sample count but cannot raise it past 768 frames or 30 fps.

Changes

  • vllm/multimodal/video.py: clamp max_frames and fps in Qwen2VLVideoBackend and Qwen3VLVideoBackend.

Codepath coverage

  • Sink is compute_frames_index_to_sample on both Qwen backends. OpenCV, TorchCodec, PyNvVideoCodec, and DeepStream all call that method, so every codec is covered.
  • Chat completions, embeddings, responses, tokenize, invocations, batch, and offline LLM all reach the same MediaConnectorVideoMediaIO → registered sampler chain.
  • video/jpeg load_base64 does not use these samplers (it keys off num_frames). The Rust frontend rejects media_io_kwargs and is unaffected.
  • Sibling loaders (Molmo2, glm5next, OpenPangu) have their own open PRs and are not changed here.

Duplicate-work check

Open/merged PRs searched for Qwen backends, max_frames, media_io_kwargs, and video sampling caps.

Tests

Existing TestGLMGASamplingCaps tests already cover the same clamping pattern on the GLMGA backend. The Qwen backends apply identical class-level _MAX_FRAMES / _MAX_FPS caps.

AI assistance

This PR was developed with AI assistance.

Made with Cursor

@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 multi-modality Related to multi-modality (#4194) qwen Related to Qwen models labels Sep 13, 2026
Comment thread tests/multimodal/test_video.py
Comment thread vllm/multimodal/video.py
* temporal_patch_size
)
n = total_frames_num / original_fps * target.fps
n = total_frames_num / original_fps * min(target.fps, cls._MAX_FPS)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think a better solution is making fps and max_frames static kwargs, which can't be overrided by arguments from request.

But anyway, let's leave it to be done in followup refactoring.

Request-level max_frames and fps were applied with no server ceiling, so
the Qwen2-VL and Qwen3-VL samplers could decode every source frame.
Clamp both knobs to the existing 768-frame and 30-fps class limits.

Co-authored-by: Cursor Agent
Signed-off-by: Juan Pérez de Algaba <jperezde@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@jperezdealgaba
jperezdealgaba force-pushed the fix/qwen-vl-sampling-caps branch from c97ebd3 to a389a7c Compare September 14, 2026 07:54
@Isotr0py
Isotr0py enabled auto-merge (squash) September 14, 2026 08:49
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 14, 2026
@Isotr0py

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #88769 for commit a389a7c9cba2.

@Isotr0py
Isotr0py merged commit ea723c8 into vllm-project:main Sep 14, 2026
105 checks passed
Shreya-gaur pushed a commit to Shreya-gaur/vllm_private that referenced this pull request Sep 14, 2026
Signed-off-by: Juan Pérez de Algaba <jperezde@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
ItsRoy69 pushed a commit to ItsRoy69/vllm that referenced this pull request Sep 15, 2026
Signed-off-by: Juan Pérez de Algaba <jperezde@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
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) qwen Related to Qwen models ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants