[Chromoting] Improve DirectX capturer to support multiple outputs

webrtc/modules/desktop_capture/win/dxgi_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.
+ */
+
+#include "webrtc/modules/desktop_capture/win/dxgi_duplicator.h"
+
+#include <string.h>
+
+#include <unknwn.h>
+#include <DXGIFormat.h>
+#include <Windows.h>
+
+#include "webrtc/base/checks.h"
+#include "webrtc/base/logging.h"
+#include "webrtc/modules/desktop_capture/win/dxgi_texture_mapping.h"
+#include "webrtc/modules/desktop_capture/win/dxgi_texture_staging.h"
+
+namespace webrtc {
+
+using Microsoft::WRL::ComPtr;
+
+namespace {
+
+// Timeout for AcquireNextFrame() call.
+const int kAcquireTimeoutMs = 10;
+
+DesktopRect RECTToDesktopRect(const RECT& rect) {

[Sergey Ulanov, 2016/07/08 22:36:54]

This is used in only one place. Do you really need it?

[Hzj_jie, 2016/07/11 00:54:59]

But it's in the constructor, we have to use it, otherwise desktop_rect_ won't be
constant.

+  return DesktopRect::MakeLTRB(
+      rect.left, rect.top, rect.right, rect.bottom);
+}
+
+}  // namespace
+
+DxgiDuplicator::DxgiDuplicator(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());
+  RTC_DCHECK(desktop_rect_.left() >= 0 && desktop_rect_.top() >= 0);
+}
+
+DxgiDuplicator::~DxgiDuplicator() {
+  if (duplication_) {
+    duplication_->ReleaseFrame();
+  }
+  texture_.reset();
+}
+
+bool DxgiDuplicator::Initialize() {
+  if (DuplicateOutput()) {
+    if (desc_.DesktopImageInSystemMemory) {
+      texture_.reset(new DxgiTextureMapping(desktop_rect_, duplication_.Get()));
+    } else {
+      texture_.reset(new DxgiTextureStaging(desktop_rect_, device_));
+    }
+    return true;
+  } else {
+    duplication_.Reset();
+    return false;
+  }
+}
+
+bool DxgiDuplicator::DuplicateOutput() {
+  RTC_DCHECK(!duplication_);
+  _com_error error = output_->DuplicateOutput(
+      static_cast<IUnknown*>(device_.d3d_device()),
+      duplication_.GetAddressOf());
+  if (error.Error() != S_OK || !duplication_) {
+    LOG(LS_WARNING) << "Failed to duplicate output from IDXGIOutput1, error "
+                    << error.ErrorMessage() << ", with code " << error.Error();
+    return false;
+  }
+
+  memset(&desc_, 0, sizeof(desc_));

[Sergey Ulanov, 2016/07/08 22:36:54]

desc_ = {};

[Hzj_jie, 2016/07/11 00:55:00]

DXGI_OUTDUPL_DESC is a complex structure (a structure with structures as its
fields), so old version of clang will raise missing-braces warning. As discussed
in
http://stackoverflow.com/questions/13905200/is-it-wise-to-ignore-gcc-clangs-w....
Though I really hate memset, we cannot resolve this issue unless clang is
updated in buildbot.

+  duplication_->GetDesc(&desc_);
+  if (desc_.ModeDesc.Format != DXGI_FORMAT_B8G8R8A8_UNORM) {
+    LOG(LS_ERROR) << "IDXGIDuplicateOutput does not use RGBA (8 bit) "
+                     "format, which is required by downstream components, "
+                     "format is "
+                  << desc_.ModeDesc.Format;
+    return false;
+  }
+
+  if (static_cast<int>(desc_.ModeDesc.Width) != desktop_rect_.width() ||
+      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;
+  }
+
+  return true;
+}
+
+bool DxgiDuplicator::ReleaseFrame() {
+  RTC_DCHECK(duplication_);
+  _com_error error = duplication_->ReleaseFrame();
+  if (error.Error() != S_OK) {
+    LOG(LS_ERROR) << "Failed to release frame from IDXGIOutputDuplication, "
+                     "error" << error.ErrorMessage() << ", code "
+                  << error.Error();
+    return false;
+  }
+  return true;
+}
+
+bool DxgiDuplicator::Duplicate(DesktopFrame* target,
+                               const DesktopFrame* last_frame,
+                               const DesktopVector& offset) {
+  RTC_DCHECK(duplication_);
+  RTC_DCHECK(texture_);
+  RTC_DCHECK(target);
+  DXGI_OUTDUPL_FRAME_INFO frame_info;
+  memset(&frame_info, 0, sizeof(frame_info));

[Sergey Ulanov, 2016/07/08 22:36:54]

frame_info = {} 

[Hzj_jie, 2016/07/11 00:54:59]

Same reason as above.

+  ComPtr<IDXGIResource> resource;
+  // We need to merge updated region with the one from last frame, since current
+  // frame contains the content one frame before.
+  DesktopRegion updated_region;
+  updated_region.AddRegion(updated_region_);
+  _com_error error = duplication_->AcquireNextFrame(
+      kAcquireTimeoutMs, &frame_info, resource.GetAddressOf());
+  if (error.Error() == S_OK) {
+    if (frame_info.AccumulatedFrames > 0) {
+      DetectUpdatedRegion(frame_info, offset);
+      updated_region.AddRegion(updated_region_);
+      if (texture_->CopyFrom(frame_info, resource.Get(), updated_region)) {
+        std::unique_ptr<DesktopFrame> source(texture_->AsDesktopFrame());
+        RTC_DCHECK(source);
+        DesktopRect target_rect(DesktopRect::MakeSize(target->size()));
+        for (DesktopRegion::Iterator it(updated_region);
+             !it.IsAtEnd();
+             it.Advance()) {
+          if (!target_rect.ContainsRect(it.rect())) {
+            // target size is not large enough to copy the pixel from texture.
+            return false;
+          }
+          target->CopyPixelsFrom(*source,
+                                 it.rect().top_left().subtract(offset),
+                                 it.rect());
+        }
+        target->mutable_updated_region()->AddRegion(updated_region);
+        return texture_->Release() && ReleaseFrame();

[Sergey Ulanov, 2016/07/08 22:36:54]

Do we really want to fail Duplicate() if Release() fails here?
Why would ReleaseFrame() fail at all?

[Hzj_jie, 2016/07/11 00:54:59]

According to MSDN, there is zero possibility Release and ReleaseFrame would
fail. But just make sure the logic covers any error code returned by Windows
APIs.

+      }
+      return false;

[Sergey Ulanov, 2016/07/08 22:36:54]

Handle errors first: 
  if (!texture_->CopyFrom(..)) {
    LOG ERROR
    return false;
  }
  copy pixels
  return true;

[Hzj_jie, 2016/07/11 00:54:59]

Done.

+    }
+  } else if (error.Error() != DXGI_ERROR_WAIT_TIMEOUT) {

[Sergey Ulanov, 2016/07/08 22:36:54]

Handle this case first. Then you wouldn't need else

[Hzj_jie, 2016/07/11 00:54:59]

Done.

+    LOG(LS_ERROR) << "Failed to capture frame, error " << error.ErrorMessage()
+                  << ", code " << error.Error();
+    return false;
+  }
+
+  if (last_frame != nullptr) {
+    // DxgiDuplicatorContainer::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 export last
+    // frame to the target.
+    updated_region_.Clear();
+    for (DesktopRegion::Iterator it(updated_region);
+         !it.IsAtEnd();
+         it.Advance()) {
+      target->CopyPixelsFrom(*last_frame, it.rect().top_left(), it.rect());
+    }
+    target->mutable_updated_region()->AddRegion(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 DxgiDuplicator::TranslatedDesktopRect(const DesktopVector& offset) {
+  DesktopRect result(DesktopRect::MakeSize(desktop_rect_.size()));
+  result.Translate(offset);
+  return result;
+}
+
+void DxgiDuplicator::DetectUpdatedRegion(
+    const DXGI_OUTDUPL_FRAME_INFO& frame_info,
+    const DesktopVector& offset) {
+  DesktopRegion untranslated;
+  if (DoDetectUpdatedRegion(frame_info, &untranslated)) {
+    updated_region_.Clear();

[Sergey Ulanov, 2016/07/08 22:36:54]

Do I understand it correctly that if there are multiple capturers they will all
share the same DxgiDuplicator instances, unless they capture different screens?
If so then this code will not set updated_region correctly, as for every change
only one of the capturers will be notified about it.

Let's discuss offline how we can solve this.

[Hzj_jie, 2016/07/11 00:54:59]

No, each monitor has its own DxgiDuplicator, and each video card has its own
DxgiAdapterDuplicator. One DxgiAdapterDuplicator has zero or more
DxgiDuplicator, and DxgiDuplicatorContainer has one or more
DxgiAdapterDuplicator. So this logic should be good. I have written it in the
doc. Feel free to discuss offline later.

[Sergey Ulanov, 2016/07/20 19:05:17]

My concern is about the case when there are two ScreenCapturer instances
capturing the _same_ monitor. In that case they will be using the same
DxgiDuplicator to capture that screen and as far as I can tell they won't work
correctly in that case.
I don't see that scenario being addressed in the design doc

[Hzj_jie, 2016/07/22 18:43:43]

Good point. I should address this issue.

+    for (DesktopRegion::Iterator it(untranslated);
+         !it.IsAtEnd();
+         it.Advance()) {
+      DesktopRect rect = it.rect();
+      rect.Translate(offset);

[Sergey Ulanov, 2016/07/08 22:36:54]

There is DesktopRegion::Translate(). You don't need to translate rects one by
one.

[Hzj_jie, 2016/07/11 00:55:00]

Done.

+      updated_region_.AddRect(rect);
+    }
+    // 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));
+  } else {
+    updated_region_.SetRect(TranslatedDesktopRect(offset));
+  }
+}
+
+bool DxgiDuplicator::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.
+    LOG(LS_ERROR) << "frame_info.AccumulatedFrames > 0, "
+                     "but TotalMetadataBufferSize == 0";
+    return false;
+  }
+
+  if (metadata.capacity() < frame_info.TotalMetadataBufferSize) {
+    metadata.clear();  // Avoid data copy
+    metadata.reserve(frame_info.TotalMetadataBufferSize);
+  }
+
+  UINT buff_size = 0;
+  DXGI_OUTDUPL_MOVE_RECT* move_rects =
+      reinterpret_cast<DXGI_OUTDUPL_MOVE_RECT*>(metadata.data());
+  size_t move_rects_count = 0;
+  _com_error error = _com_error(duplication_->GetFrameMoveRects(
+      static_cast<UINT>(metadata.capacity()), move_rects, &buff_size));
+  if (error.Error() != S_OK) {
+    LOG(LS_ERROR) << "Failed to get move rectangles, error "
+                  << error.ErrorMessage() << ", code " << error.Error();
+    return false;
+  }
+  move_rects_count = buff_size / sizeof(DXGI_OUTDUPL_MOVE_RECT);
+
+  RECT* dirty_rects =
+      reinterpret_cast<RECT*>(metadata.data() + buff_size);
+  size_t dirty_rects_count = 0;
+  error = _com_error(duplication_->GetFrameDirtyRects(
+      static_cast<UINT>(metadata.capacity()) - buff_size,
+      dirty_rects, &buff_size));
+  if (error.Error() != S_OK) {
+    LOG(LS_ERROR) << "Failed to get dirty rectangles, error "
+                  << error.ErrorMessage() << ", code " << error.Error();
+    return false;
+  }
+  dirty_rects_count = buff_size / sizeof(RECT);
+
+  while (move_rects_count > 0) {
+    updated_region->AddRect(DesktopRect::MakeXYWH(
+        move_rects->SourcePoint.x, move_rects->SourcePoint.y,
+        move_rects->DestinationRect.right - move_rects->DestinationRect.left,
+        move_rects->DestinationRect.bottom - move_rects->DestinationRect.top));
+    updated_region->AddRect(DesktopRect::MakeLTRB(
+        move_rects->DestinationRect.left, move_rects->DestinationRect.top,
+        move_rects->DestinationRect.right, move_rects->DestinationRect.bottom));
+    move_rects++;
+    move_rects_count--;
+  }
+
+  while (dirty_rects_count > 0) {
+    updated_region->AddRect(DesktopRect::MakeLTRB(
+        dirty_rects->left, dirty_rects->top,
+        dirty_rects->right, dirty_rects->bottom));
+    dirty_rects++;
+    dirty_rects_count--;
+  }
+
+  return true;
+}
+
+}  // namespace webrtc