-
Notifications
You must be signed in to change notification settings - Fork 21
AGENTS.md #91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
AGENTS.md #91
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,149 @@ | ||||||||||||||
| # AGENTS.md — LiveKit C++ Client SDK | ||||||||||||||
|
|
||||||||||||||
| ## Project Overview | ||||||||||||||
|
|
||||||||||||||
| This is **client-sdk-cpp**, the official LiveKit C++ client SDK. It wraps a Rust core (`client-sdk-rust/`) via a protobuf-based FFI bridge. All WebRTC, networking, and media logic lives in Rust; the C++ layer provides an ergonomic API for C++ consumers. | ||||||||||||||
|
|
||||||||||||||
| ## Architecture | ||||||||||||||
|
|
||||||||||||||
| ### Core Principle | ||||||||||||||
| Rust owns as much of the business logic as possible. If a feature may be used by another SDK it should be implemented in Rust. Since this is an SDK, | ||||||||||||||
| ensure backwards compatibility is maintained when possible. | ||||||||||||||
|
|
||||||||||||||
| SDK features follow pattern: | ||||||||||||||
|
|
||||||||||||||
| 1. Serialize a protobuf `FfiRequest` message. | ||||||||||||||
| 2. Send it to Rust via `FfiClient::instance().sendRequest(req)`. | ||||||||||||||
| 3. Receive a synchronous `FfiResponse` or an asynchronous `FfiEvent` callback. | ||||||||||||||
| 4. Expose the result through the C++ API. | ||||||||||||||
|
|
||||||||||||||
| When making larger scale changes, check with the developer before committing to architecture changes involving changes to the `client-sdk-rust/` submodule. | ||||||||||||||
|
|
||||||||||||||
| ### Directory Layout | ||||||||||||||
|
|
||||||||||||||
| | Path | Description | | ||||||||||||||
| |------|-------------| | ||||||||||||||
| | `include/livekit/` | Public API headers (what SDK consumers include) | | ||||||||||||||
| | `src/` | Implementation files and internal-only headers (`ffi_client.h`, `lk_log.h`, etc.) | | ||||||||||||||
| | `src/tests/` | Google Test integration and stress tests | | ||||||||||||||
| | `examples/` | In-tree example applications | | ||||||||||||||
| | `bridge/` | **Deprecated** C-style bridge layer — do not add new functionality | | ||||||||||||||
| | `client-sdk-rust/` | Git submodule — **read-only, never modify** | | ||||||||||||||
| | `client-sdk-rust/livekit-ffi/protocol/*.proto` | FFI contract (protobuf definitions, read-only reference) | | ||||||||||||||
| | `cmake/` | Build helpers (`protobuf.cmake`, `spdlog.cmake`, `LiveKitConfig.cmake.in`) | | ||||||||||||||
| | `docker/` | Dockerfile for CI and SDK distribution images | | ||||||||||||||
| | `.github/workflows/` | GitHub Actions CI workflows | | ||||||||||||||
|
|
||||||||||||||
| ### Key Types | ||||||||||||||
|
|
||||||||||||||
| - **`FfiClient`** — Singleton that sends FFI requests to Rust and dispatches event callbacks. Defined in `src/ffi_client.h` (internal). | ||||||||||||||
| - **`FfiHandle`** — RAII wrapper for Rust-owned resource handles; drop releases the FFI resource. | ||||||||||||||
| - **`Room`** — Central object for connecting to a LiveKit server room. | ||||||||||||||
| - **`RoomDelegate`** — Virtual callback interface for room lifecycle events. | ||||||||||||||
| - **`SubscriptionThreadDispatcher`** — Owns callback registrations and per-subscription reader threads for audio, video, and data tracks. `Room` delegates callback management here. | ||||||||||||||
| - **`LocalParticipant` / `RemoteParticipant`** — Participant objects for publishing and receiving tracks. | ||||||||||||||
|
|
||||||||||||||
| ## Build | ||||||||||||||
|
|
||||||||||||||
| Use `./build.sh` — always. Do not invoke CMake directly. | ||||||||||||||
|
|
||||||||||||||
|
Comment on lines
+46
to
+49
|
||||||||||||||
| ``` | ||||||||||||||
| ./build.sh debug # Debug build | ||||||||||||||
| ./build.sh debug-tests # Debug build with tests | ||||||||||||||
| ./build.sh debug-examples # Debug build with examples | ||||||||||||||
| ./build.sh release # Release build | ||||||||||||||
| ./build.sh release-tests # Release build with tests | ||||||||||||||
| ./build.sh release-examples # Release build with examples | ||||||||||||||
| ./build.sh build-all # All of the above | ||||||||||||||
| ./build.sh clean # Clean build artifacts | ||||||||||||||
| ./build.sh clean-all # Full clean (C++ + Rust targets) | ||||||||||||||
| ``` | ||||||||||||||
|
|
||||||||||||||
| **Requirements:** CMake 3.20+, C++17, Rust toolchain (cargo), protoc. On macOS: `brew install cmake ninja protobuf abseil spdlog`. On Linux: see the CI workflow for apt packages. | ||||||||||||||
|
|
||||||||||||||
| ### SDK Packaging | ||||||||||||||
|
|
||||||||||||||
| ``` | ||||||||||||||
| ./build.sh release --bundle --prefix /path/to/install | ||||||||||||||
| ``` | ||||||||||||||
|
|
||||||||||||||
| This installs headers, libraries, and CMake config files so downstream projects can use `find_package(LiveKit CONFIG)`. | ||||||||||||||
|
|
||||||||||||||
| ## Coding Conventions | ||||||||||||||
|
|
||||||||||||||
| ### Copyright Header | ||||||||||||||
|
|
||||||||||||||
| All source files must have the LiveKit Apache 2.0 copyright header. Use the current year for new files. Do not copyright externally pulled/cloned files. | ||||||||||||||
|
|
||||||||||||||
| ### Logging | ||||||||||||||
|
|
||||||||||||||
| - Use `LK_LOG_*` macros from `src/lk_log.h` (internal header, **not** public API). | ||||||||||||||
| - `LK_LOG_ERROR` — when something is broken. | ||||||||||||||
| - `LK_LOG_WARN` — when something is unexpected but recoverable. | ||||||||||||||
| - `LK_LOG_INFO` — initialization, critical state changes. | ||||||||||||||
| - `LK_LOG_DEBUG` — potentially useful diagnostic info. | ||||||||||||||
| - Do not spam logs. Keep it concise. | ||||||||||||||
| - In production/internal SDK code, do not use `std::cout`, `std::cerr`, `printf`, or raw spdlog calls; use the logging APIs instead. Tests may use standard streams sparingly for ad hoc diagnostics when appropriate. | ||||||||||||||
| - The public logging API is `include/livekit/logging.h` (`setLogLevel`, `setLogCallback`). spdlog is a **private** dependency — it must never leak into public headers. | ||||||||||||||
|
|
||||||||||||||
| ### Public API Surface | ||||||||||||||
|
|
||||||||||||||
| - Keep public APIs small. Minimize what goes into `include/livekit/`. | ||||||||||||||
| - Never introduce backwards compatibility breaking changes to the public API. | ||||||||||||||
| - `lk_log.h` lives under `src/` (internal). The public-facing logging API is `include/livekit/logging.h`. | ||||||||||||||
| - spdlog must not appear in any public header or installed header. | ||||||||||||||
|
|
||||||||||||||
| ### Error Handling | ||||||||||||||
|
|
||||||||||||||
| - Throw exceptions for broken/unrecoverable states. | ||||||||||||||
| - Use `LK_LOG_WARN` for non-fatal unexpected conditions. | ||||||||||||||
| - Use `Result<T, E>` for operations that can fail with typed errors (e.g., data track publish/subscribe). | ||||||||||||||
|
|
||||||||||||||
| ### Git Practices | ||||||||||||||
|
|
||||||||||||||
| - Use `git mv` when moving or renaming files. | ||||||||||||||
|
|
||||||||||||||
| ### CMake | ||||||||||||||
|
|
||||||||||||||
| - spdlog is linked **PRIVATE** to the `livekit` target. It must not appear in exported/installed dependencies. | ||||||||||||||
| - protobuf is vendored via FetchContent on non-Windows platforms; Windows uses vcpkg. | ||||||||||||||
| - The CMake install produces a `find_package(LiveKit CONFIG)`-compatible package with `LiveKitConfig.cmake`, `LiveKitTargets.cmake`, and `LiveKitConfigVersion.cmake`. | ||||||||||||||
|
|
||||||||||||||
| ### Readability and Performance | ||||||||||||||
| Code should be easy to read and understand. If a sacrifice is made for performance or readability, it should be documented. | ||||||||||||||
| ## Dependencies | ||||||||||||||
|
|
||||||||||||||
| | Dependency | Scope | Notes | | ||||||||||||||
| |------------|-------|-------| | ||||||||||||||
| | protobuf | Private (built-in) | Vendored via FetchContent (Unix) or vcpkg (Windows) | | ||||||||||||||
| | spdlog | **Private** | FetchContent or system package; must NOT leak into public API | | ||||||||||||||
| | client-sdk-rust | Build-time | Git submodule, built via cargo during CMake build | | ||||||||||||||
| | Google Test | Test only | FetchContent in `src/tests/CMakeLists.txt` | | ||||||||||||||
|
|
||||||||||||||
| ## Testing | ||||||||||||||
|
|
||||||||||||||
| Tests are under `src/tests/` using Google Test: | ||||||||||||||
|
|
||||||||||||||
| ``` | ||||||||||||||
| ./build.sh debug-tests | ||||||||||||||
| cd build-debug && ctest | ||||||||||||||
| ``` | ||||||||||||||
|
|
||||||||||||||
| Integration tests (`src/tests/integration/`) cover: room connections, callbacks, data tracks, RPC, logging, audio processing, and the subscription thread dispatcher. | ||||||||||||||
|
|
||||||||||||||
| When adding new client facing functionality, add a new test case to the existing test suite. | ||||||||||||||
| When adding new client facing functionality, add benchmarking to understand the limitations of the new functionality. | ||||||||||||||
|
|
||||||||||||||
| ## Formatting | ||||||||||||||
| Adhere to the formatting rules if TODO: @alan | ||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @alan-george-lk i wonder what the best way t provide this is. My reading on Agents.md leads me to believe we should just put code snippets directly inline. Does clang-tidy provide a list or would it be reasonable to ask cursor: |
||||||||||||||
|
|
||||||||||||||
|
Comment on lines
+138
to
+139
|
||||||||||||||
| Adhere to the formatting rules if TODO: @alan | |
| - Match the style of the surrounding code and keep formatting changes limited to the files you touch. | |
| - Use consistent indentation, spacing, and brace style with nearby C++ headers, sources, CMake files, and Markdown. | |
| - Keep `#include` ordering and whitespace consistent with the existing file rather than reformatting unrelated code. | |
| - If the repository or your editor provides formatting configuration, run it before submitting changes; otherwise prefer minimal, manual formatting-only edits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The document says
client-sdk-rust/is “read-only, never modify”, but also advises checking with a developer before “committing to … changes involving changes” to that submodule. Consider clarifying the intended workflow (e.g., submodule bumps are allowed via updating the pointer, but no direct edits inside the submodule).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply changes based on this feedback