[WebRTC] Two DirectX capturers cannot work concurrently

DirectX capturer won't be able to update the DesktopFrame for the second
ScreenCapturerWinDirectx instance, if AcquireNextFrame() returns timeout or
unchanged error. Current solution uses |last_frame| of the second
ScreenCapturerWinDirectx instance, which also does not contain the updated frame
between this and last AcquireNextFrame() calls. Considering following situation,
(C1: capturer 1, C2: capturer 2, update: screen updated, next AcquireNextFrame()
call will return a new frame, Fx: a frame x)
update -> C2.capture returns F1 -> update -> C1.capture returns F2 ->
C2.capture unchanged
So using F1 to update the last capture frame is not correct, we need to use F2.
Refer to design doc https://goo.gl/hU1ifG for a detail description.

The change also makes DxgiDuplicatorController work with 2+ DesktopFrame queue.
Now TwoDirectxCapturers test can pass.

BUG=314516

Review-Url: https://codereview.webrtc.org/2299663003
Cr-Commit-Position: refs/heads/master@{#14077}

webrtc/modules/desktop_capture/win/dxgi_adapter_duplicator.cc

@@ -113,11 +113,10 @@ void DxgiAdapterDuplicator::Unregister(const Context* const context) {
 }
 
 bool DxgiAdapterDuplicator::Duplicate(Context* context,
-                                      const DesktopFrame* last_frame,
+                                      SharedDesktopFrame* target) {
-                                      DesktopFrame* target) {
   RTC_DCHECK_EQ(context->contexts.size(), duplicators_.size());
   for (size_t i = 0; i < duplicators_.size(); i++) {
-    if (!duplicators_[i].Duplicate(&context->contexts[i], last_frame,
+    if (!duplicators_[i].Duplicate(&context->contexts[i],
                                    duplicators_[i].desktop_rect().top_left(),
                                    target)) {
       return false;
@@ -128,13 +127,12 @@ bool DxgiAdapterDuplicator::Duplicate(Context* context,
 
 bool DxgiAdapterDuplicator::DuplicateMonitor(Context* context,
                                              int monitor_id,
-                                             const DesktopFrame* last_frame,
+                                             SharedDesktopFrame* target) {
-                                             DesktopFrame* target) {
   RTC_DCHECK(monitor_id >= 0 &&
              monitor_id < static_cast<int>(duplicators_.size()) &&
              context->contexts.size() == duplicators_.size());
-  return duplicators_[monitor_id].Duplicate(
+  return duplicators_[monitor_id].Duplicate(&context->contexts[monitor_id],
-      &context->contexts[monitor_id], last_frame, DesktopVector(), target);
+                                            DesktopVector(), target);
 }
 
 DesktopRect DxgiAdapterDuplicator::ScreenRect(int id) const {

webrtc/modules/desktop_capture/win/dxgi_adapter_duplicator.h

@@ -15,9 +15,9 @@
 
 #include <vector>
 
-#include "webrtc/modules/desktop_capture/desktop_frame.h"
 #include "webrtc/modules/desktop_capture/desktop_geometry.h"
 #include "webrtc/modules/desktop_capture/desktop_region.h"
+#include "webrtc/modules/desktop_capture/shared_desktop_frame.h"
 #include "webrtc/modules/desktop_capture/win/d3d_device.h"
 #include "webrtc/modules/desktop_capture/win/dxgi_output_duplicator.h"
 
@@ -44,18 +44,15 @@ class DxgiAdapterDuplicator {
   // Initializes the DxgiAdapterDuplicator from a D3dDevice.
   bool Initialize();
 
-  // Sequential calls Duplicate function of all the DxgiOutputDuplicators owned
+  // Sequentially calls Duplicate function of all the DxgiOutputDuplicator
-  // by this instance.
+  // instances owned by this instance, and writes into |target|.
-  bool Duplicate(Context* context,
+  bool Duplicate(Context* context, SharedDesktopFrame* target);
-                 const DesktopFrame* last_frame,
-                 DesktopFrame* target);
 
-  // Captures one monitor and writes into target. |monitor_id| should be between
+  // Captures one monitor and writes into |target|. |monitor_id| should be
-  // [0, screen_count()).
+  // between [0, screen_count()).
   bool DuplicateMonitor(Context* context,
                         int monitor_id,
-                        const DesktopFrame* last_frame,
+                        SharedDesktopFrame* target);
-                        DesktopFrame* target);
 
   // Returns desktop rect covered by this DxgiAdapterDuplicator.
   DesktopRect desktop_rect() const { return desktop_rect_; }

webrtc/modules/desktop_capture/win/dxgi_duplicator_controller.cc

@@ -176,39 +176,33 @@ void DxgiDuplicatorController::Setup(Context* context) {
 }
 
 bool DxgiDuplicatorController::Duplicate(Context* context,
-                                         const DesktopFrame* last_frame,
+                                         SharedDesktopFrame* target) {
-                                         DesktopFrame* target) {
+  return DoDuplicate(context, -1, target);
-  return DoDuplicate(context, -1, last_frame, target);
 }
 
 bool DxgiDuplicatorController::DuplicateMonitor(Context* context,
                                                 int monitor_id,
-                                                const DesktopFrame* last_frame,
+                                                SharedDesktopFrame* target) {
-                                                DesktopFrame* target) {
   RTC_DCHECK_GE(monitor_id, 0);
-  return DoDuplicate(context, monitor_id, last_frame, target);
+  return DoDuplicate(context, monitor_id, target);
 }
 
 bool DxgiDuplicatorController::DoDuplicate(Context* context,
                                            int monitor_id,
-                                           const DesktopFrame* last_frame,
+                                           SharedDesktopFrame* target) {
-                                           DesktopFrame* target) {
   RTC_DCHECK(target);
-  if (last_frame && !target->size().equals(last_frame->size())) {
-    return false;
-  }
   target->mutable_updated_region()->Clear();
   rtc::CritScope lock(&lock_);
   if (!Initialize()) {
     // Cannot initialize COM components now, display mode may be changing.
     return false;
   }
+
   Setup(context);
   if (monitor_id < 0) {
     // Capture entire screen.
     for (size_t i = 0; i < duplicators_.size(); i++) {
-      if (!duplicators_[i].Duplicate(&context->contexts_[i], last_frame,
+      if (!duplicators_[i].Duplicate(&context->contexts_[i], target)) {
-                                     target)) {
         Deinitialize();
         return false;
       }
@@ -224,7 +218,7 @@ bool DxgiDuplicatorController::DoDuplicate(Context* context,
       monitor_id -= duplicators_[i].screen_count();
     } else {
       if (duplicators_[i].DuplicateMonitor(&context->contexts_[i], monitor_id,
-                                           last_frame, target)) {
+                                           target)) {
         target->set_dpi(dpi());
         return true;
       }

webrtc/modules/desktop_capture/win/dxgi_duplicator_controller.h

@@ -11,12 +11,13 @@
 #ifndef WEBRTC_MODULES_DESKTOP_CAPTURE_WIN_DXGI_DUPLICATOR_CONTROLLER_H_
 #define WEBRTC_MODULES_DESKTOP_CAPTURE_WIN_DXGI_DUPLICATOR_CONTROLLER_H_
 
+#include <memory>
 #include <vector>
 
 #include "webrtc/base/criticalsection.h"
-#include "webrtc/modules/desktop_capture/desktop_frame.h"
 #include "webrtc/modules/desktop_capture/desktop_geometry.h"
 #include "webrtc/modules/desktop_capture/desktop_region.h"
+#include "webrtc/modules/desktop_capture/shared_desktop_frame.h"
 #include "webrtc/modules/desktop_capture/win/d3d_device.h"
 #include "webrtc/modules/desktop_capture/win/dxgi_adapter_duplicator.h"
 
@@ -33,16 +34,6 @@ namespace webrtc {
 // but a later Duplicate() returns false, this usually means the display mode is
 // changing. Consumers should retry after a while. (Typically 50 milliseconds,
 // but according to hardware performance, this time may vary.)
-//
-// This class is normally used with double buffering, e.g. as in
-// ScreenCapturerWinDirectx, but it should work with consumers with one buffer,
-// i.e. consumers can always send nullptr for |last_frame|. Some minor changes
-// in DxgiOutputDuplicator class are nice to have to reduce size of data to copy
-// (Commented in dxgi_output_duplicator.cc). But this class won't work
-// with three or more buffers, the updated region merging logic will be broken
-// in such scenarios. If a consumer does have this requirement, one can always
-// send a new Context instance to Duplicate() function to force duplicator to
-// treat it as a new consumer.
 class DxgiDuplicatorController {
  public:
   // A context to store the status of a single consumer of
@@ -82,18 +73,15 @@ class DxgiDuplicatorController {
   // TODO(zijiehe): Windows cannot guarantee the frames returned by each
   // IDXGIOutputDuplication are synchronized. But we are using a totally
   // different threading model than the way Windows suggested, it's hard to
-  // synchronize them manually. But we should find a way to do it.
+  // synchronize them manually. We should find a way to do it.
-  bool Duplicate(Context* context,
+  bool Duplicate(Context* context, SharedDesktopFrame* target);
-                 const DesktopFrame* last_frame,
-                 DesktopFrame* target);
 
   // Captures one monitor and writes into target. |monitor_id| should >= 0. If
   // |monitor_id| is greater than the total screen count of all the Duplicators,
   // this function returns false.
   bool DuplicateMonitor(Context* context,
                         int monitor_id,
-                        const DesktopFrame* last_frame,
+                        SharedDesktopFrame* target);
-                        DesktopFrame* target);
 
   // Returns dpi of current system. Returns an empty DesktopVector if system
   // does not support DXGI based capturer.
@@ -153,8 +141,7 @@ class DxgiDuplicatorController {
   // Do the real duplication work. |monitor_id < 0| to capture entire screen.
   bool DoDuplicate(Context* context,
                    int monitor_id,
-                   const DesktopFrame* last_frame,
+                   SharedDesktopFrame* target);
-                   DesktopFrame* target);
 
   // This lock must be locked whenever accessing any of the following objects.
   rtc::CriticalSection lock_;

webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc

@@ -118,12 +118,17 @@ bool DxgiOutputDuplicator::ReleaseFrame() {
 }
 
 bool DxgiOutputDuplicator::Duplicate(Context* context,
-                                     const DesktopFrame* last_frame,
+                                     DesktopVector offset,
-                                     const DesktopVector offset,
+                                     SharedDesktopFrame* target) {
-                                     DesktopFrame* target) {
   RTC_DCHECK(duplication_);
   RTC_DCHECK(texture_);
   RTC_DCHECK(target);
+  if (!DesktopRect::MakeSize(target->size())
+           .ContainsRect(TranslatedDesktopRect(offset))) {
+    // target size is not large enough to cover current output region.
+    return false;
+  }
+
   DXGI_OUTDUPL_FRAME_INFO frame_info;
   memset(&frame_info, 0, sizeof(frame_info));
   ComPtr<IDXGIResource> resource;
@@ -140,40 +145,36 @@ bool DxgiOutputDuplicator::Duplicate(Context* context,
   // buffering implementation, as what we have in ScreenCapturerWinDirectx. If
   // a consumer uses single buffering, we should clear context->updated_region
   // after it has been merged to updated_region.
-  DesktopRegion updated_region = context->updated_region;
+  DesktopRegion updated_region;
+  updated_region.Swap(&context->updated_region);
   if (error.Error() == S_OK && frame_info.AccumulatedFrames > 0) {
     DetectUpdatedRegion(frame_info, offset, &context->updated_region);
+    if (!texture_->CopyFrom(frame_info, resource.Get(),
+                            context->updated_region)) {
+      return false;
+    }
     SpreadContextChange(context);
     updated_region.AddRegion(context->updated_region);
-    if (!texture_->CopyFrom(frame_info, resource.Get(), updated_region)) {
-      return false;
-    }
 
     const DesktopFrame& source = texture_->AsDesktopFrame();
-    DesktopRect target_rect(DesktopRect::MakeSize(target->size()));
     for (DesktopRegion::Iterator it(updated_region); !it.IsAtEnd();
          it.Advance()) {
-      if (!target_rect.ContainsRect(it.rect())) {
+      target->CopyPixelsFrom(source, SourceRect(it.rect()).top_left(),
-        // target size is not large enough to copy the pixel from texture.
+                             TargetRect(it.rect(), offset));
-        return false;
-      }
-      target->CopyPixelsFrom(source, it.rect().top_left().subtract(offset),
-                             it.rect());
     }
+    last_frame_ = target->Share();
+    last_frame_offset_ = offset;
     target->mutable_updated_region()->AddRegion(updated_region);
     return texture_->Release() && ReleaseFrame();
   }
 
-  if (last_frame != nullptr) {
+  if (last_frame_) {
-    // DxgiOutputDuplicatorContainer::Duplicate() makes sure target size and
-    // last frame size are consistent.
-    RTC_DCHECK(target->size().equals(last_frame->size()));
     // No change since last frame or AcquireNextFrame() timed out, we will
     // export last frame to the target.
-    context->updated_region.Clear();
     for (DesktopRegion::Iterator it(updated_region); !it.IsAtEnd();
          it.Advance()) {
-      target->CopyPixelsFrom(*last_frame, it.rect().top_left(), it.rect());
+      target->CopyPixelsFrom(*last_frame_, SourceRect(it.rect()).top_left(),
+                             TargetRect(it.rect(), offset));
     }
     target->mutable_updated_region()->AddRegion(updated_region);
   }
@@ -182,8 +183,7 @@ bool DxgiOutputDuplicator::Duplicate(Context* context,
   return error.Error() == DXGI_ERROR_WAIT_TIMEOUT || ReleaseFrame();
 }
 
-DesktopRect DxgiOutputDuplicator::TranslatedDesktopRect(
+DesktopRect DxgiOutputDuplicator::TranslatedDesktopRect(DesktopVector offset) {
-    const DesktopVector offset) {
   DesktopRect result(DesktopRect::MakeSize(desktop_rect_.size()));
   result.Translate(offset);
   return result;
@@ -191,7 +191,7 @@ DesktopRect DxgiOutputDuplicator::TranslatedDesktopRect(
 
 void DxgiOutputDuplicator::DetectUpdatedRegion(
     const DXGI_OUTDUPL_FRAME_INFO& frame_info,
-    const DesktopVector offset,
+    DesktopVector offset,
     DesktopRegion* updated_region) {
   if (DoDetectUpdatedRegion(frame_info, updated_region)) {
     updated_region->Translate(offset.x(), offset.y());
@@ -301,4 +301,17 @@ void DxgiOutputDuplicator::SpreadContextChange(const Context* const source) {
   }
 }
 
+DesktopRect DxgiOutputDuplicator::SourceRect(DesktopRect rect) {
+  // |texture_|->AsDesktopFrame() starts from (0, 0).
+  rect.Translate(-desktop_rect_.left(), -desktop_rect_.top());
+  return rect;
+}
+
+DesktopRect DxgiOutputDuplicator::TargetRect(DesktopRect rect,
+                                             DesktopVector offset) {
+  rect = SourceRect(rect);
+  rect.Translate(offset);
+  return rect;
+}
+
 }  // namespace webrtc

webrtc/modules/desktop_capture/win/dxgi_output_duplicator.h

@@ -21,9 +21,9 @@
 
 #include "webrtc/base/criticalsection.h"
 #include "webrtc/base/thread_annotations.h"
-#include "webrtc/modules/desktop_capture/desktop_frame.h"
 #include "webrtc/modules/desktop_capture/desktop_geometry.h"
 #include "webrtc/modules/desktop_capture/desktop_region.h"
+#include "webrtc/modules/desktop_capture/shared_desktop_frame.h"
 #include "webrtc/modules/desktop_capture/win/d3d_device.h"
 #include "webrtc/modules/desktop_capture/win/dxgi_texture.h"
 
@@ -60,17 +60,14 @@ class DxgiOutputDuplicator {
 
   // Copies the content of current IDXGIOutput to the |target|. To improve the
   // performance, this function copies only regions merged from
-  // |last_frame|.updated_region and DetectUpdatedRegion(). The |offset| decides
+  // |context|->updated_region and DetectUpdatedRegion(). The |offset| decides
-  // the
+  // the offset in the |target| where the content should be copied to. i.e. this
-  // offset in the |target| where the content should be copied to. i.e. this
   // function copies the content to the rectangle of (offset.x(), offset.y()) to
   // (offset.x() + desktop_rect_.width(), offset.y() + desktop_rect_.height()).
-  // The |last_frame| is always expected to be translated by the same offset.
   // Returns false in case of a failure.
   bool Duplicate(Context* context,
-                 const DesktopFrame* last_frame,
+                 DesktopVector offset,
-                 const DesktopVector offset,
+                 SharedDesktopFrame* target);
-                 DesktopFrame* target);
 
   // Returns the desktop rect covered by this DxgiOutputDuplicator.
   DesktopRect desktop_rect() const { return desktop_rect_; }
@@ -82,7 +79,7 @@ class DxgiOutputDuplicator {
   // function will set the |updated_region| as entire DesktopRect starts from
   // offset if it failed to execute Windows APIs.
   void DetectUpdatedRegion(const DXGI_OUTDUPL_FRAME_INFO& frame_info,
-                           const DesktopVector offset,
+                           DesktopVector offset,
                            DesktopRegion* updated_region);
 
   // Returns untranslated updated region, which are directly returned by Windows
@@ -98,7 +95,7 @@ class DxgiOutputDuplicator {
 
   // Returns a DesktopRect with the same size of desktop_size_, but translated
   // by offset.
-  DesktopRect TranslatedDesktopRect(const DesktopVector offset);
+  DesktopRect TranslatedDesktopRect(DesktopVector offset);
 
   void Setup(Context* context);
 
@@ -108,6 +105,12 @@ class DxgiOutputDuplicator {
   // contexts_.
   void SpreadContextChange(const Context* const context);
 
+  // Returns a DesktopRect in the coordinate of |texture_|->AsDesktopFrame().
+  DesktopRect SourceRect(DesktopRect rect);
+
+  // Returns a DesktopRect in the coordinate of |offset|.
+  DesktopRect TargetRect(DesktopRect rect, DesktopVector offset);
+
   const D3dDevice& device_;
   const Microsoft::WRL::ComPtr<IDXGIOutput1> output_;
   const DesktopRect desktop_rect_;
@@ -121,6 +124,12 @@ class DxgiOutputDuplicator {
   // change this time. And during next Duplicate() function call, their
   // updated_region_ will be merged and copied.
   std::vector<Context*> contexts_;
+
+  // The last full frame of this output and its offset. If on AcquireNextFrame()
+  // failed because of timeout, i.e. no update, we can copy content from
+  // |last_frame_|.
+  std::unique_ptr<SharedDesktopFrame> last_frame_;
+  DesktopVector last_frame_offset_;
 };
 
 }  // namespace webrtc

webrtc/modules/desktop_capture/win/screen_capturer_win_directx.cc

@@ -79,7 +79,7 @@ void ScreenCapturerWinDirectx::Capture(const DesktopRegion& region) {
 
   if (current_screen_id == kFullDesktopScreenId) {
     if (!DxgiDuplicatorController::Instance()->Duplicate(
-            &context_, frames_.previous_frame(), frames_.current_frame())) {
+            &context_, frames_.current_frame())) {
       // Screen size may be changed, so we need to reset the frames.
       frames_.Reset();
       callback_->OnCaptureResult(Result::ERROR_TEMPORARY, nullptr);
@@ -87,8 +87,7 @@ void ScreenCapturerWinDirectx::Capture(const DesktopRegion& region) {
     }
   } else {
     if (!DxgiDuplicatorController::Instance()->DuplicateMonitor(
-            &context_, current_screen_id, frames_.previous_frame(),
+            &context_, current_screen_id, frames_.current_frame())) {
-            frames_.current_frame())) {
       // Screen size may be changed, so we need to reset the frames.
       frames_.Reset();
       if (current_screen_id >=