Skip to content

Commit

Permalink
Reset frame queue in ScreenCapturerX11::SelectSource to fix issues wi…
Browse files Browse the repository at this point in the history
…th different sized monitors.

When Chromium displays the selection dialog for screens it gets the thumbnails by calling SelectSource for the first monitor then CaptureFrame, then SelectSource for the next monitor then CaptureFrame, and so on. With 1 or 2 screens this does not show any issues, but with 3 or more screens the program may crash.

The queue of frame buffers is actually just 2 frame buffers that get swapped every time a frame is captured. When you have one monitor both buffers will be sized for it's resolution. When you have two monitor the first buffer is sized for the first monitor and the second buffer for the second monitor. Since the monitors are selected in turn monitors and frame buffers stay matched up and things work fine. With a third monitor the first buffer is sized for the first monitor, but then later reused to capture the third monitor. If the resolution of the third monitor does not match the first we either crash or have extra junk in the frame from when we captured the first monitor.

Bug: chromium:396091
Change-Id: I7b5ee914b02fee48c09422cee1e320396c9550c7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/174520
Commit-Queue: Jamie Walch <jamiewalch@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#31229}
  • Loading branch information
Trevor Hayes authored and Commit Bot committed May 12, 2020
1 parent b856dc1 commit 2aa9356
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion modules/desktop_capture/linux/screen_capturer_x11.cc
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ void ScreenCapturerX11::CaptureFrame() {
return;
}

// If the current frame is from an older generation then allocate a new one.
// Allocate the current frame buffer only if it is not already allocated.
// Note that we can't reallocate other buffers at this point, since the caller
// may still be reading from them.
if (!queue_.current_frame()) {
Expand Down Expand Up @@ -293,6 +293,12 @@ bool ScreenCapturerX11::GetSourceList(SourceList* sources) {
}

bool ScreenCapturerX11::SelectSource(SourceId id) {
// Prevent the reuse of any frame buffers allocated for a previously selected
// source. This is required to stop crashes, or old data from appearing in
// a captured frame, when the new source is sized differently then the source
// that was selected at the time a reused frame buffer was created.
queue_.Reset();

if (!use_randr_ || id == kFullDesktopScreenId) {
selected_monitor_name_ = kFullDesktopScreenId;
selected_monitor_rect_ =
Expand Down

0 comments on commit 2aa9356

Please sign in to comment.