BlankDetectorDesktopCapturerWrapper to detect a blank DesktopFrame

webrtc/modules/desktop_capture/blank_detector_desktop_capturer_wrapper.cc

+/*
+ *  Copyright (c) 2017 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 <utility>
+
+#include "webrtc/base/checks.h"
+#include "webrtc/modules/desktop_capture/blank_detector_desktop_capturer_wrapper.h"

[Sergey Ulanov, 2017/02/25 00:05:28]

This should be the first include in this file.

[Hzj_jie, 2017/02/25 02:06:38]

Done.

+#include "webrtc/modules/desktop_capture/desktop_geometry.h"
+
+namespace webrtc {
+
+BlankDetectorDesktopCapturerWrapper::BlankDetectorDesktopCapturerWrapper(
+    std::unique_ptr<DesktopCapturer> capturer,
+    RgbaColor blank_pixel,
+    int permanent_failure_threshold)
+    : capturer_(std::move(capturer)),
+      blank_pixel_(blank_pixel),
+      permanent_failure_threshold_(permanent_failure_threshold) {
+  RTC_DCHECK(capturer_);
+}
+
+BlankDetectorDesktopCapturerWrapper::~BlankDetectorDesktopCapturerWrapper() =
+    default;
+
+void BlankDetectorDesktopCapturerWrapper::Start(
+    DesktopCapturer::Callback* callback) {
+  capturer_->Start(this);
+  callback_ = callback;
+}
+
+void BlankDetectorDesktopCapturerWrapper::SetSharedMemoryFactory(
+    std::unique_ptr<SharedMemoryFactory> shared_memory_factory) {
+  capturer_->SetSharedMemoryFactory(std::move(shared_memory_factory));
+}
+
+void BlankDetectorDesktopCapturerWrapper::CaptureFrame() {
+  RTC_DCHECK(callback_);
+  capturer_->CaptureFrame();
+}
+
+void BlankDetectorDesktopCapturerWrapper::SetExcludedWindow(WindowId window) {
+  capturer_->SetExcludedWindow(window);
+}
+
+bool BlankDetectorDesktopCapturerWrapper::GetSourceList(SourceList* sources) {
+  return capturer_->GetSourceList(sources);
+}
+
+bool BlankDetectorDesktopCapturerWrapper::SelectSource(SourceId id) {
+  return capturer_->SelectSource(id);
+}
+
+bool BlankDetectorDesktopCapturerWrapper::FocusOnSelectedSource() {
+  return capturer_->FocusOnSelectedSource();
+}
+
+void BlankDetectorDesktopCapturerWrapper::OnCaptureResult(
+    Result result,
+    std::unique_ptr<DesktopFrame> frame) {
+  RTC_DCHECK(callback_);
+  if (result != Result::SUCCESS) {
+    callback_->OnCaptureResult(result, std::move(frame));
+    return;
+  }
+
+  if (!frame) {
+    ReturnTemporaryError();
+    return;
+  }
+
+  if (permanent_failure_reached()) {
+    ReturnPermanentError();
+    return;
+  }
+
+  if (!IsBlankFrame(*frame)) {

[Sergey Ulanov, 2017/02/25 00:05:28]

Can we avoid calling it for every frame? Particularly it doesn't make sense to
check if the frame blank if it's the same as the previous frame. Maybe only
check the pixels in updated_region() or skip frames with empty updated_region()?

Also, if we know that the first frame is not black, do we need to check all
frames after it?

[Hzj_jie, 2017/02/25 02:06:38]

Done.
This class is more like the escape routes on the aircraft. If by any chance, DX
APIs fail on some certain modes, say, a specific resolution, this class can help
to find a backup plan for users automatically.

[Sergey Ulanov, 2017/02/27 23:26:25]

I don't think we should be doing this unless we know there are cases when it
solves a problem. It's impossible to workaround every possible fault in the API
implementation caused by a driver bug. E.g. what if the API always returns white
frames? What if the API returns garbage? We don't know if such bugs exist and we
don't try to workaround them a propri.

Currently there is only one specific issue that we know about: the API may
return black frame on a VirtualBox machine. To work it around it's enough to
verify that the first frame is not black. I don't think we want to extend this
workaround to other cases when the capturer may return black frames. Otherwise
all users would be paying with wasted CPU cycles to workaround a bug that may
not even exist, and there is a risk for other regressions (e.g. in the case
screen is partially black).

[Hzj_jie, 2017/02/28 00:13:17]

Done.

+    failure_count_ = 0;
+    callback_->OnCaptureResult(Result::SUCCESS, std::move(frame));
+    return;
+  }
+
+  if (permanent_failure_enabled()) {
+    failure_count_++;
+    if (permanent_failure_reached()) {
+      ReturnPermanentError();
+      return;
+    }
+  }
+  ReturnTemporaryError();
+}
+
+bool BlankDetectorDesktopCapturerWrapper::IsBlankFrame(
+    const DesktopFrame& frame) const {
+  const int size =
+      (frame.size().width() < frame.size().height() ? frame.size().width()

[Sergey Ulanov, 2017/02/25 00:05:29]

std::min(frame.size().width(), frame.size().height())

[Hzj_jie, 2017/02/25 02:06:38]

Done.

+                                                    : frame.size().height());
+  // We will check 303 pixels for a frame with 1024 x 768 resolution.
+  for (int i = 0; i < size; i += 10) {
+    const int x1 = i;
+    const int x2 = frame.size().width() - i - 1;
+    const int y1 = i;
+    const int y2 = frame.size().height() - i - 1;

[Sergey Ulanov, 2017/02/25 00:05:28]

As far as I can tell this always ignores center of the screen for non-square
screen, except for a single pixel. If the user views 9:16 photos on a 16:9
screen (note different orientation) then all pixels that are checked here will
be black. If the central pixel happens to be black then this function would
return incorrect result. Can we distribute the pixels being checked throughout
the frame. E.g. we could look at every 1000th pixel (i.e. distribute the pixels
we look at uniformly throughout the frame).

[Hzj_jie, 2017/02/25 02:06:38]

The center pixel has been checked at the end of this function.
Your suggestion seems better, I have updated the code.

+    if (!IsBlankPixel(frame, x1, y1) || !IsBlankPixel(frame, x1, y2) ||
+        !IsBlankPixel(frame, x2, y1) || !IsBlankPixel(frame, x2, y2)) {

[Sergey Ulanov, 2017/02/25 00:05:29]

I would expect performance of this code to be highly dependent on memory cache
size and behavior. It's likely to have a non-negligible effect on systems with
small L2 cache (where full frame doesn't fit in the cache). I'm not sure that
checking four pixels at the time like you do here will perform better than just
scanning the same pixels in the order in which they are stored in memory. Can
you please try that and see if it performs any different?

[Hzj_jie, 2017/02/25 02:06:38]

If we check pixels sequentially (Say, one per 1000 pixels), we should be fine
here.

+      return false;
+    }
+  }
+
+  return IsBlankPixel(frame, frame.size().width() >> 1,
+                      frame.size().height() >> 1);
+}
+
+bool BlankDetectorDesktopCapturerWrapper::IsBlankPixel(
+    const DesktopFrame& frame,
+    int x,
+    int y) const {
+  uint8_t* pixel_data = frame.GetFrameDataAtPos(DesktopVector(x, y));
+  return RgbaColor(pixel_data) == blank_pixel_;
+}
+
+bool BlankDetectorDesktopCapturerWrapper::permanent_failure_enabled() const {
+  return permanent_failure_threshold_ >= 0;
+}
+
+bool BlankDetectorDesktopCapturerWrapper::permanent_failure_reached() const {
+  return permanent_failure_enabled() &&
+      failure_count_ >= permanent_failure_threshold_;
+}
+
+void BlankDetectorDesktopCapturerWrapper::ReturnPermanentError() {
+  callback_->OnCaptureResult(Result::ERROR_PERMANENT,
+                             std::unique_ptr<DesktopFrame>());
+}
+
+void BlankDetectorDesktopCapturerWrapper::ReturnTemporaryError() {
+  callback_->OnCaptureResult(Result::ERROR_TEMPORARY,
+                             std::unique_ptr<DesktopFrame>());
+}
+
+}  // namespace webrtc