Repository navigation
[Frontend] MiniMax-H3: Add latent-mask editing to the ComfyUI extension - #7575
Conversation
|
This PR appears to be related to model: MinimaxH3. Model owners: @david6666666 @alex-jw-brooks @fhfuih Routing: @david6666666 via semantic router, model owner; @alex-jw-brooks via CODEOWNERS; @fhfuih via CODEOWNERS @avicii-forever, 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. |
179d56f to
8143c18
Compare
| 160, | ||
| 120, | ||
| 24, | ||
| 1.0 |
There was a problem hiding this comment.
Use a supported H3 duration; one second is rejected by the server.
There was a problem hiding this comment.
Thanks for the review! I'll update the example workflow to use a supported H3 duration.
There was a problem hiding this comment.
Fixed — the example workflow now uses duration: 4.458 (107 frames at 24 fps, which lies on H3's 17n+5 frame lattice); the previous 1.0 mapped to 24 frames and was rejected.
| except ImportError: | ||
| pytest.skip("ComfyUI / extension import unavailable", allow_module_level=True) | ||
|
|
||
| pytestmark = pytest.mark.skipif( |
There was a problem hiding this comment.
Move these tests under tests/e2e/features/comfyui and reuse its CPU fixtures.
There was a problem hiding this comment.
Thanks for the review! I'll move these tests under tests/e2e/features/comfyui and reuse its CPU fixtures.
There was a problem hiding this comment.
Fixed — the tests now live under tests/e2e/features/comfyui/ and reuse that directory's CPU fixtures (the conftest.py mocks for comfy_api / comfy_extras). The CUDA skip and the ComfyUI-checkout requirement are removed.
86cfa78 to
d923d61
Compare
|
Merged latest WhyThis branch was based on What changed
Verification
|
hsliuustc0106
added
frontend
End-to-end verification on real MiniMax-H3Verified EDIT-01 (#7465, server) and EDIT-02 (this PR, frontend) together on real hardware. Environment
Results
Operational note Example workflows (added to
|
Update: re-verified against the latest #7465#7465 has been rebased onto a newer Re-ran the smoke test against the updated combined branch
No regression from the rebase. |
|
Can we use another example?😂 Maybe we can refer comfyui's H3 mask edit example. |
princepride
left a comment
princepride
left a comment
There was a problem hiding this comment.
Thanks — node registration, types and colour family follow the existing VideoReferences pattern, and area-resizing the mask onto the latent grid is reasonable.
Requesting changes:
- Depends on an unsettled server contract. This client hard-codes #7465's current form fields and its 1 MiB text limit, while #7465 is unmerged and has open change requests on exactly that API surface. Please land this after #7465 and follow its final contract.
- The mock server can't catch contract drift (inline). The existing
test_comfyui_integration.pyfixture runs the real API server with a mockedAsyncOmni; please use it instead ofmock_videos_server.py. - Temporal mask mapping doesn't follow H3's VAE frame grouping (inline).
- Client re-implements server rules — both the shape lattice in
latent_mask.pyand the mask/source validation inapi_client.py(inline). Longer term it may be simpler for the server to accept any mask resolution and resize itself, so the client doesn't have to mirror the lattice.
Example workflows: the three templates are ~500 lines each and largely duplicate each other; please keep one (or two: image mask + temporal mask). They use a 160x120 test canvas (floored to 160x96 server-side) rather than a realistic H3 resolution such as 1344x768. More importantly, the default vLLM-Omni Latent Mask Editing.json feeds SolidMask 1.0, i.e. an all-generate mask, which the server treats as a no-op — so the template that says "Partially restyle the clip" doesn't edit the video at all, only the audio. Please default to a mask that actually preserves something.
| ) | ||
| if video_mask is not None: | ||
| mask_json = video_mask_to_grid_json(video_mask, width=width, height=height, num_frames=num_frames) | ||
| if len(mask_json) > 1024 * 1024: |
There was a problem hiding this comment.
This threshold mirrors the server's form-text limit (Starlette's 1 MiB max_part_size for non-file fields). Since the server accepts the mask as a JSON file part, please always send it that way and drop the magic number, so the client isn't coupled to a server-side constant.
| if video_mask is None and audio_mask is None: | ||
| raise ValueError("Latent-mask editing requires at least one mask.") | ||
|
|
||
| video_mask_trivial = video_mask is None or bool((video_mask == 1.0).all().item()) |
There was a problem hiding this comment.
The trivial-mask / required-source checks re-implement the server's validation (with a .item() on the mask), and the server already returns a clear 400 for these cases. Keeping only the "at least one mask" check would be enough on the client; scalar_mask_to_json's range check is likewise already covered by the node widget limits.
| return str(value) | ||
|
|
||
|
|
||
| def _align_frame_count(frame_count: int) -> int: |
There was a problem hiding this comment.
This mirrors the server's 17n+5 lattice, _video_latent_t and the //32 canvas floor, so any server change will silently produce wrong-shaped masks. Some dead code here too: the frame_count <= 0 branch is unreachable for a real request, and in _video_latent_t the <= 5 branch returns the same 2 the formula already gives for an aligned count of 5. The while loop can be replaced by arithmetic (n + (5 - n) % 17).
| if grid.shape[0] == 1: | ||
| grid = grid.expand(tv, gh, gw) | ||
| elif grid.shape[0] != tv: | ||
| grid = F.interpolate(grid.unsqueeze(0).unsqueeze(0), size=(tv, gh, gw), mode="nearest").squeeze(0).squeeze(0) |
There was a problem hiding this comment.
Nearest interpolation spreads mask slices uniformly over latent frames, but H3's VAE groups frames non-uniformly (first 5 frames -> 2 latents, then every 17 frames -> 5 latents). For a per-frame mask (T == num_frames, the natural ComfyUI shape), each latent picks a single frame's value, so a preserve->regenerate boundary inside a latent's frame group can end up preserved. Please map by the VAE grouping and take the max per group (regenerate wins) when T == num_frames. The relationship between T and num_frames is also not validated at the moment.
| from fastapi.responses import Response | ||
| from starlette.datastructures import UploadFile | ||
|
|
||
| app = FastAPI() |
There was a problem hiding this comment.
This mock records fields without validating them, so it can't detect client/server contract drift. For example, the 160x120 / 22-frame (~0.9 s) request in test_latent_edit_serialization would be rejected by the real server (resolve_minimax_h3_shape requires 4-15 s) but passes here. Please reuse the real-API-server fixture from test_comfyui_integration.py and drop this file.
| "source_video": ("VIDEO",), | ||
| "source_audio": ("AUDIO",), | ||
| "video_mask": ("MASK",), | ||
| "audio_mask": ("FLOAT", {"default": -1.0, "min": -1.0, "max": 1.0, "step": 0.01}), |
There was a problem hiding this comment.
Using -1.0 as an "unset" sentinel means any value in (-1, 0) is silently treated as unset. Making audio_mask an optional input socket (forceInput) would give None when unconnected and remove the sentinel.
| else: | ||
| form.add_field("video_noise_mask", mask_json) | ||
| if audio_mask is not None: | ||
| form.add_field("audio_noise_mask", scalar_mask_to_json(audio_mask)) |
There was a problem hiding this comment.
| else: | |
| form.add_field("video_noise_mask", mask_json) | |
| if audio_mask is not None: | |
| form.add_field("audio_noise_mask", scalar_mask_to_json(audio_mask)) |
Could we upload both masks as JSON files with a filename and content_type="application/json", regardless of payload size? Since #7465, the server declares both video_noise_mask and audio_noise_mask as UploadFile. The small-video-mask branch and the audio-mask field here send plain strings, which are rejected before inference.
check:
vllm-omni/vllm_omni/entrypoints/openai/video/generation/helpers.py
Lines 894 to 895 in 5a93ec1
661c49e to
435542f
Compare
|
Rebased onto latest Changes since the last review
Verification
|
|
Rebased onto latest What changedServer side (separate PR #7947)
Client side (this PR)
What I deliberately did NOT change (and why)
Support for the WF-05 workflow PR (#7898)#7898 (FayeSpica's WF-05) is branched directly from this PR, so I kept its interfaces intact and verified the combined client + WF-05 nodes on real hardware:
When #7898 rebases onto this PR, the only thing its author needs to do is drop the inherited grid-serialization helpers ( Verification
|
6ef3458 to
4fd43a8
Compare
E2E verification against #7898's WF-05 workflowRan this PR's raw-mask client + Environment: 2× RTX PRO 6000 Blackwell 96 GB (192 GB total), TP2, no offload; vllm 0.29.0 / torch 2.13.0+cu130; served model Test: loaded
Workflow portability issues (upstream in #7898's WF-05 JSON, not this PR's code) — worked around locally for the test:
|
…ask node (supports WF-05 workflows) Signed-off-by: chen hongwei <1792043268@qq.com>
Signed-off-by: chen hongwei <1792043268@qq.com>
81dc60b to
71f5a18
Compare
Omni ReviewBot: no human activity for 7 days@avicii-forever this pull request has had no human commit, comment or review since 2026-09-22. Please confirm the current plan and next step. The author or a maintainer decides whether to change the PR state. To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline. |
… uploads - Temporal Mask node now emits one slice per output frame instead of one per latent, so the server's frame-to-latent pooling preserves the intended prefix (WF-05 extension example: 32 latents instead of 9). - Area-downsample video masks by the VAE spatial stride and round values before upload so they stay within the server's 8 MiB mask limit. - Drop the audio_temporal_mask input: no node produces a [T]/[C, T] tensor and ComfyUI MASK inputs are [H, W]/[B, H, W]. - Restore the WF-05 README entry and remove the unused mock_videos_server.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Wang Zhipeng <wangzhipeng628@gmail.com>
…nvas The server now pads a short [T, H, W] video mask with its last slice instead of stretching it, so the Temporal Mask template's two-slice SolidMask batch ([0, 1]) preserved only the first latent and regenerated almost the whole clip. Build the mask with the MiniMax-H3 Temporal Mask node (one slice per output frame, continuation at preserve_fraction 0.5) as WF-05 does. Also drop the all-generate default template (SolidMask 1.0 is a no-op for the video; the Image Mask template covers the same flow) and move both remaining templates from the 160x120 test canvas to 1344x768. Add CPU tests that check the template wiring, node interfaces, canvas, and the per-frame mask source. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: princepride <wangzhipeng628@gmail.com>
The Image Mask and Temporal Mask templates duplicate WF-05: its object removal and inpainting cases send the same 2D spatial mask, and its continuation case uses the identical Load Video -> Get Video Components -> Temporal Mask wiring. Remove the two small templates and point the workflow tests at WF-05, checking every case's mask source, and that each temporal case's mode and duration match its Generate Video node. Default WF-05's Generate Video URL to :8000 like the other templates, and note in its usage guide that Load Image (as Mask) can replace a case's rectangle for an arbitrary-shape mask. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: princepride <wangzhipeng628@gmail.com>
6 participants
Purpose
Related: #7380
Add MiniMax H3 latent-mask editing to the ComfyUI extension. A new Latent Mask Editing node uploads the source media and serializes the video/audio noise masks the
/v1/videosAPI accepts, and Generate Video accepts a newlatent_editinput that forwards it to the client.source_video/source_audioupload the media to edit.video_maskis a ComfyUI mask image (0preserves,1regenerates, fractional blends). A 2D mask[H, W]is applied to every frame; a 3D mask[T, H, W]is a temporal mask for continuation or extension.audio_maskis a scalar in[0, 1].The mask is resized to the H3 latent grid using the server's own shape rules replicated client-side: the frame count snaps up to the
17n+5lattice and width/height floor to a multiple of 32, so the serialized[Tv, H/16, W/16]grid matches what the server computes. The client sendssource_video(mp4),source_audio(mp3),video_noise_mask(JSON grid), andaudio_noise_mask(JSON scalar) multipart fields. A non-trivial mask requires its matching source, and a source without a mask is rejected.Test Plan
python -m pytest tests/e2e/features/comfyui/test_latent_mask.py— raw-mask JSON serialization (2D/3D video masks, scalar/1D/2D audio masks).python -m pytest tests/e2e/features/comfyui/test_latent_mask_editing.py— asserts the exact multipart fields the client produces (masks uploaded as JSON file parts).python -m pytest tests/e2e/features/comfyui/test_comfyui_integration.py -k latent_mask— boots the real/v1/videosserver (mocked AsyncOmni) and exercises the latent-mask path.Test Result
Environment: NVIDIA H20, Python 3.12.13, pytest 9.1.1, torch 2.11.0+cu126.
Unit —
tests/e2e/features/comfyui/test_latent_mask.py:E2E —
tests/e2e/features/comfyui/test_latent_mask_editing.py(against a mock/v1/videosserver):test_latent_edit_serializationasserts the exact multipart fields the client produced:source_videofile part (video/mp4),audio_noise_maskscalar"0.5", andvideo_noise_maskresized to the aligned[Tv, H/16, W/16]grid —7x6x10for a160x120/ 22-frame request (height 120 floors to 96).