Skip to content

Add safe video generation APIs - #18

Open
zatevakhin wants to merge 2 commits into
stack/v0.0.2-04-safe-tasksfrom
stack/v0.0.2-05-video
Open

Add safe video generation APIs#18
zatevakhin wants to merge 2 commits into
stack/v0.0.2-04-safe-tasksfrom
stack/v0.0.2-05-video

Conversation

@zatevakhin

@zatevakhin zatevakhin commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Changes

  • add separate video engine ownership and checkpoint-set configuration
  • add blocking video generation with owned artifact metadata
  • add standalone owned ffmpeg argv composition without process execution

Testing

  • model-free video validation and ownership tests
  • committed mux goldens and wrong-model load coverage
  • rustdoc, formatting, and focused Clippy

Summary by CodeRabbit

  • New Features
    • Added video generation support with configurable CPU/CUDA execution, model partitions, checkpoints, dimensions, frame counts, seeds, and media inputs.
    • Added result accessors for generated frames, audio, dimensions, frame rate, and sample rate.
    • Added FFmpeg mux argument composition with support for audio and silent video workflows.
  • Bug Fixes
    • Added validation for invalid paths, unsupported media combinations, numeric limits, and unsafe native data ranges.
    • Improved handling of non-UTF-8 paths and resource cleanup.
  • Documentation
    • Documented video generation behavior, outputs, threading, filesystem effects, and caller responsibilities.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 42c8171a-e569-4fb6-83ed-72007fae11f2

📥 Commits

Reviewing files that changed from the base of the PR and between b9b5c54 and e619c19.

📒 Files selected for processing (6)
  • Justfile
  • vllm-cpp/src/abi.rs
  • vllm-cpp/src/engine.rs
  • vllm-cpp/src/lib.rs
  • vllm-cpp/tests/qwen3.rs
  • vllm-cpp/tests/safe_api.rs

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


📝 Walkthrough

Walkthrough

The PR adds public video generation and video muxing APIs. It adds native loading, generation, result conversion, validation, cleanup, ABI defaults, tests, documentation, and package inventory updates.

Changes

Video engine API

Layer / File(s) Summary
ABI defaults and package inventory
Justfile, vllm-cpp/src/abi.rs
The package inventory includes src/abi.rs. The unpacked-size limit increases to 512 KiB. Compatibility provides ABI-validated video parameter defaults and test helpers.
Video types, configuration, and validation
vllm-cpp/src/engine.rs, vllm-cpp/src/lib.rs
The crate adds video device, partition, builder, generation, result, mux, and FFmpeg argument APIs. Rust-side validation covers paths, dimensions, frame counts, steps, references, noise augmentation, FPS, and CRF values.
Native loading, generation, and result ownership
vllm-cpp/src/engine.rs
The implementation loads models, performs serialized generation, converts native results into owned Rust values, preserves supported path bytes, validates native pointer ranges, and frees native allocations on success or failure.
Validation, cleanup tests, and API documentation
vllm-cpp/src/engine.rs, vllm-cpp/tests/qwen3.rs, vllm-cpp/tests/safe_api.rs, vllm-cpp/src/lib.rs
Tests cover video marshaling, validation order, load failures, ownership, cleanup, mux argument ordering, path handling, and API traits. Documentation describes video execution and caller-controlled FFmpeg handling.

Estimated code review effort: 5 (Critical) | ~90 minutes

Merge Risk: ⚪ Minimal · up to e619c

The PR adds video generation and muxing APIs without any supplied evidence of a concrete correctness, security, availability, or deployment risk. No actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant VideoEngineBuilder
  participant NativeVideoABI as native video ABI
  participant VideoEngine
  participant VideoResult
  VideoEngineBuilder->>NativeVideoABI: load video model configuration
  NativeVideoABI-->>VideoEngineBuilder: return loaded native handle
  VideoEngineBuilder->>VideoEngine: create VideoEngine
  VideoEngine->>NativeVideoABI: generate video with validated parameters
  NativeVideoABI-->>VideoResult: return native paths and metadata
  VideoEngine->>VideoResult: convert into owned result values
``

</details>

<!-- walkthrough_end -->
<!-- pre_merge_checks_walkthrough_start -->

<details>
<summary>🚥 Pre-merge checks | ✅ 4 | ❌ 1</summary>

### ❌ Failed checks (1 warning)

|     Check name     | Status     | Explanation                                                                                                                                                                                               | Resolution                                                                         |
| :----------------: | :--------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------- |
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 130 functions across 5 files. (1 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |

<details>
<summary>✅ Passed checks (4 passed)</summary>

|         Check name         | Status   | Explanation                                                                      |
| :------------------------: | :------- | :------------------------------------------------------------------------------- |
|      Description Check     | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled.                      |
|         Title check        | ✅ Passed | The title clearly summarizes the main change: adding safe video generation APIs. |
|     Linked Issues check    | ✅ Passed | Check skipped because no linked issues were found for this pull request.         |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request.         |

</details>

<details>
<summary>Full details: Docstring Coverage</summary>

**Explanation**

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 130 functions across 5 files. (1 skipped: 1 unsupported.)

</details>

</details>

<!-- pre_merge_checks_walkthrough_end -->

- [ ] <!-- {"checkboxId":"585bb3f6-faf5-4dbf-96d2-74e382adf19a"} --> Fix all pre-merge checks with AI
<!-- finishing_touch_checkbox_start -->

<details>
<summary>✨ Finishing Touches 💡 1</summary>

<!-- finishing_touch_suggestion:docstrings -->
<details>
<summary>📝 Generate docstrings 💡</summary>

- [ ] <!-- {"checkboxId":"7962f53c-55bc-4827-bfbf-6a18da830691"} --> Create stacked PR
- [ ] <!-- {"checkboxId":"3e1879ae-f29b-4d0d-8e06-d12b7ba33d98"} --> Commit on current branch

</details>
<details>
<summary>🧪 Generate unit tests (beta)</summary>

- [ ] <!-- {"checkboxId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "radioGroupId": "utg-output-choice-group-5433741075"} -->   Create PR with unit tests
- [ ] <!-- {"checkboxId": "6ba7b810-9dad-11d1-80b4-00c04fd430c8", "radioGroupId": "utg-output-choice-group-5433741075"} -->   Commit unit tests in branch `stack/v0.0.2-05-video`

</details>

</details>

<!-- finishing_touch_checkbox_end -->
<!-- tips_start -->

---




<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>

<!-- tips_end -->
Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant