[DesktopCapture] Detect screen resolution changes in DirectX capturer

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 <utility>
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/timeutils.h"
 #include "webrtc/modules/desktop_capture/desktop_frame.h"
+#include "webrtc/modules/desktop_capture/win/screen_capture_utils.h"
 
 namespace webrtc {
 
 using Microsoft::WRL::ComPtr;
 
 // static
 bool ScreenCapturerWinDirectx::IsSupported() {
   // Forwards IsSupported() function call to DxgiDuplicatorController.
   return DxgiDuplicatorController::Instance()->IsSupported();
 }
@@ skipping 30 matching lines @@
   return DxgiDuplicatorController::Instance()
       ->ScreenRect(current_screen_id_)
       .size();
 }
 
 void ScreenCapturerWinDirectx::CaptureFrame() {
   RTC_DCHECK(callback_);
 
   int64_t capture_start_time_nanos = rtc::TimeNanos();
 
+  if (resolution_change_detector_.IsChanged(
+      GetScreenRect(kFullDesktopScreenId, L"").size())) {

[Sergey Ulanov, 2017/02/10 19:58:25]

std::wstring() for the second parameter

[Sergey Ulanov, 2017/02/10 19:58:25]

Why is it kFullDesktopScreenId instead of current_screen_id_?
There are cases when resolution of the monitor we are capturing is changed,
while total resolution of desktop stays the same.

[Sergey Ulanov, 2017/02/10 19:58:25]

indentation - this line should be indented 4 spaces more.

[Hzj_jie, 2017/02/11 01:51:22]

Done.

[Hzj_jie, 2017/02/11 01:51:22]

Done.

[Hzj_jie, 2017/02/11 01:51:22]

Three reasons,
1. it's not guaranteed GetScreenRect() returns the same size as DX APIs.
2. current_screen_id_ comes from DX APIs, which does not have a same meaning as
the parameter of GetScreenRect().
3. It's not likely the total resolution won't change if resolutions of one or
more monitors have changed. But the worst case is a temporary capture attempt,
which does not really like a big deal.

[Sergey Ulanov, 2017/02/15 01:38:52]

They maybe we should use DX API to get the screen size?
Is it possible to get displays resolutions using DX API before capturing?

[Hzj_jie, 2017/02/15 03:53:16]

Yes, technically, we can do this. But we need to initialize
DxgiDuplicatorController, which makes this issue become a cycle dependency.
Yes, we have already used DX APIs to set the resolution of frames.

[Sergey Ulanov, 2017/02/15 22:39:38]

Not sure why it's a cyclic dependency. This function should:
 1. Make sure that DxgiDuplicatorController is initialized.
 2. Get current resolution for the target screen.
 3. Verify that the resolution hasn't changed from the previous frame, and reset
frame queue if it did.
 4. Capture new frame.

If I understand correctly currently we cannot do (2) because
DxgiDuplicatorController caches screen size internally so SelectedDesktopSize()
returns out-of-date value after size changes. But I don't see any reason it
couldn't be fixed.

Anyway, I don't mind much the current approach, but at very least this code
deserves a comment to explain why it's implemented this way. Also a TODO() to
improve it.

+    // The next Duplicate() call is expected to fail. So we reset the dxgi
+    // components in-place.
+    frames_.Reset();
+    DxgiDuplicatorController::Instance()->Reset();
+    resolution_change_detector_.Reset();
+  }
+
   frames_.MoveToNextFrame();
   if (!frames_.current_frame()) {
     std::unique_ptr<DesktopFrame> new_frame;
     if (shared_memory_factory_) {
       new_frame = SharedMemoryDesktopFrame::Create(
           SelectedDesktopSize(), shared_memory_factory_.get());
     } else {
       new_frame.reset(new BasicDesktopFrame(SelectedDesktopSize()));
     }
     if (!new_frame) {
       LOG(LS_ERROR) << "Failed to allocate a new DesktopFrame.";
       // This usually means we do not have enough memory or SharedMemoryFactory
       // cannot work correctly.
       callback_->OnCaptureResult(Result::ERROR_PERMANENT, nullptr);
       return;
     }
     frames_.ReplaceCurrentFrame(SharedDesktopFrame::Wrap(std::move(new_frame)));
   }
 
   if (current_screen_id_ == kFullDesktopScreenId) {
     if (!DxgiDuplicatorController::Instance()->Duplicate(
             &context_, frames_.current_frame())) {
       // Screen size may be changed, so we need to reset the frames.
       frames_.Reset();
+      resolution_change_detector_.Reset();
       callback_->OnCaptureResult(Result::ERROR_TEMPORARY, nullptr);
       return;
     }
   } else {
     if (!DxgiDuplicatorController::Instance()->DuplicateMonitor(
             &context_, current_screen_id_, frames_.current_frame())) {
       // Screen size may be changed, so we need to reset the frames.
       frames_.Reset();
+      resolution_change_detector_.Reset();
       if (current_screen_id_ >=
           DxgiDuplicatorController::Instance()->ScreenCount()) {
         // Current monitor has been removed from the system.
         callback_->OnCaptureResult(Result::ERROR_PERMANENT, nullptr);
       } else {
         callback_->OnCaptureResult(Result::ERROR_TEMPORARY, nullptr);
       }
       return;
     }
   }
 
   std::unique_ptr<DesktopFrame> result = frames_.current_frame()->Share();
   result->set_capture_time_ms(
       (rtc::TimeNanos() - capture_start_time_nanos) /
       rtc::kNumNanosecsPerMillisec);
   callback_->OnCaptureResult(Result::SUCCESS, std::move(result));
 }
 
 bool ScreenCapturerWinDirectx::GetSourceList(SourceList* sources) {
   int screen_count = DxgiDuplicatorController::Instance()->ScreenCount();
   for (int i = 0; i < screen_count; i++) {
     sources->push_back({i});
   }
   return true;
 }
 
 bool ScreenCapturerWinDirectx::SelectSource(SourceId id) {

[Sergey Ulanov, 2017/02/10 19:58:25]

Should we also initialized duplicators and reset resolution_change_detector_
here?

[Hzj_jie, 2017/02/11 01:51:22]

The initialization is implicitly called in
DxgiDuplicatorController::Instance()->ScreenCount() function. As long as we are
using the entire screen resolution instead of a single monitor, we do not need
to reset the resolution_change_detector_ here.

[Sergey Ulanov, 2017/02/15 01:38:52]

If there are two monitors with the same resoltion and SelectScreen() is called
to select second monitor (when first one one selected previously), does the
capturer need to be reinitialized in this case? AFAICT
ScreenCapturerWinDirectx::CaptureFrame() will try to reuse old frames in this
case, and I'm not sure it's the right thing to do.

[Hzj_jie, 2017/02/15 03:53:16]

No, we only need to reinitialize if a new monitor has been attached, which is
covered by line 70, or a monitor has been removed, which is covered by line 116
& 154, or the resolution of a monitor has been changed, which is covered in
DxgiTexture.

If you are concerning about whether we should reuse the frame, I have update the
code here.

   if (id == current_screen_id_) {
     return true;
   }
 
   // Changing target screen may or may not impact frame size. So resetting
   // frames only when a Duplicate() function call returns false.
   if (id == kFullDesktopScreenId) {
     current_screen_id_ = id;
     return true;
   }
 
   int screen_count = DxgiDuplicatorController::Instance()->ScreenCount();
   if (id >= 0 && id < screen_count) {
     current_screen_id_ = id;
     return true;
   }
   return false;
 }
 
 }  // namespace webrtc