integrate livekit capture source-clock and source-pattern by stephen-derosa · Pull Request #274 · livekit/client-sdk-cpp

stephen-derosa · 2026-10-07T18:17:59Z

integrate the livekit capture source-clock and source-pattern.

This is from #227 with the gstreamer and device sources stripped.

devin-ai-integration

Devin Review found 2 potential issues.

3 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)

devin-ai-integration · 2026-10-07T18:19:51Z


 option(LIVEKIT_BUILD_EXAMPLES "Build LiveKit examples" OFF)
 option(LIVEKIT_BUILD_TESTS "Build LiveKit tests" OFF)
+option(LIVEKIT_ENABLE_CAPTURE "Build the Rust FFI with pattern and clock capture sources" OFF)


🔴 Release bundles cannot create capture sources

Official release builds leave LIVEKIT_ENABLE_CAPTURE off, so CaptureSource::create cannot create either built-in source. The release workflow uses release-examples, not the capture-enabled test presets.

Learn more

The public capture factories are always compiled, but the Rust implementation is gated by this CMake option. The official release workflow builds using the release-examples preset, which does not enable it. Consumers of a clean release bundle therefore cannot use either advertised source.

Example: A release built through ./build.sh release-examples --bundle includes CaptureSource::create, but awaiting a pattern-source creation future fails because the Rust FFI was built without capture-pattern.

Recommended fix: Enable the capture features in release and example presets, including the presets used by .github/workflows/make-release.yml and .github/workflows/builds.yml, or make capture support the default while explicitly allowing opt-out.

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration · 2026-10-07T18:19:52Z

          -DPROTOC_PATH=${Protobuf_PROTOC_EXECUTABLE}
          -DRUST_TARGET=${RUST_TARGET_TRIPLE}
          -DGCC_LIB_DIR=${GCC_LIB_DIR}
+          -DCARGO_FEATURES=${LIVEKIT_CARGO_FEATURES}


🔴 Capture-enabled builds can reuse disabled FFI

When separate build directories share a Rust target, LIVEKIT_ENABLE_CAPTURE can change without rebuilding its existing FFI artifact. The new build advertises capture support while CaptureSource::create still uses the disabled library.

Learn more

The Rust library output path is shared across configurations with the same Rust profile, but the custom command only tracks Rust source files. A second build directory can see the first one's existing artifact as up to date despite requesting different Cargo features. The generated kCaptureEnabled constant then describes CMake configuration, not the library actually loaded.

Example: Build a capture-disabled Debug configuration, then configure a different Debug build directory with LIVEKIT_ENABLE_CAPTURE=ON and build without changing Rust sources. Its FFI output already exists, so Cargo is skipped and pattern creation fails despite kCaptureEnabled == true.

Recommended fix: Give each feature configuration its own Rust target/output directory, or add a feature-specific stamp that forces Cargo to run whenever the feature selection differs from the library's actual build. Ensure header generation and the selected FFI artifact use the same feature set.

Was this helpful? React with 👍 or 👎 to provide feedback.

Copilot

🟡 Changes recommended

Capture build dependencies can become stale, and the public class layout introduces ABI constraints that should be addressed.

4 open findings

Track livekit-capture sources in custom build dependencies · New
Keep CaptureSource state behind a private implementation pointer · New
Clarify which presets enable capture · New
Document capture dimensions as positive values · New

What changed in this PR

Adds GPU-backed pattern and clock capture sources through the Rust FFI.

Changes:

Introduces the public CaptureSource API and FFI lifecycle handling.
Enables capture features in test presets and adds unit, integration, and stress tests.
Documents capture configuration, publishing, and lifecycle behavior.

File	Description
`src/tests/unit/test_capture_source.cpp`	Tests invalid configurations.
`src/tests/stress/test_capture_source_stress.cpp`	Measures capture throughput.
`src/tests/integration/test_capture_source.cpp`	Tests end-to-end publishing.
`src/ffi_client.h`	Declares capture creation support.
`src/ffi_client.cpp`	Handles capture FFI requests and events.
`src/capture_source.cpp`	Implements capture sources.
`scripts/generate-docs.sh`	Includes capture documentation.
`include/livekit/video_source.h`	Supports adopting capture handles.
`include/livekit/livekit.h`	Exposes the capture API.
`include/livekit/capture_source.h`	Defines the public capture API.
`docs/README.md`	Links capture documentation.
`docs/capture-sources.md`	Documents capture usage.
`docs/building.md`	Documents the build option.
`CMakePresets.json`	Enables capture for test presets.
`CMakeLists.txt`	Configures and builds capture support.
`build.h.in`	Exposes capture availability.
`AGENTS.md`	Documents capture architecture and threading.
`.gitattributes`	Configures preset-file whitespace handling.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+set(LIVEKIT_CARGO_FEATURES "")
+set(LIVEKIT_CAPTURE_ENABLED 0)
+if(LIVEKIT_ENABLE_CAPTURE)
+  set(LIVEKIT_CARGO_FEATURES "capture-pattern,capture-clock")


