From 090b1a4cabaf6efbc94a78a959d116e93a1d48d5 Mon Sep 17 00:00:00 2001 From: Alan George Date: Thu, 30 Apr 2026 16:18:00 -0600 Subject: [PATCH 01/24] Reintroduce LIVEKIT_USE_SYSTEM_SPDLOG --- README.md | 18 ++++++++++++++++++ cmake/spdlog.cmake | 17 ++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 6136542b..6847c1ae 100644 --- a/README.md +++ b/README.md @@ -296,6 +296,24 @@ livekit::setLogLevel(livekit::LogLevel::Debug); // show more detail livekit::setLogLevel(livekit::LogLevel::Warn); // suppress info chatter ``` +### Embedding the SDK in processes that already use spdlog (`LIVEKIT_USE_SYSTEM_SPDLOG`) + +By default on Linux/macOS the SDK vendors spdlog and statically links it into +`liblivekit`. If you embed the SDK in a process that **already loads spdlog** +itself. Two independent spdlog/fmt copies in the same process can crash at runtime. +Build the SDK with the system spdlog instead so both components share one implementation: + +```bash +cmake -B build-release -S . \ + -DCMAKE_BUILD_TYPE=Release \ + -DLIVEKIT_USE_SYSTEM_SPDLOG=ON +cmake --build build-release +``` + +Pass this only to the SDK build, not to the consumer's build. On Windows the +vcpkg path already supplies a single shared spdlog, so the option is a no-op +there. + ### Custom log callback Replace the default stderr sink with your own handler. This is the integration diff --git a/cmake/spdlog.cmake b/cmake/spdlog.cmake index b0956b00..3adb4189 100644 --- a/cmake/spdlog.cmake +++ b/cmake/spdlog.cmake @@ -1,7 +1,9 @@ # cmake/spdlog.cmake # # Windows: use vcpkg spdlog -# macOS/Linux: vendored spdlog via FetchContent (static library) +# macOS/Linux: vendored spdlog via FetchContent (static library), unless +# LIVEKIT_USE_SYSTEM_SPDLOG=ON, in which case the system-provided +# spdlog (find_package) is used. # # Exposes: # - Target spdlog::spdlog @@ -50,6 +52,10 @@ include(FetchContent) set(LIVEKIT_SPDLOG_VERSION "1.15.1" CACHE STRING "Vendored spdlog version") +option(LIVEKIT_USE_SYSTEM_SPDLOG + "Use system spdlog (find_package) instead of vendored" + OFF) + # --------------------------------------------------------------------------- # Windows: use vcpkg # --------------------------------------------------------------------------- @@ -59,6 +65,15 @@ if(WIN32 AND LIVEKIT_USE_VCPKG) return() endif() +# --------------------------------------------------------------------------- +# macOS/Linux: system spdlog (opt-in via LIVEKIT_USE_SYSTEM_SPDLOG=ON) +# --------------------------------------------------------------------------- +if(LIVEKIT_USE_SYSTEM_SPDLOG) + find_package(spdlog CONFIG REQUIRED) + message(STATUS "Using system spdlog (LIVEKIT_USE_SYSTEM_SPDLOG=ON): version=${spdlog_VERSION}") + return() +endif() + # --------------------------------------------------------------------------- # macOS/Linux: vendored spdlog via FetchContent # --------------------------------------------------------------------------- From 4068f09a352d4bd499aa2e4b6b4cdfb224272af9 Mon Sep 17 00:00:00 2001 From: Alan George Date: Thu, 7 May 2026 13:40:10 -0600 Subject: [PATCH 02/24] Export, visibility --- .github/workflows/builds.yml | 47 ++++ AGENTS.md | 31 +++ CMakeLists.txt | 29 ++- cmake/protobuf.cmake | 46 ++++ cmake/spdlog.cmake | 12 ++ include/livekit/audio_frame.h | 4 +- include/livekit/audio_processing_module.h | 3 +- include/livekit/audio_source.h | 3 +- include/livekit/audio_stream.h | 3 +- include/livekit/data_stream.h | 12 +- include/livekit/data_track_error.h | 8 +- include/livekit/data_track_frame.h | 5 +- include/livekit/data_track_stream.h | 3 +- include/livekit/e2ee.h | 8 +- include/livekit/export.h | 66 ++++++ include/livekit/ffi_handle.h | 4 +- include/livekit/livekit.h | 7 +- include/livekit/local_audio_track.h | 3 +- include/livekit/local_data_track.h | 3 +- include/livekit/local_participant.h | 3 +- include/livekit/local_track_publication.h | 3 +- include/livekit/local_video_track.h | 3 +- include/livekit/logging.h | 8 +- include/livekit/participant.h | 3 +- include/livekit/remote_audio_track.h | 3 +- include/livekit/remote_data_track.h | 3 +- include/livekit/remote_participant.h | 7 +- include/livekit/remote_track_publication.h | 3 +- include/livekit/remote_video_track.h | 3 +- include/livekit/room.h | 3 +- include/livekit/room_delegate.h | 3 +- include/livekit/rpc_error.h | 4 +- include/livekit/stats.h | 52 +++-- .../livekit/subscription_thread_dispatcher.h | 3 +- include/livekit/tracing.h | 10 +- include/livekit/track.h | 3 +- include/livekit/track_publication.h | 3 +- include/livekit/video_frame.h | 4 +- include/livekit/video_source.h | 3 +- include/livekit/video_stream.h | 3 +- scripts/check_no_private_symbols.py | 201 ++++++++++++++++++ src/ffi_client.h | 8 +- src/lk_log.h | 11 +- src/logging.cpp | 1 + src/room_proto_converter.h | 106 +++++---- src/tests/CMakeLists.txt | 46 +++- src/trace/event_tracer.h | 18 +- src/video_utils.h | 22 +- 48 files changed, 689 insertions(+), 151 deletions(-) create mode 100644 include/livekit/export.h create mode 100755 scripts/check_no_private_symbols.py diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 77fc58c5..6335af09 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -9,6 +9,8 @@ on: - cpp-example-collection/** - bridge/** - client-sdk-rust/** + - cmake/** + - scripts/** - CMakeLists.txt - build.sh - build.cmd @@ -25,6 +27,8 @@ on: - cpp-example-collection/** - bridge/** - client-sdk-rust/** + - cmake/** + - scripts/** - CMakeLists.txt - build.sh - build.cmd @@ -177,6 +181,49 @@ jobs: shell: pwsh run: ${{ matrix.build_cmd }} + # ---------- Verify exported ABI: no spdlog/fmt/protobuf/absl leaks ---------- + - name: Setup Python + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v6.0.0 + with: + python-version: '3.x' + + - name: Verify no private symbols leak (Unix) + if: runner.os != 'Windows' + shell: bash + run: | + set -eux + libdir="${{ matrix.build_dir }}/lib" + if [[ "$RUNNER_OS" == "macOS" ]]; then + lib="${libdir}/liblivekit.dylib" + if [[ ! -f "${lib}" ]]; then + lib="${{ matrix.build_dir }}/bin/liblivekit.dylib" + fi + else + lib="${libdir}/liblivekit.so" + if [[ ! -f "${lib}" ]]; then + lib="${{ matrix.build_dir }}/bin/liblivekit.so" + fi + fi + python3 scripts/check_no_private_symbols.py "${lib}" + + - name: Verify no private symbols leak (Windows) + if: runner.os == 'Windows' + shell: pwsh + run: | + $candidates = @( + "${{ matrix.build_dir }}/bin/livekit.dll", + "${{ matrix.build_dir }}/lib/livekit.dll" + ) + $lib = $null + foreach ($p in $candidates) { + if (Test-Path -LiteralPath $p) { $lib = $p; break } + } + if ($null -eq $lib) { + Write-Error "livekit.dll not found in any of: $($candidates -join ', ')" + exit 1 + } + python scripts/check_no_private_symbols.py "$lib" + # ---------- Smoke test cpp-example-collection binaries ---------- # Built under cpp-example-collection-build/ (not build-dir/bin). Visual Studio # multi-config places executables in per-target Release/ (or Debug/) subdirs. diff --git a/AGENTS.md b/AGENTS.md index 4a7051f4..a4f4454d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -166,6 +166,37 @@ All source files must have the LiveKit Apache 2.0 copyright header. Use the curr - 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`. +### Symbol Visibility / Exported ABI + +The `livekit` shared library is built with hidden symbol visibility on all +platforms. Only symbols explicitly tagged with `LIVEKIT_API` (declared in +`include/livekit/export.h`) are part of the public ABI. Vendored static +dependencies (spdlog, fmt, protobuf, absl) are also compiled with hidden +visibility so their symbols cannot leak into `liblivekit.{so,dylib,dll}`. + +Rules for new code: + +- Mark any new public class, free function, or out-of-line static method with + `LIVEKIT_API`. POD/aggregate structs and pure-inline classes do not need + annotation because they emit no out-of-line symbols. +- For internal symbols that must remain visible to in-tree tests (the tests + link the same shared library), use `LIVEKIT_INTERNAL_API`. External + consumers must not rely on these. +- Do not add `__declspec(dllexport)` or `__attribute__((visibility("default")))` + by hand; always go through `LIVEKIT_API` / `LIVEKIT_INTERNAL_API`. +- On Windows, `WINDOWS_EXPORT_ALL_SYMBOLS` is **deliberately disabled** for the + `livekit` target. Use `LIVEKIT_API` to export. + +CI enforces the policy via `scripts/check_no_private_symbols.py`, which is +also wired in as a CTest test (label `abi`): + +``` +ctest -L abi --output-on-failure +``` + +The script fails if `nm`/`dumpbin` reports any exported symbol containing +`spdlog`, `fmt::v`, `google::protobuf`, or `absl::`. + ### Readability and Performance Code should be easy to read and understand. If a sacrifice is made for performance or readability, it should be documented. diff --git a/CMakeLists.txt b/CMakeLists.txt index 6a1946b7..43c853a5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -96,6 +96,13 @@ endif() message(STATUS "Using protoc: ${Protobuf_PROTOC_EXECUTABLE}") add_library(livekit_proto OBJECT ${FFI_PROTO_FILES}) +# livekit_proto is compiled into liblivekit; apply the same hidden visibility +# policy so generated protobuf code does not leak into the SDK's exported ABI. +set_target_properties(livekit_proto PROPERTIES + CXX_VISIBILITY_PRESET hidden + C_VISIBILITY_PRESET hidden + VISIBILITY_INLINES_HIDDEN ON +) if(TARGET protobuf::libprotobuf) set(LIVEKIT_PROTOBUF_TARGET protobuf::libprotobuf) else() @@ -366,8 +373,26 @@ add_library(livekit SHARED src/trace/trace_event.h src/trace/tracing.cpp ) -if(WIN32) - set_target_properties(livekit PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS ON) +# Symbol visibility: keep liblivekit's exported ABI restricted to the public +# LiveKit API. Every TU is compiled with -fvisibility=hidden, and only +# declarations annotated with LIVEKIT_API (see include/livekit/export.h) are +# re-exposed. This prevents private dependencies (spdlog, protobuf, absl) from +# leaking into the dynamic symbol table and clashing with consumer apps that +# also link those libraries (e.g. ROS 2's rcl_logging_spdlog). +set_target_properties(livekit PROPERTIES + CXX_VISIBILITY_PRESET hidden + C_VISIBILITY_PRESET hidden + VISIBILITY_INLINES_HIDDEN ON +) +target_compile_definitions(livekit PRIVATE LIVEKIT_BUILDING_SDK=1) + +# Linker-level belt-and-suspenders for Linux: even if a static-archive object +# slipped through with default visibility, --exclude-libs,ALL strips it from +# the dynamic symbol table. ld64 (macOS) has no equivalent flag; hidden +# visibility per TU is sufficient there. On Windows, explicit dllexport via +# LIVEKIT_API replaces WINDOWS_EXPORT_ALL_SYMBOLS. +if(UNIX AND NOT APPLE) + target_link_options(livekit PRIVATE "LINKER:--exclude-libs,ALL") endif() diff --git a/cmake/protobuf.cmake b/cmake/protobuf.cmake index 42901db3..860cb23f 100644 --- a/cmake/protobuf.cmake +++ b/cmake/protobuf.cmake @@ -119,6 +119,28 @@ set(protobuf_INSTALL OFF CACHE BOOL "" FORCE) set(ABSL_ENABLE_INSTALL OFF CACHE BOOL "" FORCE) set(utf8_range_ENABLE_INSTALL OFF CACHE BOOL "" FORCE) +# Force hidden visibility on every target created by the FetchContent +# subprojects below. Setting these as CACHE BOOL FORCE before +# FetchContent_MakeAvailable propagates to every add_library() call inside +# absl / protobuf / utf8_range, so their .o files are compiled with +# -fvisibility=hidden -fvisibility-inlines-hidden. Without this, even though +# liblivekit itself is hidden, absl's static archives would carry default- +# visibility symbols that the linker happily re-exports. +# +# We snapshot the previous values so we can restore them after +# FetchContent_MakeAvailable; otherwise sibling components like the bridge +# library would also inherit hidden visibility, which would break the bridge +# unit tests that need access to internal bridge symbols. +set(_livekit_prev_cxx_visibility "${CMAKE_CXX_VISIBILITY_PRESET}") +set(_livekit_prev_c_visibility "${CMAKE_C_VISIBILITY_PRESET}") +set(_livekit_prev_inlines_hidden "${CMAKE_VISIBILITY_INLINES_HIDDEN}") +set(_livekit_prev_pic "${CMAKE_POSITION_INDEPENDENT_CODE}") + +set(CMAKE_CXX_VISIBILITY_PRESET hidden CACHE STRING "" FORCE) +set(CMAKE_C_VISIBILITY_PRESET hidden CACHE STRING "" FORCE) +set(CMAKE_VISIBILITY_INLINES_HIDDEN ON CACHE BOOL "" FORCE) +set(CMAKE_POSITION_INDEPENDENT_CODE ON CACHE BOOL "" FORCE) + set(protobuf_BUILD_TESTS OFF CACHE BOOL "" FORCE) set(protobuf_BUILD_EXAMPLES OFF CACHE BOOL "" FORCE) set(protobuf_BUILD_CONFORMANCE OFF CACHE BOOL "" FORCE) @@ -162,6 +184,30 @@ endif() # Now make protobuf available. FetchContent_MakeAvailable(livekit_protobuf) +# Restore the prior visibility cache values so the rest of the build (e.g. +# the deprecated bridge target and its tests) can use the project default +# visibility. The livekit target itself sets CXX_VISIBILITY_PRESET hidden +# explicitly via target properties, so it does not depend on these globals. +if(_livekit_prev_cxx_visibility STREQUAL "") + unset(CMAKE_CXX_VISIBILITY_PRESET CACHE) +else() + set(CMAKE_CXX_VISIBILITY_PRESET "${_livekit_prev_cxx_visibility}" + CACHE STRING "" FORCE) +endif() +if(_livekit_prev_c_visibility STREQUAL "") + unset(CMAKE_C_VISIBILITY_PRESET CACHE) +else() + set(CMAKE_C_VISIBILITY_PRESET "${_livekit_prev_c_visibility}" + CACHE STRING "" FORCE) +endif() +if(_livekit_prev_inlines_hidden STREQUAL "") + unset(CMAKE_VISIBILITY_INLINES_HIDDEN CACHE) +else() + set(CMAKE_VISIBILITY_INLINES_HIDDEN "${_livekit_prev_inlines_hidden}" + CACHE BOOL "" FORCE) +endif() +# Position-independent code remains ON globally (it was forced ON anyway). + # Protobuf targets: modern protobuf exports protobuf::protoc etc. if(TARGET protobuf::protoc) set(Protobuf_PROTOC_EXECUTABLE "$" CACHE STRING "protoc (vendored)" FORCE) diff --git a/cmake/spdlog.cmake b/cmake/spdlog.cmake index 3adb4189..2c56a916 100644 --- a/cmake/spdlog.cmake +++ b/cmake/spdlog.cmake @@ -90,4 +90,16 @@ set(SPDLOG_INSTALL OFF CACHE BOOL "" FORCE) FetchContent_MakeAvailable(livekit_spdlog) +# spdlog is linked PRIVATE into liblivekit and must not leak its symbols into +# the SDK's exported ABI. Force hidden visibility on the spdlog target so its +# object files don't carry default-visibility symbols into liblivekit. +if(TARGET spdlog) + set_target_properties(spdlog PROPERTIES + CXX_VISIBILITY_PRESET hidden + C_VISIBILITY_PRESET hidden + VISIBILITY_INLINES_HIDDEN ON + POSITION_INDEPENDENT_CODE ON + ) +endif() + message(STATUS "macOS/Linux: using vendored spdlog v${LIVEKIT_SPDLOG_VERSION}") diff --git a/include/livekit/audio_frame.h b/include/livekit/audio_frame.h index 2db2b9be..71c4a8fe 100644 --- a/include/livekit/audio_frame.h +++ b/include/livekit/audio_frame.h @@ -16,6 +16,8 @@ #pragma once +#include "livekit/export.h" + #include #include #include @@ -34,7 +36,7 @@ class OwnedAudioFrameBuffer; * number of channels, and samples per channel. It is used for capturing and * processing audio in the LiveKit SDK. */ -class AudioFrame { +class LIVEKIT_API AudioFrame { public: /** * Construct an AudioFrame from raw PCM samples. diff --git a/include/livekit/audio_processing_module.h b/include/livekit/audio_processing_module.h index 29eab332..b1267cf6 100644 --- a/include/livekit/audio_processing_module.h +++ b/include/livekit/audio_processing_module.h @@ -19,6 +19,7 @@ #include #include "livekit/audio_frame.h" +#include "livekit/export.h" #include "livekit/ffi_handle.h" namespace livekit { @@ -41,7 +42,7 @@ namespace livekit { * * Note: Audio frames must be exactly 10ms in duration. */ -class AudioProcessingModule { +class LIVEKIT_API AudioProcessingModule { public: /** * @brief Configuration options for the Audio Processing Module. diff --git a/include/livekit/audio_source.h b/include/livekit/audio_source.h index 4fbc58cc..7035d538 100644 --- a/include/livekit/audio_source.h +++ b/include/livekit/audio_source.h @@ -19,6 +19,7 @@ #include #include "livekit/audio_frame.h" +#include "livekit/export.h" #include "livekit/ffi_handle.h" namespace livekit { @@ -33,7 +34,7 @@ class FfiClient; /** * Represents a real-time audio source with an internal audio queue. */ -class AudioSource { +class LIVEKIT_API AudioSource { public: /** * Create a new native audio source. diff --git a/include/livekit/audio_stream.h b/include/livekit/audio_stream.h index d6dce270..db7e9303 100644 --- a/include/livekit/audio_stream.h +++ b/include/livekit/audio_stream.h @@ -25,6 +25,7 @@ #include #include "audio_frame.h" +#include "export.h" #include "ffi_handle.h" #include "participant.h" #include "track.h" @@ -65,7 +66,7 @@ struct AudioFrameEvent { * * stream->close(); // optional, called automatically in destructor */ -class AudioStream { +class LIVEKIT_API AudioStream { public: /// Configuration options for AudioStream creation. struct Options { diff --git a/include/livekit/data_stream.h b/include/livekit/data_stream.h index 2d5b3f0d..0ad541d1 100644 --- a/include/livekit/data_stream.h +++ b/include/livekit/data_stream.h @@ -16,6 +16,8 @@ #pragma once +#include "livekit/export.h" + #include #include #include @@ -75,7 +77,7 @@ struct ByteStreamInfo : BaseStreamInfo { /// Reader for incoming text streams. /// Created internally by the SDK when a text stream header is received. -class TextStreamReader { +class LIVEKIT_API TextStreamReader { public: /// Construct a reader from initial stream metadata. explicit TextStreamReader(TextStreamInfo info); @@ -115,7 +117,7 @@ class TextStreamReader { }; /// Reader for incoming byte streams. -class ByteStreamReader { +class LIVEKIT_API ByteStreamReader { public: /// Construct a reader from initial stream metadata. explicit ByteStreamReader(ByteStreamInfo info); @@ -151,7 +153,7 @@ class ByteStreamReader { /// Base class for sending data streams. /// Concrete subclasses are TextStreamWriter and ByteStreamWriter. -class BaseStreamWriter { +class LIVEKIT_API BaseStreamWriter { public: virtual ~BaseStreamWriter() = default; @@ -221,7 +223,7 @@ class BaseStreamWriter { }; /// Writer for outgoing text streams. -class TextStreamWriter : public BaseStreamWriter { +class LIVEKIT_API TextStreamWriter : public BaseStreamWriter { public: TextStreamWriter(LocalParticipant &local_participant, const std::string &topic = "", @@ -246,7 +248,7 @@ class TextStreamWriter : public BaseStreamWriter { }; /// Writer for outgoing byte streams. -class ByteStreamWriter : public BaseStreamWriter { +class LIVEKIT_API ByteStreamWriter : public BaseStreamWriter { public: ByteStreamWriter(LocalParticipant &local_participant, const std::string &name, const std::string &topic = "", diff --git a/include/livekit/data_track_error.h b/include/livekit/data_track_error.h index 5ec8f97f..a704334b 100644 --- a/include/livekit/data_track_error.h +++ b/include/livekit/data_track_error.h @@ -17,6 +17,8 @@ #ifndef LIVEKIT_DATA_TRACK_ERROR_H #define LIVEKIT_DATA_TRACK_ERROR_H +#include "livekit/export.h" + #include #include @@ -45,7 +47,7 @@ struct PublishDataTrackError { PublishDataTrackErrorCode code{PublishDataTrackErrorCode::UNKNOWN}; std::string message; - static PublishDataTrackError + LIVEKIT_API static PublishDataTrackError fromProto(const proto::PublishDataTrackError &error); }; @@ -61,7 +63,7 @@ struct LocalDataTrackTryPushError { LocalDataTrackTryPushErrorCode code{LocalDataTrackTryPushErrorCode::UNKNOWN}; std::string message; - static LocalDataTrackTryPushError + LIVEKIT_API static LocalDataTrackTryPushError fromProto(const proto::LocalDataTrackTryPushError &error); }; @@ -79,7 +81,7 @@ struct SubscribeDataTrackError { SubscribeDataTrackErrorCode code{SubscribeDataTrackErrorCode::UNKNOWN}; std::string message; - static SubscribeDataTrackError + LIVEKIT_API static SubscribeDataTrackError fromProto(const proto::SubscribeDataTrackError &error); }; diff --git a/include/livekit/data_track_frame.h b/include/livekit/data_track_frame.h index d4f08e6e..93bb1ee3 100644 --- a/include/livekit/data_track_frame.h +++ b/include/livekit/data_track_frame.h @@ -16,6 +16,8 @@ #pragma once +#include "livekit/export.h" + #include #include #include @@ -62,7 +64,8 @@ struct DataTrackFrame { * @param owned The proto::DataTrackFrame to create a DataTrackFrame from. * @return The created DataTrackFrame. */ - static DataTrackFrame fromOwnedInfo(const proto::DataTrackFrame &owned); + LIVEKIT_API static DataTrackFrame + fromOwnedInfo(const proto::DataTrackFrame &owned); }; } // namespace livekit diff --git a/include/livekit/data_track_stream.h b/include/livekit/data_track_stream.h index cb81054e..a2217f7e 100644 --- a/include/livekit/data_track_stream.h +++ b/include/livekit/data_track_stream.h @@ -17,6 +17,7 @@ #pragma once #include "livekit/data_track_frame.h" +#include "livekit/export.h" #include "livekit/ffi_handle.h" #include @@ -52,7 +53,7 @@ class FfiEvent; * } * } */ -class DataTrackStream { +class LIVEKIT_API DataTrackStream { public: struct Options { /// Maximum frames buffered on the Rust side. Rust defaults to 16. diff --git a/include/livekit/e2ee.h b/include/livekit/e2ee.h index e6473ae8..86f0f4e3 100644 --- a/include/livekit/e2ee.h +++ b/include/livekit/e2ee.h @@ -16,6 +16,8 @@ #pragma once +#include "livekit/export.h" + #include #include #include @@ -106,7 +108,7 @@ struct E2EEOptions { * - Providing a shared key up-front is convenient for shared-key E2EE, but is * not required by the API shape (keys may be supplied later). */ -class E2EEManager { +class LIVEKIT_API E2EEManager { public: /** If your application requires key rotation during the lifetime of a single * room or unique keys per participant (such as when implementing the MEGOLM @@ -114,7 +116,7 @@ class E2EEManager { * https://docs.livekit.io/home/client/encryption/#custom-key-provider doe * details * */ - class KeyProvider { + class LIVEKIT_API KeyProvider { public: ~KeyProvider() = default; @@ -154,7 +156,7 @@ class E2EEManager { KeyProviderOptions options_; }; - class FrameCryptor { + class LIVEKIT_API FrameCryptor { public: FrameCryptor(std::uint64_t room_handle, std::string participant_identity, int key_index, bool enabled); diff --git a/include/livekit/export.h b/include/livekit/export.h new file mode 100644 index 00000000..14983571 --- /dev/null +++ b/include/livekit/export.h @@ -0,0 +1,66 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef LIVEKIT_EXPORT_H +#define LIVEKIT_EXPORT_H + +// LIVEKIT_API marks a symbol as part of the public ABI of liblivekit. +// +// On Unix, the SDK is built with -fvisibility=hidden / -fvisibility-inlines-hidden, +// so every symbol defaults to hidden. LIVEKIT_API re-exposes the symbol with +// default visibility. Consumers see no annotation (just a normal declaration). +// +// On Windows, the SDK is built without WINDOWS_EXPORT_ALL_SYMBOLS, so symbols +// must be explicitly tagged with __declspec(dllexport) when building the SDK +// and __declspec(dllimport) when consuming it. LIVEKIT_BUILDING_SDK is defined +// only when compiling the SDK itself (set in CMakeLists.txt). + +#if defined(_WIN32) +#if defined(LIVEKIT_BUILDING_SDK) +#define LIVEKIT_API __declspec(dllexport) +#else +#define LIVEKIT_API __declspec(dllimport) +#endif +#else +#if defined(LIVEKIT_BUILDING_SDK) +#define LIVEKIT_API __attribute__((visibility("default"))) +#else +#define LIVEKIT_API +#endif +#endif + +// LIVEKIT_INTERNAL_API marks a symbol that is NOT part of the public ABI but +// must remain visible so that the in-tree test binaries (which link against +// the same shared library) can resolve it. +// +// External consumers must not rely on LIVEKIT_INTERNAL_API symbols; they may +// change or disappear without notice. +// +// On Windows, internal symbols are exported the same way as public ones +// because tests link via the standard import library; on Unix, hidden +// visibility is overridden for these specific symbols only. + +#if defined(_WIN32) +#define LIVEKIT_INTERNAL_API LIVEKIT_API +#else +#if defined(LIVEKIT_BUILDING_SDK) +#define LIVEKIT_INTERNAL_API __attribute__((visibility("default"))) +#else +#define LIVEKIT_INTERNAL_API +#endif +#endif + +#endif // LIVEKIT_EXPORT_H diff --git a/include/livekit/ffi_handle.h b/include/livekit/ffi_handle.h index 55ef6b64..6ff480a8 100644 --- a/include/livekit/ffi_handle.h +++ b/include/livekit/ffi_handle.h @@ -16,6 +16,8 @@ #pragma once +#include "livekit/export.h" + #include namespace livekit { @@ -26,7 +28,7 @@ namespace livekit { * Ensures that the handle is automatically released via * livekit_ffi_drop_handle() when the object goes out of scope. */ -class FfiHandle { +class LIVEKIT_API FfiHandle { public: explicit FfiHandle(uintptr_t h = 0) noexcept; ~FfiHandle(); diff --git a/include/livekit/livekit.h b/include/livekit/livekit.h index b662b2a8..1324b521 100644 --- a/include/livekit/livekit.h +++ b/include/livekit/livekit.h @@ -22,6 +22,7 @@ #include "audio_stream.h" #include "build.h" #include "e2ee.h" +#include "export.h" #include "local_audio_track.h" #include "local_participant.h" #include "local_track_publication.h" @@ -59,12 +60,12 @@ enum class LogSink { /// @param log_sink The log sink to use for SDK messages (default: Console). /// @returns true if initialization happened on this call, false if it was /// already initialized. -bool initialize(const LogLevel &level = LogLevel::Info, - const LogSink &log_sink = LogSink::kConsole); +LIVEKIT_API bool initialize(const LogLevel &level = LogLevel::Info, + const LogSink &log_sink = LogSink::kConsole); /// Shut down the LiveKit SDK. /// /// After shutdown, you may call initialize() again. -void shutdown(); +LIVEKIT_API void shutdown(); } // namespace livekit \ No newline at end of file diff --git a/include/livekit/local_audio_track.h b/include/livekit/local_audio_track.h index d8ad51e4..e1cd7975 100644 --- a/include/livekit/local_audio_track.h +++ b/include/livekit/local_audio_track.h @@ -17,6 +17,7 @@ #pragma once #include "audio_frame.h" +#include "export.h" #include "local_track_publication.h" #include "track.h" #include @@ -50,7 +51,7 @@ class AudioSource; * The track name provided during creation is visible to remote * participants and can be used for debugging or UI display. */ -class LocalAudioTrack : public Track { +class LIVEKIT_API LocalAudioTrack : public Track { public: /// Creates a new local audio track backed by the given `AudioSource`. /// diff --git a/include/livekit/local_data_track.h b/include/livekit/local_data_track.h index 6d128de9..daa40cbc 100644 --- a/include/livekit/local_data_track.h +++ b/include/livekit/local_data_track.h @@ -19,6 +19,7 @@ #include "livekit/data_track_error.h" #include "livekit/data_track_frame.h" #include "livekit/data_track_info.h" +#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/result.h" @@ -55,7 +56,7 @@ class OwnedLocalDataTrack; * dt->unpublishDataTrack(); * } */ -class LocalDataTrack { +class LIVEKIT_API LocalDataTrack { public: ~LocalDataTrack() = default; diff --git a/include/livekit/local_participant.h b/include/livekit/local_participant.h index a7b855d2..a45460eb 100644 --- a/include/livekit/local_participant.h +++ b/include/livekit/local_participant.h @@ -16,6 +16,7 @@ #pragma once +#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/local_audio_track.h" #include "livekit/local_data_track.h" @@ -54,7 +55,7 @@ struct RpcInvocationData { * * LocalParticipant, built on top of the participant.h base class. */ -class LocalParticipant : public Participant { +class LIVEKIT_API LocalParticipant : public Participant { public: using PublicationMap = std::unordered_map>; diff --git a/include/livekit/local_track_publication.h b/include/livekit/local_track_publication.h index f2ccd346..03bfce48 100644 --- a/include/livekit/local_track_publication.h +++ b/include/livekit/local_track_publication.h @@ -16,6 +16,7 @@ #pragma once +#include "livekit/export.h" #include "livekit/track_publication.h" namespace livekit { @@ -24,7 +25,7 @@ namespace proto { class OwnedTrackPublication; } -class LocalTrackPublication : public TrackPublication { +class LIVEKIT_API LocalTrackPublication : public TrackPublication { public: /// Note, this LocalTrackPublication is constructed internally only; /// safe to accept proto::OwnedTrackPublication. diff --git a/include/livekit/local_video_track.h b/include/livekit/local_video_track.h index 93e27837..815201f8 100644 --- a/include/livekit/local_video_track.h +++ b/include/livekit/local_video_track.h @@ -16,6 +16,7 @@ #pragma once +#include "export.h" #include "local_track_publication.h" #include "track.h" @@ -51,7 +52,7 @@ class VideoSource; * The track name provided during creation is visible to remote * participants and can be used for debugging or UI display. */ -class LocalVideoTrack : public Track { +class LIVEKIT_API LocalVideoTrack : public Track { public: /// Creates a new local video track backed by the given `VideoSource`. /// diff --git a/include/livekit/logging.h b/include/livekit/logging.h index 9d2cc564..649ec9fe 100644 --- a/include/livekit/logging.h +++ b/include/livekit/logging.h @@ -16,6 +16,8 @@ #pragma once +#include "livekit/export.h" + #include #include @@ -36,10 +38,10 @@ enum class LogLevel { /// /// Messages below this level are discarded before reaching any sink /// or callback. Thread-safe; may be called at any time after initialize(). -void setLogLevel(LogLevel level); +LIVEKIT_API void setLogLevel(LogLevel level); /// Return the current minimum log level. -LogLevel getLogLevel(); +LIVEKIT_API LogLevel getLogLevel(); /// Signature for a user-supplied log callback. /// @@ -58,6 +60,6 @@ using LogCallback = /// /// Pass nullptr / empty function to restore the default stderr sink. /// Thread-safe; may be called at any time after initialize(). -void setLogCallback(LogCallback callback); +LIVEKIT_API void setLogCallback(LogCallback callback); } // namespace livekit diff --git a/include/livekit/participant.h b/include/livekit/participant.h index 5987963e..3906b341 100644 --- a/include/livekit/participant.h +++ b/include/livekit/participant.h @@ -21,6 +21,7 @@ #include #include +#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/room_delegate.h" @@ -28,7 +29,7 @@ namespace livekit { enum class ParticipantKind { Standard = 0, Ingress, Egress, Sip, Agent }; -class Participant { +class LIVEKIT_API Participant { public: Participant(FfiHandle handle, std::string sid, std::string name, std::string identity, std::string metadata, diff --git a/include/livekit/remote_audio_track.h b/include/livekit/remote_audio_track.h index 572e62c1..70e02fb8 100644 --- a/include/livekit/remote_audio_track.h +++ b/include/livekit/remote_audio_track.h @@ -16,6 +16,7 @@ #pragma once +#include "export.h" #include "track.h" #include #include @@ -40,7 +41,7 @@ class AudioSource; * Applications generally interact with `RemoteAudioTrack` through events and * `RemoteTrackPublication`, not through direct construction. */ -class RemoteAudioTrack : public Track { +class LIVEKIT_API RemoteAudioTrack : public Track { public: /// Constructs a `RemoteAudioTrack` from an internal protocol-level /// `OwnedTrack` description provided by the signaling/FFI layer. diff --git a/include/livekit/remote_data_track.h b/include/livekit/remote_data_track.h index 762140b7..f3fcd5d8 100644 --- a/include/livekit/remote_data_track.h +++ b/include/livekit/remote_data_track.h @@ -19,6 +19,7 @@ #include "livekit/data_track_error.h" #include "livekit/data_track_info.h" #include "livekit/data_track_stream.h" +#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/result.h" @@ -50,7 +51,7 @@ class OwnedRemoteDataTrack; * } * } */ -class RemoteDataTrack { +class LIVEKIT_API RemoteDataTrack { public: ~RemoteDataTrack() = default; diff --git a/include/livekit/remote_participant.h b/include/livekit/remote_participant.h index 367061fe..c41332e3 100644 --- a/include/livekit/remote_participant.h +++ b/include/livekit/remote_participant.h @@ -16,6 +16,7 @@ #pragma once +#include "export.h" #include "participant.h" #include @@ -26,7 +27,7 @@ namespace livekit { class RemoteTrackPublication; -class RemoteParticipant : public Participant { +class LIVEKIT_API RemoteParticipant : public Participant { public: using PublicationMap = std::unordered_map>; @@ -60,7 +61,7 @@ class RemoteParticipant : public Participant { }; // Convenience for logging / streaming -std::ostream &operator<<(std::ostream &os, - const RemoteParticipant &participant); +LIVEKIT_API std::ostream &operator<<(std::ostream &os, + const RemoteParticipant &participant); } // namespace livekit diff --git a/include/livekit/remote_track_publication.h b/include/livekit/remote_track_publication.h index 212eabd3..e08127e8 100644 --- a/include/livekit/remote_track_publication.h +++ b/include/livekit/remote_track_publication.h @@ -16,6 +16,7 @@ #pragma once +#include "livekit/export.h" #include "livekit/track_publication.h" namespace livekit { @@ -24,7 +25,7 @@ namespace proto { class OwnedTrackPublication; } -class RemoteTrackPublication : public TrackPublication { +class LIVEKIT_API RemoteTrackPublication : public TrackPublication { public: /// Note, this RemoteTrackPublication is constructed internally only; /// safe to accept proto::OwnedTrackPublication. diff --git a/include/livekit/remote_video_track.h b/include/livekit/remote_video_track.h index e7a2dada..54173998 100644 --- a/include/livekit/remote_video_track.h +++ b/include/livekit/remote_video_track.h @@ -16,6 +16,7 @@ #pragma once +#include "export.h" #include "track.h" #include #include @@ -40,7 +41,7 @@ class VideoSource; * Applications generally interact with `RemoteVideoTrack` through events and * `RemoteTrackPublication`, not through direct construction. */ -class RemoteVideoTrack : public Track { +class LIVEKIT_API RemoteVideoTrack : public Track { public: /// Constructs a `RemoteVideoTrack` from an internal protocol-level /// `OwnedTrack` description provided by the signaling/FFI layer. diff --git a/include/livekit/room.h b/include/livekit/room.h index e65d7d1f..98aabdcc 100644 --- a/include/livekit/room.h +++ b/include/livekit/room.h @@ -19,6 +19,7 @@ #include "livekit/data_stream.h" #include "livekit/e2ee.h" +#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/room_event_types.h" #include "livekit/subscription_thread_dispatcher.h" @@ -94,7 +95,7 @@ struct RoomOptions { /// - participant list (local + remote) /// - track publications /// - server events forwarded to a RoomDelegate -class Room { +class LIVEKIT_API Room { public: Room(); ~Room(); diff --git a/include/livekit/room_delegate.h b/include/livekit/room_delegate.h index 2621c92c..fe981827 100644 --- a/include/livekit/room_delegate.h +++ b/include/livekit/room_delegate.h @@ -16,6 +16,7 @@ #pragma once +#include "livekit/export.h" #include "livekit/room_event_types.h" namespace livekit { @@ -31,7 +32,7 @@ class Room; * All methods provide default no-op implementations so you can override * only the callbacks you care about. */ -class RoomDelegate { +class LIVEKIT_API RoomDelegate { public: virtual ~RoomDelegate() = default; diff --git a/include/livekit/rpc_error.h b/include/livekit/rpc_error.h index 2efb988c..e643cf91 100644 --- a/include/livekit/rpc_error.h +++ b/include/livekit/rpc_error.h @@ -16,6 +16,8 @@ #pragma once +#include "livekit/export.h" + #include #include #include @@ -37,7 +39,7 @@ class RpcError; * Built-in errors are included (codes 1400–1999) but developers may use * arbitrary codes as well. */ -class RpcError : public std::runtime_error { +class LIVEKIT_API RpcError : public std::runtime_error { public: /** * Built-in error codes diff --git a/include/livekit/stats.h b/include/livekit/stats.h index de56bc05..d6b86875 100644 --- a/include/livekit/stats.h +++ b/include/livekit/stats.h @@ -16,6 +16,8 @@ #pragma once +#include "livekit/export.h" + #include #include #include @@ -505,34 +507,38 @@ struct RtcStats { // fromProto declarations // ---------------------- -RtcStatsData fromProto(const proto::RtcStatsData &); - -CodecStats fromProto(const proto::CodecStats &); -RtpStreamStats fromProto(const proto::RtpStreamStats &); -ReceivedRtpStreamStats fromProto(const proto::ReceivedRtpStreamStats &); -InboundRtpStreamStats fromProto(const proto::InboundRtpStreamStats &); -SentRtpStreamStats fromProto(const proto::SentRtpStreamStats &); -OutboundRtpStreamStats fromProto(const proto::OutboundRtpStreamStats &); -RemoteInboundRtpStreamStats +LIVEKIT_API RtcStatsData fromProto(const proto::RtcStatsData &); + +LIVEKIT_API CodecStats fromProto(const proto::CodecStats &); +LIVEKIT_API RtpStreamStats fromProto(const proto::RtpStreamStats &); +LIVEKIT_API ReceivedRtpStreamStats +fromProto(const proto::ReceivedRtpStreamStats &); +LIVEKIT_API InboundRtpStreamStats +fromProto(const proto::InboundRtpStreamStats &); +LIVEKIT_API SentRtpStreamStats fromProto(const proto::SentRtpStreamStats &); +LIVEKIT_API OutboundRtpStreamStats +fromProto(const proto::OutboundRtpStreamStats &); +LIVEKIT_API RemoteInboundRtpStreamStats fromProto(const proto::RemoteInboundRtpStreamStats &); -RemoteOutboundRtpStreamStats +LIVEKIT_API RemoteOutboundRtpStreamStats fromProto(const proto::RemoteOutboundRtpStreamStats &); -MediaSourceStats fromProto(const proto::MediaSourceStats &); -AudioSourceStats fromProto(const proto::AudioSourceStats &); -VideoSourceStats fromProto(const proto::VideoSourceStats &); -AudioPlayoutStats fromProto(const proto::AudioPlayoutStats &); -PeerConnectionStats fromProto(const proto::PeerConnectionStats &); -DataChannelStats fromProto(const proto::DataChannelStats &); -TransportStats fromProto(const proto::TransportStats &); -CandidatePairStats fromProto(const proto::CandidatePairStats &); -IceCandidateStats fromProto(const proto::IceCandidateStats &); -CertificateStats fromProto(const proto::CertificateStats &); -StreamStats fromProto(const proto::StreamStats &); +LIVEKIT_API MediaSourceStats fromProto(const proto::MediaSourceStats &); +LIVEKIT_API AudioSourceStats fromProto(const proto::AudioSourceStats &); +LIVEKIT_API VideoSourceStats fromProto(const proto::VideoSourceStats &); +LIVEKIT_API AudioPlayoutStats fromProto(const proto::AudioPlayoutStats &); +LIVEKIT_API PeerConnectionStats fromProto(const proto::PeerConnectionStats &); +LIVEKIT_API DataChannelStats fromProto(const proto::DataChannelStats &); +LIVEKIT_API TransportStats fromProto(const proto::TransportStats &); +LIVEKIT_API CandidatePairStats fromProto(const proto::CandidatePairStats &); +LIVEKIT_API IceCandidateStats fromProto(const proto::IceCandidateStats &); +LIVEKIT_API CertificateStats fromProto(const proto::CertificateStats &); +LIVEKIT_API StreamStats fromProto(const proto::StreamStats &); // High-level: -RtcStats fromProto(const proto::RtcStats &); +LIVEKIT_API RtcStats fromProto(const proto::RtcStats &); // helper if you have repeated RtcStats in proto: -std::vector fromProto(const std::vector &); +LIVEKIT_API std::vector +fromProto(const std::vector &); } // namespace livekit diff --git a/include/livekit/subscription_thread_dispatcher.h b/include/livekit/subscription_thread_dispatcher.h index 8e5fa65c..56758456 100644 --- a/include/livekit/subscription_thread_dispatcher.h +++ b/include/livekit/subscription_thread_dispatcher.h @@ -18,6 +18,7 @@ #define LIVEKIT_SUBSCRIPTION_THREAD_DISPATCHER_H #include "livekit/audio_stream.h" +#include "livekit/export.h" #include "livekit/video_stream.h" #include @@ -85,7 +86,7 @@ using DataFrameCallbackId = std::uint64_t; * kinds can be added later without pushing more thread state back into * \ref Room. */ -class SubscriptionThreadDispatcher { +class LIVEKIT_API SubscriptionThreadDispatcher { public: /// Constructs an empty dispatcher with no registered callbacks or readers. SubscriptionThreadDispatcher(); diff --git a/include/livekit/tracing.h b/include/livekit/tracing.h index 0434a4d9..f1d80795 100644 --- a/include/livekit/tracing.h +++ b/include/livekit/tracing.h @@ -16,6 +16,8 @@ #pragma once +#include "livekit/export.h" + #include #include @@ -35,8 +37,8 @@ namespace livekit { * categories. * @return true if tracing was started, false if already running or file error */ -bool startTracing(const std::string &trace_file_path, - const std::vector &categories = {}); +LIVEKIT_API bool startTracing(const std::string &trace_file_path, + const std::vector &categories = {}); /** * Stop tracing and flush remaining events to file. @@ -44,13 +46,13 @@ bool startTracing(const std::string &trace_file_path, * This blocks until all pending events are written and the file is closed. * After stopping, the trace file is complete and ready for analysis. */ -void stopTracing(); +LIVEKIT_API void stopTracing(); /** * Check if tracing is currently active. * * @return true if tracing is running */ -bool isTracingEnabled(); +LIVEKIT_API bool isTracingEnabled(); } // namespace livekit diff --git a/include/livekit/track.h b/include/livekit/track.h index 6d1e98c4..2955a32d 100644 --- a/include/livekit/track.h +++ b/include/livekit/track.h @@ -15,6 +15,7 @@ */ #pragma once +#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/stats.h" #include @@ -68,7 +69,7 @@ struct ParticipantTrackPermission { // ============================================================ // Base Track // ============================================================ -class Track { +class LIVEKIT_API Track { public: virtual ~Track() = default; diff --git a/include/livekit/track_publication.h b/include/livekit/track_publication.h index 5d0ff47f..aeb78ae5 100644 --- a/include/livekit/track_publication.h +++ b/include/livekit/track_publication.h @@ -22,6 +22,7 @@ #include #include "livekit/e2ee.h" +#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/track.h" @@ -37,7 +38,7 @@ class RemoteTrack; * Wraps the immutable publication info plus an FFI handle, and * holds a weak reference to the associated Track (if any). */ -class TrackPublication { +class LIVEKIT_API TrackPublication { public: virtual ~TrackPublication() = default; diff --git a/include/livekit/video_frame.h b/include/livekit/video_frame.h index d9632f30..8bae2408 100644 --- a/include/livekit/video_frame.h +++ b/include/livekit/video_frame.h @@ -16,6 +16,8 @@ #pragma once +#include "livekit/export.h" + #include #include #include @@ -56,7 +58,7 @@ class OwnedVideoBuffer; * - The SDK can expose the backing memory to Rust via data_ptr + layout for * the duration of a blocking FFI call (similar to AudioFrame). */ -class VideoFrame { +class LIVEKIT_API VideoFrame { public: VideoFrame(); VideoFrame(int width, int height, VideoBufferType type, diff --git a/include/livekit/video_source.h b/include/livekit/video_source.h index c015a437..2744643f 100644 --- a/include/livekit/video_source.h +++ b/include/livekit/video_source.h @@ -19,6 +19,7 @@ #include #include +#include "livekit/export.h" #include "livekit/ffi_handle.h" namespace livekit { @@ -62,7 +63,7 @@ struct VideoCaptureOptions { * Represents a real-time video source that can accept frames from the * application and feed them into the LiveKit core. */ -class VideoSource { +class LIVEKIT_API VideoSource { public: /** * Create a new native video source with a fixed resolution. diff --git a/include/livekit/video_stream.h b/include/livekit/video_stream.h index 850b5038..78a1b99e 100644 --- a/include/livekit/video_stream.h +++ b/include/livekit/video_stream.h @@ -24,6 +24,7 @@ #include #include +#include "export.h" #include "ffi_handle.h" #include "participant.h" #include "track.h" @@ -63,7 +64,7 @@ class FfiEvent; // // stream->close(); // optional, called automatically in destructor // -class VideoStream { +class LIVEKIT_API VideoStream { public: struct Options { // Maximum number of VideoFrameEvent items buffered in the internal queue. diff --git a/scripts/check_no_private_symbols.py b/scripts/check_no_private_symbols.py new file mode 100755 index 00000000..2fef9172 --- /dev/null +++ b/scripts/check_no_private_symbols.py @@ -0,0 +1,201 @@ +#!/usr/bin/env python3 +# Copyright 2026 LiveKit +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" +Verify that liblivekit's exported ABI does not leak private dependency symbols. + +The LiveKit SDK statically links several private dependencies (spdlog, fmt, +google::protobuf, absl). When those symbols escape the dynamic symbol table +of liblivekit.{so,dylib,dll}, they collide at runtime with the same libraries +loaded elsewhere in the host process (a common failure mode is ROS 2's +rcl_logging_spdlog ABI-clashing with our vendored spdlog and crashing inside +spdlog::pattern_formatter). + +This script lists exported defined symbols from the supplied shared library +using the platform-appropriate tool and fails (exit code 1) if any of them +match a forbidden pattern. + +Usage: + python3 check_no_private_symbols.py + +Optional environment variables: + LIVEKIT_SYMBOL_CHECK_VERBOSE=1 Print every leaked symbol (default: print + up to 20 examples). + LIVEKIT_SYMBOL_CHECK_EXTRA_FORBIDDEN=foo,bar Additional comma-separated + patterns to forbid. +""" + +import os +import re +import shutil +import subprocess +import sys +from pathlib import Path + +# Substring patterns that must NOT appear in any exported symbol after +# demangling. We use plain-substring semantics for readability; if you need a +# regex, switch to re.search. +DEFAULT_FORBIDDEN = [ + "spdlog::", + "fmt::v", + "google::protobuf", + "absl::", +] + +MAX_REPORTED_LEAKS = 20 + + +def _which_or_die(name: str) -> str: + path = shutil.which(name) + if not path: + sys.stderr.write(f"error: required tool '{name}' not found on PATH\n") + sys.exit(2) + return path + + +def _list_exports_macos(lib: Path) -> list[str]: + nm = _which_or_die("nm") + cxxfilt = shutil.which("c++filt") + # -gU: external (global) defined symbols. + raw = subprocess.run( + [nm, "-gU", str(lib)], + check=True, + capture_output=True, + text=True, + ).stdout + if cxxfilt: + raw = subprocess.run( + [cxxfilt], + input=raw, + check=True, + capture_output=True, + text=True, + ).stdout + return raw.splitlines() + + +def _list_exports_linux(lib: Path) -> list[str]: + nm = _which_or_die("nm") + cxxfilt = shutil.which("c++filt") + # -D: dynamic symbols (i.e., what's actually visible to the dynamic linker) + # --defined-only: drop UND entries + raw = subprocess.run( + [nm, "-D", "--defined-only", str(lib)], + check=True, + capture_output=True, + text=True, + ).stdout + if cxxfilt: + raw = subprocess.run( + [cxxfilt], + input=raw, + check=True, + capture_output=True, + text=True, + ).stdout + return raw.splitlines() + + +def _list_exports_windows(lib: Path) -> list[str]: + # dumpbin ships with MSVC; it understands import libs (.lib) and DLLs. + dumpbin = shutil.which("dumpbin") + if not dumpbin: + sys.stderr.write( + "error: 'dumpbin' not on PATH; run from a Visual Studio " + "Developer command prompt or ensure dumpbin.exe is available\n" + ) + sys.exit(2) + raw = subprocess.run( + [dumpbin, "/exports", str(lib)], + check=True, + capture_output=True, + text=True, + ).stdout + # dumpbin output lines for export entries look like + # " 1 0 00001000 ?foo@@YAHXZ = ?foo@@YAHXZ (int __cdecl foo(void))" + # We keep all of stdout: the substring search will only fire on actual + # symbol names, headers/footers are harmless. + return raw.splitlines() + + +def _list_exports(lib: Path) -> list[str]: + if sys.platform == "darwin": + return _list_exports_macos(lib) + if sys.platform.startswith("linux"): + return _list_exports_linux(lib) + if os.name == "nt" or sys.platform == "win32": + return _list_exports_windows(lib) + sys.stderr.write(f"error: unsupported platform '{sys.platform}'\n") + sys.exit(2) + + +def main(argv: list[str]) -> int: + if len(argv) != 2: + sys.stderr.write(__doc__ or "") + return 2 + + lib = Path(argv[1]).resolve() + if not lib.exists(): + sys.stderr.write(f"error: library not found: {lib}\n") + return 2 + + forbidden = list(DEFAULT_FORBIDDEN) + extra = os.environ.get("LIVEKIT_SYMBOL_CHECK_EXTRA_FORBIDDEN", "") + if extra: + forbidden.extend(p for p in extra.split(",") if p) + + verbose = bool(os.environ.get("LIVEKIT_SYMBOL_CHECK_VERBOSE")) + + print(f"[symbol-check] library : {lib}") + print(f"[symbol-check] platform: {sys.platform}") + print(f"[symbol-check] forbidden patterns: {forbidden}") + + lines = _list_exports(lib) + print(f"[symbol-check] {len(lines)} lines of nm/dumpbin output") + + # Group leaks by pattern for a tidy summary. + leaks_by_pattern: dict[str, list[str]] = {p: [] for p in forbidden} + for line in lines: + for pat in forbidden: + if pat in line: + leaks_by_pattern[pat].append(line.rstrip()) + + total_leaks = sum(len(v) for v in leaks_by_pattern.values()) + if total_leaks == 0: + print("[symbol-check] OK: no forbidden symbols exported") + return 0 + + print(f"[symbol-check] FAIL: {total_leaks} forbidden symbol(s) exported") + for pat, hits in leaks_by_pattern.items(): + if not hits: + continue + print(f" pattern {pat!r}: {len(hits)} hit(s)") + shown = hits if verbose else hits[:MAX_REPORTED_LEAKS] + for h in shown: + print(f" {h}") + if not verbose and len(hits) > MAX_REPORTED_LEAKS: + print(f" ... and {len(hits) - MAX_REPORTED_LEAKS} more " + "(set LIVEKIT_SYMBOL_CHECK_VERBOSE=1 to see all)") + + print( + "\nliblivekit must not re-export private dependency symbols.\n" + "If you intentionally added a public symbol that triggered this, mark\n" + "it with LIVEKIT_API in include/livekit/export.h and rebuild.\n" + ) + return 1 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/src/ffi_client.h b/src/ffi_client.h index 182dc143..ace63732 100644 --- a/src/ffi_client.h +++ b/src/ffi_client.h @@ -30,6 +30,7 @@ #include "data_track.pb.h" #include "livekit/data_track_error.h" +#include "livekit/export.h" #include "livekit/result.h" #include "livekit/stats.h" #include "lk_log.h" @@ -63,8 +64,11 @@ extern "C" void livekit_ffi_dispose(); extern "C" void LivekitFfiCallback(const uint8_t *buf, size_t len); // The FfiClient is used to communicate with the FFI interface of the Rust SDK -// We use the generated protocol messages to facilitate the communication -class FfiClient { +// We use the generated protocol messages to facilitate the communication. +// +// Tagged LIVEKIT_INTERNAL_API: not part of the public ABI; exposed only so the +// in-tree test binaries can call into the singleton. +class LIVEKIT_INTERNAL_API FfiClient { public: using ListenerId = int; using Listener = std::function; diff --git a/src/lk_log.h b/src/lk_log.h index 21a064ba..2197309a 100644 --- a/src/lk_log.h +++ b/src/lk_log.h @@ -17,6 +17,8 @@ #ifndef LIVEKIT_LK_LOG_H #define LIVEKIT_LK_LOG_H +#include "livekit/export.h" + #include #include @@ -25,10 +27,15 @@ namespace livekit::detail { /// Returns the shared "livekit" logger instance. /// The logger is created lazily on first access and lives until /// shutdownLogger() is called. Safe to call before initialize(). -std::shared_ptr getLogger(); +/// +/// Tagged LIVEKIT_INTERNAL_API: this is not part of the public ABI but must +/// remain visible so that in-tree test binaries that include lk_log.h (and +/// therefore expand LK_LOG_* macros referencing this symbol) can resolve it +/// against liblivekit. +LIVEKIT_INTERNAL_API std::shared_ptr getLogger(); /// Tears down the spdlog logger. Called by livekit::shutdown(). -void shutdownLogger(); +LIVEKIT_INTERNAL_API void shutdownLogger(); } // namespace livekit::detail diff --git a/src/logging.cpp b/src/logging.cpp index f8001e7b..0c33a481 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -15,6 +15,7 @@ */ #include "livekit/logging.h" +#include "lk_log.h" #include diff --git a/src/room_proto_converter.h b/src/room_proto_converter.h index 23f8b0e5..05ad9688 100644 --- a/src/room_proto_converter.h +++ b/src/room_proto_converter.h @@ -16,6 +16,7 @@ #pragma once +#include "livekit/export.h" #include "livekit/room_event_types.h" #include "room.pb.h" @@ -28,71 +29,96 @@ class RemoteParticipant; struct ByteStreamInfo; struct TextStreamInfo; -// --------- basic helper conversions --------- - -ConnectionQuality toConnectionQuality(proto::ConnectionQuality in); -ConnectionState toConnectionState(proto::ConnectionState in); -DataPacketKind toDataPacketKind(proto::DataPacketKind in); -DisconnectReason toDisconnectReason(proto::DisconnectReason in); +// All declarations below are tagged LIVEKIT_INTERNAL_API: they are NOT part of +// the public SDK ABI, but must be exported so that the in-tree test binaries +// (which include this header directly) can resolve them against liblivekit. -UserPacketData fromProto(const proto::UserPacket &in); -SipDtmfData fromProto(const proto::SipDTMF &in); -RoomInfoData fromProto(const proto::RoomInfo &in); +// --------- basic helper conversions --------- -DataStreamHeaderData fromProto(const proto::DataStream_Header &in); -DataStreamChunkData fromProto(const proto::DataStream_Chunk &in); -DataStreamTrailerData fromProto(const proto::DataStream_Trailer &in); +LIVEKIT_INTERNAL_API ConnectionQuality +toConnectionQuality(proto::ConnectionQuality in); +LIVEKIT_INTERNAL_API ConnectionState +toConnectionState(proto::ConnectionState in); +LIVEKIT_INTERNAL_API DataPacketKind toDataPacketKind(proto::DataPacketKind in); +LIVEKIT_INTERNAL_API DisconnectReason +toDisconnectReason(proto::DisconnectReason in); + +LIVEKIT_INTERNAL_API UserPacketData fromProto(const proto::UserPacket &in); +LIVEKIT_INTERNAL_API SipDtmfData fromProto(const proto::SipDTMF &in); +LIVEKIT_INTERNAL_API RoomInfoData fromProto(const proto::RoomInfo &in); + +LIVEKIT_INTERNAL_API DataStreamHeaderData +fromProto(const proto::DataStream_Header &in); +LIVEKIT_INTERNAL_API DataStreamChunkData +fromProto(const proto::DataStream_Chunk &in); +LIVEKIT_INTERNAL_API DataStreamTrailerData +fromProto(const proto::DataStream_Trailer &in); // --------- event conversions (RoomEvent.oneof message) --------- -RoomSidChangedEvent fromProto(const proto::RoomSidChanged &in); +LIVEKIT_INTERNAL_API RoomSidChangedEvent +fromProto(const proto::RoomSidChanged &in); -ConnectionStateChangedEvent fromProto(const proto::ConnectionStateChanged &in); -DisconnectedEvent fromProto(const proto::Disconnected &in); -ReconnectingEvent fromProto(const proto::Reconnecting &in); -ReconnectedEvent fromProto(const proto::Reconnected &in); -RoomEosEvent fromProto(const proto::RoomEOS &in); +LIVEKIT_INTERNAL_API ConnectionStateChangedEvent +fromProto(const proto::ConnectionStateChanged &in); +LIVEKIT_INTERNAL_API DisconnectedEvent fromProto(const proto::Disconnected &in); +LIVEKIT_INTERNAL_API ReconnectingEvent fromProto(const proto::Reconnecting &in); +LIVEKIT_INTERNAL_API ReconnectedEvent fromProto(const proto::Reconnected &in); +LIVEKIT_INTERNAL_API RoomEosEvent fromProto(const proto::RoomEOS &in); -DataStreamHeaderReceivedEvent +LIVEKIT_INTERNAL_API DataStreamHeaderReceivedEvent fromProto(const proto::DataStreamHeaderReceived &in); -DataStreamChunkReceivedEvent +LIVEKIT_INTERNAL_API DataStreamChunkReceivedEvent fromProto(const proto::DataStreamChunkReceived &in); -DataStreamTrailerReceivedEvent +LIVEKIT_INTERNAL_API DataStreamTrailerReceivedEvent fromProto(const proto::DataStreamTrailerReceived &in); -DataChannelBufferedAmountLowThresholdChangedEvent +LIVEKIT_INTERNAL_API DataChannelBufferedAmountLowThresholdChangedEvent fromProto(const proto::DataChannelBufferedAmountLowThresholdChanged &in); -ByteStreamOpenedEvent fromProto(const proto::ByteStreamOpened &in); -TextStreamOpenedEvent fromProto(const proto::TextStreamOpened &in); +LIVEKIT_INTERNAL_API ByteStreamOpenedEvent +fromProto(const proto::ByteStreamOpened &in); +LIVEKIT_INTERNAL_API TextStreamOpenedEvent +fromProto(const proto::TextStreamOpened &in); -RoomUpdatedEvent -roomUpdatedFromProto(const proto::RoomInfo &in); // room_updated -RoomMovedEvent roomMovedFromProto(const proto::RoomInfo &in); // moved +LIVEKIT_INTERNAL_API RoomUpdatedEvent +roomUpdatedFromProto(const proto::RoomInfo &in); // room_updated +LIVEKIT_INTERNAL_API RoomMovedEvent +roomMovedFromProto(const proto::RoomInfo &in); // moved // --------- room options conversions --------- -proto::AudioEncoding toProto(const AudioEncodingOptions &in); -AudioEncodingOptions fromProto(const proto::AudioEncoding &in); +LIVEKIT_INTERNAL_API proto::AudioEncoding +toProto(const AudioEncodingOptions &in); +LIVEKIT_INTERNAL_API AudioEncodingOptions +fromProto(const proto::AudioEncoding &in); -proto::VideoEncoding toProto(const VideoEncodingOptions &in); -VideoEncodingOptions fromProto(const proto::VideoEncoding &in); +LIVEKIT_INTERNAL_API proto::VideoEncoding +toProto(const VideoEncodingOptions &in); +LIVEKIT_INTERNAL_API VideoEncodingOptions +fromProto(const proto::VideoEncoding &in); -proto::TrackPublishOptions toProto(const TrackPublishOptions &in); -TrackPublishOptions fromProto(const proto::TrackPublishOptions &in); +LIVEKIT_INTERNAL_API proto::TrackPublishOptions +toProto(const TrackPublishOptions &in); +LIVEKIT_INTERNAL_API TrackPublishOptions +fromProto(const proto::TrackPublishOptions &in); // --------- room Data Packet conversions --------- -UserDataPacketEvent userDataPacketFromProto(const proto::DataPacketReceived &in, - RemoteParticipant *participant); +LIVEKIT_INTERNAL_API UserDataPacketEvent +userDataPacketFromProto(const proto::DataPacketReceived &in, + RemoteParticipant *participant); -SipDtmfReceivedEvent sipDtmfFromProto(const proto::DataPacketReceived &in, - RemoteParticipant *participant); +LIVEKIT_INTERNAL_API SipDtmfReceivedEvent +sipDtmfFromProto(const proto::DataPacketReceived &in, + RemoteParticipant *participant); // --------- room Data Stream conversions --------- -std::map +LIVEKIT_INTERNAL_API std::map toAttrMap(const proto::DataStream::Trailer &trailer); -ByteStreamInfo makeByteInfo(const proto::DataStream::Header &header); -TextStreamInfo makeTextInfo(const proto::DataStream::Header &header); +LIVEKIT_INTERNAL_API ByteStreamInfo +makeByteInfo(const proto::DataStream::Header &header); +LIVEKIT_INTERNAL_API TextStreamInfo +makeTextInfo(const proto::DataStream::Header &header); } // namespace livekit diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 34c73dba..563fd92a 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -51,10 +51,11 @@ if(UNIT_TEST_SOURCES) ${BENCHMARK_UTILS_SOURCES} ) - # On Windows, protobuf default-instance symbols (constinit globals) are not - # auto-exported from livekit.dll by WINDOWS_EXPORT_ALL_SYMBOLS. Link the - # proto object library directly so the test binary has its own copy. - if(WIN32 AND TARGET livekit_proto) + # liblivekit no longer re-exports protobuf/absl symbols (hidden visibility + + # --exclude-libs,ALL). Tests that touch livekit::proto::* types or absl + # helpers must link the proto object library and the libprotobuf static + # archive directly. + if(TARGET livekit_proto) target_sources(livekit_unit_tests PRIVATE $) endif() @@ -62,7 +63,7 @@ if(UNIT_TEST_SOURCES) PRIVATE livekit spdlog::spdlog - $<$:${LIVEKIT_PROTOBUF_TARGET}> + ${LIVEKIT_PROTOBUF_TARGET} GTest::gtest_main ) @@ -149,10 +150,11 @@ if(INTEGRATION_TEST_SOURCES) ${BENCHMARK_UTILS_SOURCES} ) - # On Windows, protobuf default-instance symbols (constinit globals) are not - # auto-exported from livekit.dll by WINDOWS_EXPORT_ALL_SYMBOLS. Link the - # proto object library directly so the test binary has its own copy. - if(WIN32 AND TARGET livekit_proto) + # liblivekit no longer re-exports protobuf/absl symbols (hidden visibility + + # --exclude-libs,ALL). Tests that touch livekit::proto::* types or absl + # helpers must link the proto object library and the libprotobuf static + # archive directly. + if(TARGET livekit_proto) target_sources(livekit_integration_tests PRIVATE $) endif() @@ -160,7 +162,7 @@ if(INTEGRATION_TEST_SOURCES) PRIVATE livekit spdlog::spdlog - $<$:${LIVEKIT_PROTOBUF_TARGET}> + ${LIVEKIT_PROTOBUF_TARGET} GTest::gtest_main ) @@ -329,3 +331,27 @@ endif() if(TARGET livekit_stress_tests) add_dependencies(run_all_tests livekit_stress_tests) endif() + +# ============================================================================ +# ABI verification: liblivekit must not re-export private dependency symbols +# ============================================================================ +# +# scripts/check_no_private_symbols.py asserts that no spdlog/fmt/protobuf/absl +# symbols are visible in the dynamic symbol table of the built shared library. +# This catches regressions where a new TU is added without -fvisibility=hidden +# or a public header forgets to gate exports through LIVEKIT_API. +find_package(Python3 COMPONENTS Interpreter) +if(Python3_Interpreter_FOUND) + add_test( + NAME livekit_no_private_symbols + COMMAND ${Python3_EXECUTABLE} + "${LIVEKIT_ROOT_DIR}/scripts/check_no_private_symbols.py" + "$" + ) + set_tests_properties(livekit_no_private_symbols PROPERTIES + LABELS "abi" + ) +else() + message(WARNING + "Python3 not found; skipping livekit_no_private_symbols ABI test") +endif() diff --git a/src/trace/event_tracer.h b/src/trace/event_tracer.h index 181abf22..f92ee705 100644 --- a/src/trace/event_tracer.h +++ b/src/trace/event_tracer.h @@ -39,16 +39,7 @@ #ifndef LIVEKIT_TRACE_EVENT_TRACER_H_ #define LIVEKIT_TRACE_EVENT_TRACER_H_ -// Platform-specific DLL export macro -#if defined(_WIN32) -#if defined(LIVEKIT_BUILDING_SDK) -#define LIVEKIT_EXPORT __declspec(dllexport) -#else -#define LIVEKIT_EXPORT __declspec(dllimport) -#endif -#else -#define LIVEKIT_EXPORT __attribute__((visibility("default"))) -#endif +#include "livekit/export.h" namespace livekit { namespace trace { @@ -66,13 +57,14 @@ typedef void (*AddTraceEventPtr)(char phase, // // This method must be called before any tracing begins. Functions // provided should be thread-safe. -LIVEKIT_EXPORT void +LIVEKIT_API void SetupEventTracer(GetCategoryEnabledPtr get_category_enabled_ptr, AddTraceEventPtr add_trace_event_ptr); // This class defines interface for the event tracing system to call -// internally. Do not call these methods directly. -class EventTracer { +// internally. Tagged LIVEKIT_INTERNAL_API so the in-tree tracing tests +// (src/tests/unit/test_tracing.cpp) can resolve the static methods. +class LIVEKIT_INTERNAL_API EventTracer { public: static const unsigned char *GetCategoryEnabled(const char *name); diff --git a/src/video_utils.h b/src/video_utils.h index fe273d66..0fdcf6e5 100644 --- a/src/video_utils.h +++ b/src/video_utils.h @@ -16,22 +16,26 @@ #pragma once +#include "livekit/export.h" #include "livekit/video_frame.h" #include "livekit/video_source.h" #include "video_frame.pb.h" namespace livekit { -// Video FFI Utils -proto::VideoBufferInfo toProto(const VideoFrame &frame); -VideoFrame fromOwnedProto(const proto::OwnedVideoBuffer &owned); -VideoFrame convertViaFfi(const VideoFrame &frame, VideoBufferType dst, - bool flip_y); -proto::VideoBufferType toProto(const VideoBufferType t); -VideoBufferType fromProto(const proto::VideoBufferType t); -std::optional +// Video FFI Utils. +// Tagged LIVEKIT_INTERNAL_API: not part of the public ABI, but exposed so +// in-tree tests that include this header can link. +LIVEKIT_INTERNAL_API proto::VideoBufferInfo toProto(const VideoFrame &frame); +LIVEKIT_INTERNAL_API VideoFrame +fromOwnedProto(const proto::OwnedVideoBuffer &owned); +LIVEKIT_INTERNAL_API VideoFrame convertViaFfi(const VideoFrame &frame, + VideoBufferType dst, bool flip_y); +LIVEKIT_INTERNAL_API proto::VideoBufferType toProto(const VideoBufferType t); +LIVEKIT_INTERNAL_API VideoBufferType fromProto(const proto::VideoBufferType t); +LIVEKIT_INTERNAL_API std::optional toProto(const std::optional &metadata); -std::optional +LIVEKIT_INTERNAL_API std::optional fromProto(const proto::FrameMetadata &metadata); } // namespace livekit From a0362ab85c46a6527862924bf10fb869f7a8ea6f Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 8 May 2026 10:05:33 -0600 Subject: [PATCH 03/24] Bundle in spdlog --- .github/workflows/builds.yml | 5 ++-- .github/workflows/tests.yml | 3 +-- AGENTS.md | 47 ++++++++++++++++++------------------ cmake/spdlog.cmake | 21 +++++----------- docker/Dockerfile.base | 1 - 5 files changed, 33 insertions(+), 44 deletions(-) diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 6335af09..51ad7944 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -133,15 +133,14 @@ jobs: libssl-dev \ libprotobuf-dev protobuf-compiler \ libabsl-dev \ - libwayland-dev libdecor-0-dev \ - libspdlog-dev + libwayland-dev libdecor-0-dev - name: Install deps (macOS) if: runner.os == 'macOS' run: | set -eux brew update - brew install cmake ninja protobuf abseil spdlog + brew install cmake ninja protobuf abseil # ---------- Rust toolchain ---------- - name: Install Rust (stable) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5033e751..95f2b753 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -115,7 +115,6 @@ jobs: libprotobuf-dev protobuf-compiler \ libabsl-dev \ libwayland-dev libdecor-0-dev \ - libspdlog-dev \ jq - name: Install deps (macOS) @@ -123,7 +122,7 @@ jobs: run: | set -eux brew update - brew install cmake ninja protobuf abseil spdlog jq + brew install cmake ninja protobuf abseil jq # ---------- Rust toolchain ---------- - name: Install Rust (stable) diff --git a/AGENTS.md b/AGENTS.md index a4f4454d..e8045aa1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,7 +110,7 @@ Updates to ./build.sh and ./build.cmd should be accompanied by updates to this f The build scripts pass an explicit job count to `cmake --build --parallel`. Set `CMAKE_BUILD_PARALLEL_LEVEL` to override the default detected logical CPU count. -**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. +**Requirements:** CMake 3.20+, C++17, Rust toolchain (cargo), protoc. On macOS: `brew install cmake ninja protobuf abseil`. On Linux: see the CI workflow for apt packages. spdlog is vendored automatically via FetchContent (or vcpkg on Windows) to avoid system conflicts, for example on ROS 2. ### SDK Packaging @@ -140,32 +140,11 @@ All source files must have the LiveKit Apache 2.0 copyright header. Use the curr ### Public API Surface - Keep public APIs small. Minimize what goes into `include/livekit/`. +- All publicly visible symbols must use `LIVEKIT_API` macro (via `include/livekit/export.h`). - 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 - -- Use `LK_LOG_WARN` for non-fatal unexpected conditions. -- Use `Result` for operations that can fail with typed errors (e.g., data track publish/subscribe). - -### Integer Types - -- Prefer fixed-width integer types from `` (`std::int32_t`, `std::uint64_t`, etc.) over raw primitive integer types when size or signedness matters. -- This applies in public APIs, FFI/protobuf-facing code, serialized payloads, handles, timestamps, IDs, and any cross-platform boundary where integer width must be explicit. -- Use raw primitive integer types only when the value is intentionally platform-sized or when preserving an existing public API is necessary for backwards compatibility. -- Do not change an existing public API from a raw primitive integer type to a fixed-width type for style consistency alone unless the compatibility impact has been reviewed. - -### 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`. - ### Symbol Visibility / Exported ABI The `livekit` shared library is built with hidden symbol visibility on all @@ -190,6 +169,28 @@ Rules for new code: CI enforces the policy via `scripts/check_no_private_symbols.py`, which is also wired in as a CTest test (label `abi`): +### Error Handling + +- Use `LK_LOG_WARN` for non-fatal unexpected conditions. +- Use `Result` for operations that can fail with typed errors (e.g., data track publish/subscribe). + +### Integer Types + +- Prefer fixed-width integer types from `` (`std::int32_t`, `std::uint64_t`, etc.) over raw primitive integer types when size or signedness matters. +- This applies in public APIs, FFI/protobuf-facing code, serialized payloads, handles, timestamps, IDs, and any cross-platform boundary where integer width must be explicit. +- Use raw primitive integer types only when the value is intentionally platform-sized or when preserving an existing public API is necessary for backwards compatibility. +- Do not change an existing public API from a raw primitive integer type to a fixed-width type for style consistency alone unless the compatibility impact has been reviewed. + +### 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`. + ``` ctest -L abi --output-on-failure ``` diff --git a/cmake/spdlog.cmake b/cmake/spdlog.cmake index 2c56a916..0ef0e191 100644 --- a/cmake/spdlog.cmake +++ b/cmake/spdlog.cmake @@ -1,9 +1,7 @@ # cmake/spdlog.cmake # # Windows: use vcpkg spdlog -# macOS/Linux: vendored spdlog via FetchContent (static library), unless -# LIVEKIT_USE_SYSTEM_SPDLOG=ON, in which case the system-provided -# spdlog (find_package) is used. +# macOS/Linux: vendored spdlog via FetchContent (static library) # # Exposes: # - Target spdlog::spdlog @@ -52,9 +50,11 @@ include(FetchContent) set(LIVEKIT_SPDLOG_VERSION "1.15.1" CACHE STRING "Vendored spdlog version") -option(LIVEKIT_USE_SYSTEM_SPDLOG - "Use system spdlog (find_package) instead of vendored" - OFF) +# Warn any existing users of old option +if(DEFINED LIVEKIT_USE_SYSTEM_SPDLOG) + message(WARNING + "LIVEKIT_USE_SYSTEM_SPDLOG is no longer supported and will be ignored") +endif() # --------------------------------------------------------------------------- # Windows: use vcpkg @@ -65,15 +65,6 @@ if(WIN32 AND LIVEKIT_USE_VCPKG) return() endif() -# --------------------------------------------------------------------------- -# macOS/Linux: system spdlog (opt-in via LIVEKIT_USE_SYSTEM_SPDLOG=ON) -# --------------------------------------------------------------------------- -if(LIVEKIT_USE_SYSTEM_SPDLOG) - find_package(spdlog CONFIG REQUIRED) - message(STATUS "Using system spdlog (LIVEKIT_USE_SYSTEM_SPDLOG=ON): version=${spdlog_VERSION}") - return() -endif() - # --------------------------------------------------------------------------- # macOS/Linux: vendored spdlog via FetchContent # --------------------------------------------------------------------------- diff --git a/docker/Dockerfile.base b/docker/Dockerfile.base index 12b83721..54559254 100644 --- a/docker/Dockerfile.base +++ b/docker/Dockerfile.base @@ -34,7 +34,6 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ libglib2.0-dev \ libprotobuf-dev \ libdecor-0-dev \ - libspdlog-dev \ libssl-dev \ libunwind-dev \ libusb-1.0-0-dev \ From 85646a8e7f05d67d1314206533fa3e0e9ad23ff2 Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 8 May 2026 10:06:07 -0600 Subject: [PATCH 04/24] Remove spdlog readme --- README.md | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/README.md b/README.md index 4bbaa607..8165d61a 100644 --- a/README.md +++ b/README.md @@ -300,24 +300,6 @@ livekit::setLogLevel(livekit::LogLevel::Debug); // show more detail livekit::setLogLevel(livekit::LogLevel::Warn); // suppress info chatter ``` -### Embedding the SDK in processes that already use spdlog (`LIVEKIT_USE_SYSTEM_SPDLOG`) - -By default on Linux/macOS the SDK vendors spdlog and statically links it into -`liblivekit`. If you embed the SDK in a process that **already loads spdlog** -itself. Two independent spdlog/fmt copies in the same process can crash at runtime. -Build the SDK with the system spdlog instead so both components share one implementation: - -```bash -cmake -B build-release -S . \ - -DCMAKE_BUILD_TYPE=Release \ - -DLIVEKIT_USE_SYSTEM_SPDLOG=ON -cmake --build build-release -``` - -Pass this only to the SDK build, not to the consumer's build. On Windows the -vcpkg path already supplies a single shared spdlog, so the option is a no-op -there. - ### Custom log callback Replace the default stderr sink with your own handler. This is the integration From 67c220e1afddda6e5ad99ea27b51674bfaef0c53 Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 8 May 2026 10:28:19 -0600 Subject: [PATCH 05/24] Rebase on main/clang format style --- include/livekit/audio_frame.h | 4 +- include/livekit/data_stream.h | 4 +- include/livekit/data_track_error.h | 8 +- include/livekit/data_track_frame.h | 4 +- include/livekit/data_track_stream.h | 3 +- include/livekit/e2ee.h | 4 +- include/livekit/ffi_handle.h | 4 +- include/livekit/livekit.h | 5 +- include/livekit/local_data_track.h | 3 +- include/livekit/local_participant.h | 3 +- include/livekit/local_video_track.h | 3 +- include/livekit/logging.h | 4 +- include/livekit/remote_audio_track.h | 3 +- include/livekit/remote_participant.h | 5 +- include/livekit/remote_video_track.h | 3 +- include/livekit/rpc_error.h | 4 +- include/livekit/stats.h | 48 ++++---- .../livekit/subscription_thread_dispatcher.h | 3 +- include/livekit/tracing.h | 8 +- include/livekit/track.h | 3 +- include/livekit/video_frame.h | 4 +- scripts/check_no_private_symbols.py | 52 ++++++++- src/lk_log.h | 11 +- src/logging.cpp | 3 +- src/room_proto_converter.h | 77 +++++++------ .../consumer_link/test_consumer_link.cpp | 109 ++++++++++++++++++ src/trace/event_tracer.h | 20 +--- src/video_utils.h | 19 +-- 28 files changed, 302 insertions(+), 119 deletions(-) create mode 100644 src/tests/consumer_link/test_consumer_link.cpp diff --git a/include/livekit/audio_frame.h b/include/livekit/audio_frame.h index a06ef65f..f66e9512 100644 --- a/include/livekit/audio_frame.h +++ b/include/livekit/audio_frame.h @@ -16,12 +16,12 @@ #pragma once -#include "livekit/export.h" - #include #include #include +#include "livekit/export.h" + namespace livekit { namespace proto { diff --git a/include/livekit/data_stream.h b/include/livekit/data_stream.h index 21d1aef1..f4bb8239 100644 --- a/include/livekit/data_stream.h +++ b/include/livekit/data_stream.h @@ -16,8 +16,6 @@ #pragma once -#include "livekit/export.h" - #include #include #include @@ -29,6 +27,8 @@ #include #include +#include "livekit/export.h" + namespace livekit { class LocalParticipant; diff --git a/include/livekit/data_track_error.h b/include/livekit/data_track_error.h index e8519a64..e25ee137 100644 --- a/include/livekit/data_track_error.h +++ b/include/livekit/data_track_error.h @@ -20,6 +20,8 @@ #include #include +#include "livekit/export.h" + namespace livekit { namespace proto { @@ -45,7 +47,7 @@ struct PublishDataTrackError { PublishDataTrackErrorCode code{PublishDataTrackErrorCode::UNKNOWN}; std::string message; - static PublishDataTrackError fromProto(const proto::PublishDataTrackError& error); + LIVEKIT_API static PublishDataTrackError fromProto(const proto::PublishDataTrackError& error); }; enum class LocalDataTrackTryPushErrorCode : std::uint32_t { @@ -60,7 +62,7 @@ struct LocalDataTrackTryPushError { LocalDataTrackTryPushErrorCode code{LocalDataTrackTryPushErrorCode::UNKNOWN}; std::string message; - static LocalDataTrackTryPushError fromProto(const proto::LocalDataTrackTryPushError& error); + LIVEKIT_API static LocalDataTrackTryPushError fromProto(const proto::LocalDataTrackTryPushError& error); }; enum class SubscribeDataTrackErrorCode : std::uint32_t { @@ -77,7 +79,7 @@ struct SubscribeDataTrackError { SubscribeDataTrackErrorCode code{SubscribeDataTrackErrorCode::UNKNOWN}; std::string message; - static SubscribeDataTrackError fromProto(const proto::SubscribeDataTrackError& error); + LIVEKIT_API static SubscribeDataTrackError fromProto(const proto::SubscribeDataTrackError& error); }; } // namespace livekit diff --git a/include/livekit/data_track_frame.h b/include/livekit/data_track_frame.h index fa37c490..ed2678d5 100644 --- a/include/livekit/data_track_frame.h +++ b/include/livekit/data_track_frame.h @@ -20,6 +20,8 @@ #include #include +#include "livekit/export.h" + namespace livekit { namespace proto { @@ -60,7 +62,7 @@ struct DataTrackFrame { * @param owned The proto::DataTrackFrame to create a DataTrackFrame from. * @return The created DataTrackFrame. */ - static DataTrackFrame fromOwnedInfo(const proto::DataTrackFrame& owned); + LIVEKIT_API static DataTrackFrame fromOwnedInfo(const proto::DataTrackFrame& owned); }; } // namespace livekit diff --git a/include/livekit/data_track_stream.h b/include/livekit/data_track_stream.h index 5a3345f0..12e08dc5 100644 --- a/include/livekit/data_track_stream.h +++ b/include/livekit/data_track_stream.h @@ -24,6 +24,7 @@ #include #include "livekit/data_track_frame.h" +#include "livekit/export.h" #include "livekit/ffi_handle.h" namespace livekit { @@ -52,7 +53,7 @@ class FfiEvent; * } * } */ -class DataTrackStream { +class LIVEKIT_API DataTrackStream { public: struct Options { /// Maximum frames buffered on the Rust side. Rust defaults to 16. diff --git a/include/livekit/e2ee.h b/include/livekit/e2ee.h index 78bcb97b..61400b38 100644 --- a/include/livekit/e2ee.h +++ b/include/livekit/e2ee.h @@ -16,14 +16,14 @@ #pragma once -#include "livekit/export.h" - #include #include #include #include #include +#include "livekit/export.h" + namespace livekit { /* Encryption algorithm type used by the underlying stack. diff --git a/include/livekit/ffi_handle.h b/include/livekit/ffi_handle.h index d0c59f79..2457cea5 100644 --- a/include/livekit/ffi_handle.h +++ b/include/livekit/ffi_handle.h @@ -16,10 +16,10 @@ #pragma once -#include "livekit/export.h" - #include +#include "livekit/export.h" + namespace livekit { /** diff --git a/include/livekit/livekit.h b/include/livekit/livekit.h index fb668611..f7200e49 100644 --- a/include/livekit/livekit.h +++ b/include/livekit/livekit.h @@ -22,6 +22,7 @@ #include "audio_stream.h" #include "build.h" #include "e2ee.h" +#include "export.h" #include "local_audio_track.h" #include "local_participant.h" #include "local_track_publication.h" @@ -59,11 +60,11 @@ enum class LogSink { /// @param log_sink The log sink to use for SDK messages (default: Console). /// @returns true if initialization happened on this call, false if it was /// already initialized. -bool initialize(const LogLevel& level = LogLevel::Info, const LogSink& log_sink = LogSink::kConsole); +LIVEKIT_API bool initialize(const LogLevel& level = LogLevel::Info, const LogSink& log_sink = LogSink::kConsole); /// Shut down the LiveKit SDK. /// /// After shutdown, you may call initialize() again. -void shutdown(); +LIVEKIT_API void shutdown(); } // namespace livekit \ No newline at end of file diff --git a/include/livekit/local_data_track.h b/include/livekit/local_data_track.h index 1e4365d7..8512d31f 100644 --- a/include/livekit/local_data_track.h +++ b/include/livekit/local_data_track.h @@ -25,6 +25,7 @@ #include "livekit/data_track_error.h" #include "livekit/data_track_frame.h" #include "livekit/data_track_info.h" +#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/result.h" @@ -55,7 +56,7 @@ class OwnedLocalDataTrack; * dt->unpublishDataTrack(); * } */ -class LocalDataTrack { +class LIVEKIT_API LocalDataTrack { public: ~LocalDataTrack() = default; diff --git a/include/livekit/local_participant.h b/include/livekit/local_participant.h index 52253d7e..ee682434 100644 --- a/include/livekit/local_participant.h +++ b/include/livekit/local_participant.h @@ -26,6 +26,7 @@ #include #include +#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/local_audio_track.h" #include "livekit/local_data_track.h" @@ -54,7 +55,7 @@ struct RpcInvocationData { * * LocalParticipant, built on top of the participant.h base class. */ -class LocalParticipant : public Participant { +class LIVEKIT_API LocalParticipant : public Participant { public: using PublicationMap = std::unordered_map>; using TrackMap = std::unordered_map>; diff --git a/include/livekit/local_video_track.h b/include/livekit/local_video_track.h index 7fc316b8..3ec97097 100644 --- a/include/livekit/local_video_track.h +++ b/include/livekit/local_video_track.h @@ -19,6 +19,7 @@ #include #include +#include "export.h" #include "local_track_publication.h" #include "track.h" @@ -51,7 +52,7 @@ class VideoSource; * The track name provided during creation is visible to remote * participants and can be used for debugging or UI display. */ -class LocalVideoTrack : public Track { +class LIVEKIT_API LocalVideoTrack : public Track { public: /// Creates a new local video track backed by the given `VideoSource`. /// diff --git a/include/livekit/logging.h b/include/livekit/logging.h index 3a623480..739ec13c 100644 --- a/include/livekit/logging.h +++ b/include/livekit/logging.h @@ -16,11 +16,11 @@ #pragma once -#include "livekit/export.h" - #include #include +#include "livekit/export.h" + namespace livekit { /// Severity levels for SDK log messages. diff --git a/include/livekit/remote_audio_track.h b/include/livekit/remote_audio_track.h index cfe8761c..e50e07cd 100644 --- a/include/livekit/remote_audio_track.h +++ b/include/livekit/remote_audio_track.h @@ -19,6 +19,7 @@ #include #include +#include "export.h" #include "track.h" namespace livekit { @@ -41,7 +42,7 @@ class AudioSource; * Applications generally interact with `RemoteAudioTrack` through events and * `RemoteTrackPublication`, not through direct construction. */ -class RemoteAudioTrack : public Track { +class LIVEKIT_API RemoteAudioTrack : public Track { public: /// Constructs a `RemoteAudioTrack` from an internal protocol-level /// `OwnedTrack` description provided by the signaling/FFI layer. diff --git a/include/livekit/remote_participant.h b/include/livekit/remote_participant.h index 938086a0..4eab33d0 100644 --- a/include/livekit/remote_participant.h +++ b/include/livekit/remote_participant.h @@ -20,13 +20,14 @@ #include #include +#include "export.h" #include "participant.h" namespace livekit { class RemoteTrackPublication; -class RemoteParticipant : public Participant { +class LIVEKIT_API RemoteParticipant : public Participant { public: using PublicationMap = std::unordered_map>; @@ -53,6 +54,6 @@ class RemoteParticipant : public Participant { }; // Convenience for logging / streaming -std::ostream& operator<<(std::ostream& os, const RemoteParticipant& participant); +LIVEKIT_API std::ostream& operator<<(std::ostream& os, const RemoteParticipant& participant); } // namespace livekit diff --git a/include/livekit/remote_video_track.h b/include/livekit/remote_video_track.h index 891d8fbd..987344e2 100644 --- a/include/livekit/remote_video_track.h +++ b/include/livekit/remote_video_track.h @@ -19,6 +19,7 @@ #include #include +#include "export.h" #include "track.h" namespace livekit { @@ -41,7 +42,7 @@ class VideoSource; * Applications generally interact with `RemoteVideoTrack` through events and * `RemoteTrackPublication`, not through direct construction. */ -class RemoteVideoTrack : public Track { +class LIVEKIT_API RemoteVideoTrack : public Track { public: /// Constructs a `RemoteVideoTrack` from an internal protocol-level /// `OwnedTrack` description provided by the signaling/FFI layer. diff --git a/include/livekit/rpc_error.h b/include/livekit/rpc_error.h index fb8d23d4..8d9f5fe9 100644 --- a/include/livekit/rpc_error.h +++ b/include/livekit/rpc_error.h @@ -16,12 +16,12 @@ #pragma once -#include "livekit/export.h" - #include #include #include +#include "livekit/export.h" + namespace livekit { namespace proto { diff --git a/include/livekit/stats.h b/include/livekit/stats.h index 70a526f8..7e8d5bf9 100644 --- a/include/livekit/stats.h +++ b/include/livekit/stats.h @@ -23,6 +23,8 @@ #include #include +#include "livekit/export.h" + namespace livekit { namespace proto { @@ -503,32 +505,32 @@ struct RtcStats { // fromProto declarations // ---------------------- -RtcStatsData fromProto(const proto::RtcStatsData&); - -CodecStats fromProto(const proto::CodecStats&); -RtpStreamStats fromProto(const proto::RtpStreamStats&); -ReceivedRtpStreamStats fromProto(const proto::ReceivedRtpStreamStats&); -InboundRtpStreamStats fromProto(const proto::InboundRtpStreamStats&); -SentRtpStreamStats fromProto(const proto::SentRtpStreamStats&); -OutboundRtpStreamStats fromProto(const proto::OutboundRtpStreamStats&); -RemoteInboundRtpStreamStats fromProto(const proto::RemoteInboundRtpStreamStats&); -RemoteOutboundRtpStreamStats fromProto(const proto::RemoteOutboundRtpStreamStats&); -MediaSourceStats fromProto(const proto::MediaSourceStats&); -AudioSourceStats fromProto(const proto::AudioSourceStats&); -VideoSourceStats fromProto(const proto::VideoSourceStats&); -AudioPlayoutStats fromProto(const proto::AudioPlayoutStats&); -PeerConnectionStats fromProto(const proto::PeerConnectionStats&); -DataChannelStats fromProto(const proto::DataChannelStats&); -TransportStats fromProto(const proto::TransportStats&); -CandidatePairStats fromProto(const proto::CandidatePairStats&); -IceCandidateStats fromProto(const proto::IceCandidateStats&); -CertificateStats fromProto(const proto::CertificateStats&); -StreamStats fromProto(const proto::StreamStats&); +LIVEKIT_API RtcStatsData fromProto(const proto::RtcStatsData&); + +LIVEKIT_API CodecStats fromProto(const proto::CodecStats&); +LIVEKIT_API RtpStreamStats fromProto(const proto::RtpStreamStats&); +LIVEKIT_API ReceivedRtpStreamStats fromProto(const proto::ReceivedRtpStreamStats&); +LIVEKIT_API InboundRtpStreamStats fromProto(const proto::InboundRtpStreamStats&); +LIVEKIT_API SentRtpStreamStats fromProto(const proto::SentRtpStreamStats&); +LIVEKIT_API OutboundRtpStreamStats fromProto(const proto::OutboundRtpStreamStats&); +LIVEKIT_API RemoteInboundRtpStreamStats fromProto(const proto::RemoteInboundRtpStreamStats&); +LIVEKIT_API RemoteOutboundRtpStreamStats fromProto(const proto::RemoteOutboundRtpStreamStats&); +LIVEKIT_API MediaSourceStats fromProto(const proto::MediaSourceStats&); +LIVEKIT_API AudioSourceStats fromProto(const proto::AudioSourceStats&); +LIVEKIT_API VideoSourceStats fromProto(const proto::VideoSourceStats&); +LIVEKIT_API AudioPlayoutStats fromProto(const proto::AudioPlayoutStats&); +LIVEKIT_API PeerConnectionStats fromProto(const proto::PeerConnectionStats&); +LIVEKIT_API DataChannelStats fromProto(const proto::DataChannelStats&); +LIVEKIT_API TransportStats fromProto(const proto::TransportStats&); +LIVEKIT_API CandidatePairStats fromProto(const proto::CandidatePairStats&); +LIVEKIT_API IceCandidateStats fromProto(const proto::IceCandidateStats&); +LIVEKIT_API CertificateStats fromProto(const proto::CertificateStats&); +LIVEKIT_API StreamStats fromProto(const proto::StreamStats&); // High-level: -RtcStats fromProto(const proto::RtcStats&); +LIVEKIT_API RtcStats fromProto(const proto::RtcStats&); // helper if you have repeated RtcStats in proto: -std::vector fromProto(const std::vector&); +LIVEKIT_API std::vector fromProto(const std::vector&); } // namespace livekit diff --git a/include/livekit/subscription_thread_dispatcher.h b/include/livekit/subscription_thread_dispatcher.h index dfe53d32..569daa46 100644 --- a/include/livekit/subscription_thread_dispatcher.h +++ b/include/livekit/subscription_thread_dispatcher.h @@ -28,6 +28,7 @@ #include #include "livekit/audio_stream.h" +#include "livekit/export.h" #include "livekit/video_stream.h" namespace livekit { @@ -83,7 +84,7 @@ using DataFrameCallbackId = std::uint64_t; * kinds can be added later without pushing more thread state back into * \ref Room. */ -class SubscriptionThreadDispatcher { +class LIVEKIT_API SubscriptionThreadDispatcher { public: /// Constructs an empty dispatcher with no registered callbacks or readers. SubscriptionThreadDispatcher(); diff --git a/include/livekit/tracing.h b/include/livekit/tracing.h index 1477bac6..369be124 100644 --- a/include/livekit/tracing.h +++ b/include/livekit/tracing.h @@ -19,6 +19,8 @@ #include #include +#include "livekit/export.h" + namespace livekit { /** @@ -35,7 +37,7 @@ namespace livekit { * categories. * @return true if tracing was started, false if already running or file error */ -bool startTracing(const std::string& trace_file_path, const std::vector& categories = {}); +LIVEKIT_API bool startTracing(const std::string& trace_file_path, const std::vector& categories = {}); /** * Stop tracing and flush remaining events to file. @@ -43,13 +45,13 @@ bool startTracing(const std::string& trace_file_path, const std::vector #include +#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/stats.h" @@ -69,7 +70,7 @@ struct ParticipantTrackPermission { // ============================================================ // Base Track // ============================================================ -class Track { +class LIVEKIT_API Track { public: virtual ~Track() = default; diff --git a/include/livekit/video_frame.h b/include/livekit/video_frame.h index bc313d0c..70faef7a 100644 --- a/include/livekit/video_frame.h +++ b/include/livekit/video_frame.h @@ -16,13 +16,13 @@ #pragma once -#include "livekit/export.h" - #include #include #include #include +#include "livekit/export.h" + namespace livekit { // Mirror of WebRTC video buffer type diff --git a/scripts/check_no_private_symbols.py b/scripts/check_no_private_symbols.py index 2fef9172..9f85e57b 100755 --- a/scripts/check_no_private_symbols.py +++ b/scripts/check_no_private_symbols.py @@ -108,13 +108,59 @@ def _list_exports_linux(lib: Path) -> list[str]: return raw.splitlines() +def _find_dumpbin_via_vswhere() -> str | None: + """Locate dumpbin.exe under the latest installed Visual Studio. + + GitHub-hosted Windows runners (and standard local VS installs) don't add + dumpbin to PATH unless the user opens a Developer Command Prompt. vswhere + ships at a fixed location with every Visual Studio install since 2017 and + is the supported way to discover the install tree from a vanilla shell. + """ + program_files_x86 = os.environ.get( + "ProgramFiles(x86)", r"C:\Program Files (x86)" + ) + vswhere = Path(program_files_x86) / "Microsoft Visual Studio" / "Installer" \ + / "vswhere.exe" + if not vswhere.exists(): + return None + try: + proc = subprocess.run( + [ + str(vswhere), + "-latest", + "-products", "*", + "-requires", "Microsoft.VisualStudio.Component.VC.Tools.x86.x64", + "-property", "installationPath", + ], + check=True, + capture_output=True, + text=True, + ) + except subprocess.CalledProcessError: + return None + install_path = proc.stdout.strip() + if not install_path: + return None + msvc_root = Path(install_path) / "VC" / "Tools" / "MSVC" + if not msvc_root.is_dir(): + return None + # Pick the highest-versioned toolchain present (lexicographic order matches + # version order for dotted MSVC versions like "14.44.35207"). + for version_dir in sorted(msvc_root.iterdir(), reverse=True): + candidate = version_dir / "bin" / "Hostx64" / "x64" / "dumpbin.exe" + if candidate.exists(): + return str(candidate) + return None + + def _list_exports_windows(lib: Path) -> list[str]: # dumpbin ships with MSVC; it understands import libs (.lib) and DLLs. - dumpbin = shutil.which("dumpbin") + dumpbin = shutil.which("dumpbin") or _find_dumpbin_via_vswhere() if not dumpbin: sys.stderr.write( - "error: 'dumpbin' not on PATH; run from a Visual Studio " - "Developer command prompt or ensure dumpbin.exe is available\n" + "error: 'dumpbin' not on PATH and could not be located via " + "vswhere; run from a Visual Studio Developer command prompt or " + "ensure dumpbin.exe is available\n" ) sys.exit(2) raw = subprocess.run( diff --git a/src/lk_log.h b/src/lk_log.h index a42a441f..2a657380 100644 --- a/src/lk_log.h +++ b/src/lk_log.h @@ -21,15 +21,22 @@ #include +#include "livekit/export.h" + namespace livekit::detail { /// Returns the shared "livekit" logger instance. /// The logger is created lazily on first access and lives until /// shutdownLogger() is called. Safe to call before initialize(). -std::shared_ptr getLogger(); +/// +/// Tagged LIVEKIT_INTERNAL_API: this is not part of the public ABI but must +/// remain visible so that in-tree test binaries that include lk_log.h (and +/// therefore expand LK_LOG_* macros referencing this symbol) can resolve it +/// against liblivekit. +LIVEKIT_INTERNAL_API std::shared_ptr getLogger(); /// Tears down the spdlog logger. Called by livekit::shutdown(). -void shutdownLogger(); +LIVEKIT_INTERNAL_API void shutdownLogger(); } // namespace livekit::detail diff --git a/src/logging.cpp b/src/logging.cpp index 1929d3e6..2e66e3a5 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -15,7 +15,6 @@ */ #include "livekit/logging.h" -#include "lk_log.h" #include #include @@ -23,6 +22,8 @@ #include +#include "lk_log.h" + namespace livekit { namespace { diff --git a/src/room_proto_converter.h b/src/room_proto_converter.h index 8df7c795..0fb8ef2a 100644 --- a/src/room_proto_converter.h +++ b/src/room_proto_converter.h @@ -18,6 +18,7 @@ #include +#include "livekit/export.h" #include "livekit/room_event_types.h" #include "room.pb.h" @@ -28,64 +29,70 @@ class RemoteParticipant; struct ByteStreamInfo; struct TextStreamInfo; +// All declarations below are tagged LIVEKIT_INTERNAL_API: they are NOT part of +// the public SDK ABI, but must be exported so that the in-tree test binaries +// (which include this header directly) can resolve them against liblivekit. + // --------- basic helper conversions --------- -ConnectionQuality toConnectionQuality(proto::ConnectionQuality in); -ConnectionState toConnectionState(proto::ConnectionState in); -DataPacketKind toDataPacketKind(proto::DataPacketKind in); -DisconnectReason toDisconnectReason(proto::DisconnectReason in); +LIVEKIT_INTERNAL_API ConnectionQuality toConnectionQuality(proto::ConnectionQuality in); +LIVEKIT_INTERNAL_API ConnectionState toConnectionState(proto::ConnectionState in); +LIVEKIT_INTERNAL_API DataPacketKind toDataPacketKind(proto::DataPacketKind in); +LIVEKIT_INTERNAL_API DisconnectReason toDisconnectReason(proto::DisconnectReason in); -UserPacketData fromProto(const proto::UserPacket& in); -SipDtmfData fromProto(const proto::SipDTMF& in); -RoomInfoData fromProto(const proto::RoomInfo& in); +LIVEKIT_INTERNAL_API UserPacketData fromProto(const proto::UserPacket& in); +LIVEKIT_INTERNAL_API SipDtmfData fromProto(const proto::SipDTMF& in); +LIVEKIT_INTERNAL_API RoomInfoData fromProto(const proto::RoomInfo& in); -DataStreamHeaderData fromProto(const proto::DataStream_Header& in); -DataStreamChunkData fromProto(const proto::DataStream_Chunk& in); -DataStreamTrailerData fromProto(const proto::DataStream_Trailer& in); +LIVEKIT_INTERNAL_API DataStreamHeaderData fromProto(const proto::DataStream_Header& in); +LIVEKIT_INTERNAL_API DataStreamChunkData fromProto(const proto::DataStream_Chunk& in); +LIVEKIT_INTERNAL_API DataStreamTrailerData fromProto(const proto::DataStream_Trailer& in); // --------- event conversions (RoomEvent.oneof message) --------- -RoomSidChangedEvent fromProto(const proto::RoomSidChanged& in); +LIVEKIT_INTERNAL_API RoomSidChangedEvent fromProto(const proto::RoomSidChanged& in); -ConnectionStateChangedEvent fromProto(const proto::ConnectionStateChanged& in); -DisconnectedEvent fromProto(const proto::Disconnected& in); -ReconnectingEvent fromProto(const proto::Reconnecting& in); -ReconnectedEvent fromProto(const proto::Reconnected& in); -RoomEosEvent fromProto(const proto::RoomEOS& in); +LIVEKIT_INTERNAL_API ConnectionStateChangedEvent fromProto(const proto::ConnectionStateChanged& in); +LIVEKIT_INTERNAL_API DisconnectedEvent fromProto(const proto::Disconnected& in); +LIVEKIT_INTERNAL_API ReconnectingEvent fromProto(const proto::Reconnecting& in); +LIVEKIT_INTERNAL_API ReconnectedEvent fromProto(const proto::Reconnected& in); +LIVEKIT_INTERNAL_API RoomEosEvent fromProto(const proto::RoomEOS& in); -DataStreamHeaderReceivedEvent fromProto(const proto::DataStreamHeaderReceived& in); -DataStreamChunkReceivedEvent fromProto(const proto::DataStreamChunkReceived& in); -DataStreamTrailerReceivedEvent fromProto(const proto::DataStreamTrailerReceived& in); +LIVEKIT_INTERNAL_API DataStreamHeaderReceivedEvent fromProto(const proto::DataStreamHeaderReceived& in); +LIVEKIT_INTERNAL_API DataStreamChunkReceivedEvent fromProto(const proto::DataStreamChunkReceived& in); +LIVEKIT_INTERNAL_API DataStreamTrailerReceivedEvent fromProto(const proto::DataStreamTrailerReceived& in); -DataChannelBufferedAmountLowThresholdChangedEvent fromProto( +LIVEKIT_INTERNAL_API DataChannelBufferedAmountLowThresholdChangedEvent fromProto( const proto::DataChannelBufferedAmountLowThresholdChanged& in); -ByteStreamOpenedEvent fromProto(const proto::ByteStreamOpened& in); -TextStreamOpenedEvent fromProto(const proto::TextStreamOpened& in); +LIVEKIT_INTERNAL_API ByteStreamOpenedEvent fromProto(const proto::ByteStreamOpened& in); +LIVEKIT_INTERNAL_API TextStreamOpenedEvent fromProto(const proto::TextStreamOpened& in); -RoomUpdatedEvent roomUpdatedFromProto(const proto::RoomInfo& in); // room_updated -RoomMovedEvent roomMovedFromProto(const proto::RoomInfo& in); // moved +LIVEKIT_INTERNAL_API RoomUpdatedEvent roomUpdatedFromProto(const proto::RoomInfo& in); // room_updated +LIVEKIT_INTERNAL_API RoomMovedEvent roomMovedFromProto(const proto::RoomInfo& in); // moved // --------- room options conversions --------- -proto::AudioEncoding toProto(const AudioEncodingOptions& in); -AudioEncodingOptions fromProto(const proto::AudioEncoding& in); +LIVEKIT_INTERNAL_API proto::AudioEncoding toProto(const AudioEncodingOptions& in); +LIVEKIT_INTERNAL_API AudioEncodingOptions fromProto(const proto::AudioEncoding& in); -proto::VideoEncoding toProto(const VideoEncodingOptions& in); -VideoEncodingOptions fromProto(const proto::VideoEncoding& in); +LIVEKIT_INTERNAL_API proto::VideoEncoding toProto(const VideoEncodingOptions& in); +LIVEKIT_INTERNAL_API VideoEncodingOptions fromProto(const proto::VideoEncoding& in); -proto::TrackPublishOptions toProto(const TrackPublishOptions& in); -TrackPublishOptions fromProto(const proto::TrackPublishOptions& in); +LIVEKIT_INTERNAL_API proto::TrackPublishOptions toProto(const TrackPublishOptions& in); +LIVEKIT_INTERNAL_API TrackPublishOptions fromProto(const proto::TrackPublishOptions& in); // --------- room Data Packet conversions --------- -UserDataPacketEvent userDataPacketFromProto(const proto::DataPacketReceived& in, RemoteParticipant* participant); +LIVEKIT_INTERNAL_API UserDataPacketEvent userDataPacketFromProto(const proto::DataPacketReceived& in, + RemoteParticipant* participant); -SipDtmfReceivedEvent sipDtmfFromProto(const proto::DataPacketReceived& in, RemoteParticipant* participant); +LIVEKIT_INTERNAL_API SipDtmfReceivedEvent sipDtmfFromProto(const proto::DataPacketReceived& in, + RemoteParticipant* participant); // --------- room Data Stream conversions --------- -std::map toAttrMap(const proto::DataStream::Trailer& trailer); -ByteStreamInfo makeByteInfo(const proto::DataStream::Header& header); -TextStreamInfo makeTextInfo(const proto::DataStream::Header& header); +LIVEKIT_INTERNAL_API std::map toAttrMap(const proto::DataStream::Trailer& trailer); +LIVEKIT_INTERNAL_API ByteStreamInfo makeByteInfo(const proto::DataStream::Header& header); +LIVEKIT_INTERNAL_API TextStreamInfo makeTextInfo(const proto::DataStream::Header& header); } // namespace livekit diff --git a/src/tests/consumer_link/test_consumer_link.cpp b/src/tests/consumer_link/test_consumer_link.cpp new file mode 100644 index 00000000..6c3bf270 --- /dev/null +++ b/src/tests/consumer_link/test_consumer_link.cpp @@ -0,0 +1,109 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Consumer-link canary +// -------------------- +// This translation unit is compiled into livekit_consumer_link_test, a target +// that *intentionally* mirrors how an external SDK consumer links against +// liblivekit: +// +// - Link line: livekit + GTest::gtest_main (no spdlog, +// no protobuf, +// no absl) +// - Include path: ${LIVEKIT_ROOT_DIR}/include (no src/, +// no generated/) +// - Defines: none of LIVEKIT_TEST_ACCESS / LIVEKIT_INTERNAL_API +// +// Two regressions we want this canary to catch fast, in every CI matrix +// entry, before the longer cpp-example-collection smoke runs: +// +// 1. A public header silently grows an `#include ` (or any +// other private dep). The compile fails because the private headers are +// not on the include path. +// 2. A public class loses its LIVEKIT_API tag and stops being exported +// from liblivekit. The link fails (Linux/macOS: undefined symbol; +// Windows: unresolved external) instead of slipping through to a +// runtime dlopen failure on a downstream user's machine. +// +// The deeper consumer-equivalent check (find_package(LiveKit CONFIG) against +// an installed SDK tree) lives in the cpp-example-collection smoke jobs. +// This canary is its lighter, in-tree, per-platform sibling. + +#include + +// Pull in every public header. The umbrella livekit.h covers most of the +// surface; the explicit includes below cover headers it does not transitively +// reference, so a regression in any single header is caught at compile time. +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +// Header inclusion alone is enough to exercise the compile path, but we still +// need to *call* a few exported symbols so the linker has to resolve them out +// of liblivekit. Pick a few that don't require server connectivity. +TEST(ConsumerLink, PublicEntryPointsResolve) { + ASSERT_TRUE(livekit::initialize(livekit::LogLevel::Warn)); + + livekit::setLogLevel(livekit::LogLevel::Warn); + EXPECT_EQ(livekit::getLogLevel(), livekit::LogLevel::Warn); + + livekit::setLogCallback(nullptr); + + EXPECT_FALSE(livekit::isTracingEnabled()); + + livekit::shutdown(); +} + +} // namespace diff --git a/src/trace/event_tracer.h b/src/trace/event_tracer.h index 2bb93e82..a0dbb1bc 100644 --- a/src/trace/event_tracer.h +++ b/src/trace/event_tracer.h @@ -39,16 +39,7 @@ #ifndef LIVEKIT_TRACE_EVENT_TRACER_H_ #define LIVEKIT_TRACE_EVENT_TRACER_H_ -// Platform-specific DLL export macro -#if defined(_WIN32) -#if defined(LIVEKIT_BUILDING_SDK) -#define LIVEKIT_EXPORT __declspec(dllexport) -#else -#define LIVEKIT_EXPORT __declspec(dllimport) -#endif -#else -#define LIVEKIT_EXPORT __attribute__((visibility("default"))) -#endif +#include "livekit/export.h" namespace livekit { namespace trace { @@ -63,12 +54,13 @@ typedef void (*AddTraceEventPtr)(char phase, const unsigned char* category_enabl // // This method must be called before any tracing begins. Functions // provided should be thread-safe. -LIVEKIT_EXPORT void SetupEventTracer(GetCategoryEnabledPtr get_category_enabled_ptr, - AddTraceEventPtr add_trace_event_ptr); +LIVEKIT_API void SetupEventTracer(GetCategoryEnabledPtr get_category_enabled_ptr, + AddTraceEventPtr add_trace_event_ptr); // This class defines interface for the event tracing system to call -// internally. Do not call these methods directly. -class EventTracer { +// internally. Tagged LIVEKIT_INTERNAL_API so the in-tree tracing tests +// (src/tests/unit/test_tracing.cpp) can resolve the static methods. +class LIVEKIT_INTERNAL_API EventTracer { public: static const unsigned char* GetCategoryEnabled(const char* name); diff --git a/src/video_utils.h b/src/video_utils.h index c2c9bcfb..76cea2fe 100644 --- a/src/video_utils.h +++ b/src/video_utils.h @@ -16,19 +16,22 @@ #pragma once +#include "livekit/export.h" #include "livekit/video_frame.h" #include "livekit/video_source.h" #include "video_frame.pb.h" namespace livekit { -// Video FFI Utils -proto::VideoBufferInfo toProto(const VideoFrame& frame); -VideoFrame fromOwnedProto(const proto::OwnedVideoBuffer& owned); -VideoFrame convertViaFfi(const VideoFrame& frame, VideoBufferType dst, bool flip_y); -proto::VideoBufferType toProto(const VideoBufferType t); -VideoBufferType fromProto(const proto::VideoBufferType t); -std::optional toProto(const std::optional& metadata); -std::optional fromProto(const proto::FrameMetadata& metadata); +// Video FFI Utils. +// Tagged LIVEKIT_INTERNAL_API: not part of the public ABI, but exposed so +// in-tree tests that include this header can link. +LIVEKIT_INTERNAL_API proto::VideoBufferInfo toProto(const VideoFrame& frame); +LIVEKIT_INTERNAL_API VideoFrame fromOwnedProto(const proto::OwnedVideoBuffer& owned); +LIVEKIT_INTERNAL_API VideoFrame convertViaFfi(const VideoFrame& frame, VideoBufferType dst, bool flip_y); +LIVEKIT_INTERNAL_API proto::VideoBufferType toProto(const VideoBufferType t); +LIVEKIT_INTERNAL_API VideoBufferType fromProto(const proto::VideoBufferType t); +LIVEKIT_INTERNAL_API std::optional toProto(const std::optional& metadata); +LIVEKIT_INTERNAL_API std::optional fromProto(const proto::FrameMetadata& metadata); } // namespace livekit From d0b703faa221b1d7ba7cc1ee33c3390de98fc7c5 Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 8 May 2026 10:53:42 -0600 Subject: [PATCH 06/24] Use visbility instead --- AGENTS.md | 9 +++++---- CMakeLists.txt | 2 +- include/livekit/audio_frame.h | 2 +- include/livekit/audio_processing_module.h | 2 +- include/livekit/audio_source.h | 2 +- include/livekit/audio_stream.h | 2 +- include/livekit/data_stream.h | 2 +- include/livekit/data_track_error.h | 2 +- include/livekit/data_track_frame.h | 2 +- include/livekit/data_track_stream.h | 2 +- include/livekit/e2ee.h | 2 +- include/livekit/ffi_handle.h | 2 +- include/livekit/livekit.h | 2 +- include/livekit/local_audio_track.h | 2 +- include/livekit/local_data_track.h | 2 +- include/livekit/local_participant.h | 2 +- include/livekit/local_track_publication.h | 2 +- include/livekit/local_video_track.h | 2 +- include/livekit/logging.h | 2 +- include/livekit/participant.h | 2 +- include/livekit/remote_audio_track.h | 2 +- include/livekit/remote_data_track.h | 2 +- include/livekit/remote_participant.h | 2 +- include/livekit/remote_track_publication.h | 2 +- include/livekit/remote_video_track.h | 2 +- include/livekit/room.h | 2 +- include/livekit/room_delegate.h | 2 +- include/livekit/rpc_error.h | 2 +- include/livekit/stats.h | 2 +- include/livekit/subscription_thread_dispatcher.h | 2 +- include/livekit/tracing.h | 2 +- include/livekit/track.h | 3 ++- include/livekit/track_publication.h | 2 +- include/livekit/video_frame.h | 2 +- include/livekit/video_source.h | 2 +- include/livekit/video_stream.h | 2 +- include/livekit/{export.h => visibility.h} | 5 +---- scripts/check_no_private_symbols.py | 2 +- src/ffi_client.h | 2 +- src/lk_log.h | 2 +- src/room_proto_converter.h | 6 +++--- src/tests/consumer_link/test_consumer_link.cpp | 7 +++---- src/trace/event_tracer.h | 5 ++--- src/video_utils.h | 2 +- 44 files changed, 54 insertions(+), 57 deletions(-) rename include/livekit/{export.h => visibility.h} (96%) diff --git a/AGENTS.md b/AGENTS.md index aa01200c..de5d307f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -140,7 +140,8 @@ All source files must have the LiveKit Apache 2.0 copyright header. Use the curr ### Public API Surface - Keep public APIs small. Minimize what goes into `include/livekit/`. -- All publicly visible symbols must use `LIVEKIT_API` macro (via `include/livekit/export.h`). +- Use `#pragma once` to guard headers. +- All publicly visible symbols must use `LIVEKIT_API` macro (via `include/livekit/visibility.h`). - 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. @@ -149,9 +150,9 @@ All source files must have the LiveKit Apache 2.0 copyright header. Use the curr The `livekit` shared library is built with hidden symbol visibility on all platforms. Only symbols explicitly tagged with `LIVEKIT_API` (declared in -`include/livekit/export.h`) are part of the public ABI. Vendored static -dependencies (spdlog, fmt, protobuf, absl) are also compiled with hidden -visibility so their symbols cannot leak into `liblivekit.{so,dylib,dll}`. +`include/livekit/visibility.h`) are part of the public ABI. Vendored static +dependencies are also compiled with hidden visibility so their symbols cannot +leak into `liblivekit.{so,dylib,dll}`. Rules for new code: diff --git a/CMakeLists.txt b/CMakeLists.txt index 43c853a5..a7d44bf6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -375,7 +375,7 @@ add_library(livekit SHARED ) # Symbol visibility: keep liblivekit's exported ABI restricted to the public # LiveKit API. Every TU is compiled with -fvisibility=hidden, and only -# declarations annotated with LIVEKIT_API (see include/livekit/export.h) are +# declarations annotated with LIVEKIT_API (see include/livekit/visibility.h) are # re-exposed. This prevents private dependencies (spdlog, protobuf, absl) from # leaking into the dynamic symbol table and clashing with consumer apps that # also link those libraries (e.g. ROS 2's rcl_logging_spdlog). diff --git a/include/livekit/audio_frame.h b/include/livekit/audio_frame.h index f66e9512..1a28d4d0 100644 --- a/include/livekit/audio_frame.h +++ b/include/livekit/audio_frame.h @@ -20,7 +20,7 @@ #include #include -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/audio_processing_module.h b/include/livekit/audio_processing_module.h index 1187ced3..ba37cc7b 100644 --- a/include/livekit/audio_processing_module.h +++ b/include/livekit/audio_processing_module.h @@ -19,8 +19,8 @@ #include #include "livekit/audio_frame.h" -#include "livekit/export.h" #include "livekit/ffi_handle.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/audio_source.h b/include/livekit/audio_source.h index bdac5910..d8c0fd6c 100644 --- a/include/livekit/audio_source.h +++ b/include/livekit/audio_source.h @@ -19,8 +19,8 @@ #include #include "livekit/audio_frame.h" -#include "livekit/export.h" #include "livekit/ffi_handle.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/audio_stream.h b/include/livekit/audio_stream.h index 096af3fd..22d91a12 100644 --- a/include/livekit/audio_stream.h +++ b/include/livekit/audio_stream.h @@ -25,10 +25,10 @@ #include #include "audio_frame.h" -#include "export.h" #include "ffi_handle.h" #include "participant.h" #include "track.h" +#include "visibility.h" namespace livekit { diff --git a/include/livekit/data_stream.h b/include/livekit/data_stream.h index f4bb8239..ed81cf83 100644 --- a/include/livekit/data_stream.h +++ b/include/livekit/data_stream.h @@ -27,7 +27,7 @@ #include #include -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/data_track_error.h b/include/livekit/data_track_error.h index e25ee137..5047a8a1 100644 --- a/include/livekit/data_track_error.h +++ b/include/livekit/data_track_error.h @@ -20,7 +20,7 @@ #include #include -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/data_track_frame.h b/include/livekit/data_track_frame.h index ed2678d5..1e82ac5a 100644 --- a/include/livekit/data_track_frame.h +++ b/include/livekit/data_track_frame.h @@ -20,7 +20,7 @@ #include #include -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/data_track_stream.h b/include/livekit/data_track_stream.h index 12e08dc5..20491029 100644 --- a/include/livekit/data_track_stream.h +++ b/include/livekit/data_track_stream.h @@ -24,8 +24,8 @@ #include #include "livekit/data_track_frame.h" -#include "livekit/export.h" #include "livekit/ffi_handle.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/e2ee.h b/include/livekit/e2ee.h index 61400b38..dfd55a60 100644 --- a/include/livekit/e2ee.h +++ b/include/livekit/e2ee.h @@ -22,7 +22,7 @@ #include #include -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/ffi_handle.h b/include/livekit/ffi_handle.h index 2457cea5..840aa384 100644 --- a/include/livekit/ffi_handle.h +++ b/include/livekit/ffi_handle.h @@ -18,7 +18,7 @@ #include -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/livekit.h b/include/livekit/livekit.h index f7200e49..a9163a80 100644 --- a/include/livekit/livekit.h +++ b/include/livekit/livekit.h @@ -22,7 +22,6 @@ #include "audio_stream.h" #include "build.h" #include "e2ee.h" -#include "export.h" #include "local_audio_track.h" #include "local_participant.h" #include "local_track_publication.h" @@ -39,6 +38,7 @@ #include "video_frame.h" #include "video_source.h" #include "video_stream.h" +#include "visibility.h" namespace livekit { diff --git a/include/livekit/local_audio_track.h b/include/livekit/local_audio_track.h index 0e6d7b77..2a2a9b02 100644 --- a/include/livekit/local_audio_track.h +++ b/include/livekit/local_audio_track.h @@ -20,9 +20,9 @@ #include #include "audio_frame.h" -#include "export.h" #include "local_track_publication.h" #include "track.h" +#include "visibility.h" namespace livekit { diff --git a/include/livekit/local_data_track.h b/include/livekit/local_data_track.h index 8512d31f..45e38611 100644 --- a/include/livekit/local_data_track.h +++ b/include/livekit/local_data_track.h @@ -25,9 +25,9 @@ #include "livekit/data_track_error.h" #include "livekit/data_track_frame.h" #include "livekit/data_track_info.h" -#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/result.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/local_participant.h b/include/livekit/local_participant.h index ee682434..bcb94bd7 100644 --- a/include/livekit/local_participant.h +++ b/include/livekit/local_participant.h @@ -26,7 +26,6 @@ #include #include -#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/local_audio_track.h" #include "livekit/local_data_track.h" @@ -34,6 +33,7 @@ #include "livekit/participant.h" #include "livekit/room_event_types.h" #include "livekit/rpc_error.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/local_track_publication.h b/include/livekit/local_track_publication.h index 13720993..50f5fd74 100644 --- a/include/livekit/local_track_publication.h +++ b/include/livekit/local_track_publication.h @@ -16,8 +16,8 @@ #pragma once -#include "livekit/export.h" #include "livekit/track_publication.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/local_video_track.h b/include/livekit/local_video_track.h index 3ec97097..a71cd9c5 100644 --- a/include/livekit/local_video_track.h +++ b/include/livekit/local_video_track.h @@ -19,9 +19,9 @@ #include #include -#include "export.h" #include "local_track_publication.h" #include "track.h" +#include "visibility.h" namespace livekit { diff --git a/include/livekit/logging.h b/include/livekit/logging.h index 739ec13c..4e5a145d 100644 --- a/include/livekit/logging.h +++ b/include/livekit/logging.h @@ -19,7 +19,7 @@ #include #include -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/participant.h b/include/livekit/participant.h index 6e637579..9858b128 100644 --- a/include/livekit/participant.h +++ b/include/livekit/participant.h @@ -21,9 +21,9 @@ #include #include -#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/room_delegate.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/remote_audio_track.h b/include/livekit/remote_audio_track.h index e50e07cd..e3973245 100644 --- a/include/livekit/remote_audio_track.h +++ b/include/livekit/remote_audio_track.h @@ -19,8 +19,8 @@ #include #include -#include "export.h" #include "track.h" +#include "visibility.h" namespace livekit { diff --git a/include/livekit/remote_data_track.h b/include/livekit/remote_data_track.h index f4a2a2be..920cb790 100644 --- a/include/livekit/remote_data_track.h +++ b/include/livekit/remote_data_track.h @@ -22,9 +22,9 @@ #include "livekit/data_track_error.h" #include "livekit/data_track_info.h" #include "livekit/data_track_stream.h" -#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/result.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/remote_participant.h b/include/livekit/remote_participant.h index 4eab33d0..ed5767e6 100644 --- a/include/livekit/remote_participant.h +++ b/include/livekit/remote_participant.h @@ -20,8 +20,8 @@ #include #include -#include "export.h" #include "participant.h" +#include "visibility.h" namespace livekit { diff --git a/include/livekit/remote_track_publication.h b/include/livekit/remote_track_publication.h index aa25480d..2568a55d 100644 --- a/include/livekit/remote_track_publication.h +++ b/include/livekit/remote_track_publication.h @@ -16,8 +16,8 @@ #pragma once -#include "livekit/export.h" #include "livekit/track_publication.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/remote_video_track.h b/include/livekit/remote_video_track.h index 987344e2..455bc7aa 100644 --- a/include/livekit/remote_video_track.h +++ b/include/livekit/remote_video_track.h @@ -19,8 +19,8 @@ #include #include -#include "export.h" #include "track.h" +#include "visibility.h" namespace livekit { diff --git a/include/livekit/room.h b/include/livekit/room.h index 6c7a3715..80b23aaa 100644 --- a/include/livekit/room.h +++ b/include/livekit/room.h @@ -23,10 +23,10 @@ #include "livekit/data_stream.h" #include "livekit/e2ee.h" -#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/room_event_types.h" #include "livekit/subscription_thread_dispatcher.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/room_delegate.h b/include/livekit/room_delegate.h index fe2d5b40..79c484aa 100644 --- a/include/livekit/room_delegate.h +++ b/include/livekit/room_delegate.h @@ -16,8 +16,8 @@ #pragma once -#include "livekit/export.h" #include "livekit/room_event_types.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/rpc_error.h b/include/livekit/rpc_error.h index 8d9f5fe9..9e449644 100644 --- a/include/livekit/rpc_error.h +++ b/include/livekit/rpc_error.h @@ -20,7 +20,7 @@ #include #include -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/stats.h b/include/livekit/stats.h index 7e8d5bf9..84674d8e 100644 --- a/include/livekit/stats.h +++ b/include/livekit/stats.h @@ -23,7 +23,7 @@ #include #include -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/subscription_thread_dispatcher.h b/include/livekit/subscription_thread_dispatcher.h index 569daa46..bdd41618 100644 --- a/include/livekit/subscription_thread_dispatcher.h +++ b/include/livekit/subscription_thread_dispatcher.h @@ -28,8 +28,8 @@ #include #include "livekit/audio_stream.h" -#include "livekit/export.h" #include "livekit/video_stream.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/tracing.h b/include/livekit/tracing.h index 369be124..a630ffe6 100644 --- a/include/livekit/tracing.h +++ b/include/livekit/tracing.h @@ -19,7 +19,7 @@ #include #include -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/track.h b/include/livekit/track.h index 43d0cfd9..8051f0fd 100644 --- a/include/livekit/track.h +++ b/include/livekit/track.h @@ -15,6 +15,7 @@ */ #pragma once + #include #include #include @@ -23,9 +24,9 @@ #include #include -#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/stats.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/track_publication.h b/include/livekit/track_publication.h index fe2418db..7a8d9ead 100644 --- a/include/livekit/track_publication.h +++ b/include/livekit/track_publication.h @@ -22,9 +22,9 @@ #include #include "livekit/e2ee.h" -#include "livekit/export.h" #include "livekit/ffi_handle.h" #include "livekit/track.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/video_frame.h b/include/livekit/video_frame.h index 70faef7a..4520aeed 100644 --- a/include/livekit/video_frame.h +++ b/include/livekit/video_frame.h @@ -21,7 +21,7 @@ #include #include -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/video_source.h b/include/livekit/video_source.h index a506291d..fa2f4378 100644 --- a/include/livekit/video_source.h +++ b/include/livekit/video_source.h @@ -19,8 +19,8 @@ #include #include -#include "livekit/export.h" #include "livekit/ffi_handle.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/video_stream.h b/include/livekit/video_stream.h index 9a2181bf..c25bd030 100644 --- a/include/livekit/video_stream.h +++ b/include/livekit/video_stream.h @@ -24,12 +24,12 @@ #include #include -#include "export.h" #include "ffi_handle.h" #include "participant.h" #include "track.h" #include "video_frame.h" #include "video_source.h" +#include "visibility.h" namespace livekit { diff --git a/include/livekit/export.h b/include/livekit/visibility.h similarity index 96% rename from include/livekit/export.h rename to include/livekit/visibility.h index 14983571..c7921fb1 100644 --- a/include/livekit/export.h +++ b/include/livekit/visibility.h @@ -14,8 +14,7 @@ * limitations under the License. */ -#ifndef LIVEKIT_EXPORT_H -#define LIVEKIT_EXPORT_H +#pragma once // LIVEKIT_API marks a symbol as part of the public ABI of liblivekit. // @@ -62,5 +61,3 @@ #define LIVEKIT_INTERNAL_API #endif #endif - -#endif // LIVEKIT_EXPORT_H diff --git a/scripts/check_no_private_symbols.py b/scripts/check_no_private_symbols.py index 9f85e57b..4412a9af 100755 --- a/scripts/check_no_private_symbols.py +++ b/scripts/check_no_private_symbols.py @@ -238,7 +238,7 @@ def main(argv: list[str]) -> int: print( "\nliblivekit must not re-export private dependency symbols.\n" "If you intentionally added a public symbol that triggered this, mark\n" - "it with LIVEKIT_API in include/livekit/export.h and rebuild.\n" + "it with LIVEKIT_API in include/livekit/visibility.h and rebuild.\n" ) return 1 diff --git a/src/ffi_client.h b/src/ffi_client.h index 725a608e..530fad74 100644 --- a/src/ffi_client.h +++ b/src/ffi_client.h @@ -30,9 +30,9 @@ #include "data_track.pb.h" #include "livekit/data_track_error.h" -#include "livekit/export.h" #include "livekit/result.h" #include "livekit/stats.h" +#include "livekit/visibility.h" #include "lk_log.h" #include "room.pb.h" diff --git a/src/lk_log.h b/src/lk_log.h index 2a657380..38e67b0a 100644 --- a/src/lk_log.h +++ b/src/lk_log.h @@ -21,7 +21,7 @@ #include -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit::detail { diff --git a/src/room_proto_converter.h b/src/room_proto_converter.h index 0fb8ef2a..1ff09bd3 100644 --- a/src/room_proto_converter.h +++ b/src/room_proto_converter.h @@ -18,8 +18,8 @@ #include -#include "livekit/export.h" #include "livekit/room_event_types.h" +#include "livekit/visibility.h" #include "room.pb.h" namespace livekit { @@ -62,8 +62,8 @@ LIVEKIT_INTERNAL_API DataStreamHeaderReceivedEvent fromProto(const proto::DataSt LIVEKIT_INTERNAL_API DataStreamChunkReceivedEvent fromProto(const proto::DataStreamChunkReceived& in); LIVEKIT_INTERNAL_API DataStreamTrailerReceivedEvent fromProto(const proto::DataStreamTrailerReceived& in); -LIVEKIT_INTERNAL_API DataChannelBufferedAmountLowThresholdChangedEvent fromProto( - const proto::DataChannelBufferedAmountLowThresholdChanged& in); +LIVEKIT_INTERNAL_API DataChannelBufferedAmountLowThresholdChangedEvent +fromProto(const proto::DataChannelBufferedAmountLowThresholdChanged& in); LIVEKIT_INTERNAL_API ByteStreamOpenedEvent fromProto(const proto::ByteStreamOpened& in); LIVEKIT_INTERNAL_API TextStreamOpenedEvent fromProto(const proto::TextStreamOpened& in); diff --git a/src/tests/consumer_link/test_consumer_link.cpp b/src/tests/consumer_link/test_consumer_link.cpp index 6c3bf270..46d211c6 100644 --- a/src/tests/consumer_link/test_consumer_link.cpp +++ b/src/tests/consumer_link/test_consumer_link.cpp @@ -47,8 +47,6 @@ // Pull in every public header. The umbrella livekit.h covers most of the // surface; the explicit includes below cover headers it does not transitively // reference, so a regression in any single header is caught at compile time. -#include - #include #include #include @@ -60,8 +58,8 @@ #include #include #include -#include #include +#include #include #include #include @@ -81,12 +79,13 @@ #include #include #include +#include #include #include -#include #include #include #include +#include namespace { diff --git a/src/trace/event_tracer.h b/src/trace/event_tracer.h index a0dbb1bc..088914a3 100644 --- a/src/trace/event_tracer.h +++ b/src/trace/event_tracer.h @@ -39,7 +39,7 @@ #ifndef LIVEKIT_TRACE_EVENT_TRACER_H_ #define LIVEKIT_TRACE_EVENT_TRACER_H_ -#include "livekit/export.h" +#include "livekit/visibility.h" namespace livekit { namespace trace { @@ -54,8 +54,7 @@ typedef void (*AddTraceEventPtr)(char phase, const unsigned char* category_enabl // // This method must be called before any tracing begins. Functions // provided should be thread-safe. -LIVEKIT_API void SetupEventTracer(GetCategoryEnabledPtr get_category_enabled_ptr, - AddTraceEventPtr add_trace_event_ptr); +LIVEKIT_API void SetupEventTracer(GetCategoryEnabledPtr get_category_enabled_ptr, AddTraceEventPtr add_trace_event_ptr); // This class defines interface for the event tracing system to call // internally. Tagged LIVEKIT_INTERNAL_API so the in-tree tracing tests diff --git a/src/video_utils.h b/src/video_utils.h index 76cea2fe..0a9c4e89 100644 --- a/src/video_utils.h +++ b/src/video_utils.h @@ -16,9 +16,9 @@ #pragma once -#include "livekit/export.h" #include "livekit/video_frame.h" #include "livekit/video_source.h" +#include "livekit/visibility.h" #include "video_frame.pb.h" namespace livekit { From af908da63f735cdd4364e2aa3e3e9651cd5af7b5 Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 8 May 2026 11:57:58 -0600 Subject: [PATCH 07/24] Maybe fix windows test --- include/livekit/remote_data_track.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/livekit/remote_data_track.h b/include/livekit/remote_data_track.h index 920cb790..edf48005 100644 --- a/include/livekit/remote_data_track.h +++ b/include/livekit/remote_data_track.h @@ -69,7 +69,7 @@ class LIVEKIT_API RemoteDataTrack { #ifdef LIVEKIT_TEST_ACCESS /// Test-only accessor for exercising lower-level FFI subscription paths. - uintptr_t testFfiHandleId() const noexcept { return ffi_handle_id(); } + LIVEKIT_API uintptr_t testFfiHandleId() const noexcept { return ffi_handle_id(); } #endif /** From 285062707144ae59ae8ae68216e5b4ddcdc65e0a Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 8 May 2026 14:10:47 -0600 Subject: [PATCH 08/24] Maybe windows fix --- include/livekit/remote_data_track.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/livekit/remote_data_track.h b/include/livekit/remote_data_track.h index edf48005..965893ca 100644 --- a/include/livekit/remote_data_track.h +++ b/include/livekit/remote_data_track.h @@ -51,7 +51,7 @@ class OwnedRemoteDataTrack; * } * } */ -class LIVEKIT_API RemoteDataTrack { +class RemoteDataTrack { public: ~RemoteDataTrack() = default; @@ -65,11 +65,11 @@ class LIVEKIT_API RemoteDataTrack { const std::string& publisherIdentity() const noexcept { return publisher_identity_; } /// Whether the track is still published by the remote participant. - bool isPublished() const; + LIVEKIT_API bool isPublished() const; #ifdef LIVEKIT_TEST_ACCESS /// Test-only accessor for exercising lower-level FFI subscription paths. - LIVEKIT_API uintptr_t testFfiHandleId() const noexcept { return ffi_handle_id(); } + uintptr_t testFfiHandleId() const noexcept { return ffi_handle_id(); } #endif /** @@ -78,7 +78,7 @@ class LIVEKIT_API RemoteDataTrack { * Returns a DataTrackStream that delivers frames via blocking * read(). Destroy the stream to unsubscribe. */ - Result, SubscribeDataTrackError> subscribe( + LIVEKIT_API Result, SubscribeDataTrackError> subscribe( const DataTrackStream::Options& options = {}); private: From 62bee737bc0a05464d851d2cf998997708f5d64a Mon Sep 17 00:00:00 2001 From: Alan George Date: Mon, 11 May 2026 08:39:45 -0600 Subject: [PATCH 09/24] Unify include strategy --- AGENTS.md | 19 ++++++++++++ include/livekit/audio_stream.h | 10 +++--- include/livekit/livekit.h | 46 ++++++++++++++-------------- include/livekit/local_audio_track.h | 8 ++--- include/livekit/local_video_track.h | 6 ++-- include/livekit/remote_audio_track.h | 4 +-- include/livekit/remote_participant.h | 4 +-- include/livekit/remote_video_track.h | 4 +-- include/livekit/video_stream.h | 12 ++++---- 9 files changed, 66 insertions(+), 47 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index de5d307f..94ff6d85 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -146,6 +146,25 @@ All source files must have the LiveKit Apache 2.0 copyright header. Use the curr - `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. +### Include Conventions + +- **Public headers (`include/livekit/*.h`) must include other public headers + with the `livekit/` prefix**: `#include "livekit/track.h"`, never the bare + `#include "track.h"` form. The prefixed spelling matches what consumers see + (`#include `), resolves through the standard `-I include/` + search path rather than the implementation-defined "current directory first" + rule of `#include "..."`, and avoids accidental name collisions with + third-party headers that happen to share short names like `track.h`. +- Use double quotes for project headers (`#include "livekit/foo.h"`) and angle + brackets for system / third-party headers (`#include `, + `#include `). +- Implementation files in `src/` may include internal headers without a + prefix (`#include "ffi_client.h"`); they are not part of the public + surface and live alongside their consumers. +- Test code (in-tree or external-style canaries) must consume public + headers via `` to mirror real consumer usage; see + `src/tests/consumer_link/test_consumer_link.cpp`. + ### Symbol Visibility / Exported ABI The `livekit` shared library is built with hidden symbol visibility on all diff --git a/include/livekit/audio_stream.h b/include/livekit/audio_stream.h index 22d91a12..57be0a89 100644 --- a/include/livekit/audio_stream.h +++ b/include/livekit/audio_stream.h @@ -24,11 +24,11 @@ #include #include -#include "audio_frame.h" -#include "ffi_handle.h" -#include "participant.h" -#include "track.h" -#include "visibility.h" +#include "livekit/audio_frame.h" +#include "livekit/ffi_handle.h" +#include "livekit/participant.h" +#include "livekit/track.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/livekit.h b/include/livekit/livekit.h index a9163a80..f7906c42 100644 --- a/include/livekit/livekit.h +++ b/include/livekit/livekit.h @@ -16,29 +16,29 @@ #pragma once -#include "audio_frame.h" -#include "audio_processing_module.h" -#include "audio_source.h" -#include "audio_stream.h" -#include "build.h" -#include "e2ee.h" -#include "local_audio_track.h" -#include "local_participant.h" -#include "local_track_publication.h" -#include "local_video_track.h" -#include "logging.h" -#include "participant.h" -#include "remote_participant.h" -#include "remote_track_publication.h" -#include "room.h" -#include "room_delegate.h" -#include "room_event_types.h" -#include "tracing.h" -#include "track_publication.h" -#include "video_frame.h" -#include "video_source.h" -#include "video_stream.h" -#include "visibility.h" +#include "livekit/audio_frame.h" +#include "livekit/audio_processing_module.h" +#include "livekit/audio_source.h" +#include "livekit/audio_stream.h" +#include "livekit/build.h" +#include "livekit/e2ee.h" +#include "livekit/local_audio_track.h" +#include "livekit/local_participant.h" +#include "livekit/local_track_publication.h" +#include "livekit/local_video_track.h" +#include "livekit/logging.h" +#include "livekit/participant.h" +#include "livekit/remote_participant.h" +#include "livekit/remote_track_publication.h" +#include "livekit/room.h" +#include "livekit/room_delegate.h" +#include "livekit/room_event_types.h" +#include "livekit/tracing.h" +#include "livekit/track_publication.h" +#include "livekit/video_frame.h" +#include "livekit/video_source.h" +#include "livekit/video_stream.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/local_audio_track.h b/include/livekit/local_audio_track.h index 2a2a9b02..53823f7b 100644 --- a/include/livekit/local_audio_track.h +++ b/include/livekit/local_audio_track.h @@ -19,10 +19,10 @@ #include #include -#include "audio_frame.h" -#include "local_track_publication.h" -#include "track.h" -#include "visibility.h" +#include "livekit/audio_frame.h" +#include "livekit/local_track_publication.h" +#include "livekit/track.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/local_video_track.h b/include/livekit/local_video_track.h index a71cd9c5..98b0b801 100644 --- a/include/livekit/local_video_track.h +++ b/include/livekit/local_video_track.h @@ -19,9 +19,9 @@ #include #include -#include "local_track_publication.h" -#include "track.h" -#include "visibility.h" +#include "livekit/local_track_publication.h" +#include "livekit/track.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/remote_audio_track.h b/include/livekit/remote_audio_track.h index e3973245..060b5825 100644 --- a/include/livekit/remote_audio_track.h +++ b/include/livekit/remote_audio_track.h @@ -19,8 +19,8 @@ #include #include -#include "track.h" -#include "visibility.h" +#include "livekit/track.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/remote_participant.h b/include/livekit/remote_participant.h index ed5767e6..67f5e9a4 100644 --- a/include/livekit/remote_participant.h +++ b/include/livekit/remote_participant.h @@ -20,8 +20,8 @@ #include #include -#include "participant.h" -#include "visibility.h" +#include "livekit/participant.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/remote_video_track.h b/include/livekit/remote_video_track.h index 455bc7aa..1df57220 100644 --- a/include/livekit/remote_video_track.h +++ b/include/livekit/remote_video_track.h @@ -19,8 +19,8 @@ #include #include -#include "track.h" -#include "visibility.h" +#include "livekit/track.h" +#include "livekit/visibility.h" namespace livekit { diff --git a/include/livekit/video_stream.h b/include/livekit/video_stream.h index c25bd030..82874ad7 100644 --- a/include/livekit/video_stream.h +++ b/include/livekit/video_stream.h @@ -24,12 +24,12 @@ #include #include -#include "ffi_handle.h" -#include "participant.h" -#include "track.h" -#include "video_frame.h" -#include "video_source.h" -#include "visibility.h" +#include "livekit/ffi_handle.h" +#include "livekit/participant.h" +#include "livekit/track.h" +#include "livekit/video_frame.h" +#include "livekit/video_source.h" +#include "livekit/visibility.h" namespace livekit { From 1827d7ac45fe20ec298cb5d768f3af392056f45b Mon Sep 17 00:00:00 2001 From: Alan George Date: Mon, 11 May 2026 09:11:59 -0600 Subject: [PATCH 10/24] PR self-review cleanup --- AGENTS.md | 23 +--- CMakeLists.txt | 11 +- src/ffi_client.h | 3 - src/lk_log.h | 5 - src/room_proto_converter.h | 4 - .../consumer_link/test_consumer_link.cpp | 108 ------------------ src/trace/event_tracer.h | 3 +- src/video_utils.h | 2 - 8 files changed, 11 insertions(+), 148 deletions(-) delete mode 100644 src/tests/consumer_link/test_consumer_link.cpp diff --git a/AGENTS.md b/AGENTS.md index 94ff6d85..a7ebab47 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,7 +110,7 @@ Updates to ./build.sh and ./build.cmd should be accompanied by updates to this f The build scripts pass an explicit job count to `cmake --build --parallel`. Set `CMAKE_BUILD_PARALLEL_LEVEL` to override the default detected logical CPU count. -**Requirements:** CMake 3.20+, C++17, Rust toolchain (cargo), protoc. On macOS: `brew install cmake ninja protobuf abseil`. On Linux: see the CI workflow for apt packages. spdlog is vendored automatically via FetchContent (or vcpkg on Windows) to avoid system conflicts, for example on ROS 2. +**Requirements:** CMake 3.20+, C++17, Rust toolchain (cargo), protoc. On macOS: `brew install cmake ninja protobuf abseil`. On Linux: see the CI workflow for apt packages. spdlog is vendored automatically via FetchContent (or vcpkg on Windows) to avoid system conflicts. ### SDK Packaging @@ -162,8 +162,7 @@ All source files must have the LiveKit Apache 2.0 copyright header. Use the curr prefix (`#include "ffi_client.h"`); they are not part of the public surface and live alongside their consumers. - Test code (in-tree or external-style canaries) must consume public - headers via `` to mirror real consumer usage; see - `src/tests/consumer_link/test_consumer_link.cpp`. + headers via `` to mirror real consumer usage ### Symbol Visibility / Exported ABI @@ -186,9 +185,6 @@ Rules for new code: - On Windows, `WINDOWS_EXPORT_ALL_SYMBOLS` is **deliberately disabled** for the `livekit` target. Use `LIVEKIT_API` to export. -CI enforces the policy via `scripts/check_no_private_symbols.py`, which is -also wired in as a CTest test (label `abi`): - ### Error Handling - Use `LK_LOG_WARN` for non-fatal unexpected conditions. @@ -211,19 +207,14 @@ also wired in as a CTest test (label `abi`): - 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`. -``` -ctest -L abi --output-on-failure -``` - -The script fails if `nm`/`dumpbin` reports any exported symbol containing -`spdlog`, `fmt::v`, `google::protobuf`, or `absl::`. - ### Readability and Performance + Code should be easy to read and understand. If a sacrifice is made for performance or readability, it should be documented. -Adhere to clang-format checks configured in `.clang-format`. Run `./scripts/clang-format.sh` if needed to confirm code styling. +Adhere to clang-format checks configured in `.clang-format`. Run `./scripts/clang-format.sh` if needed to confirm code styling, and `./scripts/clang-format.sh --fix` to auto-format generated code created. ### Static Analysis + Adhere to clang-tidy checks configured in `.clang-tidy`. Run `./scripts/clang-tidy.sh` if needed to confirm code quality. ## Dependencies @@ -249,10 +240,8 @@ Integration tests (`src/tests/integration/`) cover: room connections, callbacks, 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 -Use clang formatting that aligns with the existing codebase. - ## General C++ Development + - Do not use dynamic memory allocation after initialization - Keep each function short (roughly ≤ 60 lines) - Declare all data objects at the smallest possible level of scope diff --git a/CMakeLists.txt b/CMakeLists.txt index a7d44bf6..3f71c956 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -374,11 +374,8 @@ add_library(livekit SHARED src/trace/tracing.cpp ) # Symbol visibility: keep liblivekit's exported ABI restricted to the public -# LiveKit API. Every TU is compiled with -fvisibility=hidden, and only -# declarations annotated with LIVEKIT_API (see include/livekit/visibility.h) are -# re-exposed. This prevents private dependencies (spdlog, protobuf, absl) from -# leaking into the dynamic symbol table and clashing with consumer apps that -# also link those libraries (e.g. ROS 2's rcl_logging_spdlog). +# LiveKit API. By default everything is hidden unless marked by the LIVEKIT_API +# macro in code set_target_properties(livekit PROPERTIES CXX_VISIBILITY_PRESET hidden C_VISIBILITY_PRESET hidden @@ -386,11 +383,11 @@ set_target_properties(livekit PROPERTIES ) target_compile_definitions(livekit PRIVATE LIVEKIT_BUILDING_SDK=1) -# Linker-level belt-and-suspenders for Linux: even if a static-archive object +# Linker-level safe guard for Linux: even if a static-archive object # slipped through with default visibility, --exclude-libs,ALL strips it from # the dynamic symbol table. ld64 (macOS) has no equivalent flag; hidden # visibility per TU is sufficient there. On Windows, explicit dllexport via -# LIVEKIT_API replaces WINDOWS_EXPORT_ALL_SYMBOLS. +# LIVEKIT_API replaces WINDOWS_EXPORT_ALL_SYMBOLS if(UNIX AND NOT APPLE) target_link_options(livekit PRIVATE "LINKER:--exclude-libs,ALL") endif() diff --git a/src/ffi_client.h b/src/ffi_client.h index 530fad74..97191586 100644 --- a/src/ffi_client.h +++ b/src/ffi_client.h @@ -63,9 +63,6 @@ extern "C" void LivekitFfiCallback(const uint8_t* buf, size_t len); // The FfiClient is used to communicate with the FFI interface of the Rust SDK // We use the generated protocol messages to facilitate the communication. -// -// Tagged LIVEKIT_INTERNAL_API: not part of the public ABI; exposed only so the -// in-tree test binaries can call into the singleton. class LIVEKIT_INTERNAL_API FfiClient { public: using ListenerId = int; diff --git a/src/lk_log.h b/src/lk_log.h index 38e67b0a..b5995744 100644 --- a/src/lk_log.h +++ b/src/lk_log.h @@ -28,11 +28,6 @@ namespace livekit::detail { /// Returns the shared "livekit" logger instance. /// The logger is created lazily on first access and lives until /// shutdownLogger() is called. Safe to call before initialize(). -/// -/// Tagged LIVEKIT_INTERNAL_API: this is not part of the public ABI but must -/// remain visible so that in-tree test binaries that include lk_log.h (and -/// therefore expand LK_LOG_* macros referencing this symbol) can resolve it -/// against liblivekit. LIVEKIT_INTERNAL_API std::shared_ptr getLogger(); /// Tears down the spdlog logger. Called by livekit::shutdown(). diff --git a/src/room_proto_converter.h b/src/room_proto_converter.h index 1ff09bd3..8f502f31 100644 --- a/src/room_proto_converter.h +++ b/src/room_proto_converter.h @@ -29,10 +29,6 @@ class RemoteParticipant; struct ByteStreamInfo; struct TextStreamInfo; -// All declarations below are tagged LIVEKIT_INTERNAL_API: they are NOT part of -// the public SDK ABI, but must be exported so that the in-tree test binaries -// (which include this header directly) can resolve them against liblivekit. - // --------- basic helper conversions --------- LIVEKIT_INTERNAL_API ConnectionQuality toConnectionQuality(proto::ConnectionQuality in); diff --git a/src/tests/consumer_link/test_consumer_link.cpp b/src/tests/consumer_link/test_consumer_link.cpp deleted file mode 100644 index 46d211c6..00000000 --- a/src/tests/consumer_link/test_consumer_link.cpp +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Copyright 2026 LiveKit - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -// Consumer-link canary -// -------------------- -// This translation unit is compiled into livekit_consumer_link_test, a target -// that *intentionally* mirrors how an external SDK consumer links against -// liblivekit: -// -// - Link line: livekit + GTest::gtest_main (no spdlog, -// no protobuf, -// no absl) -// - Include path: ${LIVEKIT_ROOT_DIR}/include (no src/, -// no generated/) -// - Defines: none of LIVEKIT_TEST_ACCESS / LIVEKIT_INTERNAL_API -// -// Two regressions we want this canary to catch fast, in every CI matrix -// entry, before the longer cpp-example-collection smoke runs: -// -// 1. A public header silently grows an `#include ` (or any -// other private dep). The compile fails because the private headers are -// not on the include path. -// 2. A public class loses its LIVEKIT_API tag and stops being exported -// from liblivekit. The link fails (Linux/macOS: undefined symbol; -// Windows: unresolved external) instead of slipping through to a -// runtime dlopen failure on a downstream user's machine. -// -// The deeper consumer-equivalent check (find_package(LiveKit CONFIG) against -// an installed SDK tree) lives in the cpp-example-collection smoke jobs. -// This canary is its lighter, in-tree, per-platform sibling. - -#include - -// Pull in every public header. The umbrella livekit.h covers most of the -// surface; the explicit includes below cover headers it does not transitively -// reference, so a regression in any single header is caught at compile time. -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -namespace { - -// Header inclusion alone is enough to exercise the compile path, but we still -// need to *call* a few exported symbols so the linker has to resolve them out -// of liblivekit. Pick a few that don't require server connectivity. -TEST(ConsumerLink, PublicEntryPointsResolve) { - ASSERT_TRUE(livekit::initialize(livekit::LogLevel::Warn)); - - livekit::setLogLevel(livekit::LogLevel::Warn); - EXPECT_EQ(livekit::getLogLevel(), livekit::LogLevel::Warn); - - livekit::setLogCallback(nullptr); - - EXPECT_FALSE(livekit::isTracingEnabled()); - - livekit::shutdown(); -} - -} // namespace diff --git a/src/trace/event_tracer.h b/src/trace/event_tracer.h index 088914a3..d99bc1cc 100644 --- a/src/trace/event_tracer.h +++ b/src/trace/event_tracer.h @@ -57,8 +57,7 @@ typedef void (*AddTraceEventPtr)(char phase, const unsigned char* category_enabl LIVEKIT_API void SetupEventTracer(GetCategoryEnabledPtr get_category_enabled_ptr, AddTraceEventPtr add_trace_event_ptr); // This class defines interface for the event tracing system to call -// internally. Tagged LIVEKIT_INTERNAL_API so the in-tree tracing tests -// (src/tests/unit/test_tracing.cpp) can resolve the static methods. +// internally. class LIVEKIT_INTERNAL_API EventTracer { public: static const unsigned char* GetCategoryEnabled(const char* name); diff --git a/src/video_utils.h b/src/video_utils.h index 0a9c4e89..a6ddd3ec 100644 --- a/src/video_utils.h +++ b/src/video_utils.h @@ -24,8 +24,6 @@ namespace livekit { // Video FFI Utils. -// Tagged LIVEKIT_INTERNAL_API: not part of the public ABI, but exposed so -// in-tree tests that include this header can link. LIVEKIT_INTERNAL_API proto::VideoBufferInfo toProto(const VideoFrame& frame); LIVEKIT_INTERNAL_API VideoFrame fromOwnedProto(const proto::OwnedVideoBuffer& owned); LIVEKIT_INTERNAL_API VideoFrame convertViaFfi(const VideoFrame& frame, VideoBufferType dst, bool flip_y); From 7414f2e37f5cb80bda6181810b17531a5a2b195d Mon Sep 17 00:00:00 2001 From: Alan George Date: Mon, 11 May 2026 09:29:03 -0600 Subject: [PATCH 11/24] Move check to tests build --- .../scripts}/check_no_private_symbols.py | 0 .github/workflows/builds.yml | 43 ------------------ .github/workflows/tests.yml | 45 +++++++++++++++++++ AGENTS.md | 24 ++++++++++ src/tests/CMakeLists.txt | 24 ---------- 5 files changed, 69 insertions(+), 67 deletions(-) rename {scripts => .github/scripts}/check_no_private_symbols.py (100%) diff --git a/scripts/check_no_private_symbols.py b/.github/scripts/check_no_private_symbols.py similarity index 100% rename from scripts/check_no_private_symbols.py rename to .github/scripts/check_no_private_symbols.py diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 1f353973..e79bcde8 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -184,49 +184,6 @@ jobs: shell: pwsh run: ${{ matrix.build_cmd }} - # ---------- Verify exported ABI: no spdlog/fmt/protobuf/absl leaks ---------- - - name: Setup Python - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v6.0.0 - with: - python-version: '3.x' - - - name: Verify no private symbols leak (Unix) - if: runner.os != 'Windows' - shell: bash - run: | - set -eux - libdir="${{ matrix.build_dir }}/lib" - if [[ "$RUNNER_OS" == "macOS" ]]; then - lib="${libdir}/liblivekit.dylib" - if [[ ! -f "${lib}" ]]; then - lib="${{ matrix.build_dir }}/bin/liblivekit.dylib" - fi - else - lib="${libdir}/liblivekit.so" - if [[ ! -f "${lib}" ]]; then - lib="${{ matrix.build_dir }}/bin/liblivekit.so" - fi - fi - python3 scripts/check_no_private_symbols.py "${lib}" - - - name: Verify no private symbols leak (Windows) - if: runner.os == 'Windows' - shell: pwsh - run: | - $candidates = @( - "${{ matrix.build_dir }}/bin/livekit.dll", - "${{ matrix.build_dir }}/lib/livekit.dll" - ) - $lib = $null - foreach ($p in $candidates) { - if (Test-Path -LiteralPath $p) { $lib = $p; break } - } - if ($null -eq $lib) { - Write-Error "livekit.dll not found in any of: $($candidates -join ', ')" - exit 1 - } - python scripts/check_no_private_symbols.py "$lib" - # ---------- Smoke test cpp-example-collection binaries ---------- # Built under cpp-example-collection-build/ (not build-dir/bin). Visual Studio # multi-config places executables in per-target Release/ (or Debug/) subdirs. diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 95f2b753..ee80b658 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,6 +14,7 @@ on: - vcpkg.json - .token_helpers/** - .github/workflows/tests.yml + - .github/scripts/check_no_private_symbols.py pull_request: branches: ["main"] paths: @@ -27,6 +28,7 @@ on: - vcpkg.json - .token_helpers/** - .github/workflows/tests.yml + - .github/scripts/check_no_private_symbols.py workflow_dispatch: permissions: @@ -162,6 +164,49 @@ jobs: shell: pwsh run: ${{ matrix.build_cmd }} + # ---------- Verify exported ABI: no spdlog/fmt/protobuf/absl leaks ---------- + - name: Setup Python + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v6.0.0 + with: + python-version: '3.x' + + - name: Symbol leak check + if: runner.os != 'Windows' + shell: bash + run: | + set -eux + libdir="build-release/lib" + if [[ "$RUNNER_OS" == "macOS" ]]; then + lib="${libdir}/liblivekit.dylib" + if [[ ! -f "${lib}" ]]; then + lib="build-release/bin/liblivekit.dylib" + fi + else + lib="${libdir}/liblivekit.so" + if [[ ! -f "${lib}" ]]; then + lib="build-release/bin/liblivekit.so" + fi + fi + python3 .github/scripts/check_no_private_symbols.py "${lib}" + + - name: Symbol leak check + if: runner.os == 'Windows' + shell: pwsh + run: | + $candidates = @( + "build-release/bin/livekit.dll", + "build-release/lib/livekit.dll" + ) + $lib = $null + foreach ($p in $candidates) { + if (Test-Path -LiteralPath $p) { $lib = $p; break } + } + if ($null -eq $lib) { + Write-Error "livekit.dll not found in any of: $($candidates -join ', ')" + exit 1 + } + python .github/scripts/check_no_private_symbols.py "$lib" + # ---------- Run unit tests ---------- - name: Run unit tests (Unix) if: runner.os != 'Windows' diff --git a/AGENTS.md b/AGENTS.md index a7ebab47..318a5376 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -185,6 +185,24 @@ Rules for new code: - On Windows, `WINDOWS_EXPORT_ALL_SYMBOLS` is **deliberately disabled** for the `livekit` target. Use `LIVEKIT_API` to export. +The exported ABI is enforced by `.github/scripts/check_no_private_symbols.py`, +run from the `tests.yml` "Symbol leak check" CI step. The script fails if +`nm`/`dumpbin` reports any exported symbol matching a forbidden substring +(currently `spdlog::`, `fmt::v`, `google::protobuf`, `absl::`). To run it +locally, point it at the built shared library: + +```bash +python3 .github/scripts/check_no_private_symbols.py \ + build-release/lib/liblivekit.dylib # or .so / livekit.dll +``` + +**When adding a new third-party library or vendored dependency to the SDK, +update `.github/scripts/check_no_private_symbols.py` to add a forbidden +substring pattern for the new dependency's namespace/symbol prefix.** The +denylist is intentionally explicit — a new dep that leaks symbols will +otherwise silently pollute `liblivekit.{so,dylib,dll}` and clash at runtime +with the same library loaded elsewhere in the host process. + ### Error Handling - Use `LK_LOG_WARN` for non-fatal unexpected conditions. @@ -226,6 +244,12 @@ Adhere to clang-tidy checks configured in `.clang-tidy`. Run `./scripts/clang-ti | client-sdk-rust | Build-time | Git submodule, built via cargo during CMake build | | Google Test | Test only | FetchContent in `src/tests/CMakeLists.txt` | +When adding a new private/vendored dependency to this table, also add a +forbidden symbol pattern for it to +`.github/scripts/check_no_private_symbols.py` so the "Symbol leak check" +CI step will fail loudly if its symbols escape the public ABI of +`liblivekit`. + ## Testing Tests are under `src/tests/` using Google Test: diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 563fd92a..2bd0c09e 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -331,27 +331,3 @@ endif() if(TARGET livekit_stress_tests) add_dependencies(run_all_tests livekit_stress_tests) endif() - -# ============================================================================ -# ABI verification: liblivekit must not re-export private dependency symbols -# ============================================================================ -# -# scripts/check_no_private_symbols.py asserts that no spdlog/fmt/protobuf/absl -# symbols are visible in the dynamic symbol table of the built shared library. -# This catches regressions where a new TU is added without -fvisibility=hidden -# or a public header forgets to gate exports through LIVEKIT_API. -find_package(Python3 COMPONENTS Interpreter) -if(Python3_Interpreter_FOUND) - add_test( - NAME livekit_no_private_symbols - COMMAND ${Python3_EXECUTABLE} - "${LIVEKIT_ROOT_DIR}/scripts/check_no_private_symbols.py" - "$" - ) - set_tests_properties(livekit_no_private_symbols PROPERTIES - LABELS "abi" - ) -else() - message(WARNING - "Python3 not found; skipping livekit_no_private_symbols ABI test") -endif() From 60845b496ff51c46ab9de92e0ec4098cc84f3259 Mon Sep 17 00:00:00 2001 From: Alan George Date: Mon, 11 May 2026 10:47:30 -0600 Subject: [PATCH 12/24] CMake cleanup from self-review --- bridge/CMakeLists.txt | 12 ++++++++++++ cmake/protobuf.cmake | 38 ++------------------------------------ 2 files changed, 14 insertions(+), 36 deletions(-) diff --git a/bridge/CMakeLists.txt b/bridge/CMakeLists.txt index 549e86cb..2a697b5d 100644 --- a/bridge/CMakeLists.txt +++ b/bridge/CMakeLists.txt @@ -14,8 +14,20 @@ add_library(livekit_bridge SHARED src/rpc_controller.h ) +# Workaround for bridge deprecation: handle visbility differently to avoid +# major LIVEKIT_API changes across its code, and not affecting the SDK if(WIN32) set_target_properties(livekit_bridge PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS ON) +else() + set_target_properties(livekit_bridge PROPERTIES + CXX_VISIBILITY_PRESET default + C_VISIBILITY_PRESET default + VISIBILITY_INLINES_HIDDEN OFF + ) + target_compile_options(livekit_bridge INTERFACE + -fvisibility=default + -fno-visibility-inlines-hidden + ) endif() target_include_directories(livekit_bridge diff --git a/cmake/protobuf.cmake b/cmake/protobuf.cmake index 860cb23f..8132a084 100644 --- a/cmake/protobuf.cmake +++ b/cmake/protobuf.cmake @@ -120,22 +120,12 @@ set(ABSL_ENABLE_INSTALL OFF CACHE BOOL "" FORCE) set(utf8_range_ENABLE_INSTALL OFF CACHE BOOL "" FORCE) # Force hidden visibility on every target created by the FetchContent -# subprojects below. Setting these as CACHE BOOL FORCE before +# subprojects below. Setting these as CACHE BOOL FORCE before # FetchContent_MakeAvailable propagates to every add_library() call inside # absl / protobuf / utf8_range, so their .o files are compiled with -# -fvisibility=hidden -fvisibility-inlines-hidden. Without this, even though +# -fvisibility=hidden -fvisibility-inlines-hidden. Without this, even though # liblivekit itself is hidden, absl's static archives would carry default- # visibility symbols that the linker happily re-exports. -# -# We snapshot the previous values so we can restore them after -# FetchContent_MakeAvailable; otherwise sibling components like the bridge -# library would also inherit hidden visibility, which would break the bridge -# unit tests that need access to internal bridge symbols. -set(_livekit_prev_cxx_visibility "${CMAKE_CXX_VISIBILITY_PRESET}") -set(_livekit_prev_c_visibility "${CMAKE_C_VISIBILITY_PRESET}") -set(_livekit_prev_inlines_hidden "${CMAKE_VISIBILITY_INLINES_HIDDEN}") -set(_livekit_prev_pic "${CMAKE_POSITION_INDEPENDENT_CODE}") - set(CMAKE_CXX_VISIBILITY_PRESET hidden CACHE STRING "" FORCE) set(CMAKE_C_VISIBILITY_PRESET hidden CACHE STRING "" FORCE) set(CMAKE_VISIBILITY_INLINES_HIDDEN ON CACHE BOOL "" FORCE) @@ -184,30 +174,6 @@ endif() # Now make protobuf available. FetchContent_MakeAvailable(livekit_protobuf) -# Restore the prior visibility cache values so the rest of the build (e.g. -# the deprecated bridge target and its tests) can use the project default -# visibility. The livekit target itself sets CXX_VISIBILITY_PRESET hidden -# explicitly via target properties, so it does not depend on these globals. -if(_livekit_prev_cxx_visibility STREQUAL "") - unset(CMAKE_CXX_VISIBILITY_PRESET CACHE) -else() - set(CMAKE_CXX_VISIBILITY_PRESET "${_livekit_prev_cxx_visibility}" - CACHE STRING "" FORCE) -endif() -if(_livekit_prev_c_visibility STREQUAL "") - unset(CMAKE_C_VISIBILITY_PRESET CACHE) -else() - set(CMAKE_C_VISIBILITY_PRESET "${_livekit_prev_c_visibility}" - CACHE STRING "" FORCE) -endif() -if(_livekit_prev_inlines_hidden STREQUAL "") - unset(CMAKE_VISIBILITY_INLINES_HIDDEN CACHE) -else() - set(CMAKE_VISIBILITY_INLINES_HIDDEN "${_livekit_prev_inlines_hidden}" - CACHE BOOL "" FORCE) -endif() -# Position-independent code remains ON globally (it was forced ON anyway). - # Protobuf targets: modern protobuf exports protobuf::protoc etc. if(TARGET protobuf::protoc) set(Protobuf_PROTOC_EXECUTABLE "$" CACHE STRING "protoc (vendored)" FORCE) From 0728dddecd9632d65eb6e52cb8f01442c20bf928 Mon Sep 17 00:00:00 2001 From: Alan George Date: Mon, 11 May 2026 10:55:56 -0600 Subject: [PATCH 13/24] Test stage cleanup --- .github/workflows/tests.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ee80b658..1b040632 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -164,14 +164,15 @@ jobs: shell: pwsh run: ${{ matrix.build_cmd }} - # ---------- Verify exported ABI: no spdlog/fmt/protobuf/absl leaks ---------- + # ---------- Verify exported ABI: no third-party symbol leaks ---------- - name: Setup Python uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v6.0.0 with: python-version: '3.x' - - name: Symbol leak check + - name: Symbol leak check (Unix) if: runner.os != 'Windows' + continue-on-error: true # Still fail the build, just don't block remaining tests running shell: bash run: | set -eux @@ -189,8 +190,9 @@ jobs: fi python3 .github/scripts/check_no_private_symbols.py "${lib}" - - name: Symbol leak check + - name: Symbol leak check (Windows) if: runner.os == 'Windows' + continue-on-error: true shell: pwsh run: | $candidates = @( From a68e9845d2211b5274823cd8b38de2b1149c7b1d Mon Sep 17 00:00:00 2001 From: Alan George Date: Mon, 11 May 2026 12:42:04 -0600 Subject: [PATCH 14/24] Switch to #pragma once --- .github/workflows/tests.yml | 2 +- include/livekit/export.h | 63 +++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 include/livekit/export.h diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 1b040632..7d8d65ad 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -192,7 +192,7 @@ jobs: - name: Symbol leak check (Windows) if: runner.os == 'Windows' - continue-on-error: true + continue-on-error: true # Still fail the build, just don't block remaining tests running shell: pwsh run: | $candidates = @( diff --git a/include/livekit/export.h b/include/livekit/export.h new file mode 100644 index 00000000..c7921fb1 --- /dev/null +++ b/include/livekit/export.h @@ -0,0 +1,63 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +// LIVEKIT_API marks a symbol as part of the public ABI of liblivekit. +// +// On Unix, the SDK is built with -fvisibility=hidden / -fvisibility-inlines-hidden, +// so every symbol defaults to hidden. LIVEKIT_API re-exposes the symbol with +// default visibility. Consumers see no annotation (just a normal declaration). +// +// On Windows, the SDK is built without WINDOWS_EXPORT_ALL_SYMBOLS, so symbols +// must be explicitly tagged with __declspec(dllexport) when building the SDK +// and __declspec(dllimport) when consuming it. LIVEKIT_BUILDING_SDK is defined +// only when compiling the SDK itself (set in CMakeLists.txt). + +#if defined(_WIN32) +#if defined(LIVEKIT_BUILDING_SDK) +#define LIVEKIT_API __declspec(dllexport) +#else +#define LIVEKIT_API __declspec(dllimport) +#endif +#else +#if defined(LIVEKIT_BUILDING_SDK) +#define LIVEKIT_API __attribute__((visibility("default"))) +#else +#define LIVEKIT_API +#endif +#endif + +// LIVEKIT_INTERNAL_API marks a symbol that is NOT part of the public ABI but +// must remain visible so that the in-tree test binaries (which link against +// the same shared library) can resolve it. +// +// External consumers must not rely on LIVEKIT_INTERNAL_API symbols; they may +// change or disappear without notice. +// +// On Windows, internal symbols are exported the same way as public ones +// because tests link via the standard import library; on Unix, hidden +// visibility is overridden for these specific symbols only. + +#if defined(_WIN32) +#define LIVEKIT_INTERNAL_API LIVEKIT_API +#else +#if defined(LIVEKIT_BUILDING_SDK) +#define LIVEKIT_INTERNAL_API __attribute__((visibility("default"))) +#else +#define LIVEKIT_INTERNAL_API +#endif +#endif From fba56fbf92eeec6ed890e04a3523ff565a19745a Mon Sep 17 00:00:00 2001 From: Alan George Date: Mon, 11 May 2026 13:19:54 -0600 Subject: [PATCH 15/24] Initial set of remaining unit tests --- src/tests/unit/test_audio_source.cpp | 44 ++++++++++++ src/tests/unit/test_data_stream.cpp | 60 ++++++++++++++++ src/tests/unit/test_data_track_error.cpp | 43 +++++++++++ src/tests/unit/test_data_track_frame.cpp | 50 +++++++++++++ src/tests/unit/test_data_track_info.cpp | 37 ++++++++++ src/tests/unit/test_e2ee.cpp | 46 ++++++++++++ src/tests/unit/test_ffi_handle.cpp | 62 ++++++++++++++++ src/tests/unit/test_local_tracks.cpp | 54 ++++++++++++++ src/tests/unit/test_remote_audio_track.cpp | 38 ++++++++++ src/tests/unit/test_remote_video_track.cpp | 38 ++++++++++ src/tests/unit/test_result.cpp | 62 ++++++++++++++++ src/tests/unit/test_room_event_types.cpp | 57 +++++++++++++++ src/tests/unit/test_rpc_error.cpp | 54 ++++++++++++++ src/tests/unit/test_stats.cpp | 83 ++++++++++++++++++++++ src/tests/unit/test_streams.cpp | 59 +++++++++++++++ src/tests/unit/test_video_frame.cpp | 51 +++++++++++++ src/tests/unit/test_video_source.cpp | 44 ++++++++++++ 17 files changed, 882 insertions(+) create mode 100644 src/tests/unit/test_audio_source.cpp create mode 100644 src/tests/unit/test_data_stream.cpp create mode 100644 src/tests/unit/test_data_track_error.cpp create mode 100644 src/tests/unit/test_data_track_frame.cpp create mode 100644 src/tests/unit/test_data_track_info.cpp create mode 100644 src/tests/unit/test_e2ee.cpp create mode 100644 src/tests/unit/test_ffi_handle.cpp create mode 100644 src/tests/unit/test_local_tracks.cpp create mode 100644 src/tests/unit/test_remote_audio_track.cpp create mode 100644 src/tests/unit/test_remote_video_track.cpp create mode 100644 src/tests/unit/test_result.cpp create mode 100644 src/tests/unit/test_room_event_types.cpp create mode 100644 src/tests/unit/test_rpc_error.cpp create mode 100644 src/tests/unit/test_stats.cpp create mode 100644 src/tests/unit/test_streams.cpp create mode 100644 src/tests/unit/test_video_frame.cpp create mode 100644 src/tests/unit/test_video_source.cpp diff --git a/src/tests/unit/test_audio_source.cpp b/src/tests/unit/test_audio_source.cpp new file mode 100644 index 00000000..8f931b16 --- /dev/null +++ b/src/tests/unit/test_audio_source.cpp @@ -0,0 +1,44 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include + +namespace livekit::test { + +class AudioSourceTest : public ::testing::Test { +protected: + void SetUp() override { livekit::initialize(livekit::LogLevel::Info, livekit::LogSink::kConsole); } + void TearDown() override { livekit::shutdown(); } +}; + +TEST_F(AudioSourceTest, ConstructAndQueryProperties) { + AudioSource source(48000, 1); + EXPECT_EQ(source.sample_rate(), 48000); + EXPECT_EQ(source.num_channels(), 1); + EXPECT_NE(source.ffi_handle_id(), 0u); + EXPECT_DOUBLE_EQ(source.queuedDuration(), 0.0); +} + +TEST_F(AudioSourceTest, ClearQueueIsSafeOnFreshSource) { + AudioSource source(48000, 2, /*queue_size_ms=*/0); + source.clearQueue(); + EXPECT_DOUBLE_EQ(source.queuedDuration(), 0.0); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_data_stream.cpp b/src/tests/unit/test_data_stream.cpp new file mode 100644 index 00000000..14833025 --- /dev/null +++ b/src/tests/unit/test_data_stream.cpp @@ -0,0 +1,60 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include + +namespace livekit::test { + +TEST(DataStreamTest, TextStreamReaderConstructAndInfo) { + TextStreamInfo info; + info.stream_id = "stream-1"; + info.mime_type = "text/plain"; + info.topic = "chat"; + info.timestamp = 42; + info.attachments = {"a.txt"}; + + TextStreamReader reader(info); + EXPECT_EQ(reader.info().stream_id, "stream-1"); + EXPECT_EQ(reader.info().mime_type, "text/plain"); + EXPECT_EQ(reader.info().topic, "chat"); + EXPECT_EQ(reader.info().timestamp, 42); + ASSERT_EQ(reader.info().attachments.size(), 1u); + EXPECT_EQ(reader.info().attachments.front(), "a.txt"); +} + +TEST(DataStreamTest, ByteStreamReaderConstructAndInfo) { + ByteStreamInfo info; + info.stream_id = "stream-2"; + info.mime_type = "application/octet-stream"; + info.topic = "files"; + info.name = "data.bin"; + + ByteStreamReader reader(info); + EXPECT_EQ(reader.info().stream_id, "stream-2"); + EXPECT_EQ(reader.info().name, "data.bin"); +} + +TEST(DataStreamTest, WriterTypesAreDerivedFromBase) { + static_assert(std::is_base_of_v); + static_assert(std::is_base_of_v); + EXPECT_GT(kStreamChunkSize, 0u); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_data_track_error.cpp b/src/tests/unit/test_data_track_error.cpp new file mode 100644 index 00000000..9d4708dd --- /dev/null +++ b/src/tests/unit/test_data_track_error.cpp @@ -0,0 +1,43 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include "data_track.pb.h" + +namespace livekit::test { + +TEST(DataTrackErrorTest, PublishErrorFromEmptyProto) { + proto::PublishDataTrackError proto_err; + PublishDataTrackError err = PublishDataTrackError::fromProto(proto_err); + EXPECT_EQ(err.code, PublishDataTrackErrorCode::UNKNOWN); +} + +TEST(DataTrackErrorTest, TryPushErrorFromEmptyProto) { + proto::LocalDataTrackTryPushError proto_err; + LocalDataTrackTryPushError err = LocalDataTrackTryPushError::fromProto(proto_err); + EXPECT_EQ(err.code, LocalDataTrackTryPushErrorCode::UNKNOWN); +} + +TEST(DataTrackErrorTest, SubscribeErrorFromEmptyProto) { + proto::SubscribeDataTrackError proto_err; + SubscribeDataTrackError err = SubscribeDataTrackError::fromProto(proto_err); + EXPECT_EQ(err.code, SubscribeDataTrackErrorCode::UNKNOWN); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_data_track_frame.cpp b/src/tests/unit/test_data_track_frame.cpp new file mode 100644 index 00000000..82d05f0a --- /dev/null +++ b/src/tests/unit/test_data_track_frame.cpp @@ -0,0 +1,50 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include + +#include + +#include "data_track.pb.h" + +namespace livekit::test { + +TEST(DataTrackFrameTest, DefaultConstructed) { + DataTrackFrame frame; + EXPECT_TRUE(frame.payload.empty()); + EXPECT_FALSE(frame.user_timestamp.has_value()); +} + +TEST(DataTrackFrameTest, PayloadAndTimestampConstructor) { + std::vector payload{1, 2, 3}; + DataTrackFrame frame(std::move(payload), 12345u); + ASSERT_EQ(frame.payload.size(), 3u); + EXPECT_EQ(frame.payload[0], 1u); + ASSERT_TRUE(frame.user_timestamp.has_value()); + EXPECT_EQ(*frame.user_timestamp, 12345u); +} + +TEST(DataTrackFrameTest, FromOwnedInfoEmptyProto) { + proto::DataTrackFrame proto_frame; + DataTrackFrame frame = DataTrackFrame::fromOwnedInfo(proto_frame); + EXPECT_TRUE(frame.payload.empty()); + EXPECT_FALSE(frame.user_timestamp.has_value()); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_data_track_info.cpp b/src/tests/unit/test_data_track_info.cpp new file mode 100644 index 00000000..d1e5619c --- /dev/null +++ b/src/tests/unit/test_data_track_info.cpp @@ -0,0 +1,37 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +namespace livekit::test { + +TEST(DataTrackInfoTest, DefaultConstructed) { + DataTrackInfo info; + EXPECT_TRUE(info.name.empty()); + EXPECT_TRUE(info.sid.empty()); + EXPECT_FALSE(info.uses_e2ee); +} + +TEST(DataTrackInfoTest, AggregateInitialization) { + DataTrackInfo info{"name", "sid", true}; + EXPECT_EQ(info.name, "name"); + EXPECT_EQ(info.sid, "sid"); + EXPECT_TRUE(info.uses_e2ee); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_e2ee.cpp b/src/tests/unit/test_e2ee.cpp new file mode 100644 index 00000000..13006edd --- /dev/null +++ b/src/tests/unit/test_e2ee.cpp @@ -0,0 +1,46 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include + +namespace livekit::test { + +TEST(E2EETest, KeyProviderOptionsDefaults) { + KeyProviderOptions options; + EXPECT_FALSE(options.shared_key.has_value()); + EXPECT_FALSE(options.ratchet_salt.empty()); + EXPECT_EQ(options.ratchet_window_size, kDefaultRatchetWindowSize); + EXPECT_EQ(options.failure_tolerance, kDefaultFailureTolerance); +} + +TEST(E2EETest, E2EEOptionsDefaults) { + E2EEOptions options; + EXPECT_EQ(options.encryption_type, EncryptionType::GCM); +} + +TEST(E2EETest, FrameCryptorAccessors) { + E2EEManager::FrameCryptor cryptor(/*room_handle=*/0, "alice", /*key_index=*/3, + /*enabled=*/true); + EXPECT_EQ(cryptor.participantIdentity(), "alice"); + EXPECT_EQ(cryptor.keyIndex(), 3); + EXPECT_TRUE(cryptor.enabled()); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_ffi_handle.cpp b/src/tests/unit/test_ffi_handle.cpp new file mode 100644 index 00000000..5d62d94a --- /dev/null +++ b/src/tests/unit/test_ffi_handle.cpp @@ -0,0 +1,62 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include + +#include + +namespace livekit::test { + +TEST(FfiHandleTest, DefaultConstructedIsInvalid) { + FfiHandle handle; + EXPECT_EQ(handle.get(), 0u); + EXPECT_FALSE(handle.valid()); + EXPECT_FALSE(static_cast(handle)); +} + +TEST(FfiHandleTest, ExplicitZeroIsInvalid) { + FfiHandle handle(0); + EXPECT_FALSE(handle.valid()); +} + +TEST(FfiHandleTest, ResetToZeroIsSafe) { + FfiHandle handle; + handle.reset(); + EXPECT_FALSE(handle.valid()); +} + +TEST(FfiHandleTest, ReleaseReturnsCurrentHandleAndClears) { + FfiHandle handle; + const std::uintptr_t released = handle.release(); + EXPECT_EQ(released, 0u); + EXPECT_FALSE(handle.valid()); +} + +TEST(FfiHandleTest, MoveTransfersOwnership) { + FfiHandle src; + FfiHandle dst(std::move(src)); + EXPECT_FALSE(dst.valid()); + EXPECT_FALSE(src.valid()); // NOLINT(bugprone-use-after-move) + + FfiHandle assigned; + assigned = std::move(dst); + EXPECT_FALSE(assigned.valid()); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_local_tracks.cpp b/src/tests/unit/test_local_tracks.cpp new file mode 100644 index 00000000..b139d0d4 --- /dev/null +++ b/src/tests/unit/test_local_tracks.cpp @@ -0,0 +1,54 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include +#include + +#include +#include +#include +#include + +namespace livekit::test { + +TEST(LocalTracksTest, LocalAudioTrackFactoryIsAddressable) { + using FactoryT = std::shared_ptr (*)(const std::string&, const std::shared_ptr&); + FactoryT factory = &LocalAudioTrack::createLocalAudioTrack; + EXPECT_NE(factory, nullptr); +} + +TEST(LocalTracksTest, LocalVideoTrackFactoryIsAddressable) { + using FactoryT = std::shared_ptr (*)(const std::string&, const std::shared_ptr&); + FactoryT factory = &LocalVideoTrack::createLocalVideoTrack; + EXPECT_NE(factory, nullptr); +} + +TEST(LocalTracksTest, LocalTrackInheritsFromTrack) { + static_assert(std::is_base_of_v); + static_assert(std::is_base_of_v); + SUCCEED(); +} + +TEST(LocalTracksTest, LocalDataTrackIsNoncopyable) { + static_assert(!std::is_copy_constructible_v); + static_assert(!std::is_copy_assignable_v); + SUCCEED(); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_remote_audio_track.cpp b/src/tests/unit/test_remote_audio_track.cpp new file mode 100644 index 00000000..520f267f --- /dev/null +++ b/src/tests/unit/test_remote_audio_track.cpp @@ -0,0 +1,38 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include "track.pb.h" + +namespace livekit::test { + +TEST(RemoteAudioTrackTest, ConstructFromEmptyOwnedTrack) { + proto::OwnedTrack owned; + RemoteAudioTrack track(owned); + + EXPECT_TRUE(track.sid().empty()); + EXPECT_TRUE(track.name().empty()); + EXPECT_TRUE(track.remote()); + EXPECT_FALSE(track.has_handle()); + + const std::string description = track.to_string(); + EXPECT_FALSE(description.empty()); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_remote_video_track.cpp b/src/tests/unit/test_remote_video_track.cpp new file mode 100644 index 00000000..a3e1e875 --- /dev/null +++ b/src/tests/unit/test_remote_video_track.cpp @@ -0,0 +1,38 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include "track.pb.h" + +namespace livekit::test { + +TEST(RemoteVideoTrackTest, ConstructFromEmptyOwnedTrack) { + proto::OwnedTrack owned; + RemoteVideoTrack track(owned); + + EXPECT_TRUE(track.sid().empty()); + EXPECT_TRUE(track.name().empty()); + EXPECT_TRUE(track.remote()); + EXPECT_FALSE(track.has_handle()); + + const std::string description = track.to_string(); + EXPECT_FALSE(description.empty()); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_result.cpp b/src/tests/unit/test_result.cpp new file mode 100644 index 00000000..0a191d5d --- /dev/null +++ b/src/tests/unit/test_result.cpp @@ -0,0 +1,62 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include + +#include + +namespace livekit::test { + +struct StubError { + int code = 0; + std::string message; +}; + +TEST(ResultTest, ValueResultSuccess) { + auto r = Result::success(42); + ASSERT_TRUE(r.ok()); + EXPECT_FALSE(r.has_error()); + EXPECT_TRUE(static_cast(r)); + EXPECT_EQ(r.value(), 42); +} + +TEST(ResultTest, ValueResultFailure) { + auto r = Result::failure(StubError{7, "nope"}); + ASSERT_FALSE(r.ok()); + EXPECT_TRUE(r.has_error()); + EXPECT_EQ(r.error().code, 7); + EXPECT_EQ(r.error().message, "nope"); +} + +TEST(ResultTest, VoidResultSuccess) { + auto r = Result::success(); + ASSERT_TRUE(r.ok()); + EXPECT_FALSE(r.has_error()); + r.value(); +} + +TEST(ResultTest, VoidResultFailure) { + auto r = Result::failure(StubError{1, "bad"}); + ASSERT_FALSE(r.ok()); + EXPECT_EQ(r.error().code, 1); + StubError moved = std::move(r).error(); + EXPECT_EQ(moved.message, "bad"); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_room_event_types.cpp b/src/tests/unit/test_room_event_types.cpp new file mode 100644 index 00000000..89a10650 --- /dev/null +++ b/src/tests/unit/test_room_event_types.cpp @@ -0,0 +1,57 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +namespace livekit::test { + +TEST(RoomEventTypesTest, EnumValuesAreReachable) { + EXPECT_NE(ConnectionState::Connected, ConnectionState::Disconnected); + EXPECT_NE(DataPacketKind::Reliable, DataPacketKind::Lossy); + EXPECT_NE(EncryptionState::New, EncryptionState::Ok); + EXPECT_NE(DisconnectReason::Unknown, DisconnectReason::ClientInitiated); + EXPECT_NE(ConnectionQuality::Poor, ConnectionQuality::Excellent); +} + +TEST(RoomEventTypesTest, RoomInfoDataDefaults) { + RoomInfoData info; + EXPECT_TRUE(info.name.empty()); + EXPECT_TRUE(info.metadata.empty()); + EXPECT_FALSE(info.active_recording); + EXPECT_EQ(info.creation_time, 0); +} + +TEST(RoomEventTypesTest, AttributeEntryConstruction) { + AttributeEntry entry("k", "v"); + EXPECT_EQ(entry.key, "k"); + EXPECT_EQ(entry.value, "v"); +} + +TEST(RoomEventTypesTest, TrackPublishOptionsDefaults) { + TrackPublishOptions options; + EXPECT_FALSE(options.packet_trailer_features.user_timestamp); + EXPECT_FALSE(options.packet_trailer_features.frame_id); +} + +TEST(RoomEventTypesTest, UserPacketDataDefaults) { + UserPacketData packet; + EXPECT_TRUE(packet.data.empty()); + EXPECT_FALSE(packet.topic.has_value()); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_rpc_error.cpp b/src/tests/unit/test_rpc_error.cpp new file mode 100644 index 00000000..8467719a --- /dev/null +++ b/src/tests/unit/test_rpc_error.cpp @@ -0,0 +1,54 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +namespace livekit::test { + +TEST(RpcErrorTest, NumericConstructor) { + RpcError err(1234, "boom", "{\"detail\":\"x\"}"); + EXPECT_EQ(err.code(), 1234u); + EXPECT_EQ(err.message(), "boom"); + EXPECT_EQ(err.data(), "{\"detail\":\"x\"}"); +} + +TEST(RpcErrorTest, EnumConstructor) { + RpcError err(RpcError::ErrorCode::APPLICATION_ERROR, "handler failed"); + EXPECT_EQ(err.code(), static_cast(RpcError::ErrorCode::APPLICATION_ERROR)); + EXPECT_EQ(err.message(), "handler failed"); + EXPECT_TRUE(err.data().empty()); +} + +TEST(RpcErrorTest, BuiltInFactory) { + RpcError err = RpcError::builtIn(RpcError::ErrorCode::CONNECTION_TIMEOUT); + EXPECT_EQ(err.code(), static_cast(RpcError::ErrorCode::CONNECTION_TIMEOUT)); + EXPECT_FALSE(err.message().empty()); + EXPECT_TRUE(err.data().empty()); +} + +TEST(RpcErrorTest, ThrowableAsRuntimeError) { + try { + throw RpcError(RpcError::ErrorCode::SEND_FAILED, "send failed"); + } catch (const std::runtime_error& e) { + SUCCEED() << "caught as std::runtime_error: " << e.what(); + return; + } + FAIL() << "RpcError did not propagate as std::runtime_error"; +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_stats.cpp b/src/tests/unit/test_stats.cpp new file mode 100644 index 00000000..5d4b39b9 --- /dev/null +++ b/src/tests/unit/test_stats.cpp @@ -0,0 +1,83 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include + +#include "stats.pb.h" + +namespace livekit::test { + +TEST(StatsTest, ScalarFromProtoOverloads) { + proto::RtcStatsData data; + proto::CodecStats codec; + proto::RtpStreamStats rtp; + proto::ReceivedRtpStreamStats received; + proto::InboundRtpStreamStats inbound; + proto::SentRtpStreamStats sent; + proto::OutboundRtpStreamStats outbound; + proto::RemoteInboundRtpStreamStats remote_inbound; + proto::RemoteOutboundRtpStreamStats remote_outbound; + proto::MediaSourceStats media_source; + proto::AudioSourceStats audio_source; + proto::VideoSourceStats video_source; + proto::AudioPlayoutStats audio_playout; + proto::PeerConnectionStats peer; + proto::DataChannelStats data_channel; + proto::TransportStats transport; + proto::CandidatePairStats candidate_pair; + proto::IceCandidateStats ice_candidate; + proto::CertificateStats certificate; + proto::StreamStats stream; + + (void)fromProto(data); + (void)fromProto(codec); + (void)fromProto(rtp); + (void)fromProto(received); + (void)fromProto(inbound); + (void)fromProto(sent); + (void)fromProto(outbound); + (void)fromProto(remote_inbound); + (void)fromProto(remote_outbound); + (void)fromProto(media_source); + (void)fromProto(audio_source); + (void)fromProto(video_source); + (void)fromProto(audio_playout); + (void)fromProto(peer); + (void)fromProto(data_channel); + (void)fromProto(transport); + (void)fromProto(candidate_pair); + (void)fromProto(ice_candidate); + (void)fromProto(certificate); + (void)fromProto(stream); +} + +TEST(StatsTest, HighLevelRtcStatsFromProto) { + proto::RtcStats rtc; + RtcStats stats = fromProto(rtc); + (void)stats; +} + +TEST(StatsTest, RepeatedRtcStatsFromProto) { + std::vector protos(2); + std::vector stats = fromProto(protos); + EXPECT_EQ(stats.size(), 2u); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_streams.cpp b/src/tests/unit/test_streams.cpp new file mode 100644 index 00000000..24fcaeb3 --- /dev/null +++ b/src/tests/unit/test_streams.cpp @@ -0,0 +1,59 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include +#include +#include +#include + +namespace livekit::test { + +TEST(StreamOptionsTest, AudioStreamOptionsDefaults) { + AudioStream::Options options; + EXPECT_EQ(options.capacity, 0u); + EXPECT_TRUE(options.noise_cancellation_module.empty()); + EXPECT_TRUE(options.noise_cancellation_options_json.empty()); +} + +TEST(StreamOptionsTest, VideoStreamOptionsDefaults) { + VideoStream::Options options; + EXPECT_EQ(options.capacity, 0u); + EXPECT_EQ(options.format, VideoBufferType::RGBA); +} + +TEST(StreamOptionsTest, DataTrackStreamOptionsDefaults) { + DataTrackStream::Options options; + EXPECT_FALSE(options.buffer_size.has_value()); +} + +TEST(StreamOptionsTest, AudioFrameEventIsConstructible) { + AudioFrameEvent ev; + (void)ev; + SUCCEED(); +} + +TEST(StreamOptionsTest, VideoFrameEventIsConstructible) { + VideoFrameEvent ev; + ev.timestamp_us = 0; + ev.rotation = VideoRotation::VIDEO_ROTATION_0; + EXPECT_FALSE(ev.metadata.has_value()); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_video_frame.cpp b/src/tests/unit/test_video_frame.cpp new file mode 100644 index 00000000..9bf63b5c --- /dev/null +++ b/src/tests/unit/test_video_frame.cpp @@ -0,0 +1,51 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include + +#include + +namespace livekit::test { + +TEST(VideoFrameTest, DefaultConstructed) { + VideoFrame frame; + EXPECT_EQ(frame.dataSize(), 0u); +} + +TEST(VideoFrameTest, ConstructFromBuffer) { + std::vector data(std::size_t{16} * 16 * 4, 0xAB); + VideoFrame frame(16, 16, VideoBufferType::RGBA, std::move(data)); + EXPECT_EQ(frame.width(), 16); + EXPECT_EQ(frame.height(), 16); + EXPECT_EQ(frame.type(), VideoBufferType::RGBA); + EXPECT_EQ(frame.dataSize(), 16u * 16u * 4u); +} + +TEST(VideoFrameTest, FactoryCreate) { + VideoFrame frame = VideoFrame::create(8, 8, VideoBufferType::ARGB); + EXPECT_EQ(frame.width(), 8); + EXPECT_EQ(frame.height(), 8); + EXPECT_EQ(frame.type(), VideoBufferType::ARGB); + EXPECT_GT(frame.dataSize(), 0u); + + const auto planes = frame.planeInfos(); + EXPECT_FALSE(planes.empty()); +} + +} // namespace livekit::test diff --git a/src/tests/unit/test_video_source.cpp b/src/tests/unit/test_video_source.cpp new file mode 100644 index 00000000..522ead90 --- /dev/null +++ b/src/tests/unit/test_video_source.cpp @@ -0,0 +1,44 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include + +namespace livekit::test { + +class VideoSourceTest : public ::testing::Test { +protected: + void SetUp() override { livekit::initialize(livekit::LogLevel::Info, livekit::LogSink::kConsole); } + void TearDown() override { livekit::shutdown(); } +}; + +TEST_F(VideoSourceTest, ConstructAndQueryProperties) { + VideoSource source(640, 480); + EXPECT_EQ(source.width(), 640); + EXPECT_EQ(source.height(), 480); + EXPECT_NE(source.ffi_handle_id(), 0u); +} + +TEST_F(VideoSourceTest, VideoCaptureOptionsDefaults) { + VideoCaptureOptions options; + EXPECT_EQ(options.timestamp_us, 0); + EXPECT_EQ(options.rotation, VideoRotation::VIDEO_ROTATION_0); + EXPECT_FALSE(options.metadata.has_value()); +} + +} // namespace livekit::test From 2d8817fb61f0a72d2be8be7e66816ec7b5c619d1 Mon Sep 17 00:00:00 2001 From: Alan George Date: Tue, 12 May 2026 13:33:35 -0600 Subject: [PATCH 16/24] Fix formatting --- src/tests/unit/test_audio_source.cpp | 1 - src/tests/unit/test_data_stream.cpp | 3 +-- src/tests/unit/test_data_track_error.cpp | 1 - src/tests/unit/test_data_track_frame.cpp | 3 +-- src/tests/unit/test_data_track_info.cpp | 1 - src/tests/unit/test_e2ee.cpp | 3 +-- src/tests/unit/test_ffi_handle.cpp | 3 +-- src/tests/unit/test_local_tracks.cpp | 9 ++++----- src/tests/unit/test_remote_audio_track.cpp | 1 - src/tests/unit/test_remote_video_track.cpp | 1 - src/tests/unit/test_result.cpp | 6 +++--- src/tests/unit/test_room_event_types.cpp | 1 - src/tests/unit/test_rpc_error.cpp | 1 - src/tests/unit/test_stats.cpp | 3 +-- src/tests/unit/test_streams.cpp | 5 ++--- src/tests/unit/test_video_frame.cpp | 3 +-- src/tests/unit/test_video_source.cpp | 1 - 17 files changed, 15 insertions(+), 31 deletions(-) diff --git a/src/tests/unit/test_audio_source.cpp b/src/tests/unit/test_audio_source.cpp index 8f931b16..bd2fc577 100644 --- a/src/tests/unit/test_audio_source.cpp +++ b/src/tests/unit/test_audio_source.cpp @@ -15,7 +15,6 @@ */ #include - #include #include diff --git a/src/tests/unit/test_data_stream.cpp b/src/tests/unit/test_data_stream.cpp index 14833025..840f5e02 100644 --- a/src/tests/unit/test_data_stream.cpp +++ b/src/tests/unit/test_data_stream.cpp @@ -15,11 +15,10 @@ */ #include +#include #include -#include - namespace livekit::test { TEST(DataStreamTest, TextStreamReaderConstructAndInfo) { diff --git a/src/tests/unit/test_data_track_error.cpp b/src/tests/unit/test_data_track_error.cpp index 9d4708dd..cc41fbd0 100644 --- a/src/tests/unit/test_data_track_error.cpp +++ b/src/tests/unit/test_data_track_error.cpp @@ -15,7 +15,6 @@ */ #include - #include #include "data_track.pb.h" diff --git a/src/tests/unit/test_data_track_frame.cpp b/src/tests/unit/test_data_track_frame.cpp index 82d05f0a..96323b18 100644 --- a/src/tests/unit/test_data_track_frame.cpp +++ b/src/tests/unit/test_data_track_frame.cpp @@ -15,12 +15,11 @@ */ #include +#include #include #include -#include - #include "data_track.pb.h" namespace livekit::test { diff --git a/src/tests/unit/test_data_track_info.cpp b/src/tests/unit/test_data_track_info.cpp index d1e5619c..0e0eb03c 100644 --- a/src/tests/unit/test_data_track_info.cpp +++ b/src/tests/unit/test_data_track_info.cpp @@ -15,7 +15,6 @@ */ #include - #include namespace livekit::test { diff --git a/src/tests/unit/test_e2ee.cpp b/src/tests/unit/test_e2ee.cpp index 13006edd..678c8f62 100644 --- a/src/tests/unit/test_e2ee.cpp +++ b/src/tests/unit/test_e2ee.cpp @@ -15,11 +15,10 @@ */ #include +#include #include -#include - namespace livekit::test { TEST(E2EETest, KeyProviderOptionsDefaults) { diff --git a/src/tests/unit/test_ffi_handle.cpp b/src/tests/unit/test_ffi_handle.cpp index 5d62d94a..a6ca3109 100644 --- a/src/tests/unit/test_ffi_handle.cpp +++ b/src/tests/unit/test_ffi_handle.cpp @@ -15,12 +15,11 @@ */ #include +#include #include #include -#include - namespace livekit::test { TEST(FfiHandleTest, DefaultConstructedIsInvalid) { diff --git a/src/tests/unit/test_local_tracks.cpp b/src/tests/unit/test_local_tracks.cpp index b139d0d4..4b23d316 100644 --- a/src/tests/unit/test_local_tracks.cpp +++ b/src/tests/unit/test_local_tracks.cpp @@ -15,16 +15,15 @@ */ #include - -#include -#include -#include - #include #include #include #include +#include +#include +#include + namespace livekit::test { TEST(LocalTracksTest, LocalAudioTrackFactoryIsAddressable) { diff --git a/src/tests/unit/test_remote_audio_track.cpp b/src/tests/unit/test_remote_audio_track.cpp index 520f267f..d7a7c961 100644 --- a/src/tests/unit/test_remote_audio_track.cpp +++ b/src/tests/unit/test_remote_audio_track.cpp @@ -15,7 +15,6 @@ */ #include - #include #include "track.pb.h" diff --git a/src/tests/unit/test_remote_video_track.cpp b/src/tests/unit/test_remote_video_track.cpp index a3e1e875..6e5777dd 100644 --- a/src/tests/unit/test_remote_video_track.cpp +++ b/src/tests/unit/test_remote_video_track.cpp @@ -15,7 +15,6 @@ */ #include - #include #include "track.pb.h" diff --git a/src/tests/unit/test_result.cpp b/src/tests/unit/test_result.cpp index 0a191d5d..fb0853d9 100644 --- a/src/tests/unit/test_result.cpp +++ b/src/tests/unit/test_result.cpp @@ -15,14 +15,12 @@ */ #include +#include #include #include -#include - namespace livekit::test { - struct StubError { int code = 0; std::string message; @@ -49,6 +47,8 @@ TEST(ResultTest, VoidResultSuccess) { ASSERT_TRUE(r.ok()); EXPECT_FALSE(r.has_error()); r.value(); + + auto& e = r.error(); } TEST(ResultTest, VoidResultFailure) { diff --git a/src/tests/unit/test_room_event_types.cpp b/src/tests/unit/test_room_event_types.cpp index 89a10650..db423142 100644 --- a/src/tests/unit/test_room_event_types.cpp +++ b/src/tests/unit/test_room_event_types.cpp @@ -15,7 +15,6 @@ */ #include - #include namespace livekit::test { diff --git a/src/tests/unit/test_rpc_error.cpp b/src/tests/unit/test_rpc_error.cpp index 8467719a..02f020e8 100644 --- a/src/tests/unit/test_rpc_error.cpp +++ b/src/tests/unit/test_rpc_error.cpp @@ -15,7 +15,6 @@ */ #include - #include namespace livekit::test { diff --git a/src/tests/unit/test_stats.cpp b/src/tests/unit/test_stats.cpp index 5d4b39b9..434907e9 100644 --- a/src/tests/unit/test_stats.cpp +++ b/src/tests/unit/test_stats.cpp @@ -15,11 +15,10 @@ */ #include +#include #include -#include - #include "stats.pb.h" namespace livekit::test { diff --git a/src/tests/unit/test_streams.cpp b/src/tests/unit/test_streams.cpp index 24fcaeb3..114d8c47 100644 --- a/src/tests/unit/test_streams.cpp +++ b/src/tests/unit/test_streams.cpp @@ -15,14 +15,13 @@ */ #include - -#include - #include #include #include #include +#include + namespace livekit::test { TEST(StreamOptionsTest, AudioStreamOptionsDefaults) { diff --git a/src/tests/unit/test_video_frame.cpp b/src/tests/unit/test_video_frame.cpp index 9bf63b5c..5849ea1b 100644 --- a/src/tests/unit/test_video_frame.cpp +++ b/src/tests/unit/test_video_frame.cpp @@ -15,12 +15,11 @@ */ #include +#include #include #include -#include - namespace livekit::test { TEST(VideoFrameTest, DefaultConstructed) { diff --git a/src/tests/unit/test_video_source.cpp b/src/tests/unit/test_video_source.cpp index 522ead90..c2d9419b 100644 --- a/src/tests/unit/test_video_source.cpp +++ b/src/tests/unit/test_video_source.cpp @@ -15,7 +15,6 @@ */ #include - #include #include From 7a830bd4ab0a0c7868a58d9db757d3e343499b99 Mon Sep 17 00:00:00 2001 From: Alan George Date: Tue, 12 May 2026 13:37:35 -0600 Subject: [PATCH 17/24] Includes in coverage --- .github/workflows/tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ed0ae82f..97844f97 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -392,6 +392,7 @@ jobs: --root . \ --gcov-executable "${GCOV_EXECUTABLE}" \ --gcov-ignore-parse-errors=all \ + --filter 'include/livekit/' \ --filter 'src/' \ --exclude 'src/tests/' \ --exclude '.*\.pb\.' \ From 827367841ced4abb82a33d092f93dba1ad5c5713 Mon Sep 17 00:00:00 2001 From: Alan George Date: Tue, 12 May 2026 13:54:23 -0600 Subject: [PATCH 18/24] Maintain same test case name --- src/tests/unit/test_result.cpp | 48 +++++++++++++++++----------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/tests/unit/test_result.cpp b/src/tests/unit/test_result.cpp index 7743e6d5..980bdbe6 100644 --- a/src/tests/unit/test_result.cpp +++ b/src/tests/unit/test_result.cpp @@ -46,43 +46,43 @@ struct SimpleError { // Result — success path // --------------------------------------------------------------------------- -TEST(ResultTest, SuccessOkIsTrue) { +TEST(ResultTest, Int_SuccessOkIsTrue) { auto r = Result::success(42); EXPECT_TRUE(r.ok()); } -TEST(ResultTest, SuccessHasErrorIsFalse) { +TEST(ResultTest, Int_SuccessHasErrorIsFalse) { auto r = Result::success(42); EXPECT_FALSE(r.has_error()); } -TEST(ResultTest, SuccessBoolConversionIsTrue) { +TEST(ResultTest, Int_SuccessBoolConversionIsTrue) { auto r = Result::success(42); EXPECT_TRUE(static_cast(r)); } -TEST(ResultTest, SuccessValueMatchesInput) { +TEST(ResultTest, Int_SuccessValueMatchesInput) { auto r = Result::success(99); EXPECT_EQ(r.value(), 99); } -TEST(ResultTest, SuccessConstValueMatchesInput) { +TEST(ResultTest, Int_SuccessConstValueMatchesInput) { const auto r = Result::success(7); EXPECT_EQ(r.value(), 7); } -TEST(ResultTest, SuccessValueCanBeMutated) { +TEST(ResultTest, Int_SuccessValueCanBeMutated) { auto r = Result::success(1); r.value() = 100; EXPECT_EQ(r.value(), 100); } -TEST(ResultTest, SuccessStringValue) { +TEST(ResultTest, Int_SuccessStringValue) { auto r = Result::success("hello"); EXPECT_EQ(r.value(), "hello"); } -TEST(ResultTest, SuccessMoveValueTransfersOwnership) { +TEST(ResultTest, Int_SuccessMoveValueTransfersOwnership) { auto r = Result, SimpleError>::success(std::make_unique(55)); auto ptr = std::move(r).value(); ASSERT_NE(ptr, nullptr); @@ -93,40 +93,40 @@ TEST(ResultTest, SuccessMoveValueTransfersOwnership) { // Result — failure path // --------------------------------------------------------------------------- -TEST(ResultTest, FailureOkIsFalse) { +TEST(ResultTest, Int_FailureOkIsFalse) { auto r = Result::failure(SimpleError{1, "oops"}); EXPECT_FALSE(r.ok()); } -TEST(ResultTest, FailureHasErrorIsTrue) { +TEST(ResultTest, Int_FailureHasErrorIsTrue) { auto r = Result::failure(SimpleError{1, "oops"}); EXPECT_TRUE(r.has_error()); } -TEST(ResultTest, FailureBoolConversionIsFalse) { +TEST(ResultTest, Int_FailureBoolConversionIsFalse) { auto r = Result::failure(SimpleError{1, "oops"}); EXPECT_FALSE(static_cast(r)); } -TEST(ResultTest, FailureErrorCodeMatchesInput) { +TEST(ResultTest, Int_FailureErrorCodeMatchesInput) { auto r = Result::failure(SimpleError{42, "bad"}); EXPECT_EQ(r.error().code, 42); EXPECT_EQ(r.error().message, "bad"); } -TEST(ResultTest, FailureConstErrorMatchesInput) { +TEST(ResultTest, Int_FailureConstErrorMatchesInput) { const auto r = Result::failure(SimpleError{3, "err"}); EXPECT_EQ(r.error().code, 3); } -TEST(ResultTest, FailureMoveErrorTransfersOwnership) { +TEST(ResultTest, Int_FailureMoveErrorTransfersOwnership) { auto r = Result>::failure(std::make_unique(SimpleError{9, "moved"})); auto err = std::move(r).error(); ASSERT_NE(err, nullptr); EXPECT_EQ(err->code, 9); } -TEST(ResultTest, FailureStringError) { +TEST(ResultTest, Int_FailureStringError) { auto r = Result::failure("something went wrong"); EXPECT_EQ(r.error(), "something went wrong"); } @@ -135,22 +135,22 @@ TEST(ResultTest, FailureStringError) { // Result — success path // --------------------------------------------------------------------------- -TEST(ResultVoidTest, SuccessOkIsTrue) { +TEST(ResultTest, Void_SuccessOkIsTrue) { auto r = Result::success(); EXPECT_TRUE(r.ok()); } -TEST(ResultVoidTest, SuccessHasErrorIsFalse) { +TEST(ResultTest, Void_SuccessHasErrorIsFalse) { auto r = Result::success(); EXPECT_FALSE(r.has_error()); } -TEST(ResultVoidTest, SuccessBoolConversionIsTrue) { +TEST(ResultTest, Void_SuccessBoolConversionIsTrue) { auto r = Result::success(); EXPECT_TRUE(static_cast(r)); } -TEST(ResultVoidTest, SuccessValueIsCallable) { +TEST(ResultTest, Void_SuccessValueIsCallable) { auto r = Result::success(); EXPECT_NO_THROW(r.value()); } @@ -159,28 +159,28 @@ TEST(ResultVoidTest, SuccessValueIsCallable) { // Result — failure path // --------------------------------------------------------------------------- -TEST(ResultVoidTest, FailureOkIsFalse) { +TEST(ResultTest, Void_FailureOkIsFalse) { auto r = Result::failure(SimpleError{5, "void fail"}); EXPECT_FALSE(r.ok()); } -TEST(ResultVoidTest, FailureHasErrorIsTrue) { +TEST(ResultTest, Void_FailureHasErrorIsTrue) { auto r = Result::failure(SimpleError{5, "void fail"}); EXPECT_TRUE(r.has_error()); } -TEST(ResultVoidTest, FailureBoolConversionIsFalse) { +TEST(ResultTest, Void_FailureBoolConversionIsFalse) { auto r = Result::failure(SimpleError{5, "void fail"}); EXPECT_FALSE(static_cast(r)); } -TEST(ResultVoidTest, FailureErrorMatchesInput) { +TEST(ResultTest, Void_FailureErrorMatchesInput) { auto r = Result::failure(SimpleError{7, "nope"}); EXPECT_EQ(r.error().code, 7); EXPECT_EQ(r.error().message, "nope"); } -TEST(ResultVoidTest, FailureMoveError) { +TEST(ResultTest, Void_FailureMoveError) { auto r = Result::failure("void error"); auto msg = std::move(r).error(); EXPECT_EQ(msg, "void error"); From 9159f3e5d3785f144f295e24d50635565e3f5999 Mon Sep 17 00:00:00 2001 From: Alan George Date: Tue, 12 May 2026 15:30:39 -0600 Subject: [PATCH 19/24] Move room unit tests out, add isInit check --- include/livekit/livekit.h | 3 + src/livekit.cpp | 5 ++ src/room.cpp | 6 ++ src/tests/integration/test_room.cpp | 87 ++---------------------- src/tests/unit/test_room.cpp | 101 ++++++++++++++++++++++++++++ 5 files changed, 119 insertions(+), 83 deletions(-) create mode 100644 src/tests/unit/test_room.cpp diff --git a/include/livekit/livekit.h b/include/livekit/livekit.h index f7906c42..a292a2c7 100644 --- a/include/livekit/livekit.h +++ b/include/livekit/livekit.h @@ -62,6 +62,9 @@ enum class LogSink { /// already initialized. LIVEKIT_API bool initialize(const LogLevel& level = LogLevel::Info, const LogSink& log_sink = LogSink::kConsole); +/// Returns true if the LiveKit SDK is initialized, false otherwise. +LIVEKIT_API bool isInitialized(); + /// Shut down the LiveKit SDK. /// /// After shutdown, you may call initialize() again. diff --git a/src/livekit.cpp b/src/livekit.cpp index 1bcd9df9..a79756cb 100644 --- a/src/livekit.cpp +++ b/src/livekit.cpp @@ -27,6 +27,11 @@ bool initialize(const LogLevel& level, const LogSink& log_sink) { return ffi_client.initialize(log_sink == LogSink::kCallback); } +bool isInitialized() { + auto& ffi_client = FfiClient::instance(); + return ffi_client.isInitialized(); +} + void shutdown() { auto& ffi_client = FfiClient::instance(); ffi_client.shutdown(); diff --git a/src/room.cpp b/src/room.cpp index d9e96a44..3a7eac35 100644 --- a/src/room.cpp +++ b/src/room.cpp @@ -23,6 +23,7 @@ #include "ffi_client.h" #include "livekit/audio_stream.h" #include "livekit/e2ee.h" +#include "livekit/livekit.h" #include "livekit/local_data_track.h" #include "livekit/local_participant.h" #include "livekit/local_track_publication.h" @@ -105,6 +106,11 @@ void Room::setDelegate(RoomDelegate* delegate) { bool Room::Connect(const std::string& url, const std::string& token, const RoomOptions& options) { TRACE_EVENT0("livekit", "Room::Connect"); + if (!livekit::isInitialized()) { + LK_LOG_ERROR("Room::Connect failed: LiveKit is not initialized"); + return false; + } + { const std::scoped_lock g(lock_); if (connection_state_ != ConnectionState::Disconnected) { diff --git a/src/tests/integration/test_room.cpp b/src/tests/integration/test_room.cpp index 011bdf23..29bffcd7 100644 --- a/src/tests/integration/test_room.cpp +++ b/src/tests/integration/test_room.cpp @@ -19,78 +19,8 @@ namespace livekit::test { -class RoomTest : public ::testing::Test { -protected: - void SetUp() override { livekit::initialize(livekit::LogLevel::Info, livekit::LogSink::kConsole); } - - void TearDown() override { livekit::shutdown(); } -}; - -TEST_F(RoomTest, CreateRoom) { - Room room; - // Room should be created without issues - EXPECT_EQ(room.localParticipant(), nullptr) << "Local participant should be null before connect"; -} - -TEST_F(RoomTest, RoomOptionsDefaults) { - RoomOptions options; - - EXPECT_TRUE(options.auto_subscribe) << "auto_subscribe should default to true"; - EXPECT_FALSE(options.dynacast) << "dynacast should default to false"; - EXPECT_FALSE(options.rtc_config.has_value()) << "rtc_config should not have a value by default"; - EXPECT_FALSE(options.encryption.has_value()) << "encryption should not have a value by default"; -} - -TEST_F(RoomTest, RtcConfigDefaults) { - RtcConfig config; - - EXPECT_EQ(config.ice_transport_type, 0); - EXPECT_EQ(config.continual_gathering_policy, 0); - EXPECT_TRUE(config.ice_servers.empty()); -} - -TEST_F(RoomTest, IceServerConfiguration) { - IceServer server; - server.url = "stun:stun.l.google.com:19302"; - server.username = "user"; - server.credential = "pass"; - - EXPECT_EQ(server.url, "stun:stun.l.google.com:19302"); - EXPECT_EQ(server.username, "user"); - EXPECT_EQ(server.credential, "pass"); -} - -TEST_F(RoomTest, RoomWithCustomRtcConfig) { - RoomOptions options; - options.auto_subscribe = false; - options.dynacast = true; - - RtcConfig rtc_config; - rtc_config.ice_servers.push_back({"stun:stun.l.google.com:19302", "", ""}); - rtc_config.ice_servers.push_back({"turn:turn.example.com:3478", "user", "pass"}); - - options.rtc_config = rtc_config; - - EXPECT_FALSE(options.auto_subscribe); - EXPECT_TRUE(options.dynacast); - EXPECT_TRUE(options.rtc_config.has_value()); - EXPECT_EQ(options.rtc_config->ice_servers.size(), 2); -} - -TEST_F(RoomTest, RemoteParticipantsEmptyBeforeConnect) { - Room room; - auto participants = room.remoteParticipants(); - EXPECT_TRUE(participants.empty()) << "Remote participants should be empty before connect"; -} - -TEST_F(RoomTest, RemoteParticipantLookupBeforeConnect) { - Room room; - auto participant = room.remoteParticipant("nonexistent"); - EXPECT_EQ(participant, nullptr) << "Looking up participant before connect should return nullptr"; -} - // Server-dependent tests - require LIVEKIT_URL and LIVEKIT_TOKEN_A env vars -class RoomServerTest : public ::testing::Test { +class RoomTest : public ::testing::Test { protected: void SetUp() override { livekit::initialize(livekit::LogLevel::Info, livekit::LogSink::kConsole); @@ -112,12 +42,7 @@ class RoomServerTest : public ::testing::Test { std::string token_; }; -TEST_F(RoomServerTest, ConnectToServer) { - if (!server_available_) { - GTEST_SKIP() << "LIVEKIT_URL and LIVEKIT_TOKEN_A not set, skipping server " - "connection test"; - } - +TEST_F(RoomTest, ConnectToServer) { Room room; RoomOptions options; @@ -129,11 +54,7 @@ TEST_F(RoomServerTest, ConnectToServer) { } } -TEST_F(RoomServerTest, ConnectWithInvalidToken) { - if (!server_available_) { - GTEST_SKIP() << "LIVEKIT_URL not set, skipping invalid token test"; - } - +TEST_F(RoomTest, ConnectWithInvalidToken) { Room room; RoomOptions options; @@ -141,7 +62,7 @@ TEST_F(RoomServerTest, ConnectWithInvalidToken) { EXPECT_FALSE(connected) << "Should fail to connect with invalid token"; } -TEST_F(RoomServerTest, ConnectWithInvalidUrl) { +TEST_F(RoomTest, ConnectWithInvalidUrl) { Room room; RoomOptions options; diff --git a/src/tests/unit/test_room.cpp b/src/tests/unit/test_room.cpp new file mode 100644 index 00000000..3bf896c3 --- /dev/null +++ b/src/tests/unit/test_room.cpp @@ -0,0 +1,101 @@ +/* + * Copyright 2025 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +namespace livekit::test { + +class RoomTest : public ::testing::Test { +protected: + void SetUp() override { livekit::initialize(livekit::LogLevel::Info, livekit::LogSink::kConsole); } + + void TearDown() override { livekit::shutdown(); } +}; + +TEST_F(RoomTest, ConnectWithoutInitialize) { + // Test fixture initializes by default, do this to emulate lack of initialization + livekit::shutdown(); + + Room room; + bool result = room.Connect("wss://localhost:7880", "test", livekit::RoomOptions()); + EXPECT_FALSE(result) << "Connecting without initializing should return false"; +} + +TEST_F(RoomTest, CreateRoom) { + Room room; + // Room should be created without issues + EXPECT_EQ(room.localParticipant(), nullptr) << "Local participant should be null before connect"; +} + +TEST_F(RoomTest, RoomOptionsDefaults) { + RoomOptions options; + + EXPECT_TRUE(options.auto_subscribe) << "auto_subscribe should default to true"; + EXPECT_FALSE(options.dynacast) << "dynacast should default to false"; + EXPECT_FALSE(options.rtc_config.has_value()) << "rtc_config should not have a value by default"; + EXPECT_FALSE(options.encryption.has_value()) << "encryption should not have a value by default"; +} + +TEST_F(RoomTest, RtcConfigDefaults) { + RtcConfig config; + + EXPECT_EQ(config.ice_transport_type, 0); + EXPECT_EQ(config.continual_gathering_policy, 0); + EXPECT_TRUE(config.ice_servers.empty()); +} + +TEST_F(RoomTest, IceServerConfiguration) { + IceServer server; + server.url = "stun:stun.l.google.com:19302"; + server.username = "user"; + server.credential = "pass"; + + EXPECT_EQ(server.url, "stun:stun.l.google.com:19302"); + EXPECT_EQ(server.username, "user"); + EXPECT_EQ(server.credential, "pass"); +} + +TEST_F(RoomTest, RoomWithCustomRtcConfig) { + RoomOptions options; + options.auto_subscribe = false; + options.dynacast = true; + + RtcConfig rtc_config; + rtc_config.ice_servers.push_back({"stun:stun.l.google.com:19302", "", ""}); + rtc_config.ice_servers.push_back({"turn:turn.example.com:3478", "user", "pass"}); + + options.rtc_config = rtc_config; + + EXPECT_FALSE(options.auto_subscribe); + EXPECT_TRUE(options.dynacast); + EXPECT_TRUE(options.rtc_config.has_value()); + EXPECT_EQ(options.rtc_config->ice_servers.size(), 2); +} + +TEST_F(RoomTest, RemoteParticipantsEmptyBeforeConnect) { + Room room; + auto participants = room.remoteParticipants(); + EXPECT_TRUE(participants.empty()) << "Remote participants should be empty before connect"; +} + +TEST_F(RoomTest, RemoteParticipantLookupBeforeConnect) { + Room room; + auto participant = room.remoteParticipant("nonexistent"); + EXPECT_EQ(participant, nullptr) << "Looking up participant before connect should return nullptr"; +} + +} // namespace livekit::test From 92159e989549fe3aae4610005e4b66845335ea31 Mon Sep 17 00:00:00 2001 From: Alan George Date: Tue, 12 May 2026 18:50:22 -0600 Subject: [PATCH 20/24] Moved FfiClient instance to .cpp to ensure only one copy exists owned by the lib, added unit tests --- src/ffi_client.cpp | 5 + src/ffi_client.h | 11 +- src/livekit.cpp | 7 +- src/tests/unit/test_ffi_client.cpp | 351 +++++++++++++++++++++++++++++ 4 files changed, 366 insertions(+), 8 deletions(-) create mode 100644 src/tests/unit/test_ffi_client.cpp diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index 5e719dc6..29f5dee9 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -146,6 +146,11 @@ std::optional ExtractAsyncId(const proto::FfiEvent& event) { } // namespace +FfiClient& FfiClient::instance() noexcept { + static FfiClient instance; + return instance; +} + // clang-tidy flags this as a trivial destructor in release mode // due to the assert being pre-processed out // NOLINTNEXTLINE(modernize-use-equals-default) diff --git a/src/ffi_client.h b/src/ffi_client.h index dc00d565..32fd337d 100644 --- a/src/ffi_client.h +++ b/src/ffi_client.h @@ -74,10 +74,13 @@ class LIVEKIT_INTERNAL_API FfiClient { FfiClient(FfiClient&&) = delete; FfiClient& operator=(FfiClient&&) = delete; - static FfiClient& instance() noexcept { - static FfiClient instance; - return instance; - } + // Defined out-of-line in ffi_client.cpp so the process has exactly one + // FfiClient. An inline definition would, under the SDK's hidden inline + // visibility, produce a separate function-local static in every TU that + // includes this header (notably: in-tree test executables), giving each + // binary its own "singleton" and silently desynchronizing the dylib-side + // and test-exe-side initialization flags. + static FfiClient& instance() noexcept; // Must be called before any other FFI usage bool initialize(bool capture_logs); diff --git a/src/livekit.cpp b/src/livekit.cpp index a79756cb..83f3044a 100644 --- a/src/livekit.cpp +++ b/src/livekit.cpp @@ -22,19 +22,18 @@ namespace livekit { bool initialize(const LogLevel& level, const LogSink& log_sink) { + // Initializes logger if singleton instance is not already initialized setLogLevel(level); auto& ffi_client = FfiClient::instance(); return ffi_client.initialize(log_sink == LogSink::kCallback); } bool isInitialized() { - auto& ffi_client = FfiClient::instance(); - return ffi_client.isInitialized(); + return FfiClient::instance().isInitialized(); } void shutdown() { - auto& ffi_client = FfiClient::instance(); - ffi_client.shutdown(); + FfiClient::instance().shutdown(); detail::shutdownLogger(); } diff --git a/src/tests/unit/test_ffi_client.cpp b/src/tests/unit/test_ffi_client.cpp new file mode 100644 index 00000000..476a2817 --- /dev/null +++ b/src/tests/unit/test_ffi_client.cpp @@ -0,0 +1,351 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/// @file test_ffi_client.cpp +/// @brief Unit tests for FfiClient (the internal singleton that bridges the +/// C++ SDK to the Rust FFI runtime). +/// +/// Scope: +/// - Singleton identity +/// - initialize() / shutdown() / isInitialized() state machine +/// - AddListener() / RemoveListener() lifecycle and ID uniqueness +/// - sendRequest() invariants that are reachable without a live FFI +/// roundtrip (empty-serialization failure path) +/// +/// Out of scope (covered by integration tests with a live Rust runtime): +/// - Real sendRequest()/sendRequestAsync() responses +/// - Listener invocation from the FFI callback thread +/// +/// DISABLED_ tests in this file are stubs for the "isInitialized() gate" +/// design proposal (see project notes). Strip the DISABLED_ prefix once +/// the corresponding gate lands in production. + +#include +#include + +#include +#include + +#include "ffi.pb.h" +#include "ffi_client.h" + +namespace livekit::test { + +class FfiClientTest : public ::testing::Test { +protected: + void SetUp() override { + // Ensure the singleton for this test case starts up uninitialized + livekit::shutdown(); + + // This assert helps test the livekit::shutdown() <-> FFI client interface + ASSERT_FALSE(FfiClient::instance().isInitialized()); + } + + void TearDown() override { livekit::shutdown(); } +}; + +TEST_F(FfiClientTest, Singleton) { + auto& a = FfiClient::instance(); + auto& b = FfiClient::instance(); + EXPECT_EQ(&a, &b); +} + +// --------------------------------------------------------------------------- +// Initialization state +// --------------------------------------------------------------------------- + +TEST_F(FfiClientTest, DefaultUninitialized) { + EXPECT_FALSE(FfiClient::instance().isInitialized()); +} + +TEST_F(FfiClientTest, Initialize) { + EXPECT_TRUE(FfiClient::instance().initialize(false)); + EXPECT_TRUE(FfiClient::instance().isInitialized()); +} + +TEST_F(FfiClientTest, DoubleInitialize) { + ASSERT_TRUE(FfiClient::instance().initialize(false)); + EXPECT_FALSE(FfiClient::instance().initialize(false)) + << "second initialize() on an already-initialized client must be a no-op"; + EXPECT_TRUE(FfiClient::instance().isInitialized()); +} + +TEST_F(FfiClientTest, Shutdown) { + ASSERT_TRUE(FfiClient::instance().initialize(false)); + ASSERT_TRUE(FfiClient::instance().isInitialized()); + + FfiClient::instance().shutdown(); + EXPECT_FALSE(FfiClient::instance().isInitialized()); +} + +TEST_F(FfiClientTest, ShutdownWithoutInitialize) { + EXPECT_NO_THROW(FfiClient::instance().shutdown()); + EXPECT_FALSE(FfiClient::instance().isInitialized()); +} + +TEST_F(FfiClientTest, RepeatedShutdown) { + FfiClient::instance().initialize(false); + EXPECT_NO_THROW(FfiClient::instance().shutdown()); + EXPECT_NO_THROW(FfiClient::instance().shutdown()); + EXPECT_NO_THROW(FfiClient::instance().shutdown()); +} + +TEST_F(FfiClientTest, ReinitializeAfterShutdown) { + ASSERT_TRUE(FfiClient::instance().initialize(false)); + FfiClient::instance().shutdown(); + ASSERT_FALSE(FfiClient::instance().isInitialized()); + + EXPECT_TRUE(FfiClient::instance().initialize(false)); + EXPECT_TRUE(FfiClient::instance().isInitialized()); +} + +// --------------------------------------------------------------------------- +// AddListener / RemoveListener +// +// These touch only C++-side state (a std::unordered_map under a mutex), so +// they are safe to exercise whether or not the FFI runtime is initialized. +// --------------------------------------------------------------------------- + +TEST_F(FfiClientTest, AddListenerReturnsNonZeroId) { + const auto id = FfiClient::instance().AddListener([](const proto::FfiEvent&) {}); + EXPECT_NE(id, 0); + FfiClient::instance().RemoveListener(id); +} + +TEST_F(FfiClientTest, AddListenerReturnsUniqueIds) { + constexpr int kCount = 16; + std::unordered_set ids; + ids.reserve(kCount); + for (int i = 0; i < kCount; ++i) { + const auto id = FfiClient::instance().AddListener([](const proto::FfiEvent&) {}); + EXPECT_TRUE(ids.insert(id).second) << "duplicate listener id: " << id; + } + for (auto id : ids) { + FfiClient::instance().RemoveListener(id); + } +} + +TEST_F(FfiClientTest, RemoveListenerWithUnknownIdIsSafe) { + EXPECT_NO_THROW(FfiClient::instance().RemoveListener(/*never registered=*/424242)); +} + +TEST_F(FfiClientTest, RemoveListenerIsIdempotent) { + const auto id = FfiClient::instance().AddListener([](const proto::FfiEvent&) {}); + EXPECT_NO_THROW(FfiClient::instance().RemoveListener(id)); + EXPECT_NO_THROW(FfiClient::instance().RemoveListener(id)); +} + +TEST_F(FfiClientTest, ListenerRegistrationSurvivesShutdownReinitCycle) { + FfiClient::instance().initialize(false); + const auto id = FfiClient::instance().AddListener([](const proto::FfiEvent&) {}); + EXPECT_NE(id, 0); + + // shutdown() does not clear the C++-side listener map today; document that + // contract here so a future refactor that changes it is a deliberate choice. + FfiClient::instance().shutdown(); + EXPECT_NO_THROW(FfiClient::instance().RemoveListener(id)); +} + +// --------------------------------------------------------------------------- +// sendRequest() — invariants reachable without a live FFI +// --------------------------------------------------------------------------- + +TEST_F(FfiClientTest, SendRequestThrowsOnEmptyRequest) { + // A default-constructed FfiRequest has no oneof populated and serializes + // to zero bytes, which sendRequest treats as a serialization failure. + // This path is reachable regardless of initialization state. + proto::FfiRequest req; + EXPECT_THROW(FfiClient::instance().sendRequest(req), std::runtime_error); +} + +// --------------------------------------------------------------------------- +// PROPOSED: sendRequest() isInitialized() gate (Tier 1 #1) +// +// These tests describe the *proposed* behavior: sendRequest() should throw +// std::runtime_error when the SDK has not been initialized, instead of +// calling livekit_ffi_request() on a torn-down/never-started Rust runtime. +// +// They are DISABLED today because, without the gate, calling sendRequest() +// on an uninitialized client invokes Rust FFI behavior that is undefined +// and may crash the test process. Once the gate lands in +// FfiClient::sendRequest(), strip the DISABLED_ prefix. +// --------------------------------------------------------------------------- + +TEST_F(FfiClientTest, DISABLED_SendRequestThrowsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + + proto::FfiRequest req; + // Populate any oneof so we get past the empty-bytes serialization check + // and hit the proposed init gate instead. + (void)req.mutable_dispose(); + + EXPECT_THROW(FfiClient::instance().sendRequest(req), std::runtime_error); +} + +TEST_F(FfiClientTest, DISABLED_SendRequestThrowsAfterShutdown) { + ASSERT_TRUE(FfiClient::instance().initialize(false)); + FfiClient::instance().shutdown(); + ASSERT_FALSE(FfiClient::instance().isInitialized()); + + proto::FfiRequest req; + (void)req.mutable_dispose(); + + EXPECT_THROW(FfiClient::instance().sendRequest(req), std::runtime_error); +} + +// --------------------------------------------------------------------------- +// PROPOSED: *Async helper isInitialized() gates (Tier 1 #3) +// +// Each async helper (connectAsync, publishTrackAsync, captureAudioFrameAsync, +// publishDataAsync, performRpcAsync, publishDataTrackAsync, etc.) should +// surface "not initialized" through its return channel: +// - std::future helpers: the future should resolve with an exception +// (fut.get() throws std::runtime_error). +// - Result helpers (subscribeDataTrack, publishDataTrackAsync's +// Result payload): the Result should be a failure with an appropriate +// error code. +// +// Each stub below pins down the *contract* for one helper. They are kept +// DISABLED for the same reason as sendRequest above: without the gate, the +// helper either calls into Rust unsafely or registers a pending waiter that +// will never complete. +// +// As you add each gate, strip the DISABLED_ prefix one test at a time. +// --------------------------------------------------------------------------- + +TEST_F(FfiClientTest, DISABLED_ConnectAsyncFailsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + + RoomOptions options; + auto fut = FfiClient::instance().connectAsync("wss://localhost:7880", "fake-token", options); + EXPECT_THROW(fut.get(), std::runtime_error); +} + +TEST_F(FfiClientTest, DISABLED_PublishTrackAsyncFailsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + + // Handle values are placeholders — the gate must fail before they are + // ever forwarded to Rust. + // TrackPublishOptions opts; + // auto fut = FfiClient::instance().publishTrackAsync(/*participant=*/1, /*track=*/2, opts); + // EXPECT_THROW(fut.get(), std::runtime_error); + GTEST_SKIP() << "TODO: enable once publishTrackAsync gate lands"; +} + +TEST_F(FfiClientTest, DISABLED_UnpublishTrackAsyncFailsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + // auto fut = FfiClient::instance().unpublishTrackAsync(/*participant=*/1, "sid", true); + // EXPECT_THROW(fut.get(), std::runtime_error); + GTEST_SKIP() << "TODO: enable once unpublishTrackAsync gate lands"; +} + +TEST_F(FfiClientTest, DISABLED_PublishDataAsyncFailsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + // const std::uint8_t payload[1] = {0}; + // auto fut = FfiClient::instance().publishDataAsync( + // /*participant=*/1, payload, 1, /*reliable=*/true, /*destinations=*/{}, /*topic=*/""); + // EXPECT_THROW(fut.get(), std::runtime_error); + GTEST_SKIP() << "TODO: enable once publishDataAsync gate lands"; +} + +TEST_F(FfiClientTest, DISABLED_PublishSipDtmfAsyncFailsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + // auto fut = FfiClient::instance().publishSipDtmfAsync(/*participant=*/1, /*code=*/1, "1", {}); + // EXPECT_THROW(fut.get(), std::runtime_error); + GTEST_SKIP() << "TODO: enable once publishSipDtmfAsync gate lands"; +} + +TEST_F(FfiClientTest, DISABLED_SetLocalMetadataAsyncFailsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + // auto fut = FfiClient::instance().setLocalMetadataAsync(/*participant=*/1, "metadata"); + // EXPECT_THROW(fut.get(), std::runtime_error); + GTEST_SKIP() << "TODO: enable once setLocalMetadataAsync gate lands"; +} + +TEST_F(FfiClientTest, DISABLED_CaptureAudioFrameAsyncFailsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + // proto::AudioFrameBufferInfo buf; + // auto fut = FfiClient::instance().captureAudioFrameAsync(/*source=*/1, buf); + // EXPECT_THROW(fut.get(), std::runtime_error); + GTEST_SKIP() << "TODO: enable once captureAudioFrameAsync gate lands"; +} + +TEST_F(FfiClientTest, DISABLED_PerformRpcAsyncFailsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + // auto fut = FfiClient::instance().performRpcAsync( + // /*participant=*/1, "dest", "method", "payload", std::nullopt); + // EXPECT_THROW(fut.get(), std::runtime_error); + GTEST_SKIP() << "TODO: enable once performRpcAsync gate lands"; +} + +TEST_F(FfiClientTest, DISABLED_GetTrackStatsAsyncFailsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + // auto fut = FfiClient::instance().getTrackStatsAsync(/*track=*/1); + // EXPECT_THROW(fut.get(), std::runtime_error); + GTEST_SKIP() << "TODO: enable once getTrackStatsAsync gate lands"; +} + +TEST_F(FfiClientTest, DISABLED_PublishDataTrackAsyncReturnsFailureWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + + // publishDataTrackAsync's future yields a Result<..., PublishDataTrackError>, + // so the convention here is "failure result" rather than a thrown exception. + // + // auto fut = FfiClient::instance().publishDataTrackAsync(/*participant=*/1, "name"); + // auto result = fut.get(); + // EXPECT_FALSE(result.ok()); + // EXPECT_EQ(result.error().code, PublishDataTrackErrorCode::INVALID_HANDLE); // or NOT_INITIALIZED + GTEST_SKIP() << "TODO: enable once publishDataTrackAsync gate lands; " + "decide between INVALID_HANDLE and a new NOT_INITIALIZED code"; +} + +TEST_F(FfiClientTest, DISABLED_SubscribeDataTrackReturnsFailureWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + + // subscribeDataTrack returns Result<..., SubscribeDataTrackError> directly. + // + // auto result = FfiClient::instance().subscribeDataTrack(/*track=*/1); + // EXPECT_FALSE(result.ok()); + // EXPECT_EQ(result.error().code, SubscribeDataTrackErrorCode::INVALID_HANDLE); // or NOT_INITIALIZED + GTEST_SKIP() << "TODO: enable once subscribeDataTrack gate lands; " + "decide between INVALID_HANDLE and a new NOT_INITIALIZED code"; +} + +TEST_F(FfiClientTest, DISABLED_SendStreamHeaderAsyncFailsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + // proto::DataStream::Header header; + // auto fut = FfiClient::instance().sendStreamHeaderAsync(/*participant=*/1, header, {}, "sender"); + // EXPECT_THROW(fut.get(), std::runtime_error); + GTEST_SKIP() << "TODO: enable once sendStreamHeaderAsync gate lands"; +} + +TEST_F(FfiClientTest, DISABLED_SendStreamChunkAsyncFailsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + // proto::DataStream::Chunk chunk; + // auto fut = FfiClient::instance().sendStreamChunkAsync(/*participant=*/1, chunk, {}, "sender"); + // EXPECT_THROW(fut.get(), std::runtime_error); + GTEST_SKIP() << "TODO: enable once sendStreamChunkAsync gate lands"; +} + +TEST_F(FfiClientTest, DISABLED_SendStreamTrailerAsyncFailsWhenNotInitialized) { + ASSERT_FALSE(FfiClient::instance().isInitialized()); + // proto::DataStream::Trailer trailer; + // auto fut = FfiClient::instance().sendStreamTrailerAsync(/*participant=*/1, trailer, "sender"); + // EXPECT_THROW(fut.get(), std::runtime_error); + GTEST_SKIP() << "TODO: enable once sendStreamTrailerAsync gate lands"; +} + +} // namespace livekit::test From 18a6b122afdd13825605cf5980dfb7198a91a6fd Mon Sep 17 00:00:00 2001 From: Alan George Date: Wed, 13 May 2026 11:03:18 -0600 Subject: [PATCH 21/24] Throw when not initialized --- src/ffi_client.cpp | 6 ++++++ src/livekit.cpp | 4 +--- src/tests/unit/test_ffi_client.cpp | 31 +++++++++--------------------- 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index 29f5dee9..cbfda71d 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -192,6 +192,12 @@ void FfiClient::RemoveListener(ListenerId id) { } proto::FfiResponse FfiClient::sendRequest(const proto::FfiRequest& request) const { + // The Rust FFI will lazily initialize the FFI client when the first request is sent, + // but if not initialized none of the async operations will work. Guarding against that here + if (!isInitialized()) { + throw std::runtime_error("FfiClient::sendRequest failed: LiveKit is not initialized"); + } + std::string bytes; if (!request.SerializeToString(&bytes) || bytes.empty()) { throw std::runtime_error("failed to serialize FfiRequest"); diff --git a/src/livekit.cpp b/src/livekit.cpp index 83f3044a..1417213e 100644 --- a/src/livekit.cpp +++ b/src/livekit.cpp @@ -28,9 +28,7 @@ bool initialize(const LogLevel& level, const LogSink& log_sink) { return ffi_client.initialize(log_sink == LogSink::kCallback); } -bool isInitialized() { - return FfiClient::instance().isInitialized(); -} +bool isInitialized() { return FfiClient::instance().isInitialized(); } void shutdown() { FfiClient::instance().shutdown(); diff --git a/src/tests/unit/test_ffi_client.cpp b/src/tests/unit/test_ffi_client.cpp index 476a2817..9ad31cec 100644 --- a/src/tests/unit/test_ffi_client.cpp +++ b/src/tests/unit/test_ffi_client.cpp @@ -51,7 +51,7 @@ class FfiClientTest : public ::testing::Test { livekit::shutdown(); // This assert helps test the livekit::shutdown() <-> FFI client interface - ASSERT_FALSE(FfiClient::instance().isInitialized()); + ASSERT_FALSE(FfiClient::instance().isInitialized()); } void TearDown() override { livekit::shutdown(); } @@ -67,15 +67,18 @@ TEST_F(FfiClientTest, Singleton) { // Initialization state // --------------------------------------------------------------------------- -TEST_F(FfiClientTest, DefaultUninitialized) { - EXPECT_FALSE(FfiClient::instance().isInitialized()); -} +TEST_F(FfiClientTest, DefaultUninitialized) { EXPECT_FALSE(FfiClient::instance().isInitialized()); } TEST_F(FfiClientTest, Initialize) { EXPECT_TRUE(FfiClient::instance().initialize(false)); EXPECT_TRUE(FfiClient::instance().isInitialized()); } +TEST_F(FfiClientTest, InitializeFromSDK) { + EXPECT_TRUE(livekit::initialize(livekit::LogLevel::Info, livekit::LogSink::kConsole)); + EXPECT_TRUE(FfiClient::instance().isInitialized()); +} + TEST_F(FfiClientTest, DoubleInitialize) { ASSERT_TRUE(FfiClient::instance().initialize(false)); EXPECT_FALSE(FfiClient::instance().initialize(false)) @@ -114,9 +117,6 @@ TEST_F(FfiClientTest, ReinitializeAfterShutdown) { // --------------------------------------------------------------------------- // AddListener / RemoveListener -// -// These touch only C++-side state (a std::unordered_map under a mutex), so -// they are safe to exercise whether or not the FFI runtime is initialized. // --------------------------------------------------------------------------- TEST_F(FfiClientTest, AddListenerReturnsNonZeroId) { @@ -171,20 +171,7 @@ TEST_F(FfiClientTest, SendRequestThrowsOnEmptyRequest) { EXPECT_THROW(FfiClient::instance().sendRequest(req), std::runtime_error); } -// --------------------------------------------------------------------------- -// PROPOSED: sendRequest() isInitialized() gate (Tier 1 #1) -// -// These tests describe the *proposed* behavior: sendRequest() should throw -// std::runtime_error when the SDK has not been initialized, instead of -// calling livekit_ffi_request() on a torn-down/never-started Rust runtime. -// -// They are DISABLED today because, without the gate, calling sendRequest() -// on an uninitialized client invokes Rust FFI behavior that is undefined -// and may crash the test process. Once the gate lands in -// FfiClient::sendRequest(), strip the DISABLED_ prefix. -// --------------------------------------------------------------------------- - -TEST_F(FfiClientTest, DISABLED_SendRequestThrowsWhenNotInitialized) { +TEST_F(FfiClientTest, SendRequestWhenNotInitialized) { ASSERT_FALSE(FfiClient::instance().isInitialized()); proto::FfiRequest req; @@ -192,7 +179,7 @@ TEST_F(FfiClientTest, DISABLED_SendRequestThrowsWhenNotInitialized) { // and hit the proposed init gate instead. (void)req.mutable_dispose(); - EXPECT_THROW(FfiClient::instance().sendRequest(req), std::runtime_error); + // EXPECT_THROW(FfiClient::instance().sendRequest(req), std::runtime_error); } TEST_F(FfiClientTest, DISABLED_SendRequestThrowsAfterShutdown) { From a0b76020f321d95b072bf9cb4e41ee054fe57ed5 Mon Sep 17 00:00:00 2001 From: Alan George Date: Wed, 13 May 2026 17:57:10 -0600 Subject: [PATCH 22/24] Additional FfiClient tests for error states --- src/ffi_client.cpp | 3 +- src/tests/unit/test_ffi_client.cpp | 180 +++++++++-------------------- 2 files changed, 54 insertions(+), 129 deletions(-) diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index e59e8478..bd982de1 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -188,7 +188,8 @@ void FfiClient::RemoveListener(ListenerId id) { proto::FfiResponse FfiClient::sendRequest(const proto::FfiRequest& request) const { // The Rust FFI will lazily initialize the FFI client when the first request is sent, - // but if not initialized none of the async operations will work. Guarding against that here + // but if not initialized none of the async operations will work. Guarding against that here. + // Improvement ticket added to the Rust SDK to discuss this if (!isInitialized()) { throw std::runtime_error("FfiClient::sendRequest failed: LiveKit is not initialized"); } diff --git a/src/tests/unit/test_ffi_client.cpp b/src/tests/unit/test_ffi_client.cpp index 9ad31cec..f6bf9d67 100644 --- a/src/tests/unit/test_ffi_client.cpp +++ b/src/tests/unit/test_ffi_client.cpp @@ -14,25 +14,6 @@ * limitations under the License. */ -/// @file test_ffi_client.cpp -/// @brief Unit tests for FfiClient (the internal singleton that bridges the -/// C++ SDK to the Rust FFI runtime). -/// -/// Scope: -/// - Singleton identity -/// - initialize() / shutdown() / isInitialized() state machine -/// - AddListener() / RemoveListener() lifecycle and ID uniqueness -/// - sendRequest() invariants that are reachable without a live FFI -/// roundtrip (empty-serialization failure path) -/// -/// Out of scope (covered by integration tests with a live Rust runtime): -/// - Real sendRequest()/sendRequestAsync() responses -/// - Listener invocation from the FFI callback thread -/// -/// DISABLED_ tests in this file are stubs for the "isInitialized() gate" -/// design proposal (see project notes). Strip the DISABLED_ prefix once -/// the corresponding gate lands in production. - #include #include @@ -139,7 +120,7 @@ TEST_F(FfiClientTest, AddListenerReturnsUniqueIds) { } TEST_F(FfiClientTest, RemoveListenerWithUnknownIdIsSafe) { - EXPECT_NO_THROW(FfiClient::instance().RemoveListener(/*never registered=*/424242)); + EXPECT_NO_THROW(FfiClient::instance().RemoveListener(424242)); } TEST_F(FfiClientTest, RemoveListenerIsIdempotent) { @@ -160,7 +141,7 @@ TEST_F(FfiClientTest, ListenerRegistrationSurvivesShutdownReinitCycle) { } // --------------------------------------------------------------------------- -// sendRequest() — invariants reachable without a live FFI +// These tests ensure FfiClient methods throw in various error conditions // --------------------------------------------------------------------------- TEST_F(FfiClientTest, SendRequestThrowsOnEmptyRequest) { @@ -171,18 +152,7 @@ TEST_F(FfiClientTest, SendRequestThrowsOnEmptyRequest) { EXPECT_THROW(FfiClient::instance().sendRequest(req), std::runtime_error); } -TEST_F(FfiClientTest, SendRequestWhenNotInitialized) { - ASSERT_FALSE(FfiClient::instance().isInitialized()); - - proto::FfiRequest req; - // Populate any oneof so we get past the empty-bytes serialization check - // and hit the proposed init gate instead. - (void)req.mutable_dispose(); - - // EXPECT_THROW(FfiClient::instance().sendRequest(req), std::runtime_error); -} - -TEST_F(FfiClientTest, DISABLED_SendRequestThrowsAfterShutdown) { +TEST_F(FfiClientTest, SendRequestThrowsAfterShutdown) { ASSERT_TRUE(FfiClient::instance().initialize(false)); FfiClient::instance().shutdown(); ASSERT_FALSE(FfiClient::instance().isInitialized()); @@ -193,146 +163,100 @@ TEST_F(FfiClientTest, DISABLED_SendRequestThrowsAfterShutdown) { EXPECT_THROW(FfiClient::instance().sendRequest(req), std::runtime_error); } -// --------------------------------------------------------------------------- -// PROPOSED: *Async helper isInitialized() gates (Tier 1 #3) -// -// Each async helper (connectAsync, publishTrackAsync, captureAudioFrameAsync, -// publishDataAsync, performRpcAsync, publishDataTrackAsync, etc.) should -// surface "not initialized" through its return channel: -// - std::future helpers: the future should resolve with an exception -// (fut.get() throws std::runtime_error). -// - Result helpers (subscribeDataTrack, publishDataTrackAsync's -// Result payload): the Result should be a failure with an appropriate -// error code. -// -// Each stub below pins down the *contract* for one helper. They are kept -// DISABLED for the same reason as sendRequest above: without the gate, the -// helper either calls into Rust unsafely or registers a pending waiter that -// will never complete. -// -// As you add each gate, strip the DISABLED_ prefix one test at a time. -// --------------------------------------------------------------------------- - -TEST_F(FfiClientTest, DISABLED_ConnectAsyncFailsWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_ConnectAsyncThrows) { ASSERT_FALSE(FfiClient::instance().isInitialized()); RoomOptions options; - auto fut = FfiClient::instance().connectAsync("wss://localhost:7880", "fake-token", options); - EXPECT_THROW(fut.get(), std::runtime_error); + EXPECT_THROW(FfiClient::instance().connectAsync("wss://localhost:7880", "fake-token", options), std::runtime_error); } -TEST_F(FfiClientTest, DISABLED_PublishTrackAsyncFailsWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_PublishTrackAsyncThrows) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // Handle values are placeholders — the gate must fail before they are - // ever forwarded to Rust. - // TrackPublishOptions opts; - // auto fut = FfiClient::instance().publishTrackAsync(/*participant=*/1, /*track=*/2, opts); - // EXPECT_THROW(fut.get(), std::runtime_error); - GTEST_SKIP() << "TODO: enable once publishTrackAsync gate lands"; + TrackPublishOptions options; + EXPECT_THROW(FfiClient::instance().publishTrackAsync(1, 2, options), std::runtime_error); } -TEST_F(FfiClientTest, DISABLED_UnpublishTrackAsyncFailsWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_UnpublishTrackAsyncThrows) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // auto fut = FfiClient::instance().unpublishTrackAsync(/*participant=*/1, "sid", true); - // EXPECT_THROW(fut.get(), std::runtime_error); - GTEST_SKIP() << "TODO: enable once unpublishTrackAsync gate lands"; + + EXPECT_THROW(FfiClient::instance().unpublishTrackAsync(1, "sid", true), std::runtime_error); } -TEST_F(FfiClientTest, DISABLED_PublishDataAsyncFailsWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_PublishDataAsyncThrows) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // const std::uint8_t payload[1] = {0}; - // auto fut = FfiClient::instance().publishDataAsync( - // /*participant=*/1, payload, 1, /*reliable=*/true, /*destinations=*/{}, /*topic=*/""); - // EXPECT_THROW(fut.get(), std::runtime_error); - GTEST_SKIP() << "TODO: enable once publishDataAsync gate lands"; + + const std::uint8_t payload[1] = {0}; + EXPECT_THROW(FfiClient::instance().publishDataAsync(1, payload, 1, true, {}, ""), std::runtime_error); } -TEST_F(FfiClientTest, DISABLED_PublishSipDtmfAsyncFailsWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_PublishSipDtmfAsyncThrows) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // auto fut = FfiClient::instance().publishSipDtmfAsync(/*participant=*/1, /*code=*/1, "1", {}); - // EXPECT_THROW(fut.get(), std::runtime_error); - GTEST_SKIP() << "TODO: enable once publishSipDtmfAsync gate lands"; + + EXPECT_THROW(FfiClient::instance().publishSipDtmfAsync(1, 1, "1", {}), std::runtime_error); } -TEST_F(FfiClientTest, DISABLED_SetLocalMetadataAsyncFailsWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_SetLocalMetadataAsyncThrows) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // auto fut = FfiClient::instance().setLocalMetadataAsync(/*participant=*/1, "metadata"); - // EXPECT_THROW(fut.get(), std::runtime_error); - GTEST_SKIP() << "TODO: enable once setLocalMetadataAsync gate lands"; + + EXPECT_THROW(FfiClient::instance().setLocalMetadataAsync(1, "metadata"), std::runtime_error); } -TEST_F(FfiClientTest, DISABLED_CaptureAudioFrameAsyncFailsWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_CaptureAudioFrameAsyncThrows) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // proto::AudioFrameBufferInfo buf; - // auto fut = FfiClient::instance().captureAudioFrameAsync(/*source=*/1, buf); - // EXPECT_THROW(fut.get(), std::runtime_error); - GTEST_SKIP() << "TODO: enable once captureAudioFrameAsync gate lands"; + + proto::AudioFrameBufferInfo buf; + EXPECT_THROW(FfiClient::instance().captureAudioFrameAsync(1, buf), std::runtime_error); } -TEST_F(FfiClientTest, DISABLED_PerformRpcAsyncFailsWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_PerformRpcAsyncThrows) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // auto fut = FfiClient::instance().performRpcAsync( - // /*participant=*/1, "dest", "method", "payload", std::nullopt); - // EXPECT_THROW(fut.get(), std::runtime_error); - GTEST_SKIP() << "TODO: enable once performRpcAsync gate lands"; + + EXPECT_THROW(FfiClient::instance().performRpcAsync(1, "dest", "method", "payload", std::nullopt), std::runtime_error); } -TEST_F(FfiClientTest, DISABLED_GetTrackStatsAsyncFailsWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_GetTrackStatsAsyncThrows) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // auto fut = FfiClient::instance().getTrackStatsAsync(/*track=*/1); - // EXPECT_THROW(fut.get(), std::runtime_error); - GTEST_SKIP() << "TODO: enable once getTrackStatsAsync gate lands"; + + EXPECT_THROW(FfiClient::instance().getTrackStatsAsync(1), std::runtime_error); } -TEST_F(FfiClientTest, DISABLED_PublishDataTrackAsyncReturnsFailureWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_PublishDataTrackAsyncFails) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // publishDataTrackAsync's future yields a Result<..., PublishDataTrackError>, - // so the convention here is "failure result" rather than a thrown exception. - // - // auto fut = FfiClient::instance().publishDataTrackAsync(/*participant=*/1, "name"); - // auto result = fut.get(); - // EXPECT_FALSE(result.ok()); - // EXPECT_EQ(result.error().code, PublishDataTrackErrorCode::INVALID_HANDLE); // or NOT_INITIALIZED - GTEST_SKIP() << "TODO: enable once publishDataTrackAsync gate lands; " - "decide between INVALID_HANDLE and a new NOT_INITIALIZED code"; + auto fut_result = FfiClient::instance().publishDataTrackAsync(1, "name"); + auto result = fut_result.get(); + EXPECT_FALSE(result.ok()); + EXPECT_EQ(result.error().code, PublishDataTrackErrorCode::INTERNAL); } -TEST_F(FfiClientTest, DISABLED_SubscribeDataTrackReturnsFailureWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_SubscribeDataTrackFails) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // subscribeDataTrack returns Result<..., SubscribeDataTrackError> directly. - // - // auto result = FfiClient::instance().subscribeDataTrack(/*track=*/1); - // EXPECT_FALSE(result.ok()); - // EXPECT_EQ(result.error().code, SubscribeDataTrackErrorCode::INVALID_HANDLE); // or NOT_INITIALIZED - GTEST_SKIP() << "TODO: enable once subscribeDataTrack gate lands; " - "decide between INVALID_HANDLE and a new NOT_INITIALIZED code"; + auto result = FfiClient::instance().subscribeDataTrack(1); + EXPECT_FALSE(result.ok()); + EXPECT_EQ(result.error().code, SubscribeDataTrackErrorCode::INTERNAL); } -TEST_F(FfiClientTest, DISABLED_SendStreamHeaderAsyncFailsWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_SendStreamHeaderAsyncThrows) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // proto::DataStream::Header header; - // auto fut = FfiClient::instance().sendStreamHeaderAsync(/*participant=*/1, header, {}, "sender"); - // EXPECT_THROW(fut.get(), std::runtime_error); - GTEST_SKIP() << "TODO: enable once sendStreamHeaderAsync gate lands"; + + proto::DataStream::Header header; + EXPECT_THROW(FfiClient::instance().sendStreamHeaderAsync(1, header, {}, "sender"), std::runtime_error); } -TEST_F(FfiClientTest, DISABLED_SendStreamChunkAsyncFailsWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_SendStreamChunkAsyncThrows) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // proto::DataStream::Chunk chunk; - // auto fut = FfiClient::instance().sendStreamChunkAsync(/*participant=*/1, chunk, {}, "sender"); - // EXPECT_THROW(fut.get(), std::runtime_error); - GTEST_SKIP() << "TODO: enable once sendStreamChunkAsync gate lands"; + + proto::DataStream::Chunk chunk; + EXPECT_THROW(FfiClient::instance().sendStreamChunkAsync(1, chunk, {}, "sender"), std::runtime_error); } -TEST_F(FfiClientTest, DISABLED_SendStreamTrailerAsyncFailsWhenNotInitialized) { +TEST_F(FfiClientTest, NotInitialized_SendStreamTrailerAsyncThrows) { ASSERT_FALSE(FfiClient::instance().isInitialized()); - // proto::DataStream::Trailer trailer; - // auto fut = FfiClient::instance().sendStreamTrailerAsync(/*participant=*/1, trailer, "sender"); - // EXPECT_THROW(fut.get(), std::runtime_error); - GTEST_SKIP() << "TODO: enable once sendStreamTrailerAsync gate lands"; + + proto::DataStream::Trailer trailer; + EXPECT_THROW(FfiClient::instance().sendStreamTrailerAsync(1, trailer, "sender"), std::runtime_error); } } // namespace livekit::test From 39012efff245109d4221c5160c63f521752ed7b2 Mon Sep 17 00:00:00 2001 From: Alan George Date: Wed, 13 May 2026 18:54:04 -0600 Subject: [PATCH 23/24] Revert tests.yml --- .github/workflows/tests.yml | 49 +------------------------------------ 1 file changed, 1 insertion(+), 48 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 97844f97..610eb1c2 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,7 +14,6 @@ on: - vcpkg.json - .token_helpers/** - .github/workflows/tests.yml - - .github/scripts/check_no_private_symbols.py pull_request: branches: ["main"] paths: @@ -28,7 +27,6 @@ on: - vcpkg.json - .token_helpers/** - .github/workflows/tests.yml - - .github/scripts/check_no_private_symbols.py workflow_dispatch: permissions: @@ -164,51 +162,6 @@ jobs: shell: pwsh run: ${{ matrix.build_cmd }} - # ---------- Verify exported ABI: no third-party symbol leaks ---------- - - name: Setup Python - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v6.0.0 - with: - python-version: '3.x' - - - name: Symbol leak check (Unix) - if: runner.os != 'Windows' - continue-on-error: true # Still fail the build, just don't block remaining tests running - shell: bash - run: | - set -eux - libdir="build-release/lib" - if [[ "$RUNNER_OS" == "macOS" ]]; then - lib="${libdir}/liblivekit.dylib" - if [[ ! -f "${lib}" ]]; then - lib="build-release/bin/liblivekit.dylib" - fi - else - lib="${libdir}/liblivekit.so" - if [[ ! -f "${lib}" ]]; then - lib="build-release/bin/liblivekit.so" - fi - fi - python3 .github/scripts/check_no_private_symbols.py "${lib}" - - - name: Symbol leak check (Windows) - if: runner.os == 'Windows' - continue-on-error: true # Still fail the build, just don't block remaining tests running - shell: pwsh - run: | - $candidates = @( - "build-release/bin/livekit.dll", - "build-release/lib/livekit.dll" - ) - $lib = $null - foreach ($p in $candidates) { - if (Test-Path -LiteralPath $p) { $lib = $p; break } - } - if ($null -eq $lib) { - Write-Error "livekit.dll not found in any of: $($candidates -join ', ')" - exit 1 - } - python .github/scripts/check_no_private_symbols.py "$lib" - # ---------- Run unit tests ---------- - name: Run unit tests (Unix) if: runner.os != 'Windows' @@ -392,7 +345,7 @@ jobs: --root . \ --gcov-executable "${GCOV_EXECUTABLE}" \ --gcov-ignore-parse-errors=all \ - --filter 'include/livekit/' \ + --filter 'include/' \ --filter 'src/' \ --exclude 'src/tests/' \ --exclude '.*\.pb\.' \ From 8fc5988252bd3706c5a093bca82776728b7d8aaf Mon Sep 17 00:00:00 2001 From: Alan George Date: Thu, 14 May 2026 17:26:03 -0600 Subject: [PATCH 24/24] Fix AI comment --- src/ffi_client.h | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/ffi_client.h b/src/ffi_client.h index 32fd337d..d16f3d90 100644 --- a/src/ffi_client.h +++ b/src/ffi_client.h @@ -74,12 +74,8 @@ class LIVEKIT_INTERNAL_API FfiClient { FfiClient(FfiClient&&) = delete; FfiClient& operator=(FfiClient&&) = delete; - // Defined out-of-line in ffi_client.cpp so the process has exactly one - // FfiClient. An inline definition would, under the SDK's hidden inline - // visibility, produce a separate function-local static in every TU that - // includes this header (notably: in-tree test executables), giving each - // binary its own "singleton" and silently desynchronizing the dylib-side - // and test-exe-side initialization flags. + // Access the singleton instance of the FfiClient + // Note: lazily created, not thread safe static FfiClient& instance() noexcept; // Must be called before any other FFI usage