Skip to content

fix(vllm/omni): pass response_format into video EngineInputs (#14667) - #14844

Merged
rmccorm4 merged 2 commits into
ai-dynamo:mainfrom
Prudctual:cursor/omni-video-response-format-fbb4
Sep 18, 2026
Merged

rmccorm4 merged 2 commits into
ai-dynamo:mainfrom
Prudctual:cursor/omni-video-response-format-fbb4

Conversation

@Prudctual

@Prudctual Prudctual commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Overview

Pass response_format from the video request into EngineInputs so b64_json is not dropped for T2V/I2V.

Details

_engine_inputs_from_video() omitted response_format, so the formatter defaulted to a local file:// URL. Image/audio already forwarded delivery format. I2V shares the video builder. FFmpeg color corruption (issue part B) is out of scope.

Where should the reviewer start?

components/src/dynamo/vllm/omni/omni_handler.py then the new unit tests in test_omni_handler.py.

Related Issues

Summary by CodeRabbit

  • Bug Fixes
    • Video generation now respects the requested response format when returning results.
    • Requests specifying Base64-encoded JSON or a URL receive responses in the selected format.
    • Consistent response-format handling is supported across text-to-video and image-to-video generation workflows.

_engine_inputs_from_video dropped req.response_format, so b64_json
requests fell back to a pod-local file URL. Image and audio builders
already forward the delivery format; T2V and I2V share this path.

Signed-off-by: Jasim Kareem <mj9034812@gmail.com>

Co-authored-by: Jasim Kareem <mj9034812@gmail.com>
@Prudctual
Prudctual requested review from a team as code owners September 15, 2026 07:45
@copy-pr-bot

copy-pr-bot Bot commented Sep 15, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@Prudctual
Prudctual deployed to external_collaborator September 15, 2026 07:45 — with GitHub Actions Active
@Prudctual
Prudctual deployed to external_collaborator September 15, 2026 07:45 — with GitHub Actions Active
@github-actions github-actions Bot added the fix label Sep 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

👋 Hi Prudctual! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

@github-actions github-actions Bot added backend::vllm Relates to the vllm backend multimodal external-contribution Pull request is from an external contributor labels Sep 15, 2026
@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 25dad06d-2286-4782-b888-83af1297b47c

📥 Commits

Reviewing files that changed from the base of the PR and between b6a5b70 and e757801.

📒 Files selected for processing (2)
  • components/src/dynamo/vllm/omni/omni_handler.py
  • components/src/dynamo/vllm/tests/omni/test_omni_handler.py

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The video request handler now forwards req.response_format into EngineInputs. Tests verify b64_json and url for image generation, text-to-video, and image-to-video requests.

Changes

Response format propagation

Layer / File(s) Summary
Engine input forwarding and coverage
components/src/dynamo/vllm/omni/omni_handler.py, components/src/dynamo/vllm/tests/omni/test_omni_handler.py
Video request handling includes response_format in EngineInputs. Parameterized tests verify b64_json and url values for image and video generation paths.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~5 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to e7578

No supported merge-blocking risk remains for this change.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning Issue #14667 requires response-format preservation, decodable inline MP4 delivery for b64_json, URL delivery, FFmpeg/runtime color correction, and a model-free RGB-quadrant codec regression test. Th… Implement the required FFmpeg/runtime and x86 build changes. Add the model-free RGB-quadrant codec regression test. Add focused coverage or other reviewable evidence that a b64_json video response is a decodable inline MP4 while configure…
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description includes the required Overview, Details, reviewer-start, and Related Issues sections. It explains the change, identifies the files to review, links issue #14667, and states the out-of-…
Title check ✅ Passed The title clearly and concisely describes the main change: passing response_format into video EngineInputs in vllm/omni.
Out of Scope Changes check ✅ Passed The changed source line propagates response_format into video EngineInputs. The added tests verify response-format propagation for image, text-to-video, and image-to-video requests. These changes …
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.
Full details: Linked Issues check

Explanation

Issue #14667 requires response-format preservation, decodable inline MP4 delivery for b64_json, URL delivery, FFmpeg/runtime color correction, and a model-free RGB-quadrant codec regression test. The PR adds response_format=req.response_format to video EngineInputs. It adds tests for b64_json and url propagation for image, text-to-video, and image-to-video requests. The PR does not change the FFmpeg/runtime encoding path and does not add the required RGB-quadrant codec regression test. The available evidence also does not demonstrate decodable MP4 output.

Resolution

Implement the required FFmpeg/runtime and x86 build changes. Add the model-free RGB-quadrant codec regression test. Add focused coverage or other reviewable evidence that a b64_json video response is a decodable inline MP4 while configured URL delivery remains correct.

  • Fix all pre-merge checks with AI

Comment @coderabbitai help to get the list of available commands.

Comment thread components/src/dynamo/vllm/tests/omni/test_omni_handler.py Outdated
Comment thread components/src/dynamo/vllm/tests/omni/test_omni_handler.py Outdated
Comment thread components/src/dynamo/vllm/tests/omni/test_omni_handler.py
@Jont828

Jont828 commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

@Prudctual Thanks for picking this up! Can you sign the commit, seems like it's unverified.

@rmccorm4

Copy link
Copy Markdown
Contributor

/ok to test e757801

Copy link
Copy Markdown
Contributor Author

thanks for the look @Jont828. ill push a signed commit once signing is set up on my side (current one is unverified). noted the test nits as non-blocking and will leave those cases as is for now. appreciate the /ok to test from @rmccorm4 as well.

@rmccorm4

Copy link
Copy Markdown
Contributor

One small comment, otherwise LGTM: #14844 (comment)

Keep video and I2V coverage for b64_json and url. This PR only
threads response_format through the video engine path.

Signed-off-by: Jasim Kareem <mj9034812@gmail.com>

Co-authored-by: Jasim Kareem <mj9034812@gmail.com>
@pull-request-size pull-request-size Bot added size/S and removed size/M labels Sep 17, 2026
@cursor
cursor Bot deployed to external_collaborator September 17, 2026 19:35 Active
@rmccorm4
rmccorm4 enabled auto-merge (squash) September 17, 2026 19:57
@rmccorm4

Copy link
Copy Markdown
Contributor

/ok to test 4ffdc68

@rmccorm4
rmccorm4 merged commit 3264d8f into ai-dynamo:main Sep 18, 2026
108 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend::vllm Relates to the vllm backend external-contribution Pull request is from an external contributor fix multimodal size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: vllm-omni-video-agg on 1.4.2 drops base64 delivery and corrupts video colors

3 participants