Skip to content

Visibility controls on public API#121

Draft
alan-george-lk wants to merge 10 commits intomainfrom
bugfix/truly_private_spdlog
Draft

Visibility controls on public API#121
alan-george-lk wants to merge 10 commits intomainfrom
bugfix/truly_private_spdlog

Conversation

@alan-george-lk
Copy link
Copy Markdown
Collaborator

@alan-george-lk alan-george-lk commented May 7, 2026

This PR changes the following:

  • Define LIVEKIT_API macro to expose LiveKit API symbols across platforms (common convention, example)
  • Update relevant class/method symbols with this new macro
  • Hide third-party symbols from consumers
    • spdlog was installed at the system level in CI (and documented for users to install at that level), but was brought in via FetchContent/vcpkg. This is no longer needed (system install). Removed that flow and deprecated the CMake argument/functionality for using system level spdlog. It is now entirely within the SDK and fully private/no symbols exposed to clash

Testing

  • Built against/linked in downstream project that originally exposed this issue (suspected spdlog clash with third-party lib)
  • Unit/integration tests are standalone binaries that practice this linking, ensured they still worked via CI

@alan-george-lk alan-george-lk changed the title Visibility/export controls on public API Visibility controls on public API May 8, 2026
Comment thread .github/workflows/builds.yml
#include "audio_frame.h"
#include "local_track_publication.h"
#include "track.h"
#include "visibility.h"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some places i see:
#include "livekit/visibility.h" in other locations #include "visibility.h"

Copy link
Copy Markdown
Collaborator Author

@alan-george-lk alan-george-lk May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, I think Opus added it like this to match convention already in the files. We should come up with a standard convention (using the livekit/ prefix or not) and commit to that. See how the ones above this line don't have the prefix, but this file does use them even though it's the same folder:

#include "livekit/ffi_handle.h"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, this is probably AI run away, i think we should be using #include "livekit/*

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants