[armaanamatya]

Purpose

Fixes #7375.

vllm bench omni-duplex-eval evaluate sends judge frames at source resolution. Image tokens grow with pixel count, so on 4K clips (3734x2100 in the report, ~7.7k tokens per frame) 10-13 frames already exceed a 64K-token judge, and those samples fail with HTTP 500 every time (77k-101k token prompts in the report, 5 of 660 samples dropped).

This bounds frame resolution before JPEG encoding:

• extract_jpeg(..., max_side=None): if the longer side is larger than max_side, scale the frame down with the aspect ratio kept. Smaller frames are unchanged. The default is None, so the generate-side runner (iter_jpegs) is not affected.
• evaluate_sample(..., judge_frame_max_side=1920): applied to the temporal-window frames, the frame-sample content frames, and the frame-sample fallback that [Bugfix][Benchmark][Duplex] Fall back to frame-sample judging when the video exceeds the judge context #9101 added for judge context overflow in video_url mode. Without the cap that fallback's 16 frames of a 4K clip would still be ~123k tokens.
• CLI: --judge-frame-max-side (default 1920, 0 keeps source resolution, negative is rejected).
• Each score file records judge_frame_max_side, and summarize reports it. evaluate skips samples that already have a score, so a rerun with another cap would otherwise average different judge inputs; summarize now raises a ValueError for a mixed score root. Scores written before this PR have no field and used source resolution, so they count as None.

With the 1920 cap, the reported clip goes to 1920x1080, ~2k tokens per frame, so the 16-frame content prompt is ~33k tokens.

What changes and what doesn't:

• 1080p and smaller clips produce the same bytes as before, so their scores are unchanged.
• Temporal frames are extracted in both judge modes, so for clips larger than 1080p the judge input changes in video_url mode too. For those clips this departs from the pinned protocol, which is why the value is written to the score file. --judge-frame-max-side 0 restores the old behavior.
• Not covered: the temporal frame count is still window length x judge_fps with no cap, and there is no token-budget pre-check or skip/degrade path (items 1-2 in the issue). This PR only fixes the resolution part, which is what the reported failures come from.

Related: #9101 (merged) retries the content prompt with at most 16 sampled frames when the judge rejects the full video for context length; this PR bounds the resolution of those frames as well. #7387 (draft) handles the frame-count side with an opt-in retry using fewer frames after a context-length error. The two don't overlap in behavior and can land in either order, but they touch the same files (omni_duplex_eval_eval.py, the CLI, test_omni_duplex_eval_cli.py), so whichever lands second will need a rebase.

Test Plan

CUDA_VISIBLE_DEVICES="" python -m pytest -q tests/benchmarks/duplex \
  tests/e2e/features/fullduplex/test_omni_duplex_eval_cli.py \
  tests/e2e/accuracy/omni_duplex_eval/test_omni_duplex_eval_ci.py

New tests:

