Skip to content

[4/N] Unify worker-side MP4 encoding and wire Wan into it - #7048

Merged
hsliuustc0106 merged 13 commits into
vllm-project:mainfrom
specture724:post-VAE-overlap-4-of-4-wan-consumer
Sep 17, 2026
Merged

hsliuustc0106 merged 13 commits into
vllm-project:mainfrom
specture724:post-VAE-overlap-4-of-4-wan-consumer

Conversation

@specture724

@specture724 specture724 commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

[4/4] Unify worker-side MP4 encoding and wire Wan into it

Part of the RFC #6872 implementation stack (4/4), stacked on #7016, #7017, and
#7018. GitHub can't set a fork branch as another PR's base, so this targets
main like the rest of the stack; the diff includes the earlier PRs' commits
until they merge.

Summary

[1/N] added the Wan VAE chunk callback but nothing called it. This wires it up
and, now that a second consumer exists, moves the consumer itself into the
framework.

Producing chunks is the model's business: Wan walks a causal frame-by-frame
loop, H3 walks overlapping clips, and Wan S2V decodes one clip per
autoregressive iteration. What happens after a chunk is committed is not --
every one of them quantizes to uint8, moves to the host once, and lands in a
bounded encoder. Each model carried its own copy of that, so a capability meant
to be model-agnostic had a per-backend consumer bolted onto it.

diffusion/utils/chunked_video.py now owns that half:

  • ChunkedVideoMP4Session for producers that drive themselves (Wan S2V pushes
    finished clips out of its autoregressive loop).
  • decode_to_mp4() for producers that can be driven through
    SupportsChunkedVAEDecode.

Batching, per-batch-entry encoders, audio muxing, the requested-size crop, and
quantization from the producer's declared chunk_value_range are parameters.
H3, Wan T2V/I2V, and Wan S2V share it: wan2_2/chunked_mp4.py drops from 219
to 77 lines (only request-option plumbing remains) and H3's decode_to_mp4
keeps only what is H3-specific -- decoding audio first, the crop, the fixed
rate. A model that wants worker-side encoding now implements one method and
declares one pixel range.

With preencode_mp4, each committed chunk leaves the accelerator while later
chunks are still decoding, so host transfer and H.264 encoding overlap the
remaining decode instead of following it. The pre-encoded path emits MP4 bytes,
so it rejects rather than silently downgrades what it cannot serve:
enable_frame_interpolation, which needs the frames it no longer materializes,
and non-array output types.

Validation

Two measurements, at different scopes. They are not comparable to each other.

VAE decode stage, T2V — real Wan2.1-T2V-1.3B VAE weights, L20X, latent
[1,16,65,60,104] → 257 frames at 480×832. Denoising is excluded, so this
isolates the stage the callback changes:

Path Total (avg of 3) Host transfers
Full decode + one-shot mux ~13.05 s 1
Wired pre-encode, per chunk ~9.68 s 65
Wired pre-encode, 17-frame batches ~9.81 s 13

~26% off the decode-plus-encode stage at an identical 257-frame count.
Consumer batching trades 65 host transfers for 13; wall clock is within noise of
the per-chunk path on this GPU, so the default is about transfer count, not
latency.

Full request, S2V — real Wan2.2-S2V-14B weights, L20X, TP=1, 448×448, 77
frames, 4 steps, warmed up, both orders run to expose ordering effects. The
baseline's MP4 encoding happens downstream of generate(), so it is measured
and added:

Path Run 1 Run 2
Baseline generate() 15.95 s 15.86 s
Baseline downstream mux 0.48 s 0.46 s
Baseline end to end 16.44 s 16.31 s
Pre-encode end to end 15.80 s 15.86 s

~3% end to end. The win is small here because denoising dominates a full S2V
request and the mux is only ~0.47 s of ~16.4 s; it is the same ~0.5 s of
encoding, now absorbed into the decode loop. An earlier unwarmed run showed
~32%, which was entirely first-call overhead — that number is not real.

Output equivalence: 77 frames on both paths, audio stream present in the
pre-encoded container. The consumer move is behavior-preserving: real
Wan2.1-T2V-1.3B weights decode to the same 257 frames with an unchanged
pre-encode SHA. Decoded pixels differ by lossy H.264 re-encoding
(max_abs 37, mean_abs 1.70), which is expected; the bit-exact signal is the
pre-encode tensor comparison in [1/3].

