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 "webrtc/modules/desktop_capture/blank_detector_desktop_capturer_wrapper.h"
+
+#include <algorithm>
+#include <utility>
+
+#include "webrtc/base/checks.h"
+#include "webrtc/modules/desktop_capture/desktop_geometry.h"
+#include "webrtc/system_wrappers/include/metrics.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();

[Sergey Ulanov, 2017/03/01 22:18:32]

Maybe check permanent_failure_reached() here instead of in OnCaptureResult()?

[Hzj_jie, 2017/03/01 23:31:37]

Done.

+}
+
+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 || non_blank_frame_received_) {
+    callback_->OnCaptureResult(result, std::move(frame));
+    return;
+  }
+
+  if (!frame) {

[Sergey Ulanov, 2017/03/01 22:18:32]

This is a bug in the underlying capturer. DCHECK() would be more appropriate

[Hzj_jie, 2017/03/01 23:31:38]

Done.

+    ReturnTemporaryError();
+    return;
+  }
+
+  if (permanent_failure_reached()) {
+    ReturnPermanentError();
+    return;
+  }
+
+  // If nothing has been changed in current frame, we do not need to check it
+  // again.
+  if (!frame->updated_region().is_empty() || is_first_frame_) {
+    last_frame_is_blank_ = IsBlankFrame(*frame);
+    is_first_frame_ = false;
+  }
+  RTC_HISTOGRAM_BOOLEAN("WebRTC.DesktopCapture.BlankFrameDetected",
+                        last_frame_is_blank_);
+  if (!last_frame_is_blank_) {
+    non_blank_frame_received_ = true;
+    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 {
+  // We will check 768 pixels for a frame with 1024 x 768 resolution.
+  for (int i = 0; i < frame.size().width() * frame.size().height(); i += 1000) {

[Sergey Ulanov, 2017/03/01 22:18:32]

Hm. I think this filter can still be problematic in some cases as it verifies
only limited portion of the screen when width is close to 1000. Something like
this may work better:
 y = i / W
 x = (i + y * P) % W
Here P is some arbitrary number. I think a prime number under 100 would work
well, e.g. 31.
WDYT?

Also did you verify if this gives significant performance boost? Since we only
need to check a few first frames it may be appropriate to verify the whole
frame.

[Hzj_jie, 2017/03/01 23:31:37]

According to DesktopCapturerDifferWrapperTest, it takes ~10ms to compare two
full frames. But BlankDetectorDesktopCapturerWrapper takes only 0.05ms. I think
a simpler choice is to decrease the 1000 to a composite number ~100. i.e.
Usually it's not divisible by a screen width to avoid we fall back into a
vertical line. 96, 105 or 108 could be good choice. It will take around 0.5ms,
but still cover a very large range of the screen.

+    const int x = i % frame.size().width();
+    const int y = i / frame.size().width();
+    if (!IsBlankPixel(frame, x, y)) {
+      return false;
+    }
+  }
+
+  return IsBlankPixel(frame, frame.size().width() >> 1,

[Sergey Ulanov, 2017/03/01 22:18:32]

Add a comment that we are verifying the pixel in the center as well. Also
replace >> 1 with / 2. It's more readable and the compiler should know to
optimize it with shift.

[Hzj_jie, 2017/03/01 23:31:37]

Done.

+                      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_;

[Sergey Ulanov, 2017/03/01 22:18:32]

Do we want to verify the alpha byte? Is it guaranteed to be set?
It may work with DX capturer, but the behavior is not consistent across other
capturers (IIRC some set it to 0xff, others return 0).

[Hzj_jie, 2017/03/01 23:31:37]

Yes, so the constructor of BlankDetectorDesktopCapturerWrapper has a parameter
to define the "blank_pixel". It's a platform dependent value.

+}
+
+bool BlankDetectorDesktopCapturerWrapper::permanent_failure_enabled() const {
+  return permanent_failure_threshold_ >= 0;

[Sergey Ulanov, 2017/03/01 22:18:32]

It's not documented in the header that permanent_failure_threshold=0 allows to
disable permanent failure. Do we actually need this feature? If we do then
please add a comment about it in the header.

[Hzj_jie, 2017/03/01 23:31:38]

Done.

+}
+
+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