+  FfiHandle handle_;
+  CaptureSourceKind kind_ = CaptureSourceKind::Pixel;
+  int width_ = 0;
+  int height_ = 0;
+  std::optional<VideoCodec> codec_;


Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Clarify which build presets enable LIVEKIT_ENABLE_CAPTURE and the requirements for source creation. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

alan-george-lk · 2026-10-07T21:08:56Z

+  VideoSource(FfiHandle&& handle, int width, int height) noexcept
+      : handle_(std::move(handle)), width_(width), height_(height) {}


Should this be exposed as public API? Do we have precedent for passing in a FfiHandle in elsewhere?

alan-george-lk · 2026-10-07T21:19:06Z

+  do {                                                                                                               \
+    if constexpr (!kCaptureEnabled) {                                                                                \
+      GTEST_SKIP() << "livekit-ffi built without the 'capture' feature; configure with -DLIVEKIT_ENABLE_CAPTURE=ON"; \
+    }                                                                                                                \
+  } while (false)


This is a bit smelly, I feel like there's gotta be a better way to check if capture is enabled. Might be worth researching C++ best practices on dynamic library availability checking

alan-george-lk · 2026-10-07T21:22:15Z

+  using namespace std::chrono_literals;
+  if constexpr (!kCaptureEnabled) {
+    GTEST_SKIP() << "capture sources are disabled in this build";
+  }


This is cleaner than https://github.com/livekit/client-sdk-cpp/pull/274/changes#r4212095152

alan-george-lk · 2026-10-07T21:23:09Z

+  /// @param config Clock source configuration.
+  /// @return A future that resolves to the created capture source.
+  /// @throws CaptureSourceError When awaiting the future if creation fails.
+  static std::future<std::shared_ptr<CaptureSource>> create(ClockVideoSourceConfig config);


Help me understand why these should be async via the std::future, I get there's some async required to set things up under the hood, but compared to something like local_video_track that's synchronous: https://github.com/livekit/client-sdk-cpp/blob/main/include/livekit/local_video_track.h#L66-L67

Additionally, every example of it being called in this PR (test code, or example docs) immediately calls .get()

Not a critical comment, but am curious/want to make sure API is aligned

alan-george-lk · 2026-10-07T21:30:51Z


 option(LIVEKIT_BUILD_EXAMPLES "Build LiveKit examples" OFF)
 option(LIVEKIT_BUILD_TESTS "Build LiveKit tests" OFF)
+option(LIVEKIT_ENABLE_CAPTURE "Build the Rust FFI with pattern and clock capture sources" OFF)


LIVEKIT_ENABLE_CAPTURE -> LIVEKIT_BUILD_CAPTURE?

Also should we have additional CI release artifacts with capture? like <SDK>_capture.tar.gz

alan-george-lk · 2026-10-07T21:48:17Z

+namespace livekit {
+
+/// @brief Whether the Rust FFI was built with capture-source support.
+inline constexpr bool kCaptureEnabled = "@LIVEKIT_CAPTURE_ENABLED@"[0] == '1';


I'd prefer #define LIVEKIT_CAPTURE_ENABLED over a global bool, even though more out dated

integrate livekit capture source-clock and source-pattern

1c9e30d

stephen-derosa requested a balanced review from Copilot October 7, 2026 18:17

stephen-derosa self-assigned this Oct 7, 2026

stephen-derosa requested review from a team and xianshijing-lk as code owners October 7, 2026 18:18

Copilot started reviewing on behalf of stephen-derosa October 7, 2026 18:18 View session

devin-ai-integration Bot reviewed Oct 7, 2026

View reviewed changes

Copilot AI reviewed Oct 7, 2026

View reviewed changes

stephen-derosa and others added 2 commits October 7, 2026 15:17

Update comment for output resolution requirement

c1a01d5

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Update capture sources documentation for build presets

711f3ba

Clarify which build presets enable LIVEKIT_ENABLE_CAPTURE and the requirements for source creation. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

alan-george-lk reviewed Oct 7, 2026

View reviewed changes

Navigation Menu

Repository navigation

integrate livekit capture source-clock and source-pattern - #274

integrate livekit capture source-clock and source-pattern#274
stephen-derosa wants to merge 3 commits into
mainfrom
sderosa/initial-capture-source

stephen-derosa commented Oct 7, 2026

Uh oh!

devin-ai-integration Bot left a comment

Uh oh!

devin-ai-integration Bot Oct 7, 2026

Uh oh!

devin-ai-integration Bot Oct 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

alan-george-lk Oct 7, 2026

Uh oh!

alan-george-lk Oct 7, 2026

Uh oh!

alan-george-lk Oct 7, 2026

Uh oh!

alan-george-lk Oct 7, 2026

Uh oh!

alan-george-lk Oct 7, 2026

Uh oh!

alan-george-lk Oct 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		VideoSource(FfiHandle&& handle, int width, int height) noexcept
		: handle_(std::move(handle)), width_(width), height_(height) {}

Conversation

stephen-derosa commented Oct 7, 2026

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

alan-george-lk Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

alan-george-lk Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

alan-george-lk Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

alan-george-lk Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

alan-george-lk Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

alan-george-lk Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants