Repository navigation
[Feature] Add LTX-2.3 keyframe interpolation (FLF2V / FMLF) one-stage pipeline - #5457
lucasbarrez wants to merge 2 commits into
Conversation
92cf89e to
41a4636
Compare
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
| clean_latents = torch.zeros_like(latents) | ||
| for cond, strength, latent_idx in zip(condition_latents, condition_strengths, condition_indices, strict=True): | ||
| num_cond_tokens = cond.size(1) | ||
| start_token_idx = latent_idx * latent_height * latent_width |
There was a problem hiding this comment.
Non-first anchors cannot replace the base latent timeline this way. The current Diffusers LTX-2.3 condition pipeline appends every non-zero-index anchor as extra keyframe tokens with its own positional coordinates; apply_visual_conditioning() only overwrites index 0. Here a middle/last anchor gets base-frame RoPE positions, so FLF/FMLF conditioning is not equivalent. Please port the keyframe-token/coordinate path and add a non-zero-index regression test.
There was a problem hiding this comment.
The rebased keyframe-token/coordinate path and focused regression test address this. Looks good now.
| """ | ||
| if not req.prompts: | ||
| return None | ||
| first = req.prompts[0] |
There was a problem hiding this comment.
LTX23Pipeline supports request batching, but this resolver reads only req.prompts[0] and _prepare_condition_latents() then applies that condition set to the whole batch. A batched request will therefore use the first request's anchors for every item. Please resolve anchors per request (or disable batching for this pipeline) and cover mixed condition sets.
There was a problem hiding this comment.
Disabling request batching for this pipeline addresses this. Looks good now.
… keyframe tokens Non-first-frame anchors (latent_idx > 0) were overwritten in-place at their base-timeline slot, matching diffusers 0.38.0 but giving them base-frame RoPE positions. Diffusers main instead appends them as extra keyframe tokens with their own coordinates (VideoConditionByKeyframeIndex). Port that: - apply_first_frame_conditioning() now only overwrites latent_idx == 0. - _prepare_keyframe_coords() computes per-keyframe pixel-space RoPE coords. - The keyframe tokens/coords/mask are appended to the sequence in _prepare_denoise_context_for_cfg and trimmed in _unpack_and_denormalize_stage (both overridable hooks; no shared runtime file changed). - supports_request_batch = False: conditions are resolved from the first request only, so batching could apply one item's anchors to every output. Addresses review feedback on PR vllm-project#5457. Signed-off-by: lucasbarrez <lucasbarrez12@gmail.com>
… keyframe tokens Non-first-frame anchors (latent_idx > 0) were overwritten in-place at their base-timeline slot, matching diffusers 0.38.0 but giving them base-frame RoPE positions. Diffusers main instead appends them as extra keyframe tokens with their own coordinates (VideoConditionByKeyframeIndex). Port that: - apply_first_frame_conditioning() now only overwrites latent_idx == 0. - _prepare_keyframe_coords() computes per-keyframe pixel-space RoPE coords. - The keyframe tokens/coords/mask are appended to the sequence in _prepare_denoise_context_for_cfg and trimmed in _unpack_and_denormalize_stage (both overridable hooks; no shared runtime file changed). - supports_request_batch = False: conditions are resolved from the first request only, so batching could apply one item's anchors to every output. Addresses review feedback on PR vllm-project#5457. Signed-off-by: lucasbarrez <lucasbarrez12@gmail.com>
|
Thanks @david6666666 — you're right, and good catch. I'd initially matched diffusers 0.38.0 (the version vLLM-Omni pins), where Ported that behavior in 0e0271d:
Validated FLF2V / FMLF end-to-end on H100. Thanks for the thorough review! |
|
Heads up — I see the LTX unification landed ( |
…ipeline Re-expressed on the unified LTXRuntime (post the LTX unification refactor): LTX2ConditionPipeline composes a new LTXMultiAnchorConditioningMixin over LTX2Pipeline. First-frame anchors overwrite the base timeline; non-first-frame anchors are appended as keyframe tokens with their own RoPE coordinates (matching diffusers main's VideoConditionByKeyframeIndex) and trimmed before VAE decode. Anchor pinning runs through the _step_video_latents_i2v hook as an x0-space blend, so no shared runtime file is modified. Degrades to pure T2V when no conditions are supplied; supports_request_batch = False. Addresses the keyframe-interpolation item of vllm-project#4985. Signed-off-by: lucasbarrez <lucasbarrez12@gmail.com>
0e0271d to
02ece25
Compare
|
@mglyn ptal thx |
…ation API Add a VideoConditionAnchor request schema (mirroring ImageReference) and a `conditions` field on VideoGenerationRequest, decode each anchor to a PIL image in the video form parser, and thread the anchors into `multi_modal_data["conditions"]` for the pipeline to consume. Makes LTX2ConditionPipeline usable over the OpenAI-compatible video API (FLF2V / FMLF). The request schema is provisional pending the conditioning contract discussion in vllm-project#4985. Signed-off-by: lucasbarrez <lucasbarrez12@gmail.com>
43fa41c to
f18ec2c
Compare
| frame_interpolation_model_path: str | None = Form(default=None), | ||
| lora: str | None = Form(default=None), | ||
| extra_params: str | None = Form(default=None), | ||
| conditions: str | None = Form(default=None), |
There was a problem hiding this comment.
This adds a public multipart API path, but the only new tests cover the pipeline directly. Please add endpoint coverage for decoding and forwarding anchors, plus a 400 response for invalid URLs or indices, for both sync and async video routes.
| through the ``_step_video_latents_i2v`` hook (which the shared step adapter calls | ||
| whenever a conditioning mask is present), so no shared runtime file is modified. | ||
| """ | ||
|
|
There was a problem hiding this comment.
#5148 already introduced the phase-aware LTXI2VConditioningMixin in
ltx2_conditioning.py, shared by one- and two-stage entries. Generalize that
visual-conditioning path into first-frame, combined, and guiding-keyframe
policies instead of duplicating preprocessing, VAE, mask, timestep/guidance,
and Euler-conditioning hooks in a parallel module.
| raise ValueError( | ||
| f"Condition index {cond.index} resolves to {resolved}, " | ||
| f"outside the valid range [0, {latent_num_frames})." | ||
| ) |
There was a problem hiding this comment.
This differs from the current Diffusers/LTX reference preprocessing:
aspect-fill, center crop, F.interpolate, and duration-aware trimming.
VideoProcessor stretches the aspect ratio and leaves overlong clips intact.
| Only conditions with ``latent_idx == 0`` are applied here (matching Diffusers' | ||
| ``apply_first_frame_conditioning``); non-first-frame anchors are appended as | ||
| keyframe tokens instead and are skipped. ``conditioning_mask`` is ``(B, N)``. | ||
| """ |
There was a problem hiding this comment.
Replacing frame 0 and appending only non-zero anchors matches Diffusers
and combined_image_conditionings, not the specialized official keyframe
pipeline, which appends frame 0 as a guiding latent as well.
| latents = base_scaled * noise + (1.0 - base_scaled) * latents | ||
|
|
||
| # Renoise keyframe tokens with the same formula (strength=1 keeps them clean). | ||
| if keyframe is not None: |
There was a problem hiding this comment.
Preserve a separate clean copy before noising. The current x0
blend targets the initially noised keyframe for strength < 1;
Diffusers keeps a separate kf_clean_list as the clean target.
| return super()._prepare_video_latents_stage( | ||
| request_inputs, prompt_context, device=device, noise_scale=noise_scale, image=image | ||
| ) | ||
|
|
There was a problem hiding this comment.
num_videos_per_prompt can make B > 1 while every keyframe tensor has
B=1, so sequence-dimension concatenation fails. Repeat anchors across
the output batch or explicitly reject multiple outputs.
| @@ -36,6 +37,15 @@ class LTX2Pipeline(LTXI2VConditioningMixin, LTXRuntime): | |||
| supports_request_batch = True | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
This is a Diffusers-style one-stage pipeline. The official Lightricks
pipeline is two-stage, re-applies keyframes after upsampling, and uses the
distilled LoRA in Stage 2; do not claim full official parity yet.
| @@ -140,13 +154,27 @@ async def _run_and_extract( | |||
| target_size = (vp.width, vp.height) | |||
| if input_image.size != target_size: | |||
| input_image = input_image.resize(target_size, Image.Resampling.LANCZOS) | |||
There was a problem hiding this comment.
Online uses PIL/LANCZOS while
offline uses VideoProcessor/bilinear, producing different, distorted
inputs. Normalize once in the pipeline with reference preprocessing.
|
|
||
| await VIDEO_STORE.update_fields(video_id, {"status": VideoGenerationStatus.IN_PROGRESS}) | ||
| started_at = time.perf_counter() | ||
| try: |
There was a problem hiding this comment.
This keyword is passed even when None, but not every existing
handler/test double accepts it; test_video_server.py currently fails
with BlockingVideoHandler.generate_video_bytes() TypeError.
| raise HTTPException(400, detail=str(exc)) from exc | ||
| reference_audio = ReferenceAudio(path=audio_path) | ||
|
|
||
| return request, handler, effective_model_name, reference_image, reference_video, reference_audio |
There was a problem hiding this comment.
Validate pipeline capability before decoding anchors and reject mixing
conditions with ordinary references. Otherwise conditions, or the ordinary
image in LTX2ConditionPipeline, can be silently ignored.
|
Thanks so much @mglyn and @david6666666 for the thorough and thoughtful review, I really appreciate the time you took digging into the details. 🙏 @david6666666, glad the keyframe token/coordinate rework and the batching change addressed your points, and good call on the endpoint tests. @mglyn, this all makes complete sense and I'm fully on board with the direction. A shared LTX conditioning framework (generic latent conditioning, masks/positions, per-phase conditions, reference tokens, one-/two-stage reuse) is clearly the right foundation, much cleaner than a parallel pipeline, especially with two-stage, retake, IC-LoRA, A2V and LipDub all on the way. Happy to keep this PR open and re-express the work as a small integration + tests on top of the new interfaces once they land. I'll also fix the test_video_server.py regression I introduced in the meantime. Really looking forward to the new infrastructure, thanks for letting me continue contributing here. If there's any way I can help shape or test the conditioning interfaces as they come together, I'd be glad to. 🚀 |
|
I agree with the direction in the latest discussion: keeping a parallel LTX-2.3 pipeline would now duplicate the shared conditioning/runtime architecture. Would it make sense to re-express this as a small integration on the current conditioning framework and validate it against the official LTX-2.5 keyframe semantics? If that is not planned, closing the current branch may avoid maintaining two implementations. |
Omni ReviewBot: no human activity for 15 days@lucasbarrez this pull request has had no human commit, comment or review since 2026-08-24. Per repository policy it may be closed if it stays inactive. 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. |
Omni ReviewBot: no human activity for 30 days@lucasbarrez this pull request has had no human commit, comment or review since 2026-08-24. Please consider closing this PR if the work is no longer planned. 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. |
Omni ReviewBot routing recordAssigned Strict on cursor (cursor-grok-4.6-high) under experiment |
Omni ReviewBot attempt recordReview attempt ended as timeout (Strict attempt outlived its budget; falling back to direct/cursor/auto). |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
Changes since the previous review
- 0 new inline finding(s); 9 finding(s) below.
CI at
f18ec2c67449(2026-10-09T19:42:43.735206+00:00): required check(s) blocking:buildkite/vllm-omni(missing).
Note: The assigned review arm
strict/cursor/cursor-grok-4.6-highcould not complete this review, so it was produced by the fallback armdirect/cursor/auto. It is excluded from the routing experiment.
Full review analysis
PR description
This change adds a one-stage LTX-2 / LTX-2.3 keyframe-interpolation pipeline, LTX2ConditionPipeline, and registers it beside the existing one-stage LTX pipeline. First-frame anchors overwrite the base latent timeline; later anchors are appended as extra keyframe tokens with their own RoPE coordinates, then trimmed before decode. The frozen diff also opens a public multipart conditions field on the sync and async video routes and forwards decoded anchors through multi_modal_data, which is wider than the PR text that still describes HTTP serving as a follow-up.
Change flow
flowchart LR
Form["[CHANGED] Video form conditions"]:::changed
Proto["[CHANGED] VideoConditionAnchor"]:::changed
Serve["[CHANGED] serving_video anchor resize"]:::changed
Mix["[NEW] LTXMultiAnchorConditioningMixin"]:::new
Pipe["[NEW] LTX2ConditionPipeline"]:::new
Reg["[CHANGED] diffusion registry"]:::changed
Denoise["[EXISTING] LTX one-stage denoise"]:::existing
Form --> Proto --> Serve --> Mix --> Pipe --> Denoise
Pipe --> Reg
classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px
Findings
- [P1] Non-zero anchors blend toward noised latents when strength is below 1 —
vllm_omni/diffusion/models/ltx2/ltx2_condition.py:450
Existing thread: #5457 (comment)
Evidence for Non-zero anchors blend toward noised latents when strength is below 1
_prepare_condition_latents noises keyframe.tokens in place (lines 407-412) and _prepare_video_latents_stage then uses that same tensor as the x0 clean target (full_clean = torch.cat([clean_latents, keyframe.tokens], dim=1) at line 450). First-frame anchors keep a separate clean copy before the base noise mix, so strength 1.0 on a later anchor stays clean only because 1 - mask is zero. Any advertised strength below 1.0, or a supplied-latent noise_scale with a partial mask, makes the Euler blend pull middle/last anchors toward the already noised keyframe instead of the VAE-encoded image. Snapshot the packed keyframe latents before renoising and concatenate that clean copy into _pending_clean_latents.
Evidence for Keyframe concat crashes when num_videos_per_prompt is greater than 1
Request batching is disabled, but _prepare_video_latents_stage still sizes the base latents and mask with prompt_context.batch_size * request_inputs.num_videos_per_prompt (line 433). Encoded anchors and _prepare_keyframe_coords stay batch 1. The very next torch.cat of conditioning_mask with keyframe.mask (line 449), and the later latent/coord concat at lines 498-501, require equal batch sizes, so FLF2V/FMLF with num_videos_per_prompt > 1 raises before denoise. Repeat anchor tokens, coords, and masks across that output batch, or reject num_videos_per_prompt != 1 up front. The new tests never construct this batch.
Evidence for Conditions are decoded for every video model and can be mixed with ordinary references
_parse_video_form decodes request.conditions whenever the field is set (line 3078) and serving_video._run_and_extract always stores them on multi_modal_data while still accepting an image or video reference (lines 170-177). There is no check that the loaded pipeline is LTX2ConditionPipeline, and no 400 when anchors are combined with image_reference / video_reference. Other pipelines never read conditions, so the request succeeds and the anchors are ignored. On the condition pipeline, a concurrent image is also ignored once conditions resolve. Reject unsupported pipelines and mixed references before decode_image_url. Index errors raised later as ValueError are not OmniClientError, so the sync handler's generic Exception arm (line 3222) turns them into HTTP 500.
Evidence for Video generation always passes reference_conditions into every handler
_run_video_generation_job and create_video_sync now call generate_video_bytes(..., reference_conditions=reference_conditions) even when the value is None (lines 2860 and 3205). OmniOpenAIServingVideo accepts the keyword, but this PR's changed-files set does not update tests/entrypoints. On this head, review already reported BlockingVideoHandler.generate_video_bytes() raising TypeError for that keyword. The stub module is not in this sparse checkout, so its body was not re-read; the call site and the absent test edit still match that failure. Pass the keyword only when anchors exist, or update every video handler and test double to accept it.
Evidence for New conditions form field has no sync or async endpoint tests
The multipart conditions field (line 2956) is decoded and forwarded on both POST /v1/videos and POST /v1/videos/sync, but the only new test module is tests/diffusion/models/ltx2/test_ltx2_condition.py. Nothing in the diff asserts anchor decoding, forwarding into multi_modal_data, or a 400 for a bad image URL or index on either route. Buildkite step "Entrypoints Test" in .buildkite/cuda/test-ready.yml runs pytest -sv tests/entrypoints/ -m 'core_model and cuda' --run-level 'core_model'. The PR Testing section reports core_model/cpu pipeline unit tests and does not report that entrypoint selector. Add those route tests, or drop the HTTP surface until they exist.
Evidence for Anchor preprocess stretches frames instead of the LTX reference crop
preprocess_conditions (line 111) sends every anchor through VideoProcessor.preprocess_video, which resizes to the exact request height and width and leaves an over-long clip intact. The LTX/Diffusers condition path this module says it mirrors uses aspect-fill, center crop, F.interpolate, and duration-aware trimming. Stretched anchors are then VAE-encoded and pinned, so FLF/FMLF conditioning is not the reference geometry. The HTTP path also resizes with PIL LANCZOS in serving_video.py before this bilinear processor runs, so online and direct callers start from different pixels. Do the reference preprocess once inside the pipeline and stop resizing in serving.
Evidence for Serving resizes anchors with LANCZOS before the pipeline bilinear processor
When width and height are set, _run_and_extract replaces each anchor with a PIL LANCZOS resize (lines 157-168) and then the pipeline preprocesses again with VideoProcessor (resample="bilinear", mixin __init__). Direct LTX2VideoCondition calls only hit the bilinear processor. The two ingresses therefore pin different pixels for the same source image, and a second resample runs on the online path whenever the sizes already match the LANCZOS output. Leave resize to the single pipeline preprocess.
Evidence for Multi-anchor conditioning duplicates the existing LTX I2V mixin
LTXMultiAnchorConditioningMixin (line 185) reimplements validation, VAE encode/normalize/pack, conditioning masks, timestep masking, x0 guidance sigma, and the Euler pin as a sibling of LTXI2VConditioningMixin, which pipeline_ltx2.py already imports from ltx2_conditioning.py. ltx2_conditioning.py is not in this sparse checkout, so its hook bodies were not compared line by line. The new class still owns a parallel copy of those seams, and the clean-target and batch bugs above sit in that copy. Fold first-frame, combined, and guiding-keyframe policies into the existing conditioning mixin instead of keeping a second module.
Evidence for Frame 0 is overwritten in place and omitted from the keyframe token sequence
apply_first_frame_conditioning continues past every latent_idx != 0 (line 174) and writes index 0 over the base timeline. The append loop later skips index 0 as well (line 376). That matches Diffusers' combined in-place first frame, and the module docstring says so. The official LTX keyframe pipeline also appends frame 0 as a guiding latent with its own coordinates. This entry point is documented as FLF2V/FMLF and offers no policy switch, so first-frame anchors cannot follow that guiding-token path. Either append frame 0 as a guiding token or expose an explicit policy and stop presenting one behavior as both.
🤖 This review was generated by InferMatrix Copilot, an open-source repo-maintenance agent for PR review, CI debugging and issue triage. Try it on your own repo, and ⭐ star it if it helped!
Omni ReviewBot: finding feedback[p1] New conditions form field has no sync or async endpoint tests — See the review for details. If you are the PR author and disagree, react 👎 here; the maintainer will see your disagreement. |
Omni ReviewBot: finding feedback[p1] Non-zero anchors blend toward noised latents when strength is below 1 — See the review for details. If you are the PR author and disagree, react 👎 here; the maintainer will see your disagreement. |
Omni ReviewBot: finding feedback[p1] Keyframe concat crashes when num_videos_per_prompt is greater than 1 — See the review for details. If you are the PR author and disagree, react 👎 here; the maintainer will see your disagreement. |
Omni ReviewBot: finding feedback[p1] Video generation always passes reference_conditions into every handler — See the review for details. If you are the PR author and disagree, react 👎 here; the maintainer will see your disagreement. |
Omni ReviewBot: finding feedback[p1] Conditions are decoded for every video model and can be mixed with ordinary references — See the review for details. If you are the PR author and disagree, react 👎 here; the maintainer will see your disagreement. |
What
Adds
LTX23ConditionPipeline, a one-stage multi-anchor keyframe-interpolationpipeline for LTX-2.3 (FLF2V / FMLF), built on the unified
LTXPipelineRuntimefrom #5147. Anchors pin arbitrary frame indices (each with its own strength) and
the model interpolates the motion between them under text + audio conditioning.
Addresses the keyframe interpolation item of #4985.
How
LTXMultiAnchorConditioningMixin— sibling toLTXI2VConditioningMixin.Where I2V pins only frame 0, this generalizes to a set of anchor indices,
reusing the existing
conditioning_mask/ video-audio-scheduler / timestephooks. No shared runtime file is modified.
LTX23ConditionPipeline(LTXMultiAnchorConditioningMixin, LTX23Pipeline)—thin MRO composition; inherits x0 guidance, component profile and denoise loop.
Degrades to pure T2V when no conditions are supplied.
ConditionVideoAudioScheduler— applies the x0-space anchor blend duringthe loop; the mask and clean latents are captured at construction, so no
mutable per-step pipeline state is introduced.
ltx2_condition.py.Files
vllm_omni/diffusion/models/ltx2/ltx2_condition.py(new)vllm_omni/diffusion/models/ltx2/pipeline_ltx2.py,__init__.py,vllm_omni/diffusion/registry.py(register + export)tests/diffusion/models/ltx2/test_ltx2_condition.py(new)docs/models/supported_models.mdTesting
core_model/cpu, no GPU): MRO / guidance / hook /registry contracts, condition validation,
apply_visual_conditioningtokenpinning.
multi_modal_data,pure-T2V fallback when no conditions, and the anchor timestep-masking hook
(
timestep = t·(1 − mask), audio unmasked).video + audio with anchors correctly pinned at first / middle / last frames.
weights + GPU CI), in the spirit of
tests/e2e/accuracy/test_ltx2_3_video_similarity.py.Scope / follow-ups
the Standard / Full-distilled recipe layer lands, so it builds on that rather
than the legacy
LTX2TwoStagesPipelinepath.conditions(request schema →multi_modal_data["conditions"])is a follow-up PR. The pipeline reads conditions defensively, so it is
independently mergeable. Two open contract questions for the serving layer:
multi_modal_datapath asI2V, or do you want a dedicated anchor schema?
LTX23ConditionPipelineclass as the entry point,mirroring
LTX23ImageToVideoPipeline?cc @mglyn — following up on #4985, where you noted keyframe interpolation would be
welcome once it can build on the shared pipeline framework. This is that work,
re-expressed on the merged
LTXPipelineRuntime. Happy to adjust the entry-pointand request-schema contract per your answer to the two questions above.