From 831bd96138e93fa9829e9f4be47ae6b4728720bc Mon Sep 17 00:00:00 2001 From: Julien Isorce Date: Thu, 31 Jan 2019 15:55:26 -0800 Subject: [PATCH] Remove unnecessary memset to DesktopFrame With a recent change, the webrtc::DesktopFrame is created and initialized with 0. So it's not necessary to call memset() to the frame again any more. See https://webrtc-review.googlesource.com/c/src/+/97184/ Bug: webrtc:9703 Change-Id: I27d096058ead075765f4c49bf60cb8d725da1afd Reviewed-on: https://webrtc-review.googlesource.com/c/120700 Commit-Queue: Brave Yao Reviewed-by: Brave Yao Cr-Commit-Position: refs/heads/master@{#26504} --- .../cropped_desktop_frame_unittest.cc | 13 ++++++++++ .../desktop_and_cursor_composer_unittest.cc | 1 - modules/desktop_capture/desktop_frame.h | 1 + modules/desktop_capture/win/dxgi_frame.cc | 24 ++++++++++--------- .../desktop_capture/window_capturer_win.cc | 1 - 5 files changed, 27 insertions(+), 13 deletions(-) diff --git a/modules/desktop_capture/cropped_desktop_frame_unittest.cc b/modules/desktop_capture/cropped_desktop_frame_unittest.cc index 34aff14274..fdf51aeaff 100644 --- a/modules/desktop_capture/cropped_desktop_frame_unittest.cc +++ b/modules/desktop_capture/cropped_desktop_frame_unittest.cc @@ -64,4 +64,17 @@ TEST(CroppedDesktopFrameTest, SetTopLeft) { ASSERT_EQ(frame->top_left().y(), 203); } +TEST(CroppedDesktopFrameTest, InitializedWithZeros) { + std::unique_ptr frame = CreateTestFrame(); + const DesktopVector frame_origin = frame->top_left(); + const DesktopSize frame_size = frame->size(); + std::unique_ptr cropped = CreateCroppedDesktopFrame( + std::move(frame), DesktopRect::MakeOriginSize(frame_origin, frame_size)); + for (int j = 0; j < cropped->size().height(); ++j) { + for (int i = 0; i < cropped->stride(); ++i) { + ASSERT_EQ(cropped->data()[i + j * cropped->stride()], 0); + } + } +} + } // namespace webrtc diff --git a/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc b/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc index b1430f97ed..f1a2860461 100644 --- a/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc +++ b/modules/desktop_capture/desktop_and_cursor_composer_unittest.cc @@ -126,7 +126,6 @@ class FakeMouseMonitor : public MouseCursorMonitor { std::unique_ptr image( new BasicDesktopFrame(DesktopSize(kCursorWidth, kCursorHeight))); uint32_t* data = reinterpret_cast(image->data()); - memset(data, 0, image->stride() * kCursorHeight); // Set four pixels near the hotspot and leave all other blank. for (int y = 0; y < kTestCursorSize; ++y) { diff --git a/modules/desktop_capture/desktop_frame.h b/modules/desktop_capture/desktop_frame.h index 803a2f4b94..5c595a58e1 100644 --- a/modules/desktop_capture/desktop_frame.h +++ b/modules/desktop_capture/desktop_frame.h @@ -138,6 +138,7 @@ class RTC_EXPORT DesktopFrame { // A DesktopFrame that stores data in the heap. class RTC_EXPORT BasicDesktopFrame : public DesktopFrame { public: + // The entire data buffer used for the frame is initialized with zeros. explicit BasicDesktopFrame(DesktopSize size); ~BasicDesktopFrame() override; diff --git a/modules/desktop_capture/win/dxgi_frame.cc b/modules/desktop_capture/win/dxgi_frame.cc index 85d708c82c..13d5b4b62e 100644 --- a/modules/desktop_capture/win/dxgi_frame.cc +++ b/modules/desktop_capture/win/dxgi_frame.cc @@ -41,20 +41,22 @@ bool DxgiFrame::Prepare(DesktopSize size, DesktopCapturer::SourceId source_id) { std::unique_ptr frame; if (factory_) { frame = SharedMemoryDesktopFrame::Create(size, factory_); + + if (!frame) { + RTC_LOG(LS_WARNING) << "DxgiFrame cannot create a new DesktopFrame."; + return false; + } + + // DirectX capturer won't paint each pixel in the frame due to its one + // capturer per monitor design. So once the new frame is created, we + // should clear it to avoid the legacy image to be remained on it. See + // http://crbug.com/708766. + RTC_DCHECK_EQ(frame->stride(), + frame->size().width() * DesktopFrame::kBytesPerPixel); + memset(frame->data(), 0, frame->stride() * frame->size().height()); } else { frame.reset(new BasicDesktopFrame(size)); } - if (!frame) { - RTC_LOG(LS_WARNING) << "DxgiFrame cannot create a new DesktopFrame."; - return false; - } - // DirectX capturer won't paint each pixel in the frame due to its one - // capturer per monitor design. So once the new frame is created, we should - // clear it to avoid the legacy image to be remained on it. See - // http://crbug.com/708766. - RTC_DCHECK_EQ(frame->stride(), - frame->size().width() * DesktopFrame::kBytesPerPixel); - memset(frame->data(), 0, frame->stride() * frame->size().height()); frame_ = SharedDesktopFrame::Wrap(std::move(frame)); } diff --git a/modules/desktop_capture/window_capturer_win.cc b/modules/desktop_capture/window_capturer_win.cc index 5dc3d95439..52b2bac83e 100644 --- a/modules/desktop_capture/window_capturer_win.cc +++ b/modules/desktop_capture/window_capturer_win.cc @@ -249,7 +249,6 @@ void WindowCapturerWin::CaptureFrame() { !window_capture_helper_.IsWindowVisibleOnCurrentDesktop(window_)) { std::unique_ptr frame( new BasicDesktopFrame(DesktopSize(1, 1))); - memset(frame->data(), 0, frame->stride() * frame->size().height()); previous_size_ = frame->size(); window_size_map_[window_] = previous_size_;