Skip to content

Remove registration of video/audio callbacks based on TrackSource#110

Open
stephen-derosa wants to merge 2 commits intolivekit:mainfrom
stephen-derosa:sderosa/remove-source-type-callback-registration
Open

Remove registration of video/audio callbacks based on TrackSource#110
stephen-derosa wants to merge 2 commits intolivekit:mainfrom
stephen-derosa:sderosa/remove-source-type-callback-registration

Conversation

@stephen-derosa
Copy link
Copy Markdown
Contributor

@stephen-derosa stephen-derosa commented Apr 23, 2026

bridge changes were only applied to truly purge the public API of this improper implementation.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the SDK callback registration model to be keyed by (participant_identity, track_name) (removing TrackSource-based registration), and extends the video pipeline to optionally carry packet-trailer metadata (user timestamps / frame IDs) through capture, publish options, and receive callbacks.

Changes:

  • Removed TrackSource-keyed audio/video callback registration and updated Room/dispatcher/bridge APIs and tests to use track names.
  • Added video frame metadata support via VideoFrameMetadata / VideoCaptureOptions, protobuf conversion helpers, and a richer VideoFrameEvent callback path.
  • Added unit + integration tests for metadata round-tripping and updated test build/linking (notably for Windows protobuf symbol handling).

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/video_utils.h Adds proto conversions for optional video frame metadata.
src/video_utils.cpp Implements metadata conversions and minor const-correctness tweaks.
src/video_stream.cpp Populates VideoFrameEvent with optional received metadata.
src/video_source.cpp Adds VideoCaptureOptions overload and forwards optional metadata to FFI.
src/video_frame.cpp Simplifies pointer casts and minor formatting changes in owned-buffer copy path.
src/tests/unit/test_video_frame_metadata.cpp New unit tests for metadata/proto conversions and publish options trailer features.
src/tests/unit/test_subscription_thread_dispatcher.cpp Updates dispatcher tests to use track-name keyed callbacks.
src/tests/unit/test_room_callbacks.cpp Updates room callback tests to use track-name keyed callbacks.
src/tests/integration/test_video_frame_metadata.cpp New integration test validating metadata round-trip to receiver callback.
src/tests/CMakeLists.txt Adjusts test linking/includes, including Windows protobuf symbol handling.
src/subscription_thread_dispatcher.cpp Removes source-keyed paths; adds event callback registration and dispatch.
src/room.cpp Routes subscribe/unsubscribe events using track-name keyed dispatcher API.
src/room_proto_converter.cpp Adds proto conversion for PacketTrailerFeatures within TrackPublishOptions.
include/livekit/video_stream.h Extends VideoFrameEvent with optional metadata and clarifies timestamp semantics.
include/livekit/video_source.h Introduces VideoFrameMetadata + VideoCaptureOptions and new capture overload.
include/livekit/subscription_thread_dispatcher.h Updates public dispatcher API to track-name keys; adds VideoFrameEventCallback.
include/livekit/room.h Updates public Room callback APIs (track-name key) and adds video frame event callback setter.
include/livekit/room_event_types.h Adds PacketTrailerFeatures and wires into TrackPublishOptions.
bridge/tests/test_livekit_bridge.cpp Updates bridge callback tests to use track names.
bridge/src/livekit_bridge.cpp Updates bridge callback registration/clearing to use track names.
bridge/README.md Updates bridge documentation to describe track-name keyed callbacks.
bridge/include/livekit_bridge/livekit_bridge.h Updates bridge public header docs and signatures to use track names.
AGENTS.md Adds guidelines on fixed-width integer type usage.
.token_helpers/set_data_track_test_tokens.bash Adds helper script to generate and export integration-test tokens.
.token_helpers/README.md Adds brief documentation for token helper scripts.
.token_helpers/gen_and_set.bash Adds helper script to generate and export a single token for local use.
.github/workflows/builds.yml Disables clang-tidy thread comments in CI configuration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/room.cpp Outdated
Comment thread include/livekit/video_source.h Outdated
Comment thread .token_helpers/gen_and_set.bash Outdated
Comment thread .token_helpers/set_data_track_test_tokens.bash Outdated
@stephen-derosa stephen-derosa force-pushed the sderosa/remove-source-type-callback-registration branch 2 times, most recently from 0eeab14 to ade85fa Compare April 23, 2026 21:53
@stephen-derosa stephen-derosa changed the title Remove registration of video/audio callbacks based on Source Remove registration of video/audio callbacks based on TrackSource Apr 23, 2026
@stephen-derosa stephen-derosa force-pushed the sderosa/remove-source-type-callback-registration branch from 5b78d6b to ac93d6b Compare April 23, 2026 22:40
Copy link
Copy Markdown
Contributor

@alan-george-lk alan-george-lk left a comment

Choose a reason for hiding this comment

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

LGTM, just confirming (other than bridge) all of the header changes are in private scope (I think they are but hard to tell from PR diff)

@stephen-derosa
Copy link
Copy Markdown
Contributor Author

LGTM, just confirming (other than bridge) all of the header changes are in private scope (I think they are but hard to tell from PR diff)

these changes touch public api code -- it breaks usage. We will major bump next release

Comment thread README.md

# Deprecation
- livekit_bridge (bridge/ folder) is deprecated. Avoid using it. Migrate to the base SDK. This will be removed on 06/01/2026.
- setOn*FrameCallback with TrackSource is deprecated. Use track name instead. This will be removed on 06/01/2026.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added this chunk here to keep track of deprecated and breaking changes. If we decide to not introduce breaking changes right now, we can just commit the README changes and officially remove with the bridge on 06/01.

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.

3 participants