[WebRTC] Add ScreenCapturerDifferWrapper to share Differ across ScreenCapturers

webrtc/modules/desktop_capture/screen_capturer_differ_wrapper.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/screen_capturer_differ_wrapper.h"
+
+#include <algorithm>
+#include <utility>
+
+#include "webrtc/base/checks.h"
+#include "webrtc/base/timeutils.h"
+#include "webrtc/modules/desktop_capture/desktop_geometry.h"
+#include "webrtc/modules/desktop_capture/differ_block.h"
+
+namespace webrtc {
+
+namespace {
+
+static const int kBlockXOffset = kBlockSize * DesktopFrame::kBytesPerPixel;
+
+// Checks whether (0, 0) - (|width|, |height|) area in |prev| and |curr| are
+// equal. |stride| is used to indicate the bytes count between two rows in |old|
+// and |new|. |width| or |height| should be less than 32 (defined by
+// kBlockSize), otherwise BlockDifference() should be used.

[Sergey Ulanov, 2016/08/12 05:25:51]

The comment should make it clear if true result indicates that the block are
different or if they are the same. Or it would be better to rename the function
to make it clear (e.g. PartialBlockDiffer() as in differ.cc)

[Hzj_jie, 2016/08/15 00:38:06]

Considering Differ class will be removed, I would prefer to make this function
consistent with differ_block.h. The comment has been updated.

+bool PartialBlockDifference(const uint8_t* prev,

[Sergey Ulanov, 2016/08/12 05:25:51]

Style guide discourages names like 'prev' and 'curr'.
maybe old_buffer, new_buffer?
Also the comment above mentions |old| and |new|

[Hzj_jie, 2016/08/15 00:38:06]

Done.

+                            const uint8_t* curr,
+                            int stride,

[Sergey Ulanov, 2016/08/12 05:25:50]

potentially stride may be different for the two buffers. Pass two different
values?
Previously the capturers could ensure that it never happens, but you cannot
assume it here.

[Hzj_jie, 2016/08/15 00:38:06]

These functions are only called from CompareFrames function, which guarantees
old_frame.stride() equals to new_frame.stride() by using a RTC_DCHECK_EQ.

[Sergey Ulanov, 2016/08/15 22:04:50]

Mention it in screen_capturer_differ_wrapper.h?

[Hzj_jie, 2016/08/16 07:10:04]

Done.

+                            int width,
+                            int height) {
+  RTC_DCHECK(width < kBlockSize || height < kBlockSize);

[Sergey Ulanov, 2016/08/12 05:25:50]

Why do you need to check it here? 

Also, I think code in differ_block.cc can be updated to handle blocks of any
arbitrary height (only per-row comparison is optimized there), then this
function would only be useful the case when width !=32.

[Hzj_jie, 2016/08/15 00:38:06]

This seems not true for NEON (though we have not implemented one.) In NEON, we
need a 32 bit * 4 x 4 block to use its SIMD instructions.

[Sergey Ulanov, 2016/08/15 22:04:50]

Why is it 4x4 blocks? can you point me instructions/intrinsics that we would use
for NEON and that require 4x4 blocks?
In either case, currently we don't have optimized version for NEON and I don't
think we are going to need it in foreseeable future. But even if we ever
implement it, it will still be possible to make that code work with arbitrary
height. It looks wrong to sacrifice code simplicity for slightly more optimal
code on architecture where this code is not used. The change I suggested would
not only allow to avoid code duplication but will also make it possible to take
advantage of SSE optimizations where we are not currently doing so (as it will
be used for all blocks of size 32, not just those that are 32 pixels high), and
I think it's important as with this change we will have much higher percentage
of "partial" block comparisons.

[Hzj_jie, 2016/08/16 07:10:04]

Not really, this class will try to use as many block comparisons as possible. So
the partial block comparisons happen only at the right and bottom of the entire
frame. The disadvantage is in the worst case, we will compare one block for four
times, as we discarded differ-info boolean array.

I have tried to use VectorDifference, but found DesktopRegion::AddRect took too
much time to add (x, 1) rectangles. So a trade-off is to use BlockDifference
with variable height.

+  const int width_bytes = width * DesktopFrame::kBytesPerPixel;
+  for (int y = 0; y < height; y++) {
+    if (memcmp(prev, curr, width_bytes) != 0)
+      return true;
+    prev += stride;
+    curr += stride;
+  }
+
+  return false;
+}
+
+// Compares block-columns in the range of [0, |x_end| - |x_start|), in
+// block-rows start from |prev| and |curr|, and outputs dirty regions into
+// |output|. |row_top| is the original position of the top of the row in the
+// DesktopFrame, |width| is the DesktopFrame::size()::width(), |stride| is the
+// DesktopFrame::stride().
+void CompareBlockRow(const uint8_t* prev,
+                     const uint8_t* curr,
+                     const int x_start,
+                     const int x_end,
+                     const int row_top,
+                     const int width,
+                     const int stride,
+                     DesktopRegion* const output) {
+  const int row_bottom = row_top + kBlockSize;
+  const int row_height = kBlockSize;
+  const int column_right = std::min(x_end * kBlockSize, width);
+
+  // The first block-column in a continuous dirty area in current block-row.
+  int first_dirty_x_block = -1;
+
+  // We always need to add dirty area into |output| in the last block, so handle
+  // it separatedly.
+  for (int x = x_start; x < x_end - 1; x++) {
+    if (BlockDifference(prev, curr, stride)) {
+      if (first_dirty_x_block == -1) {
+        // This is the first dirty block in a continuous dirty area.
+        first_dirty_x_block = x;
+      }
+    } else if (first_dirty_x_block != -1) {
+      // The block on the left is the last dirty block in a continuous
+      // dirty area.
+      output->AddRect(DesktopRect::MakeLTRB(first_dirty_x_block * kBlockSize,
+                                            row_top, x * kBlockSize,
+                                            row_bottom));
+      first_dirty_x_block = -1;
+    }
+    prev += kBlockXOffset;
+    curr += kBlockXOffset;
+  }
+
+  bool last_block_diff;
+  if (x_end * kBlockSize > width) {
+    // The last one is a partial block.
+    last_block_diff = PartialBlockDifference(
+        prev, curr, stride, column_right - (x_end - 1) * kBlockSize,
+        row_height);
+  } else {
+    last_block_diff = BlockDifference(prev, curr, stride);
+  }
+  if (last_block_diff) {
+    if (first_dirty_x_block == -1) {
+      output->AddRect(DesktopRect::MakeLTRB((x_end - 1) * kBlockSize, row_top,
+                                            column_right, row_bottom));
+    } else {
+      output->AddRect(DesktopRect::MakeLTRB(first_dirty_x_block * kBlockSize,
+                                            row_top, column_right, row_bottom));
+    }
+  } else if (first_dirty_x_block != -1) {
+    output->AddRect(DesktopRect::MakeLTRB(first_dirty_x_block * kBlockSize,
+                                          row_top, (x_end - 1) * kBlockSize,
+                                          row_bottom));
+  }
+}
+
+// Compares block-columns in the range of [0, |x_end| - |x_start|), in partial
+// block-rows start from |prev| and |curr|, and outputs dirty regions into
+// |output|.
+void ComparePartialBlockRow(const uint8_t* prev,

[Sergey Ulanov, 2016/08/12 05:25:50]

See my previous comment. I think the optimized block differ can be changed to
handle blocks of arbitrary height, and then you wouldn't need to duplicate bunch
of code here and in CompareBlockRow().

[Hzj_jie, 2016/08/15 00:38:06]

I am happy to do so, if we are on the same page for NEON's 4 x 4 block issue.
According to current block_differ_sse2 implementation, indeed we can change
block differ to handle 32 x 1 block.

+                            const uint8_t* curr,
+                            const int x_start,
+                            const int x_end,
+                            const int row_top,
+                            const int width,
+                            const int height,
+                            const int stride,
+                            DesktopRegion* const output) {
+  const int row_bottom = height;
+  const int row_height = height - row_top;
+  const int column_right = std::min(x_end * kBlockSize, width);
+
+  // The first block-column in a continuous dirty area in current block-row.
+  int first_dirty_x_block = -1;
+
+  // We always need to add dirty area into |output| in the last block, so handle
+  // it separatedly.
+  for (int x = x_start; x < x_end - 1; x++) {
+    if (PartialBlockDifference(prev, curr, stride, kBlockSize, row_height)) {
+      if (first_dirty_x_block == -1) {
+        // This is the first dirty block in a continuous dirty area.
+        first_dirty_x_block = x;
+      }
+    } else if (first_dirty_x_block != -1) {
+      // The block on the left is the last dirty block in a continuous
+      // dirty area.
+      output->AddRect(DesktopRect::MakeLTRB(first_dirty_x_block * kBlockSize,
+                                            row_top, x * kBlockSize,
+                                            row_bottom));
+      first_dirty_x_block = -1;
+    }
+    prev += kBlockXOffset;
+    curr += kBlockXOffset;
+  }
+
+  if (PartialBlockDifference(prev, curr, stride,
+                             column_right - (x_end - 1) * kBlockSize,
+                             row_height)) {
+    if (first_dirty_x_block == -1) {
+      output->AddRect(DesktopRect::MakeLTRB((x_end - 1) * kBlockSize, row_top,
+                                            column_right, row_bottom));
+    } else {
+      output->AddRect(DesktopRect::MakeLTRB(first_dirty_x_block * kBlockSize,
+                                            row_top, column_right, row_bottom));
+    }
+  } else if (first_dirty_x_block != -1) {
+    output->AddRect(DesktopRect::MakeLTRB(first_dirty_x_block * kBlockSize,
+                                          row_top, (x_end - 1) * kBlockSize,
+                                          row_bottom));
+  }
+}
+
+// Compares |rect| area in |old_frame| and |new_frame|, and outputs dirty
+// regions into |output|.
+void CompareFrames(const DesktopFrame& old_frame,
+                   const DesktopFrame& new_frame,
+                   const DesktopRect rect,

[Sergey Ulanov, 2016/08/12 05:25:51]

DesktopRect&

[Hzj_jie, 2016/08/15 00:38:06]

Done.

+                   DesktopRegion* const output) {
+  RTC_DCHECK(old_frame.size().equals(new_frame.size()));
+  RTC_DCHECK(DesktopRect::MakeSize(old_frame.size()).ContainsRect(rect));
+  RTC_DCHECK_EQ(old_frame.stride(), new_frame.stride());
+
+  const int x_start = rect.left() / kBlockSize;
+  const int y_start = rect.top() / kBlockSize;
+  const int x_end = (rect.right() + kBlockSize - 1) / kBlockSize;
+  const int y_end = (rect.bottom() + kBlockSize - 1) / kBlockSize;
+  const DesktopVector block_top_left(x_start * kBlockSize,
+                                     y_start * kBlockSize);
+
+  // Offset from the start of one block-row to the next.
+  const int block_y_stride = old_frame.stride() * kBlockSize;
+
+  const uint8_t* prev_block_row_start =
+      old_frame.GetFrameDataAtPos(block_top_left);
+  const uint8_t* curr_block_row_start =
+      new_frame.GetFrameDataAtPos(block_top_left);
+
+  int row_top = y_start * kBlockSize;
+  // The last row may be a partial row, so we handle it separately.
+  for (int y = y_start; y < y_end - 1; y++) {
+    CompareBlockRow(prev_block_row_start, curr_block_row_start, x_start, x_end,
+                    row_top, old_frame.size().width(), old_frame.stride(),
+                    output);
+    row_top += kBlockSize;
+    prev_block_row_start += block_y_stride;
+    curr_block_row_start += block_y_stride;
+  }
+  if (row_top + kBlockSize > old_frame.size().height()) {
+    // The last row is a partial row.
+    ComparePartialBlockRow(prev_block_row_start, curr_block_row_start, x_start,
+                           x_end, row_top, old_frame.size().width(),
+                           old_frame.size().height(), old_frame.stride(),
+                           output);
+  } else {
+    CompareBlockRow(prev_block_row_start, curr_block_row_start, x_start, x_end,
+                    row_top, old_frame.size().width(), old_frame.stride(),
+                    output);
+  }
+}
+
+}  // namespace
+
+ScreenCapturerDifferWrapper::ScreenCapturerDifferWrapper(
+    std::unique_ptr<ScreenCapturer> base_capturer,
+    bool diff_entire_frame)
+    : base_capturer_(std::move(base_capturer)),
+      diff_entire_frame_(diff_entire_frame) {
+  RTC_DCHECK(base_capturer_);
+}
+
+ScreenCapturerDifferWrapper::~ScreenCapturerDifferWrapper() = default;

[Sergey Ulanov, 2016/08/12 05:25:50]

 {} instead of = default; would be cleaner here

[Hzj_jie, 2016/08/15 00:38:06]

Done.

+
+void ScreenCapturerDifferWrapper::Start(DesktopCapturer::Callback* callback) {
+  callback_ = callback;
+  base_capturer_->Start(this);
+}
+
+void ScreenCapturerDifferWrapper::SetSharedMemoryFactory(
+    std::unique_ptr<SharedMemoryFactory> shared_memory_factory) {
+  base_capturer_->SetSharedMemoryFactory(std::move(shared_memory_factory));
+}
+
+void ScreenCapturerDifferWrapper::Capture(const DesktopRegion& region) {
+  base_capturer_->Capture(region);
+}
+
+bool ScreenCapturerDifferWrapper::GetScreenList(ScreenList* screens) {
+  return base_capturer_->GetScreenList(screens);
+}
+
+bool ScreenCapturerDifferWrapper::SelectScreen(ScreenId id) {
+  return base_capturer_->SelectScreen(id);
+}
+
+void ScreenCapturerDifferWrapper::OnCaptureResult(
+    Result result,
+    std::unique_ptr<DesktopFrame> frame) {
+  int64_t start_time_nanos = rtc::TimeNanos();
+  if (result == Result::SUCCESS && frame) {
+    if (last_frame_ &&
+        (last_frame_->size().width() != frame->size().width() ||
+         last_frame_->size().height() != frame->size().height() ||
+         last_frame_->stride() != frame->stride())) {
+      last_frame_.reset();
+    }
+
+    if (last_frame_) {
+      if (diff_entire_frame_) {
+        CompareFrames(*last_frame_, *frame,
+                      DesktopRect::MakeSize(frame->size()),
+                      frame->mutable_updated_region());
+      } else {
+        DesktopRegion hints;
+        hints.Swap(frame->mutable_updated_region());
+        for (DesktopRegion::Iterator it(hints); !it.IsAtEnd(); it.Advance()) {
+          // Note, here we always expect the underlying capturer won't return a
+          // dirty region which excceeds the DesktopFrame::size().
+          CompareFrames(*last_frame_, *frame, it.rect(),
+                        frame->mutable_updated_region());
+        }
+      }
+    } else {
+      frame->mutable_updated_region()->SetRect(
+          DesktopRect::MakeSize(frame->size()));
+    }
+    last_frame_ = static_cast<SharedDesktopFrame*>(frame.get())->Share();
+  }
+
+  frame->set_capture_time_ms(frame->capture_time_ms() +

[Sergey Ulanov, 2016/08/12 05:25:50]

frame may be nullptr here. Handle errors case first in top of this function.

[Hzj_jie, 2016/08/15 00:38:06]

Done.

+                             (rtc::TimeNanos() - start_time_nanos) /
+                                 rtc::kNumNanosecsPerMillisec);
+  callback_->OnCaptureResult(result, std::move(frame));
+}
+
+}  // namespace webrtc