Tests: 11 new cases for the framework consumer, covering per-batch-entry
containers, the crop, audio muxing, ranks that own no output, a VAE that does
not declare the capability, and the property the shared quantizer exists for --
a Wan [-1, 1] chunk and an H3 [0, 1] chunk of the same picture quantize to
the same pixels. Wan's own cases cover transfer coalescing and every rejected
combination. Two more drive the real S2V forward() over a two-clip loop on
CPU, checking that both clips reach the encoder, that every output decodes as a
playable container with audio, and that each request's waveform lands on its own
outputs.

Known limitation

With VAE patch parallelism, S2V broadcasts decoded frames to every rank because
the autoregressive loop needs them, so every rank encodes its own identical
copy. This matches the existing full-decode path, which likewise concatenates
the same tensor on every rank, and is left alone here rather than changing which
rank produces output.

🤖 Generated with Claude Code

@chatgpt-codex-connector

Copy link
Copy Markdown

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

Copy link
Copy Markdown

This PR appears to be related to model: Wan.

Model owners: @bjf-frz

@specture724, 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.

@specture724 specture724 changed the title [4/4] Give the Wan chunk callback its production consumer [4/N] Give the Wan chunk callback its production consumer Sep 4, 2026
@specture724
specture724 force-pushed the post-VAE-overlap-4-of-4-wan-consumer branch from 7df80e3 to f847196 Compare September 4, 2026 07:50
@hsliuustc0106 hsliuustc0106 added diffusion codes related to diffusion models enhancement New feature or request high priority high priority issue, needs to be done asap labels Sep 5, 2026
@specture724
specture724 force-pushed the post-VAE-overlap-4-of-4-wan-consumer branch from f847196 to d8806eb Compare September 8, 2026 03:09
@specture724
specture724 marked this pull request as draft September 8, 2026 03:14
@specture724
specture724 force-pushed the post-VAE-overlap-4-of-4-wan-consumer branch 3 times, most recently from 9b79bf9 to ab7e0fc Compare September 8, 2026 09:13
@specture724 specture724 changed the title [4/N] Give the Wan chunk callback its production consumer [4/N] Unify worker-side MP4 encoding and wire Wan into it Sep 8, 2026
@hsliuustc0106

Copy link
Copy Markdown
Collaborator

[P1] Abort the S2V MP4 session when clip generation fails. After an earlier clip starts encoders, a later denoise/VAE failure exits forward() without aborting the session. The encoder threads retain their containers/audio and wait for a sentinel that never arrives. An injected second-clip failure through the real S2V forward, using the CPU model fixture and real encoders, left all four encoders alive; session.abort() closed all four. Please wrap the session lifetime in cleanup covering every unsuccessful exit, including push/finish failures. pipeline_wan2_2_s2v.py:1492–1503.

[P2] Preserve serving codec defaults for Wan pre-encoding. With only extra_params={"preencode_mp4": true}, this helper returns None. Serving normally supplies preset=ultrafast, threads=0, but skips its encoder when the worker returns bytes. Enabling pre-encoding therefore changes the codec preset and encoding cost. Please apply the serving defaults when the key is absent, while preserving explicit null/empty/custom options. H3 already carries these defaults into worker encoding. chunked_mp4.py:63–66.

Reviewed fee2684b836f against ecdd762ff576, distinguishing this PR's changes from the inherited #7018 work. 210 targeted CPU tests passed across the shared consumer and Wan/S2V/H3 contracts. The peer-VAE-rank fix is present; the earlier #7018 metadata finding is inherited.

Local A/B used one reserved L20X, Python 3.12, torch 2.13.0, vLLM 0.28.0, diffusers 0.40.0, and the real Wan2.1-T2V-1.3B VAE (revision 0fad780a534b6463e45facd96134c9f345acfa5b). Controls: BF16, seed 7048 latent [1,16,65,60,104], 257 frames at 480×832/24fps, no audio/tiling/offload, identical H264 CRF18/ultrafast/threads=0. Baseline uses normal decode/postprocess/serving encode with eight converter workers; head uses decode_to_mp4, batch threshold 17. One warmup and two measured runs per side; loading and file writes excluded, shared caches retained.

