Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add APIs for converting between native VideoFrameBuffers and their underlying CVPixelBuffer on macOS #355

Merged
merged 6 commits into from
Jun 26, 2024

Conversation

maxbrunsfeld
Copy link
Contributor

Depends on #354 (this branch contains that change as well).

Background

In the Zed editor, we're switching over from using the LiveKit Swift SDK to this crate. The main motivation is that we want our LiveKit-based screen-sharing and audio features to work on Linux (and someday Windows), not just macOS.

In order to implement screen sharing efficiently (as it was before, when using the Swift SDK), we want to pass video frames directly from the ScreenCaptureKit framework to this crate's NativeVideoSource::capture_frame method, without copying them into intermediate buffers. Similarly, when playing remote video tracks, we want to take decoded video frames directly from the NativeVideoStream and use them as GPU textures without performing an extra copy.

Change

To support both of these use cases, we have added two new macOS-specific APIs to the video_frame::NativeBuffer type: from_cv_pixel_buffer, and get_cv_pixel_buffer. I think it makes sense to have this functionality, since AFAICT, the purpose of the Native variant of VideoFrameBuffer is to adapt a platform-specific video frame representation into the WebRTC's logic.

Details

  • We haven't attempted to make the APIs type safe. This would drag in a dependency on the macOS CoreMedia framework, so we decided to leave the interface in terms of *mut c_void.
  • The get_cv_pixel_buffer can return NULL if the native buffer is not actually backed by a CVPixelBuffer. Rather than implicitly perform some potentially expensive conversion implicitly, we decided to leave that to the caller. The only point of these method is to enable a fast path in which we minimize conversions.

Let me know if there is a different solution to this problem that you'd prefer. Thanks!

@maxbrunsfeld maxbrunsfeld force-pushed the native-video-frame-buffer branch 3 times, most recently from fc222ea to 0867d5b Compare June 26, 2024 03:08
@maxbrunsfeld maxbrunsfeld marked this pull request as ready for review June 26, 2024 03:09
@theomonnom theomonnom merged commit 2487755 into livekit:main Jun 26, 2024
12 of 16 checks passed
@theomonnom
Copy link
Member

theomonnom commented Jun 26, 2024

Awesome, thanks! 🙌

@maxbrunsfeld maxbrunsfeld deleted the native-video-frame-buffer branch June 26, 2024 15:13
SomeoneToIgnore added a commit to zed-industries/zed that referenced this pull request Nov 15, 2024
See livekit/rust-sdks#355

Todo:

* [x] make `call` / `live_kit_client` crates use the livekit rust sdk
* [x] create a fake version of livekit rust API for integration tests
* [x] capture local audio
* [x] play remote audio
* [x] capture local video tracks
* [x] play remote video tracks
* [x] tests passing
* bugs
* [x] deafening does not work
(livekit/rust-sdks#359)
* [x] mute and speaking status are not replicated properly:
(livekit/rust-sdks#358)
* [x] **linux** - crash due to symbol conflict between WebRTC's
BoringSSL and libcurl's openssl
(livekit/rust-sdks#89)
* [x] **linux** - libwebrtc-sys adds undesired dependencies on `libGL`
and `libXext`
* [x] **windows** - linker error, maybe related to the C++ stdlib
(livekit/rust-sdks#364)
        ```
libwebrtc_sys-54978c6ad5066a35.rlib(video_frame.obj) : error LNK2038:
mismatch detected for 'RuntimeLibrary': value 'MT_StaticRelease' doesn't
match value 'MD_DynamicRelease' in
libtree_sitter_yaml-df6b0adf8f009e8f.rlib(2e40c9e35e9506f4-scanner.o)
        ```
    * [x] audio problems

Release Notes:

- Switch from Swift to Rust LiveKit SDK 🦀

---------

Co-authored-by: Mikayla Maki <[email protected]>
Co-authored-by: Conrad Irwin <[email protected]>
Co-authored-by: Kirill Bulatov <[email protected]>
Co-authored-by: Michael Sloan <[email protected]>
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