DirectX capturer may crash after switching shared screen

webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 39 matching lines @@
     case DXGI_MODE_ROTATION_ROTATE180:
       return Rotation::CLOCK_WISE_180;
     case DXGI_MODE_ROTATION_ROTATE270:
       return Rotation::CLOCK_WISE_270;
   }
 
   RTC_NOTREACHED();
   return Rotation::CLOCK_WISE_0;
 }
 
-// Translates |rect| with the reverse of |offset|
-DesktopRect ReverseTranslate(DesktopRect rect, DesktopVector offset) {
-  rect.Translate(-offset.x(), -offset.y());
-  return rect;
-}
-
 }  // namespace
 
 DxgiOutputDuplicator::DxgiOutputDuplicator(const D3dDevice& device,
                                            const ComPtr<IDXGIOutput1>& output,
                                            const DXGI_OUTPUT_DESC& desc)
     : device_(device),
       output_(output),
       desktop_rect_(RECTToDesktopRect(desc.DesktopCoordinates)) {
   RTC_DCHECK(output_);
   RTC_DCHECK(!desktop_rect_.is_empty());
@@ skipping 49 matching lines @@
       static_cast<int>(desc_.ModeDesc.Height) != desktop_rect_.height()) {
     LOG(LS_ERROR) << "IDXGIDuplicateOutput does not return a same size as its "
                      "IDXGIOutput1, size returned by IDXGIDuplicateOutput is "
                   << desc_.ModeDesc.Width << " x " << desc_.ModeDesc.Height
                   << ", size returned by IDXGIOutput1 is "
                   << desktop_rect_.width() << " x " << desktop_rect_.height();
     return false;
   }
 
   rotation_ = DxgiRotationToRotation(desc_.Rotation);