Measured decode-to-MP4 times Peak allocated GPU memory
Base 7.288 / 7.290 s 8222.0 MiB
Head 6.708 / 6.661 s 7679.2 MiB

Mean stage latency fell 8.29%, but output equivalence did not hold: both pairs retained 257 frames, with decoded RGB MAE 3.426/255, maximum difference 56, and PSNR 34.56 dB. These two observations do not establish output-quality equivalence or a full-request speedup; denoising, HTTP and IPC were not measured.

@specture724
specture724 force-pushed the post-VAE-overlap-4-of-4-wan-consumer branch from fee2684 to af8aa1c Compare September 12, 2026 14:56
@specture724
specture724 marked this pull request as ready for review September 12, 2026 14:59
@hsliuustc0106

Copy link
Copy Markdown
Collaborator

Re-reviewed af8aa1c66fa3 against 7e520f9cc47e. Both findings from the previous review remain unresolved:

[P1] Abort the S2V MP4 session when clip generation fails. A second-clip denoise failure still exits forward() without aborting the session. Reproducing this through the real S2V forward with the CPU model fixture and real encoders left all four encoder threads alive and unclosed, retaining their containers/audio while waiting for more input. Explicit session.abort() closed all four. Please guard the entire session lifetime and abort on unsuccessful exits, including push/finish failures. pipeline_wan2_2_s2v.py:1492–1503.

[P2] Preserve serving codec defaults for Wan pre-encoding. With only extra_params={"preencode_mp4": true}, the helper still returns None. The normal serving paths supply preset=ultrafast, threads=0, but bypass encoding once the worker returns bytes. Enabling pre-encoding therefore changes codec settings and their cost/compression behavior. Please apply the serving defaults when video_codec_options is absent, while preserving explicit null/empty/custom options. chunked_mp4.py:63–66.

The update is mostly a rebase onto merged #7018. The T2V adaptation preserves typed media on the output-owning rank and the legacy empty placeholder on non-owner ranks; I found no additional regression in that change.

Validation: 246 focused CPU tests passed across shared chunked video, Wan/S2V, H3, T2V forward and typed-media contracts. An additional T2V smoke check with model/VAE stubs and real CPU encoding passed for two requests × two outputs: the owner produced four playable MP4s; the simulated non-owner returned empty outputs without a typed-media batch error. Both earlier findings were reproduced on this head, and all reproducer-owned encoder threads were cleaned up. Python 3.12, torch 2.13.0, vLLM 0.29.0, PyAV 18.1.0.

The earlier GPU A/B was not rerun and is not current-head performance evidence. Full S2V denoising, HTTP end-to-end behavior and multi-GPU execution were not tested in this re-review.

@hsliuustc0106 hsliuustc0106 added the ready label to trigger buildkite CI label Sep 13, 2026
@hsliuustc0106

Copy link
Copy Markdown
Collaborator

@MuweiZ any more comments?

if current_omni_platform.is_available():
current_omni_platform.empty_cache()
if mp4_session is not None:
mp4_session.push(clip_video)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P3] Encode only on the output-owning VAE-PP rank. After the broadcast every rank of _vae_pp_group holds the clip, so every rank pushes into its own session and runs quantize + D2H + a full H.264/AAC encode per clip — but the executor collects only rank 0's forward response (num_responses = 1 in multiproc_executor.py), so peer output is discarded. T2V/I2V/H3 all encode owner-only because their peer ranks receive no chunks. The broadcast itself must stay (the AR motion loop needs frames on all ranks); gating the push on the pp-group owner leaves peers with an empty session, and finish() == [] is already tolerated downstream (the T2V non-owner path).

@hsliuustc0106

Copy link
Copy Markdown
Collaborator

Re-reviewed 015a2a5e5d with an architecture/performance focus. Both prior findings are fixed:

  • [P1 → fixed] Session abort. The clip loop and output assembly are wrapped in try/except BaseException → mp4_session.abort(). I traced abort() on every exit: idempotent after a failed finish() (already-closed encoders only join), a no-op before the first flush, and an encoder-worker failure drains its own queue so a blocked push cannot deadlock. The new test drives the second-clip failure through the real forward and asserts all four encoder threads exit.
  • [P2 → fixed] Codec defaults. An absent video_codec_options now inherits the one-shot serving encoder's preset=ultrafast, threads=0; explicit None/{}/custom stay caller choices, all four covered by tests.