• test_max_side_scales_down_keeping_aspect_ratio: a 320x180 clip with max_side=160 gives 160x90.
• test_max_side_leaves_frames_within_limit_unchanged[equal|larger]: same bytes as without max_side.
• test_frame_sample_content_passes_frames: both temporal and content extraction receive the default max side, and the score file records it.
• test_cli_evaluate_forwards_judge_frame_max_side (default / 768 / 0 -> None) and test_cli_evaluate_rejects_negative_judge_frame_max_side.
• test_summary_records_a_uniform_judge_frame_max_side[source|capped], test_summary_rejects_mixed_judge_frame_max_side and test_summary_treats_a_score_without_the_field_as_source_resolution.
• test_content_prompt_falls_back_to_frame_sample_on_judge_context_overflow (from [Bugfix][Benchmark][Duplex] Fall back to frame-sample judging when the video exceeds the judge context #9101) now also checks that the fallback frames get the default max side.

vLLM Version: 0.32.0

vLLM-Omni Commit: c679762 (on main d6bd0fd)

Test Result

478 passed (CPU, WSL2 Ubuntu 24.04, Python 3.12, vLLM 0.32.0), both at c679762 and on a trial merge with main 4560304. ruff check / ruff format (0.14.10) clean.

The PR also passes max_side=judge_frame_max_side on the #9101 fallback's frame extraction, checked by the assertion above. The rebase onto d6bd0fd resolved conflicts with #9390 (judge_result wrapper) and #9056 (GT references) in omni_duplex_eval_eval.py, keeping main's code; the _extract_frames stub in #9390's test_rtd_requires_complete_scores_before_publishing now accepts the max_side keyword.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-17T02:50:15.200100Z	c1b49de	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[vllm-omni-review-bot]

This PR appears to belong to: docs/design/module/benchmarking.md.

Module owners: @alex-jw-brooks @Bounty-hunter @NickCao

Routing: @alex-jw-brooks via module of the changed files, CODEOWNERS; @Bounty-hunter via module of the changed files, CODEOWNERS; @NickCao via CODEOWNERS

@armaanamatya, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer.

Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment.

[armaanamatya]

Rechecked on current main (aa3fd1d9b, vLLM 0.32.0): head c1b49de92 merges cleanly, and tests/benchmarks/duplex + tests/e2e/features/fullduplex/test_omni_duplex_eval_cli.py give 128 passed (CPU). Main's #9145 removed test_omni_duplex_eval_ci_mock.py from the test plan's list; this PR doesn't touch that file.

@alex-jw-brooks @Bounty-hunter @yenuo26 @NickCao could one of you review? It caps judge frame resolution (default 1920 on the long side, 0 disables) so 4K clips stop overflowing the judge's context (#7375).

@vllm-omni-review-bot please review.

[armaanamatya]

Rebased onto main bd50aef74; head is now 05d6a4f24. The conflicts came from #9101 (frame-sample fallback on judge context overflow), #9175 and #8869 in omni_duplex_eval_eval.py and test_omni_duplex_eval_cli.py.

Beyond moving the PR onto the new code, the fallback's frame extraction now also gets max_side=judge_frame_max_side. #9101 caps the fallback at 16 frames but not their resolution, so 16 frames of a 4K clip would still be about 123k tokens; the #9101 fallback test now asserts the cap is applied there too.

tests/benchmarks/duplex + test_omni_duplex_eval_cli.py: 172 passed (CPU, vLLM 0.32.0), ruff clean. Description updated; its test plan drops test_omni_duplex_eval_ci_mock.py, which #9145 removed.

[armaanamatya]

@alex-jw-brooks @Bounty-hunter @yenuo26 @NickCao this is rebased onto today's main (05d6a4f24, see the comment above) and now also bounds the frames in the #9101 fallback path. Could one of you review? It is a small change: one max_side parameter on extract_jpeg, threaded through evaluate_sample and the CLI.

@vllm-omni-review-bot please review.

[armaanamatya]

Rebased onto main d6bd0fd16; head is now 1008060d7. The conflicts were with #9390 (judge_result around the temporal judge) and #9056 (GT references) in omni_duplex_eval_eval.py; the PR keeps main's code and only adds max_side=judge_frame_max_side to the three frame extractions. #9390's _extract_frames stub in test_omni_duplex_eval_judge.py now accepts the keyword; without that its four cases fail with a TypeError on this branch.

tests/benchmarks/duplex + test_omni_duplex_eval_cli.py: 256 passed (CPU, vLLM 0.32.0). Description updated.

@alex-jw-brooks @Bounty-hunter @yenuo26 @NickCao could one of you review? It is one max_side parameter on extract_jpeg, threaded through evaluate_sample and the CLI.

@vllm-omni-review-bot please review.

[jiangmengyu18]

Individual scores record judge_frame_max_side, but summaries neither retain nor validate it. Partial reruns with different caps can silently mix different judge inputs. Preserve a uniform setting or explicitly group/reject mixed settings.

[armaanamatya]

Fixed in c679762e3. summarize now reports judge_frame_max_side and raises a ValueError if the score root mixes values, naming them and pointing at --overwrite. Scores from before this PR have no field and used source resolution, so they count as None. The serving path always scores into a fresh root, so only the CLI can hit this.

The same commit renames the max_side test and covers both the equal and larger cases, per the nit from yenuo26. 478 passed (CPU), also on a trial merge with main 456030431.

[yenuo26]

Reviewed 1008060d (static; duplex unit/CLI suites not re-run here).

Looks good / CLEAN for self-approve. Bounding judge JPEG longer-side via extract_jpeg(..., max_side=) + evaluate_sample/CLI plumbing addresses #7375 without touching generate-side iter_jpegs (default None). Cap applied on temporal, frame-sample content, and the #9101 context-overflow fallback — the important coverage set. Score files record judge_frame_max_side, and --judge-frame-max-side 0 restores source resolution.

Tests: scale-down aspect (320×180 → 160×90), leave-unchanged at equal max_side, CLI default/768/0 + negative reject, and fallback max_side assertion all look right. _extract_frames keyword form is what production uses.

Soft nits (non-blocking):

1. test_max_side_leaves_smaller_frames_unchanged actually covers “not larger than” (max(size) > max_side is false for equal sides too). A one-line rename/comment would match the predicate.
2. Protocol note for video_url on >1080p clips is already in the PR body + score field — good; no further ask.

No helper-reuse or redundant Buildkite concerns. Fine once CI is green.

[yenuo26]

Re-reviewed tip c679762e (static; duplex suites not re-run here).

Looks good / CLEAN for self-approve. Tip addresses both open threads:

1. jiangmengyu18 — summarize_scores now records a uniform judge_frame_max_side and raises on mixed caps (legacy missing field ≡ None / source resolution). Covered by test_summary_records_a_uniform_judge_frame_max_side, test_summary_rejects_mixed_judge_frame_max_side, and test_summary_treats_a_score_without_the_field_as_source_resolution.
2. Prior soft nit — leave-unchanged test renamed to test_max_side_leaves_frames_within_limit_unchanged with equal|larger cases matching max(size) > max_side.

Production path unchanged in spirit: extract_jpeg(..., max_side=) + evaluate/CLI plumbing; #9101 fallback still capped. _extract_frames stub accepts **kwargs. No new helper-reuse or Buildkite concerns. Fine once CI is green.