Fix/windows desktop capture - #263
Merged
Merged
Conversation
…tch) Windows screen/window capture has been completely broken since the branch-heads/7339 -> 7977 (m140 -> m152) bump: zero frames captured, and the "screens" enumeration returned application windows. Root cause is a DesktopCaptureOptions ABI mismatch between webrtc.lib and the JNI wrapper. gn builds the lib with its default rtc_enable_win_wgc=true (webrtc.gni: rtc_enable_win_wgc = is_win), so the lib-side class contains the RTC_ENABLE_WIN_WGC-guarded members; the wrapper compiled without the define sees a smaller class (64 vs 80 bytes on 7977) with different member offsets for everything after the WGC block. DesktopCaptureOptions::CreateDefault() and the copy constructor are defined out-of-line in the lib, so `auto options = CreateDefault()` in the wrapper writes an 80-byte object into a 64-byte stack slot: 16 bytes of adjacent stack are zero-smashed on every capturer construction. That deterministic smash is what zeroed jni::DesktopCapturer's spilled screenCapturer argument (every ScreenCapturer.initialize() logged screenCapturer=0 and constructed the window capturer instead - the "Screens tab lists applications" bug), and general field-offset skew after the WGC block broke frame delivery entirely. The mismatch already existed on 7339, but the guarded block was then just 5 bools at the tail-adjacent position: an 8-byte zero overflow and skewed reads of cosmetic flags (disable_effects_), which happened to be survivable - which is why 0.14.0 worked. 7977 grew the block (7 bools + LUID) and added std::optional<Environment> env_ after it, making the corruption fatal. Fix: define RTC_ENABLE_WIN_WGC for the wrapper so both sides agree on the layout. This is the same ABI bug class already fixed on Linux with WEBRTC_USE_X11 (devopvoid#236) and WEBRTC_USE_PIPEWIRE (devopvoid#261). All allow_wgc_* options default to false, so capturer selection behavior is unchanged (DirectX/GDI as before).
With the RTC_ENABLE_WIN_WGC ABI fix in place, screen capture works but window (application) capture still delivered no frames: neither thumbnails (DesktopCaptureCallback) nor the live share (VideoTrackDesktopSource). Both files feed libyuv::ConvertToI420 with the cropped output height (i420Buffer/buffer->height() == crop_h) in the src_height slot. libyuv bounds-checks crop_y + crop_height <= src_height, so the conversion fails with -1 whenever crop_y > 0. The Windows border-crop branch sets crop_y = -top_left().y() for frames that start above the screen origin, which is the case for every MAXIMIZED window (its frame sits at -SM_CXPADDEDBORDER). Screen frames never enter that branch at all (exact stride, `fullscreen == true`), which is why only window capture appeared broken. Confirmed at runtime on a diagnostic build (Komm, Windows 11): size=1928x1043 crop_y=7 crop_w=1920 crop_h=1032 Failed to convert desktop frame to I420, libyuv result=-1 Also propagate non-SUCCESS capture results to the Java callback (with a null frame) instead of silently returning, so callers don't have to burn a timeout to notice a failed capture.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes Windows desktop capture, which is completely broken since the branch-heads/7339 → 7977 (m140 → m152) WebRTC bump: zero frames from both screen and window capture, and
ScreenCapturer.getDesktopSources()returning application windows instead of monitors.Two independent bugs:
1.
DesktopCaptureOptionsABI mismatch (RTC_ENABLE_WIN_WGC)gn builds
webrtc.libwith its defaultrtc_enable_win_wgc = is_win(seewebrtc.gni), so every lib-side translation unit compilesDesktopCaptureOptionswith theRTC_ENABLE_WIN_WGC-guarded members. The JNI wrapper compiles the same header without the define, and therefore sees a different class layout: on branch-heads/7977 the sizes are 80 bytes (lib) vs 64 bytes (wrapper), and every member after the WGC block (use_update_notifications_,disable_effects_,prefer_cursor_embedded_,env_, …) sits at different offsets.DesktopCaptureOptions::CreateDefault()and the copy constructor are defined out-of-line in the lib, soauto options = DesktopCaptureOptions::CreateDefault();in the wrapper writes an 80-byte object into a 64-byte stack slot — deterministically zero-smashing 16 bytes of adjacent stack on every capturer construction. On our machine that smash clobbered the spilledscreenCapturerargument ofjni::DesktopCapturer's constructor tofalse, which is why everyScreenCapturerbehaved as aWindowCapturer(monitor list showing windows), and general offset skew broke frame delivery entirely.The mismatch already existed on 7339, but the guarded block was then just 5 bools directly before the trailing bools — an 8-byte zero overflow and misreads of cosmetic flags, which happened to be survivable. 7977 grew the block (7 bools + a
LUID) and addedstd::optional<Environment> env_after it, making the corruption fatal.Fix: define
RTC_ENABLE_WIN_WGCfor the wrapper on Windows so both sides agree on the layout. This is the same ABI bug class already fixed on Linux withWEBRTC_USE_X11(#236) and documented forWEBRTC_USE_PIPEWIRE(#261). Allallow_wgc_*options default tofalse, so capturer selection is unchanged (DirectX/GDI as before) — but the WGC capturers become opt-in-able as a bonus.2. Wrong
src_heightpassed tolibyuv::ConvertToI420(window capture)Both JNI conversion sites (
DesktopCaptureCallback.cppandVideoTrackDesktopSource.cpp) pass the cropped output height (i420Buffer->height()/buffer->height()==crop_h) inConvertToI420'ssrc_heightparameter, which must describe the full source frame. libyuv bounds-checkscrop_y + crop_height <= src_heightinternally, so the conversion fails with-1whenevercrop_y > 0.The
WEBRTC_WIN"crop black window borders" branch setscrop_y = -top_left().y()for frames starting above the screen origin — which is the case for every maximized window, whose frame sits at-SM_CXPADDEDBORDER. Screen frames have exact stride (fullscreen == true) and never enter that branch, which is why only window capture appeared broken. Confirmed at runtime with a diagnostic build:Fix: pass the actual frame height. Additionally,
DesktopCaptureCallbacknow forwards non-SUCCESScapture results to the Java callback (with anullframe) instead of silently returning, so callers can react to a failed capture instead of having to rely on timeouts. (Java callbacks should null-check the frame — theDesktopCapturer.Resultparameter has always implied that a frame may be absent.)Testing
Verified on Windows 11 (x64) with a JavaFX app using
ScreenCapturer,WindowCapturerandVideoDesktopSourceagainst a LiveKit SFU:Linux and macOS are unaffected (fix 1 is inside the
WIN32branch; fix 2's crop branch isWEBRTC_WIN-only, and passing the true source height is correct on every platform).