Architecture: the owner-only encoding convention holds for T2V/I2V/H3 — peers receive no chunks, decode_to_mp4 returns [], and H3's wrapper returns b"" (pipeline_minimax_h3.py:2213), which resolves the 09-09 non-owner-rank thread here. One gap remains on S2V (inline comment): it pushes on every VAE-PP rank while only rank 0's output is consumed.

Performance: the body's claims match the implementation. chunk_to_uint8_frames quantizes on-device and transfers once per flush batch; the synchronous D2H inside the producer callback and the chunk-count (not byte) bound on pending memory are now stated in the docstrings, consistent with the 09-09 threads — per-chunk timestamps and max_pending_bytes are reasonable future transport-layer work. S2V's per-clip .cpu() becomes an equivalent batched transfer, so there is no new critical-path stall, and encode overlaps the next clip's denoise.

CI on this head: main / AMD / NPU / build 3.11+3.12 / pre-commit / RTD green. vllm-omni-intel-ci failed in 54s; the log is not anonymously readable and the duration points at setup rather than tests — worth a rerun. I could not run the new tests locally (no torch+PyAV environment on the review host), so test evidence here is CI plus reading.

)
num_steps = common.num_inference_steps or num_inference_steps
num_outputs_per_prompt = common.num_outputs_per_prompt or 1
preencode_mp4 = resolve_wan_preencode_mp4(common, output_type=output_type or "np")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Validate common.output_type here so preencoding rejects pil, pt, and latent requests.

@specture724
specture724 force-pushed the post-VAE-overlap-4-of-4-wan-consumer branch from 71b4892 to b2d7c0f Compare September 16, 2026 02:47
specture724 and others added 12 commits September 17, 2026 00:27
Give the Wan chunk callback its production consumer. With
preencode_mp4, each committed temporal chunk is quantized on the
accelerator, transferred once, and pushed to one bounded encoder per
batch entry, so host transfer and H.264 encoding overlap the remaining
decode instead of following it.

The pre-encoded path emits MP4 bytes, so it rejects rather than silently
downgrades what it cannot serve: frame interpolation, which needs the
frames it no longer materializes, and non-array output types. Encoding
fps mirrors the fallback the serving layer applies, so the bytes carry
the rate the response advertises without reporting one.

Wan2.1-T2V-1.3B VAE weights, L20X, latent [1,16,65,60,104] -> 257
frames at 480x832: full decode plus one-shot mux ~13.05 s, wired
pre-encode ~9.68 s, a ~26% reduction at an identical frame count.
Consumer batching trades 65 host transfers for 13; wall clock is within
noise of the per-chunk path on this GPU.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: specture724 <specture724@gmail.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: specture724 <specture724@gmail.com>
… them

S2V already decodes one clip per iteration and needs each clip's trailing
frames to build the next clip's motion latents, so it has no use for the
finer temporal callback. Hand each finished clip to a bounded encoder
instead of accumulating clips on the host and concatenating: H.264
encoding then overlaps the next clip's denoise and decode, and the full
video is never materialized. The waveform is muxed into each container,
so nothing downstream re-attaches audio.

Covered by driving the real forward() over a two-clip loop on CPU: both
clips reach the encoder, every output decodes as a playable container
with an audio stream, and each request's waveform lands on its own
outputs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: specture724 <specture724@gmail.com>
[1/N] landed the seam as a dedicated decode_with_chunks method behind
the SupportsChunkedVAEDecode protocol, rather than an on_chunk keyword
on decode(), so the consumer called a signature that no longer exists.
Call the capability method and gate on the published type guard, which
turns an unsupported VAE into a clear error instead of a TypeError from
deep inside the decode call.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: specture724 <specture724@gmail.com>
Producing chunks is the model's business: Wan walks a causal
frame-by-frame loop, H3 walks overlapping clips, and Wan S2V decodes one
clip per autoregressive iteration. What happens after a chunk is
committed is not -- every one of them quantizes to uint8, moves to the
host once, and lands in a bounded encoder -- yet each model carried its
own copy of that, so a capability meant to be model-agnostic had a
per-backend consumer bolted onto it.

