DirectX based screen capturer logic

webrtc/modules/desktop_capture/win/screen_capturer_win_directx.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/screen_capturer_win_directx.h"
+
+#include <string.h>
+
+#include <comdef.h>
+#include <wincodec.h>
+#include <DXGI.h>
+
+#include "webrtc/base/checks.h"
+#include "webrtc/base/criticalsection.h"
+#include "webrtc/modules/desktop_capture/win/screen_capture_utils.h"
+#include "webrtc/system_wrappers/include/logging.h"
+#include "webrtc/system_wrappers/include/tick_util.h"
+
+namespace webrtc {
+
+using Microsoft::WRL::ComPtr;
+using rtc::CriticalSection;
+using rtc::CritScope;
+
+namespace {
+
+// In milliseconds, consistent with windows api.

[Sergey Ulanov, 2016/04/19 23:51:07]

The comment should make it clear the purpose of this constant. E.g. 
  // Timeout for AcquireNextFrame() call.

Don't need to mention that it's in ms - it's clear from the name

[Hzj_jie, 2016/04/26 23:00:07]

Done.

+const int kAcquireTimeoutMs = 10;
+
+// Wait time between two DuplicateOutput operations, DuplicateOutput may fail if
+// display mode is changing.
+const int kDuplicateOutputWait = 50;

[Sergey Ulanov, 2016/04/19 23:51:08]

Units should be clear from the name. As I understand it is in ms, then 50ms
seems to be awfully long for a frame to be rendered. Can we lower this, e.g. to
2 ms? Potentially we could increase delay after every failed attempt.

[Hzj_jie, 2016/04/26 23:00:08]

AFAICT, the DuplicateOutput should only fail when the display mode is changing.
i.e. full screen to window, or resolution changes. So in most of cases, this
won't impact the delay of rendering. And I believe Windows cannot do it fast, 2
milliseconds is too short.

+
+// How many times we attempt to DuplicateOutput before returning an error to
+// upstream components.
+const int kDuplicateOutputAttempts = 10;
+
+rtc::GlobalLockPod g_initialize_lock;
+
+// A container of all the objects we need to call Windows API. Note, one
+// application can only have one IDXGIOutputDuplication instance, that's the
+// reason the container is singleton.
+struct DxgiContainer {
+  CriticalSection duplication_lock;
+  CriticalSection acquire_lock;
+  bool initialize_result GUARDED_BY(g_initialize_lock) = false;
+  ID3D11Device* device GUARDED_BY(g_initialize_lock) = nullptr;
+  ID3D11DeviceContext* context GUARDED_BY(g_initialize_lock) = nullptr;
+  IDXGIOutput1* output1 GUARDED_BY(g_initialize_lock) = nullptr;
+  ComPtr<IDXGIOutputDuplication> duplication
+      GUARDED_BY(duplication_lock);
+  DesktopSize desktop_size;

[Sergey Ulanov, 2016/04/19 23:51:08]

Why do we need to store desktop_size here? What if there are multiple monitors
and multiple capturers, one for each monitor?

[Hzj_jie, 2016/04/26 23:00:08]

I did not see any comments on msdn to describe the multi monitors scenario. But
this is for the default dummy frame, it should be removed.

+  std::vector<BYTE> metadata GUARDED_BY(acquire_lock);

[Sergey Ulanov, 2016/04/19 23:51:08]

please use uint8_t instead of BYTE

[Hzj_jie, 2016/04/26 23:00:07]

Done.

+};
+
+DxgiContainer* g_container GUARDED_BY(g_initialize_lock);
+
+// A DesktopFrame which does not own the data buffer, and also does not have
+// shared memory. This uses in IT2ME scenario only.
+class DxgiDesktopFrame : public DesktopFrame {
+ public:
+  DxgiDesktopFrame(DesktopSize size, int stride, uint8_t* data) :
+      DesktopFrame(size, stride, data, nullptr) {}
+
+  // A valid but empty frame, uses before we captured the first screenshot.
+  DxgiDesktopFrame()
+      : DesktopFrame(g_container->desktop_size, 0, nullptr, nullptr) {}
+
+  virtual ~DxgiDesktopFrame() {}
+};
+
+}  // namespace
+
+// A pair of an ID3D11Texture2D and an IDXGISurface. We need an
+// ID3D11Texture2D instance to copy GPU texture to RAM, but an IDXGISurface to
+// map the texture into a bitmap buffer. These two instances are always
+// pointing to a same object.
+//
+// This class is not thread safe.
+class ScreenCapturerWinDirectx::Texture {
+ public:
+  // Captures a frame represented by frame_info and resource. Returns false if
+  // anything wrong.
+  // Before the first Capture is called, this instance is still workable, but
+  // the data in underlying buffer (SharedMemory) is undefined.
+  bool Capture(const DXGI_OUTDUPL_FRAME_INFO& frame_info,

[Sergey Ulanov, 2016/04/19 23:51:07]

This method copies the image from GPU to in-memory texture, so I don't think we
want to call it differently, e.g. CopyFrom().

[Hzj_jie, 2016/04/26 23:00:08]

Done.

+               IDXGIResource* resource) {
+    if (!resource || frame_info.AccumulatedFrames == 0) {
+      // Nothing updated, but current data is still valid.
+      return false;
+    }
+
+    ComPtr<ID3D11Texture2D> texture;
+    _com_error error = resource->QueryInterface(
+        __uuidof(ID3D11Texture2D),
+        reinterpret_cast<void**>(texture.GetAddressOf()));
+    if (error.Error() != S_OK || !texture) {
+      LOG(LS_ERROR) << "Failed to convert IDXGIResource to ID3D11Texture2D, "
+                       "error "
+                    << error.ErrorMessage() << ", code " << error.Error();
+      return false;
+    }
+
+    // AcquireNextFrame returns a CPU inaccessible IDXGIResource, so we need to
+    // make a copy.
+    if (!CreateTexture(texture.Get())) {

[Sergey Ulanov, 2016/04/19 23:51:08]

It looks strange that you have CreateTexture() function in Texture class and
also that this function actually doesn't always create a texture.  I think it
would be cleaner to have one-to-one relationship between Texture objects and the
corresponding textures. I.e. Texture constructor should be responsible for
creation of a texture, and so a new Texture object would need to be created
whenever screen size changes.

[Hzj_jie, 2016/04/26 23:00:07]

Not only screen size change, there are bunch of parameters in an ID3D11Texture2D
to impact the behavior, so if we have a constructor out of Texture class,
stage_, surface_ and desktop_size_ will need to be public to this function. But
I am happy to change the name of this function into InitializeStage.

+      return false;
+    }
+
+    g_container->context->CopyResource(

[Sergey Ulanov, 2016/04/19 23:51:08]

Can we use CopySubresourceRegion() here? (see
https://msdn.microsoft.com/en-us/library/windows/desktop/ff476394(v=vs.85).aspx
)
This should allow to copy only updated rects instead of the whole surface.

[Hzj_jie, 2016/04/26 23:00:07]

Done.

+        static_cast<ID3D11Resource*>(stage_.Get()),
+        static_cast<ID3D11Resource*>(texture.Get()));
+
+    rect_ = {0};
+    error = _com_error(surface_->Map(&rect_, DXGI_MAP_READ));
+    if (error.Error() != S_OK) {
+      LOG(LS_ERROR) << "Failed to map the IDXGISurface to a bitmap, error "
+                    << error.ErrorMessage() << ", code " << error.Error();
+      return false;
+    }
+
+    // surface_->Unmap() will be called next time we capture an image to avoid
+    // memory copy without shared_memory.
+    return true;
+  }
+
+  // Create a DesktopFrame from current instance.
+  std::unique_ptr<DesktopFrame> CreateDesktopFrame() const {
+    return std::unique_ptr<DesktopFrame>(
+        new DxgiDesktopFrame(desktop_size(), pitch(), bits()));
+  }
+
+  uint8_t* bits() const { return static_cast<uint8_t*>(rect_.pBits); }
+  int pitch() const { return static_cast<int>(rect_.Pitch); }
+  const DesktopSize& desktop_size() const { return desktop_size_; }

[Sergey Ulanov, 2016/04/19 23:51:08]

Please call this size(), instead of desktop_size() for consistency with
DesktopFrame.

[Hzj_jie, 2016/04/26 23:00:07]

Done.

+
+ private:
+  // Initializes stage_ from a CPU inaccessible IDXGIResource. Returns false
+  // if it fails to execute windows api.
+  bool CreateTexture(ID3D11Texture2D* texture) {
+    RTC_DCHECK(texture);
+    D3D11_TEXTURE2D_DESC desc = {0};
+    texture->GetDesc(&desc);
+    desc.BindFlags = 0;
+    desc.CPUAccessFlags = D3D11_CPU_ACCESS_READ;
+    desc.MiscFlags = 0;
+    desc.Usage = D3D11_USAGE_STAGING;
+    if (stage_) {
+      // Make sure stage_ and surface_ are always pointing to a same object.
+      // We need an ID3D11Texture2D instance for
+      // ID3D11DeviceContext::CopySubresourceRegion, but an IDXGISurface for
+      // IDXGISurface::Map.
+      {
+        ComPtr<IUnknown> left;
+        ComPtr<IUnknown> right;
+        bool left_result = SUCCEEDED(stage_.As(&left));
+        bool right_result = SUCCEEDED(surface_.As(&right));
+        RTC_DCHECK(left_result);
+        RTC_DCHECK(right_result);
+        RTC_DCHECK(left.Get() == right.Get());
+      }
+
+      // This buffer should be used already.
+      _com_error error = _com_error(surface_->Unmap());
+      if (error.Error() == S_OK) {
+        D3D11_TEXTURE2D_DESC orgi_desc;

[Sergey Ulanov, 2016/04/19 23:51:08]

orgi?
Also style guide discourages names like orig_desc. Maybe call it current_desc?

[Hzj_jie, 2016/04/26 23:00:07]

Sorry, typo. Updated.

+        stage_->GetDesc(&orgi_desc);
+        if (memcmp(&desc, &orgi_desc, sizeof(D3D11_TEXTURE2D_DESC)) == 0) {
+          return true;
+        }
+      } else {
+        // Let's recreate stage_ and surface_ later.
+        LOG(LS_ERROR) << "Failed to unmap surface, error "
+                      << error.ErrorMessage() << ", code " << error.Error();
+      }
+
+      stage_.Reset();
+      surface_.Reset();
+    } else {
+      RTC_DCHECK(!surface_);
+    }
+
+    _com_error error = _com_error(g_container->device->CreateTexture2D(
+        &desc, nullptr, stage_.GetAddressOf()));
+    if (error.Error() != S_OK || !stage_) {
+      LOG(LS_ERROR) << "Failed to create a new ID3D11Texture2D as stage, "
+                       "error "
+                    << error.ErrorMessage() << ", code " << error.Error();
+      return false;
+    }
+
+    error = _com_error(stage_.As(&surface_));
+    if (error.Error() != S_OK || !surface_) {
+      LOG(LS_ERROR) << "Failed to convert ID3D11Texture2D to IDXGISurface, "
+                       "error "
+                    << error.ErrorMessage() << ", code " << error.Error();
+      return false;
+    }
+
+    desktop_size_.set(static_cast<int>(desc.Width),
+                      static_cast<int>(desc.Height));

[Sergey Ulanov, 2016/04/19 23:51:08]

I don't think you need these casts

[Hzj_jie, 2016/04/26 23:00:07]

Done.

+    return true;
+  }
+
+  Microsoft::WRL::ComPtr<ID3D11Texture2D> stage_;
+  Microsoft::WRL::ComPtr<IDXGISurface> surface_;
+  DXGI_MAPPED_RECT rect_;
+  DesktopSize desktop_size_;
+};
+
+bool ScreenCapturerWinDirectx::Initialize() {
+  if (!g_container) {
+    rtc::GlobalLockScope lock(&g_initialize_lock);
+    if (!g_container) {
+      g_container = new DxgiContainer();
+      g_container->initialize_result = DoInitialize();
+      if (g_container->initialize_result) {
+        return true;
+      }
+
+      // Clean up if DirectX cannot work on the system.
+      if (g_container->duplication) {
+        g_container->duplication.Reset();
+      }
+
+      if (g_container->output1) {
+        g_container->output1->Release();
+        g_container->output1 = nullptr;
+      }
+
+      if (g_container->context) {
+        g_container->context->Release();
+        g_container->context = nullptr;
+      }
+
+      if (g_container->device) {
+        g_container->device->Release();
+        g_container->device = nullptr;
+      }
+
+      return false;
+    }
+  }
+
+  return g_container->initialize_result;
+}
+
+bool ScreenCapturerWinDirectx::DoInitialize() {
+  D3D_FEATURE_LEVEL feature_level;
+  _com_error error = D3D11CreateDevice(
+      nullptr, D3D_DRIVER_TYPE_HARDWARE, nullptr,
+      D3D11_CREATE_DEVICE_BGRA_SUPPORT | D3D11_CREATE_DEVICE_SINGLETHREADED,
+      nullptr, 0, D3D11_SDK_VERSION, &g_container->device, &feature_level,
+      &g_container->context);
+  if (error.Error() != S_OK || !g_container->device || !g_container->context) {
+    LOG(LS_WARNING) << "D3D11CreateDeivce returns error "
+                    << error.ErrorMessage() << " with code " << error.Error();
+    return false;
+  }
+
+  if (feature_level < D3D_FEATURE_LEVEL_11_0) {
+    LOG(LS_WARNING) << "D3D11CreateDevice returns an instance without DirectX "
+                       "11 support, level "
+                    << feature_level;
+    return false;
+  }
+
+  ComPtr<IDXGIDevice> device;
+  error = _com_error(g_container->device->QueryInterface(
+      __uuidof(IDXGIDevice), reinterpret_cast<void**>(device.GetAddressOf())));
+  if (error.Error() != S_OK || !device) {
+    LOG(LS_WARNING) << "ID3D11Device is not an implementation of IDXGIDevice, "
+                       "this usually means the system does not support DirectX "
+                       "11";
+    return false;
+  }
+
+  ComPtr<IDXGIAdapter> adapter;
+  error = _com_error(device->GetAdapter(adapter.GetAddressOf()));
+  if (error.Error() != S_OK || !adapter) {
+    LOG(LS_WARNING) << "Failed to get an IDXGIAdapter implementation from "
+                       "IDXGIDevice.";
+    return false;
+  }
+
+  ComPtr<IDXGIOutput> output;
+  for (int i = 0;; i++) {
+    error = _com_error(adapter->EnumOutputs(i, output.GetAddressOf()));
+    if (error.Error() == DXGI_ERROR_NOT_FOUND) {
+      LOG(LS_WARNING) << "No output detected.";
+      return false;
+    }
+    if (error.Error() == S_OK && output) {
+      DXGI_OUTPUT_DESC desc;
+      error = _com_error(output->GetDesc(&desc));
+      if (error.Error() == S_OK) {
+        if (desc.AttachedToDesktop) {
+          // Current output instance is the device attached to desktop.
+          break;
+        }
+      } else {
+        LOG(LS_WARNING) << "Failed to get output description of device " << i
+                        << ", ignore.";
+      }
+    }
+  }
+
+  RTC_DCHECK(output);
+  error = _com_error(output.CopyTo(
+      __uuidof(IDXGIOutput1), reinterpret_cast<void**>(&g_container->output1)));
+  if (error.Error() != S_OK || !g_container->output1) {
+    LOG(LS_WARNING) << "Failed to convert IDXGIOutput to IDXGIOutput1, this "
+                       "usually means the system does not support DirectX 11";
+    return false;
+  }
+
+  return DuplicateOutput();
+}
+
+bool ScreenCapturerWinDirectx::DuplicateOutput() {
+  // We are updating the instance.
+  CritScope lock(&g_container->duplication_lock);
+  // Make sure nobody is using current instance.
+  CritScope lock2(&g_container->acquire_lock);
+  if (g_container->duplication) {
+    g_container->duplication->ReleaseFrame();
+    g_container->duplication.Reset();
+  }
+
+  for (int i = 0; i < kDuplicateOutputAttempts; i++) {
+    _com_error error = g_container->output1->DuplicateOutput(
+        static_cast<IUnknown*>(g_container->device),
+        g_container->duplication.GetAddressOf());
+    if (error.Error() == S_OK && g_container->duplication) {
+      DXGI_OUTDUPL_DESC desc;
+      g_container->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;
+      }
+
+      g_container->desktop_size.set(desc.ModeDesc.Width, desc.ModeDesc.Height);
+      return true;
+    } else {
+      // Make sure we have correct signal and duplicate the output next time.
+      g_container->duplication.Reset();
+      LOG(LS_WARNING) << "Failed to duplicate output from IDXGIOutput1, error "
+                      << error.ErrorMessage() << ", with code "
+                      << error.Error();
+      Sleep(kDuplicateOutputWait);
+    }
+  }
+
+  return false;
+}
+
+ScreenCapturerWinDirectx::ScreenCapturerWinDirectx(
+    const DesktopCaptureOptions& options)
+    : callback_(nullptr), set_thread_execution_state_failed_(false) {
+  RTC_DCHECK(g_container && g_container->initialize_result);
+
+  // Texture instance won't change forever.
+  while (!surfaces_.current()) {
+    surfaces_.ReplaceCurrent(new Texture());
+    surfaces_.MoveToNext();
+  }
+
+  // Make sure we always have a valid DesktopFrame instance to emit.
+  // Not similar as other methods, this class may not always be able to capture

[Sergey Ulanov, 2016/04/19 23:51:07]

s/Not similar as other methods/Unlike other capturers/

[Hzj_jie, 2016/04/26 23:00:07]

Done.

+  // desktop frames, the desktop may not change. So we create a set of default
+  // frames to emit, if first several attempts do not capture anything.
+  while (!frames_.current_frame()) {
+    frames_.ReplaceCurrentFrame(new DxgiDesktopFrame());

[Sergey Ulanov, 2016/04/19 23:51:07]

Why do we need this? We can keep the queue empty until we capture the first
frame. Also it's better to call OnCaptureCompleted(nullptr) instead of returning
dummy frames like this one.

[Hzj_jie, 2016/04/26 23:00:07]

I doubt this decision, several DesktopFrame::Callback implementations do not
expect to have a nullptr frame returned, they are simply crashes the service.
(Such as DesktopSessionAgent.) But this is a common scenario in DirectX based
capturer, i.e. the first several AcquireNextFrame attempts usually do not return
frames. So I need to prepare some dummy frames to avoid crash.

+    frames_.MoveToNextFrame();
+  }
+}
+
+ScreenCapturerWinDirectx::~ScreenCapturerWinDirectx() {}
+
+void ScreenCapturerWinDirectx::Start(Callback* callback) {
+  RTC_DCHECK(!callback_);
+  RTC_DCHECK(callback);
+
+  callback_ = callback;
+}
+
+void ScreenCapturerWinDirectx::SetSharedMemoryFactory(
+    rtc::scoped_ptr<SharedMemoryFactory> shared_memory_factory) {
+  shared_memory_factory_ =
+      rtc::ScopedToUnique(std::move(shared_memory_factory));
+}
+
+bool ScreenCapturerWinDirectx::DetectUpdatedRegion(
+    const DXGI_OUTDUPL_FRAME_INFO& frame_info,
+    DesktopFrame* frame) {
+  RTC_DCHECK(g_container->duplication);
+  RTC_DCHECK(frame);
+  DesktopRegion& updated_region = *frame->mutable_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 (g_container->metadata.size() < frame_info.TotalMetadataBufferSize) {
+    g_container->metadata.clear();  // Avoid data copy
+    g_container->metadata.reserve(frame_info.TotalMetadataBufferSize);

[Sergey Ulanov, 2016/04/19 23:51:07]

reserve() doesn't actually change the size of the vector<>, so the clear() call
above won't do anything as well (size() is always 0). I think in this case it's
more appropriate to use a simple dynamically allocated buffer, i.e.
std::unique_ptr<uint8_t[]>. There is no reason to use vector as you don't need
to change size dynamically. But if you do use vector<> then this should be
resize() instead of reserve()

[Hzj_jie, 2016/04/26 23:00:07]

resize will fill the following data with a default value, but reverse won't. And
since I always use capacity, though the size is not correct, the memory buffer
is safe. But yes, I have made a mistake above, I should check
g_container->metadata.capacity() < frame_info.TotalMetadataBufferSize.

+  }
+
+  UINT buff_size = 0;
+  DXGI_OUTDUPL_MOVE_RECT* move_rects = nullptr;
+  size_t move_rects_count = 0;
+  RECT* dirty_rects = nullptr;
+  size_t dirty_rects_count = 0;
+  for (int i = 0; i < 2; i++) {

[Sergey Ulanov, 2016/04/19 23:51:08]

I commented on this before: I don't think we want to use a loop here. It makes
it harder to understand this code. If you want to share the error handling logic
then please add a helper function for that.

[Hzj_jie, 2016/04/26 23:00:07]

Done.

+    _com_error error = S_OK;
+    if (i == 0) {
+      move_rects = reinterpret_cast<DXGI_OUTDUPL_MOVE_RECT*>(
+           g_container->metadata.data());
+      error = _com_error(g_container->duplication->GetFrameMoveRects(
+          static_cast<UINT>(g_container->metadata.capacity()),
+          move_rects, &buff_size));
+    } else {
+      dirty_rects = reinterpret_cast<RECT*>(
+          g_container->metadata.data() + buff_size);
+      error = _com_error(g_container->duplication->GetFrameDirtyRects(
+          static_cast<UINT>(g_container->metadata.capacity()) - buff_size,
+          dirty_rects, &buff_size));
+    }
+    if (error.Error() != S_OK) {
+      if (error.Error() == DXGI_ERROR_ACCESS_LOST) {
+        DuplicateOutput();
+      } else {
+        LOG(LS_ERROR) << "Failed to get " << (i == 0 ? "move" : "dirty")
+                      << " rectangles, error " << error.ErrorMessage()
+                      << ", code " << error.Error();
+      }
+      // Send entire desktop as we cannot get dirty or move rectangles.
+      return false;
+    }
+    if (i == 0) {
+      move_rects_count = buff_size / sizeof(DXGI_OUTDUPL_MOVE_RECT);
+    } else {
+      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;
+}
+
+bool ScreenCapturerWinDirectx::ProcessFrame(
+    const DXGI_OUTDUPL_FRAME_INFO& frame_info,
+    IDXGIResource* resource) {
+  RTC_DCHECK(resource);
+  RTC_DCHECK(frame_info.AccumulatedFrames > 0);
+  // We have something to update, so move to next surface.
+  surfaces_.MoveToNext();
+  RTC_DCHECK(surfaces_.current());
+  if (!surfaces_.current()->Capture(frame_info, resource)) {
+    return false;
+  }
+
+  frames_.MoveToNextFrame();
+  if (shared_memory_factory_) {
+    // When using shared_memory_factory_, try to avoid reallocate memory.

[Sergey Ulanov, 2016/04/19 23:51:07]

Suggest rewording:
  // When using shared memory |frames_| is used to store a queue of
  // SharedMemoryDesktopFrame's.

[Hzj_jie, 2016/04/26 23:00:08]

Done.

+    SharedMemoryDesktopFrame* frame =
+        reinterpret_cast<SharedMemoryDesktopFrame*>(frames_.current_frame());
+    if (!frame || !frame->size().equals(surfaces_.current()->desktop_size()) ||
+        frame->stride() != surfaces_.current()->pitch()) {
+      // Current frame does not have a same size as last captured surface.
+      // Note, Windows does not guarantee to return exactly the same buffer size
+      // as the width * height * 4. The pitch may be larger than width * 4.
+      std::unique_ptr<DesktopFrame> new_frame =
+          SharedMemoryDesktopFrame::Create(surfaces_.current()->desktop_size(),
+              surfaces_.current()->pitch(), shared_memory_factory_.get());

[Sergey Ulanov, 2016/04/19 23:51:07]

why does the SharedMemoryDesktopFrame need to have the same stride as the
surface allocated by DirectX?

[Hzj_jie, 2016/04/26 23:00:07]

Done.

+      if (!new_frame) {
+        LOG(LS_ERROR) << "Failed to allocate a new SharedMemoryDesktopFrame";
+        return false;
+      }
+      frames_.ReplaceCurrentFrame(new_frame.release());
+    }
+  } else {
+    frames_.ReplaceCurrentFrame(
+        surfaces_.current()->CreateDesktopFrame().release());
+  }
+
+  if (!DetectUpdatedRegion(frame_info, frames_.current_frame())) {
+    frames_.current_frame()->mutable_updated_region()->Clear();
+    frames_.current_frame()->mutable_updated_region()->AddRect(

[Sergey Ulanov, 2016/04/19 23:51:08]

When using SharedDesktopFrame there is no reason to put updated rect in the
shared frame itself. Instead you can call Share() and then store updated_region
there. That would allow to ensure that we don't emit the same updated_region
multiple times (the problem I pointed out in Capture() down below). Also we
don't need to use |frames_| at all when not using shared memory. So this
function could look like follows:

unique_ptr<DesktopFrame> ProcessFrame() {
 DesktopRegion updated_region = DetectUpdatedRegion();

 unique_ptr<DesktopFrame> result;
 if (shared_memory_factory_) {
   .. Create frame if size changed
   .. Copy updated_region from the old frames
   result = frames_.CurrentFrame()->Share();
 } else {
   result = surfaces_.current()->CreateDesktopFrame();
 }

 result->mutable_updated_region.swap(&updated_region);
 return result;
}

This also allows to have only one if block for shared_memory_factory_.

[Hzj_jie, 2016/04/26 23:00:08]

Done.

+        DesktopRect::MakeSize(surfaces_.current()->desktop_size()));
+  }
+
+  if (shared_memory_factory_) {
+    // There is a shared memory provided, we need to copy data to it.
+    std::unique_ptr<DesktopFrame> frame(
+        surfaces_.current()->CreateDesktopFrame());
+    for (DesktopRegion::Iterator it(frames_.current_frame()->updated_region());
+         !it.IsAtEnd(); it.Advance()) {
+      // VideoEncoderVpx expects to use 8 pixels (for v9) or 3 pixels (for v8)
+      // more area to encode final video. So we need to copy a little bit more
+      // than the real changed area.
+      // In AlignRect function, 1 more pixel may be applied to right and bottom,
+      // so we add 9 pixels here.
+      DesktopRect rect = it.rect();
+      rect.Expand(9);
+      rect.IntersectWith(DesktopRect::MakeSize(frame->size()));
+      frames_.current_frame()->CopyPixelsFrom(*frame, rect.top_left(), rect);
+    }
+  }
+
+  return true;
+}
+
+void ScreenCapturerWinDirectx::Capture(const DesktopRegion& region) {
+  RTC_DCHECK(g_container->duplication);
+  RTC_DCHECK(callback_);
+
+  if (!g_container->duplication && !DuplicateOutput()) {
+    // Receive a capture request when application is shutting down, or between
+    // mode change.
+    callback_->OnCaptureCompleted(nullptr);
+    return;
+  }
+
+  TickTime capture_start_time = TickTime::Now();
+
+  if (!SetThreadExecutionState(ES_DISPLAY_REQUIRED | ES_SYSTEM_REQUIRED)) {
+    if (!set_thread_execution_state_failed_) {
+      set_thread_execution_state_failed_ = true;
+      LOG(LS_WARNING) << "Failed to make system & display power assertion: "
+                      << GetLastError();
+    }
+  }
+
+  DXGI_OUTDUPL_FRAME_INFO frame_info;
+  memset(&frame_info, 0, sizeof(DXGI_OUTDUPL_FRAME_INFO));
+  ComPtr<IDXGIResource> resource;
+  CritScope lock(&g_container->acquire_lock);
+  _com_error error = g_container->duplication->AcquireNextFrame(
+      kAcquireTimeoutMs, &frame_info, resource.GetAddressOf());
+  if (error.Error() == DXGI_ERROR_ACCESS_LOST) {
+    LOG(LS_ERROR) << "Access lost " << error.ErrorMessage();
+    if (DuplicateOutput()) {
+      EmitCurrentFrame();
+    } else {
+      callback_->OnCaptureCompleted(nullptr);
+    }
+    return;
+  }
+
+  if (error.Error() == DXGI_ERROR_WAIT_TIMEOUT) {
+    // Nothing changed.
+    EmitCurrentFrame();

[Sergey Ulanov, 2016/04/19 23:51:07]

This will emit current frame with the old updated_region() while
updated_region() should be empty.

[Hzj_jie, 2016/04/26 23:00:08]

Done.

+    return;
+  }
+
+  if (error.Error() != S_OK) {
+    LOG(LS_ERROR) << "Failed to capture frame, error " << error.ErrorMessage()
+                  << ", code " << error.Error();
+    callback_->OnCaptureCompleted(nullptr);
+    return;
+  }
+
+  if (frame_info.AccumulatedFrames == 0) {
+    EmitCurrentFrame();

[Sergey Ulanov, 2016/04/19 23:51:07]

Same as above. We don't want to emit the same frame with the same
updated_region()

[Hzj_jie, 2016/04/26 23:00:08]

Done.

+    g_container->duplication->ReleaseFrame();

[Sergey Ulanov, 2016/04/19 23:51:07]

Move this before EmitCurrentFrame(). OnCaptureCompleted() should be the the last
thing you do before returning, in case the callback deletes the capturer from
OnCaptureCompleted().

[Hzj_jie, 2016/04/26 23:00:08]

Done.

+    return;
+  }
+
+  // Everything looks good so far, build next frame.
+  bool result = ProcessFrame(frame_info, resource.Get());
+  // DetectUpdatedRegion may release last g_container->duplication. But
+  // DuplicateOutput function will always release last frame, so there is no
+  // potential leak.
+  if (g_container->duplication) {
+    g_container->duplication->ReleaseFrame();
+  }
+  if (result) {
+    frames_.current_frame()->set_capture_time_ms(
+        (TickTime::Now() - capture_start_time).Milliseconds());
+    EmitCurrentFrame();
+  } else {
+    callback_->OnCaptureCompleted(nullptr);
+  }
+}
+
+bool ScreenCapturerWinDirectx::GetScreenList(ScreenList* screens) {
+  return true;
+}
+
+bool ScreenCapturerWinDirectx::SelectScreen(ScreenId id) {
+  return id == kFullDesktopScreenId;

[Sergey Ulanov, 2016/04/19 23:51:08]

Add a comment that only full desktop capture is supported at the moment

[Hzj_jie, 2016/04/26 23:00:07]

Done.

+}
+
+void ScreenCapturerWinDirectx::EmitCurrentFrame() {
+  callback_->OnCaptureCompleted(frames_.current_frame()->Share());
+}
+
+}  // namespace webrtc