-  unrotated_size_ =
-      RotateSize(desktop_rect_.size(), ReverseRotation(rotation_));
+  unrotated_size_ = RotateSize(desktop_size(), ReverseRotation(rotation_));
 
   return true;
 }
 
 bool DxgiOutputDuplicator::ReleaseFrame() {
   RTC_DCHECK(duplication_);
   _com_error error = duplication_->ReleaseFrame();
   if (error.Error() != S_OK) {
     LOG(LS_ERROR) << "Failed to release frame from IDXGIOutputDuplication, "
                      "error"
@@ skipping 21 matching lines @@
   _com_error error = duplication_->AcquireNextFrame(
       kAcquireTimeoutMs, &frame_info, resource.GetAddressOf());
   if (error.Error() != S_OK && error.Error() != DXGI_ERROR_WAIT_TIMEOUT) {
     LOG(LS_ERROR) << "Failed to capture frame, error " << error.ErrorMessage()
                   << ", code " << error.Error();
     return false;
   }
 
   // We need to merge updated region with the one from context, but only spread
   // updated region from current frame. So keeps a copy of updated region from
-  // context here.
+  // context here. The |updated_region| always starts from (0, 0).
   DesktopRegion updated_region;
   updated_region.Swap(&context->updated_region);
   if (error.Error() == S_OK &&
       frame_info.AccumulatedFrames > 0 &&
       resource) {
-    DetectUpdatedRegion(frame_info, offset, &context->updated_region);
+    DetectUpdatedRegion(frame_info, &context->updated_region);
+    SpreadContextChange(context);
     if (!texture_->CopyFrom(frame_info, resource.Get())) {
       return false;
     }
-    SpreadContextChange(context);
     updated_region.AddRegion(context->updated_region);
+    // TODO(zijiehe): Why clearing context->updated_region() here triggers

[Sergey Ulanov, 2017/04/05 19:47:57]

nit: s/Why/Why does/ and s/triggers/trigger/. Or alternatively make this a
statement instead of a question, e.g. "Figure out why ..."

[Hzj_jie, 2017/04/05 21:16:59]

Done.

+    // screen flickering?
 
     const DesktopFrame& source = texture_->AsDesktopFrame();
     if (rotation_ != Rotation::CLOCK_WISE_0) {
       for (DesktopRegion::Iterator it(updated_region); !it.IsAtEnd();
            it.Advance()) {
-        const DesktopRect source_rect =
-            RotateRect(ReverseTranslate(it.rect(), offset),
-                       desktop_rect().size(), ReverseRotation(rotation_));
+        // The |updated_region| returned by Windows is rotated, but the |source|
+        // frame is not. So we need to rotate it reversely.
+        const DesktopRect source_rect = RotateRect(
+            it.rect(), desktop_size(), ReverseRotation(rotation_));
         RotateDesktopFrame(source, source_rect, rotation_, offset, target);
       }
     } else {
       for (DesktopRegion::Iterator it(updated_region); !it.IsAtEnd();
            it.Advance()) {
-        target->CopyPixelsFrom(
-            source, ReverseTranslate(it.rect(), offset).top_left(), it.rect());
+        // The DesktopRect in |source|, starts from (0, 0).
+        const DesktopRect source_rect = it.rect();

[Sergey Ulanov, 2017/04/05 19:47:57]

Maybe remove this var and just use it.rect() below?

[Hzj_jie, 2017/04/05 21:16:58]

Definitely.

+        // The DesktopRect in |target|, starts from offset.
+        DesktopRect dest_rect = source_rect;
+        dest_rect.Translate(offset);
+        target->CopyPixelsFrom(source, source_rect.top_left(), dest_rect);
       }
     }
     last_frame_ = target->Share();
     last_frame_offset_ = offset;
+    updated_region.Translate(offset.x(), offset.y());
     target->mutable_updated_region()->AddRegion(updated_region);
     num_frames_captured_++;
     return texture_->Release() && ReleaseFrame();
   }
 
   if (last_frame_) {
     // No change since last frame or AcquireNextFrame() timed out, we will
     // export last frame to the target.
     for (DesktopRegion::Iterator it(updated_region); !it.IsAtEnd();
          it.Advance()) {
-      target->CopyPixelsFrom(*last_frame_, it.rect().top_left(), it.rect());
+      // The DesktopRect in |source|, starts from last_frame_offset_.
+      DesktopRect source_rect = it.rect();
+      // The DesktopRect in |target|, starts from offset.
+      DesktopRect target_rect = source_rect;
+      source_rect.Translate(last_frame_offset_);
+      target_rect.Translate(offset);
+      target->CopyPixelsFrom(*last_frame_, source_rect.top_left(), target_rect);
     }
+    updated_region.Translate(offset.x(), offset.y());
     target->mutable_updated_region()->AddRegion(updated_region);
   } else {
     // If we were at the very first frame, and capturing failed, the
     // context->updated_region should be kept unchanged for next attempt.
     context->updated_region.Swap(&updated_region);
   }
   // If AcquireNextFrame() failed with timeout error, we do not need to release
   // the frame.
   return error.Error() == DXGI_ERROR_WAIT_TIMEOUT || ReleaseFrame();
 }
 
-DesktopRect DxgiOutputDuplicator::TranslatedDesktopRect(DesktopVector offset) {
-  DesktopRect result(DesktopRect::MakeSize(desktop_rect_.size()));
+DesktopRect DxgiOutputDuplicator::TranslatedDesktopRect(

[Sergey Ulanov, 2017/04/05 19:47:57]

GetTranslatedDesktopRect()?

[Hzj_jie, 2017/04/05 21:16:59]

Done.
Also changed UntranslatedDesktopRect() into GetUntranslatedDesktopRect().

+    DesktopVector offset) const {
+  DesktopRect result(DesktopRect::MakeSize(desktop_size()));
   result.Translate(offset);
   return result;
 }
 
+DesktopRect DxgiOutputDuplicator::UntranslatedDesktopRect() const {

[Sergey Ulanov, 2017/04/05 19:47:57]

desktop_rect()?

[Hzj_jie, 2017/04/05 21:16:59]

It may be too confusing, IMO.

+  return TranslatedDesktopRect(DesktopVector());

[Sergey Ulanov, 2017/04/05 19:47:57]

I think just "DesktopRect::MakeSize(desktop_size())" would be more readable.

[Hzj_jie, 2017/04/05 21:16:58]

Done.

+}
+
 void DxgiOutputDuplicator::DetectUpdatedRegion(
     const DXGI_OUTDUPL_FRAME_INFO& frame_info,
-    DesktopVector offset,
     DesktopRegion* updated_region) {
   if (DoDetectUpdatedRegion(frame_info, updated_region)) {
-    updated_region->Translate(offset.x(), offset.y());
     // Make sure even a region returned by Windows API is out of the scope of
     // desktop_rect_, we still won't export it to the target DesktopFrame.
-    updated_region->IntersectWith(TranslatedDesktopRect(offset));
+    updated_region->IntersectWith(UntranslatedDesktopRect());
   } else {
-    updated_region->SetRect(TranslatedDesktopRect(offset));
+    updated_region->SetRect(UntranslatedDesktopRect());
   }
 }
 
 bool DxgiOutputDuplicator::DoDetectUpdatedRegion(
     const DXGI_OUTDUPL_FRAME_INFO& frame_info,
     DesktopRegion* updated_region) {
   RTC_DCHECK(updated_region);
   updated_region->Clear();
   if (frame_info.TotalMetadataBufferSize == 0) {
     // This should not happen, since frame_info.AccumulatedFrames > 0.
@@ skipping 74 matching lines @@
     dirty_rects++;
     dirty_rects_count--;
   }
 
   return true;
 }
 
 void DxgiOutputDuplicator::Setup(Context* context) {
   RTC_DCHECK(context->updated_region.is_empty());
   // Always copy entire monitor during the first Duplicate() function call.
-  context->updated_region.AddRect(desktop_rect_);
+  context->updated_region.AddRect(UntranslatedDesktopRect());
   RTC_DCHECK(std::find(contexts_.begin(), contexts_.end(), context) ==
              contexts_.end());
   contexts_.push_back(context);
 }
 
 void DxgiOutputDuplicator::Unregister(const Context* const context) {
   auto it = std::find(contexts_.begin(), contexts_.end(), context);
   RTC_DCHECK(it != contexts_.end());
   contexts_.erase(it);
 }
 
 void DxgiOutputDuplicator::SpreadContextChange(const Context* const source) {
   for (Context* dest : contexts_) {
     RTC_DCHECK(dest);
     if (dest != source) {
       dest->updated_region.AddRegion(source->updated_region);
     }
   }
 }
 
+DesktopSize DxgiOutputDuplicator::desktop_size() const {
+  return desktop_rect_.size();
+}
+
 int64_t DxgiOutputDuplicator::num_frames_captured() const {
 #if !defined(NDEBUG)
   RTC_DCHECK_EQ(!!last_frame_, num_frames_captured_ > 0);
 #endif
   return num_frames_captured_;
 }
 
 }  // namespace webrtc