Move the consumer into diffusion/utils/chunked_video.py:
ChunkedVideoMP4Session for producers that drive themselves, and
decode_to_mp4() for producers that can be driven through the capability.
Batching, per-batch-entry encoders, audio muxing, the requested-size
crop, and quantization from the producer's declared pixel range are all
parameters now. H3, Wan T2V/I2V, and Wan S2V share it; a model that
wants worker-side encoding implements one method and declares one range.

Real Wan2.1-T2V-1.3B weights decode to the same 257 frames with an
unchanged pre-encode SHA, so the move is behavior-preserving.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: specture724 <specture724@gmail.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: specture724 <specture724@gmail.com>
Signed-off-by: specture724 <specture724@gmail.com>
… request

Every rank of a distributed H3 VAE group drives the temporal collectives, but
only the owner receives chunks, so the shared consumer returns no container on
a peer. The H3 wrapper treated that as a violated single-output invariant and
raised, turning normal peer-rank silence into a request failure. Return empty
bytes there instead -- the pre-encoded counterpart of the empty tensor the full
decode leaves on a peer -- and enforce the single-container invariant only on
the owner. Two new cases drive the real H3 decode_to_mp4 and post_decode chain
on a peer-rank VAE rather than the shared fixture.

Also correct two descriptions that claimed more than the code does: the chunk
transfer is a synchronous .cpu() inside the producer callback, so what overlaps
the remaining decode is the CPU encode of earlier chunks rather than the copy;
and the encoder's bound covers pending raw frames in chunks, not total request
memory, since the muxed container still grows with output duration.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Signed-off-by: specture724 <specture724@gmail.com>
Signed-off-by: specture724 <specture724@gmail.com>
Under VAE patch parallelism every rank of the VAE group receives each
decoded S2V clip by broadcast, because the autoregressive motion loop
needs the frames everywhere. Each rank then also pushed the clip into its
own MP4 session and paid quantization, a host transfer, and a full
H.264/AAC encode, although the executor keeps only the owning rank's
response.

Record ownership from the decode itself, where peers get an empty
placeholder, and push only on the owner. The broadcast and motion loop
are unchanged. A peer now finishes with no containers, the same empty
result T2V, I2V, and H3 non-owners already return, which the per-request
split and downstream paths tolerate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Signed-off-by: specture724 <specture724@gmail.com>
S2V resolved the pre-encode guard against the forward keyword argument,
which defaults to np, instead of the request's own output_type. A request
asking for pil, pt, or latent therefore passed the guard and received MP4
bytes, while T2V and I2V reject the same combination.

Read output_type from the sampling params first, as T2V does, and keep the
keyword as the default for direct callers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Signed-off-by: specture724 <specture724@gmail.com>
… reads

Rebasing onto main picked up the UniPC single-shift fix, which passes
self._flow_shift into set_timesteps. The pre-encode stub builds its
pipeline with object.__new__ and never set that attribute, so every
pre-encode test raised AttributeError on the new call.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Signed-off-by: specture724 <specture724@gmail.com>
@specture724
specture724 force-pushed the post-VAE-overlap-4-of-4-wan-consumer branch from b2d7c0f to eaad9eb Compare September 16, 2026 16:33
@hsliuustc0106 hsliuustc0106 added ready label to trigger buildkite CI cuda-test Used to trigger vllm-omni cuda CI separately. and removed ready label to trigger buildkite CI labels Sep 16, 2026
@hsliuustc0106
hsliuustc0106 merged commit 2ab5d17 into vllm-project:main Sep 17, 2026
6 checks passed
mlaneuville pushed a commit to mlaneuville/vllm-omni that referenced this pull request Sep 22, 2026
…ct#7048)

Signed-off-by: specture724 <specture724@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Signed-off-by: Matthieu Laneuville <matthieu.laneuville@surf.nl>
khairulkabir1661 pushed a commit to khairulkabir1661/vllm-omni that referenced this pull request Sep 25, 2026
…ct#7048)

Signed-off-by: specture724 <specture724@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda-test Used to trigger vllm-omni cuda CI separately. diffusion codes related to diffusion models enhancement New feature or request high priority high priority issue, needs to be done asap ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants