pre-encoded ingest - #260
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 2 potential issues.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| /// Optional video encoder backend. Pre-encoded sources must select | ||
| /// @ref VideoEncoderBackend::PreEncoded. | ||
| std::optional<VideoEncoderBackend> video_encoder; |
There was a problem hiding this comment.
🔴 Existing binaries overread publish options
After a library-only upgrade, publishTrack receives options compiled with the previous public layout. toProto then reads the new trailing field out of bounds. Existing applications can select an arbitrary encoder or crash.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (frame.width == 0 || frame.height == 0) { | ||
| throw std::invalid_argument("EncodedVideoSource: frame dimensions must be non-zero"); | ||
| } |
There was a problem hiding this comment.
🟡 Oversized frame dimensions become negative
captureFrame accepts nonzero uint32_t dimensions above the downstream signed range. capture_encoded_frame casts them to i32, turning those values negative. The frame enters WebRTC with invalid geometry and can fail or crash.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🔵 Needs a closer look
It adds new public API surface and new/updated FFI message usage that requires careful cross-language contract validation beyond what can be fully confirmed from the C++ diffs alone.
Pull request overview
Adds pre-encoded video ingestion support to the C++ SDK by introducing a new EncodedVideoSource that submits already-encoded access units via the Rust FFI layer, plus the associated publish options, tests, and documentation.
Changes:
- Introduces
livekit::EncodedVideoSource(public API) for publishing encoded access units and polling encoder feedback. - Extends
TrackPublishOptionswithVideoEncoderBackend(includingPreEncoded) and wires it through proto conversion. - Adds unit/integration/stress coverage and updates Doxygen + docs to document the new workflow.
File summaries
| File | Description |
|---|---|
| src/video_source.cpp | Adds an internal SourceType path so VideoSource can create encoded-capable sources via FFI. |
| src/encoded_video_source.cpp | Implements synchronous submission of encoded frames + feedback polling over FFI. |
| src/room_proto_converter.cpp | Serializes/deserializes the new TrackPublishOptions::video_encoder field. |
| include/livekit/video_source.h | Adds private SourceType and constructor for encoded sources; friends EncodedVideoSource. |
| include/livekit/encoded_video_source.h | New public header defining the encoded ingestion API and data types. |
| include/livekit/room_event_types.h | Defines VideoCodec and adds VideoEncoderBackend + video_encoder publish option. |
| include/livekit/livekit.h | Exposes the new public header via the umbrella include. |
| src/tests/unit/test_video_frame_metadata.cpp | Adds round-trip unit test coverage for VideoEncoderBackend. |
| src/tests/unit/test_encoded_video_source.cpp | New unit tests for EncodedVideoSource construction, validation, feedback, and capture. |
| src/tests/stress/test_encoded_video_ingestion_stress.cpp | New stress test measuring sustained encoded frame submission throughput. |
| src/tests/integration/test_encoded_video_ingestion.cpp | New integration test publishing pre-encoded H.264 and verifying decoded reception. |
| docs/pre-encoded-video.md | New user-facing documentation for pre-encoded publishing and feedback handling. |
| docs/README.md | Adds a link to the new pre-encoded video documentation. |
| docs/doxygen/Doxyfile | Adds the new markdown doc to the Doxygen input set. |
| scripts/generate-docs.sh | Ensures the generated Doxygen config includes the new doc page. |
| CMakeLists.txt | Adds src/encoded_video_source.cpp to the shared library build. |
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
4d41d21 to
127f263
Compare
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 Documentation generation always fails
Deleting the guide leaves generate-docs.sh referencing it as a Doxygen input. Doxygen treats the missing file as fatal, blocking documentation CI and releases.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
Point Rust SDK at pre-encoded ingest FFI
127f263 to
e6545df
Compare
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
4 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| /// Optional video encoder backend. Pre-encoded sources must select | ||
| /// @ref VideoEncoderBackend::PreEncoded. | ||
| std::optional<VideoEncoderBackend> video_encoder; |
There was a problem hiding this comment.
🟡 Convenience publishing misconfigures encoded video
publishVideoTrack accepts an EncodedVideoSource but publishes it without video_codec or PreEncoded. Its default options violate both requirements, so encoded publication can fail or advertise the wrong codec.
Prompt for agents
LocalParticipant::publishVideoTrack accepts shared_ptr<VideoSource>, which also accepts the new EncodedVideoSource, but its generated TrackPublishOptions only sets source. Encoded publication requires VideoEncoderBackend::PreEncoded and a matching VideoCodec. Update the convenience publishing API or source/track metadata flow so an EncodedVideoSource passed through this public helper automatically supplies both settings, or reject this source type clearly before sending the publish request. Preserve normal VideoSource behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
Overview
Pre-Encoded Ingestion