Skip to content

Handle dmabuf using existing capture path#1279

Open
ladvoc wants to merge 3 commits into
mainfrom
ladvoc/uniform-frame-capture
Open

Handle dmabuf using existing capture path#1279
ladvoc wants to merge 3 commits into
mainfrom
ladvoc/uniform-frame-capture

Conversation

@ladvoc

@ladvoc ladvoc commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Previously, dedicated methods on NativeVideoSource were used to capture dmabuf; these have been deprecated and dmabuf is now handled by NativeBuffer (also handles analogous zero-copy CVPixelBuffer for Apple platforms). See the updated example for new usage.

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Changeset ✓

This PR includes a changeset covering all affected packages:

Package Bump
libwebrtc minor
livekit patch
livekit-ffi patch
webrtc-sys minor

@ladvoc
ladvoc force-pushed the ladvoc/uniform-frame-capture branch from 5013593 to ac1b1d7 Compare July 23, 2026 18:58
@ladvoc
ladvoc marked this pull request as ready for review July 23, 2026 19:25
@ladvoc
ladvoc requested a review from cloudwebrtc as a code owner July 23, 2026 19:25
@ladvoc
ladvoc requested a review from chenosaurus July 23, 2026 19:25

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment on lines 96 to 98
pub fn capture_frame<T: AsRef<dyn VideoBuffer>>(&self, frame: &VideoFrame<T>) {
self.handle.capture_frame(frame)
}

/// Captures one pre-encoded video access unit.
pub fn capture_encoded_frame(&self, frame: &EncodedVideoFrame<'_>) -> bool {
self.handle.capture_encoded_frame(frame)
}

/// Returns and clears the pending keyframe request raised by the
/// pass-through encoder (PLI/FIR or reconfiguration).
pub fn take_keyframe_request(&self) -> bool {
self.handle.take_keyframe_request()
}

/// Returns and clears the pending rate-control target raised by the
/// pass-through encoder.
pub fn take_rate_control_request(&self) -> Option<EncodedRateControl> {
self.handle.take_rate_control_request()
self.handle.capture_frame(frame);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Recommended replacement for capturing camera buffers can no longer report dropped frames

The public capture method (capture_frame at libwebrtc/src/video_source.rs:96-98) throws away the accept/drop result returned by the lower layer, so callers migrating from the now-deprecated dmabuf method (which returned a boolean) can no longer tell whether a submitted frame was dropped.
Impact: Code that relied on knowing a frame was dropped for pacing/statistics loses that signal after following the recommended migration.

Return-value swallowed at the public wrapper

The inner implementation was changed to return bool and explicitly documented as "Returns false if the frame was dropped by the adapter" (libwebrtc/src/native/video_source.rs:125-127,160). The deprecated capture_dmabuf_frame_with_metadata (libwebrtc/src/video_source.rs:133-157) returns bool, and its #[deprecated] note tells users to migrate to capture_frame. However the public capture_frame at libwebrtc/src/video_source.rs:96-98 terminates the call with ; and returns (), discarding the boolean. Migrating dmabuf users therefore lose the drop-detection return value that the deprecated API provided. Propagating the bool here would be a (nearly) non-breaking change since existing capture_frame(&f); statement callers are unaffected.

Suggested change
pub fn capture_frame<T: AsRef<dyn VideoBuffer>>(&self, frame: &VideoFrame<T>) {
self.handle.capture_frame(frame)
}
/// Captures one pre-encoded video access unit.
pub fn capture_encoded_frame(&self, frame: &EncodedVideoFrame<'_>) -> bool {
self.handle.capture_encoded_frame(frame)
}
/// Returns and clears the pending keyframe request raised by the
/// pass-through encoder (PLI/FIR or reconfiguration).
pub fn take_keyframe_request(&self) -> bool {
self.handle.take_keyframe_request()
}
/// Returns and clears the pending rate-control target raised by the
/// pass-through encoder.
pub fn take_rate_control_request(&self) -> Option<EncodedRateControl> {
self.handle.take_rate_control_request()
self.handle.capture_frame(frame);
}
pub fn capture_frame<T: AsRef<dyn VideoBuffer>>(&self, frame: &VideoFrame<T>) -> bool {
self.handle.capture_frame(frame)
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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.

1 participant