From 79b2e06782a4115ebc46d1d44ddbf7a63a405926 Mon Sep 17 00:00:00 2001 From: Peter Kasting Date: Mon, 18 May 2015 14:39:14 -0700 Subject: [PATCH] Make the BlockDifference() functions return DiffInfo as their callers expect. This prevents the compiler from warning about possible type truncations at the callsites. BUG=chromium:81439 TEST=none R=wez@chromium.org Review URL: https://webrtc-codereview.appspot.com/41059004 Cr-Commit-Position: refs/heads/master@{#9211} --- webrtc/modules/desktop_capture/differ.cc | 77 ++++++++++--------- webrtc/modules/desktop_capture/differ.h | 20 +++-- .../modules/desktop_capture/differ_block.cc | 16 ++-- webrtc/modules/desktop_capture/differ_block.h | 10 ++- .../desktop_capture/differ_block_sse2.cc | 20 ++--- .../desktop_capture/differ_block_sse2.h | 12 +-- .../desktop_capture/differ_unittest.cc | 64 +++++++-------- 7 files changed, 111 insertions(+), 108 deletions(-) diff --git a/webrtc/modules/desktop_capture/differ.cc b/webrtc/modules/desktop_capture/differ.cc index 5a347a7dd6..0f53b78e0c 100644 --- a/webrtc/modules/desktop_capture/differ.cc +++ b/webrtc/modules/desktop_capture/differ.cc @@ -28,13 +28,14 @@ Differ::Differ(int width, int height, int bpp, int stride) { // One additional row/column is added as a boundary on the right & bottom. diff_info_width_ = ((width_ + kBlockSize - 1) / kBlockSize) + 1; diff_info_height_ = ((height_ + kBlockSize - 1) / kBlockSize) + 1; - diff_info_size_ = diff_info_width_ * diff_info_height_ * sizeof(DiffInfo); - diff_info_.reset(new DiffInfo[diff_info_size_]); + diff_info_size_ = diff_info_width_ * diff_info_height_ * sizeof(bool); + diff_info_.reset(new bool[diff_info_size_]); } Differ::~Differ() {} -void Differ::CalcDirtyRegion(const void* prev_buffer, const void* curr_buffer, +void Differ::CalcDirtyRegion(const uint8_t* prev_buffer, + const uint8_t* curr_buffer, DesktopRegion* region) { // Identify all the blocks that contain changed pixels. MarkDirtyBlocks(prev_buffer, curr_buffer); @@ -44,7 +45,8 @@ void Differ::CalcDirtyRegion(const void* prev_buffer, const void* curr_buffer, MergeBlocks(region); } -void Differ::MarkDirtyBlocks(const void* prev_buffer, const void* curr_buffer) { +void Differ::MarkDirtyBlocks(const uint8_t* prev_buffer, + const uint8_t* curr_buffer) { memset(diff_info_.get(), 0, diff_info_size_); // Calc number of full blocks. @@ -60,18 +62,16 @@ void Differ::MarkDirtyBlocks(const void* prev_buffer, const void* curr_buffer) { // Offset from the start of one block-row to the next. int block_y_stride = (width_ * bytes_per_pixel_) * kBlockSize; // Offset from the start of one diff_info row to the next. - int diff_info_stride = diff_info_width_ * sizeof(DiffInfo); + int diff_info_stride = diff_info_width_ * sizeof(bool); - const uint8_t* prev_block_row_start = - static_cast(prev_buffer); - const uint8_t* curr_block_row_start = - static_cast(curr_buffer); - DiffInfo* diff_info_row_start = static_cast(diff_info_.get()); + const uint8_t* prev_block_row_start = prev_buffer; + const uint8_t* curr_block_row_start = curr_buffer; + bool* diff_info_row_start = diff_info_.get(); for (int y = 0; y < y_full_blocks; y++) { const uint8_t* prev_block = prev_block_row_start; const uint8_t* curr_block = curr_block_row_start; - DiffInfo* diff_info = diff_info_row_start; + bool* diff_info = diff_info_row_start; for (int x = 0; x < x_full_blocks; x++) { // Mark this block as being modified so that it gets incorporated into @@ -79,15 +79,15 @@ void Differ::MarkDirtyBlocks(const void* prev_buffer, const void* curr_buffer) { *diff_info = BlockDifference(prev_block, curr_block, bytes_per_row_); prev_block += block_x_offset; curr_block += block_x_offset; - diff_info += sizeof(DiffInfo); + diff_info += sizeof(bool); } // If there is a partial column at the end, handle it. // This condition should rarely, if ever, occur. if (partial_column_width != 0) { - *diff_info = DiffPartialBlock(prev_block, curr_block, bytes_per_row_, - partial_column_width, kBlockSize); - diff_info += sizeof(DiffInfo); + *diff_info = !PartialBlocksEqual(prev_block, curr_block, bytes_per_row_, + partial_column_width, kBlockSize); + diff_info += sizeof(bool); } // Update pointers for next row. @@ -102,74 +102,75 @@ void Differ::MarkDirtyBlocks(const void* prev_buffer, const void* curr_buffer) { if (partial_row_height != 0) { const uint8_t* prev_block = prev_block_row_start; const uint8_t* curr_block = curr_block_row_start; - DiffInfo* diff_info = diff_info_row_start; + bool* diff_info = diff_info_row_start; for (int x = 0; x < x_full_blocks; x++) { - *diff_info = DiffPartialBlock(prev_block, curr_block, - bytes_per_row_, - kBlockSize, partial_row_height); + *diff_info = !PartialBlocksEqual(prev_block, curr_block, + bytes_per_row_, + kBlockSize, partial_row_height); prev_block += block_x_offset; curr_block += block_x_offset; - diff_info += sizeof(DiffInfo); + diff_info += sizeof(bool); } if (partial_column_width != 0) { - *diff_info = DiffPartialBlock(prev_block, curr_block, bytes_per_row_, - partial_column_width, partial_row_height); - diff_info += sizeof(DiffInfo); + *diff_info = !PartialBlocksEqual(prev_block, curr_block, bytes_per_row_, + partial_column_width, + partial_row_height); + diff_info += sizeof(bool); } } } -DiffInfo Differ::DiffPartialBlock(const uint8_t* prev_buffer, - const uint8_t* curr_buffer, - int stride, int width, int height) { +bool Differ::PartialBlocksEqual(const uint8_t* prev_buffer, + const uint8_t* curr_buffer, + int stride, int width, int height) { int width_bytes = width * bytes_per_pixel_; for (int y = 0; y < height; y++) { if (memcmp(prev_buffer, curr_buffer, width_bytes) != 0) - return 1; + return false; prev_buffer += bytes_per_row_; curr_buffer += bytes_per_row_; } - return 0; + return true; } void Differ::MergeBlocks(DesktopRegion* region) { region->Clear(); - uint8_t* diff_info_row_start = static_cast(diff_info_.get()); - int diff_info_stride = diff_info_width_ * sizeof(DiffInfo); + bool* diff_info_row_start = diff_info_.get(); + int diff_info_stride = diff_info_width_ * sizeof(bool); for (int y = 0; y < diff_info_height_; y++) { - uint8_t* diff_info = diff_info_row_start; + bool* diff_info = diff_info_row_start; for (int x = 0; x < diff_info_width_; x++) { - if (*diff_info != 0) { + if (*diff_info) { // We've found a modified block. Look at blocks to the right and below // to group this block with as many others as we can. int left = x * kBlockSize; int top = y * kBlockSize; int width = 1; int height = 1; - *diff_info = 0; + *diff_info = false; // Group with blocks to the right. // We can keep looking until we find an unchanged block because we // have a boundary block which is never marked as having diffs. - uint8_t* right = diff_info + 1; + bool* right = diff_info + 1; while (*right) { - *right++ = 0; + *right++ = false; width++; } // Group with blocks below. // The entire width of blocks that we matched above much match for // each row that we add. - uint8_t* bottom = diff_info; + bool* bottom = diff_info; bool found_new_row; do { found_new_row = true; bottom += diff_info_stride; right = bottom; for (int x2 = 0; x2 < width; x2++) { - if (*right++ == 0) { + if (!*right++) { found_new_row = false; } } @@ -181,7 +182,7 @@ void Differ::MergeBlocks(DesktopRegion* region) { // try to add these blocks a second time. right = bottom; for (int x2 = 0; x2 < width; x2++) { - *right++ = 0; + *right++ = false; } } } while (found_new_row); diff --git a/webrtc/modules/desktop_capture/differ.h b/webrtc/modules/desktop_capture/differ.h index d5e21db953..224c6913af 100644 --- a/webrtc/modules/desktop_capture/differ.h +++ b/webrtc/modules/desktop_capture/differ.h @@ -18,8 +18,6 @@ namespace webrtc { -typedef uint8_t DiffInfo; - // TODO(sergeyu): Simplify differ now that we are working with DesktopRegion. // diff_info_ should no longer be needed, as we can put our data directly into // the region that we are calculating. @@ -40,7 +38,7 @@ class Differ { // Given the previous and current screen buffer, calculate the dirty region // that encloses all of the changed pixels in the new screen. - void CalcDirtyRegion(const void* prev_buffer, const void* curr_buffer, + void CalcDirtyRegion(const uint8_t* prev_buffer, const uint8_t* curr_buffer, DesktopRegion* region); private: @@ -48,21 +46,21 @@ class Differ { friend class DifferTest; // Identify all of the blocks that contain changed pixels. - void MarkDirtyBlocks(const void* prev_buffer, const void* curr_buffer); + void MarkDirtyBlocks(const uint8_t* prev_buffer, const uint8_t* curr_buffer); // After the dirty blocks have been identified, this routine merges adjacent // blocks into a region. // The goal is to minimize the region that covers the dirty blocks. void MergeBlocks(DesktopRegion* region); - // Check for diffs in upper-left portion of the block. The size of the portion - // to check is specified by the |width| and |height| values. + // Checks whether the upper-left portions of the buffers are equal. The size + // of the portion to check is specified by the |width| and |height| values. // Note that if we force the capturer to always return images whose width and // height are multiples of kBlockSize, then this will never be called. - DiffInfo DiffPartialBlock(const uint8_t* prev_buffer, - const uint8_t* curr_buffer, - int stride, - int width, int height); + bool PartialBlocksEqual(const uint8_t* prev_buffer, + const uint8_t* curr_buffer, + int stride, + int width, int height); // Dimensions of screen. int width_; @@ -76,7 +74,7 @@ class Differ { int bytes_per_row_; // Diff information for each block in the image. - rtc::scoped_ptr diff_info_; + rtc::scoped_ptr diff_info_; // Dimensions and total size of diff info array. int diff_info_width_; diff --git a/webrtc/modules/desktop_capture/differ_block.cc b/webrtc/modules/desktop_capture/differ_block.cc index 3d1b229541..abae0d66f4 100644 --- a/webrtc/modules/desktop_capture/differ_block.cc +++ b/webrtc/modules/desktop_capture/differ_block.cc @@ -18,22 +18,24 @@ namespace webrtc { -int BlockDifference_C(const uint8_t* image1, - const uint8_t* image2, - int stride) { +bool BlockDifference_C(const uint8_t* image1, + const uint8_t* image2, + int stride) { int width_bytes = kBlockSize * kBytesPerPixel; for (int y = 0; y < kBlockSize; y++) { if (memcmp(image1, image2, width_bytes) != 0) - return 1; + return true; image1 += stride; image2 += stride; } - return 0; + return false; } -int BlockDifference(const uint8_t* image1, const uint8_t* image2, int stride) { - static int (*diff_proc)(const uint8_t*, const uint8_t*, int) = NULL; +bool BlockDifference(const uint8_t* image1, + const uint8_t* image2, + int stride) { + static bool (*diff_proc)(const uint8_t*, const uint8_t*, int) = NULL; if (!diff_proc) { #if defined(ARCH_CPU_ARM_FAMILY) || defined(ARCH_CPU_MIPS_FAMILY) diff --git a/webrtc/modules/desktop_capture/differ_block.h b/webrtc/modules/desktop_capture/differ_block.h index 2b43f4ed06..e1d487d68b 100644 --- a/webrtc/modules/desktop_capture/differ_block.h +++ b/webrtc/modules/desktop_capture/differ_block.h @@ -11,7 +11,7 @@ #ifndef WEBRTC_MODULES_DESKTOP_CAPTURE_DIFFER_BLOCK_H_ #define WEBRTC_MODULES_DESKTOP_CAPTURE_DIFFER_BLOCK_H_ -#include "webrtc/typedefs.h" +#include namespace webrtc { @@ -22,9 +22,11 @@ const int kBlockSize = 32; // Format: BGRA 32 bit. const int kBytesPerPixel = 4; -// Low level functions to compare 2 blocks of pixels. Zero means the blocks -// are identical. One - the blocks are different. -int BlockDifference(const uint8_t* image1, const uint8_t* image2, int stride); +// Low level function to compare 2 blocks of pixels of size +// (kBlockSize, kBlockSize). Returns whether the blocks differ. +bool BlockDifference(const uint8_t* image1, + const uint8_t* image2, + int stride); } // namespace webrtc diff --git a/webrtc/modules/desktop_capture/differ_block_sse2.cc b/webrtc/modules/desktop_capture/differ_block_sse2.cc index 7f31bd3a84..8d35df2226 100644 --- a/webrtc/modules/desktop_capture/differ_block_sse2.cc +++ b/webrtc/modules/desktop_capture/differ_block_sse2.cc @@ -21,9 +21,9 @@ namespace webrtc { -extern int BlockDifference_SSE2_W16(const uint8_t* image1, - const uint8_t* image2, - int stride) { +extern bool BlockDifference_SSE2_W16(const uint8_t* image1, + const uint8_t* image2, + int stride) { __m128i acc = _mm_setzero_si128(); __m128i v0; __m128i v1; @@ -54,16 +54,16 @@ extern int BlockDifference_SSE2_W16(const uint8_t* image1, sad = _mm_adds_epu16(sad, acc); int diff = _mm_cvtsi128_si32(sad); if (diff) - return 1; + return true; image1 += stride; image2 += stride; } - return 0; + return false; } -extern int BlockDifference_SSE2_W32(const uint8_t* image1, - const uint8_t* image2, - int stride) { +extern bool BlockDifference_SSE2_W32(const uint8_t* image1, + const uint8_t* image2, + int stride) { __m128i acc = _mm_setzero_si128(); __m128i v0; __m128i v1; @@ -110,11 +110,11 @@ extern int BlockDifference_SSE2_W32(const uint8_t* image1, sad = _mm_adds_epu16(sad, acc); int diff = _mm_cvtsi128_si32(sad); if (diff) - return 1; + return true; image1 += stride; image2 += stride; } - return 0; + return false; } } // namespace webrtc diff --git a/webrtc/modules/desktop_capture/differ_block_sse2.h b/webrtc/modules/desktop_capture/differ_block_sse2.h index 081e6fa235..90426dafab 100644 --- a/webrtc/modules/desktop_capture/differ_block_sse2.h +++ b/webrtc/modules/desktop_capture/differ_block_sse2.h @@ -19,14 +19,14 @@ namespace webrtc { // Find block difference of dimension 16x16. -extern int BlockDifference_SSE2_W16(const uint8_t* image1, - const uint8_t* image2, - int stride); +extern bool BlockDifference_SSE2_W16(const uint8_t* image1, + const uint8_t* image2, + int stride); // Find block difference of dimension 32x32. -extern int BlockDifference_SSE2_W32(const uint8_t* image1, - const uint8_t* image2, - int stride); +extern bool BlockDifference_SSE2_W32(const uint8_t* image1, + const uint8_t* image2, + int stride); } // namespace webrtc diff --git a/webrtc/modules/desktop_capture/differ_unittest.cc b/webrtc/modules/desktop_capture/differ_unittest.cc index 019e952e9c..3b6f859a7c 100644 --- a/webrtc/modules/desktop_capture/differ_unittest.cc +++ b/webrtc/modules/desktop_capture/differ_unittest.cc @@ -51,7 +51,7 @@ class DifferTest : public testing::Test { } // Here in DifferTest so that tests can access private methods of Differ. - void MarkDirtyBlocks(const void* prev_buffer, const void* curr_buffer) { + void MarkDirtyBlocks(const uint8_t* prev_buffer, const uint8_t* curr_buffer) { differ_->MarkDirtyBlocks(prev_buffer, curr_buffer); } @@ -71,7 +71,7 @@ class DifferTest : public testing::Test { // Convenience wrapper for Differ's DiffBlock that calculates the appropriate // offset to the start of the desired block. - DiffInfo DiffBlock(int block_x, int block_y) { + bool DiffBlock(int block_x, int block_y) { // Offset from upper-left of buffer to upper-left of requested block. int block_offset = ((block_y * stride_) + (block_x * bytes_per_pixel_)) * kBlockSize; @@ -114,8 +114,8 @@ class DifferTest : public testing::Test { } // Get the value in the |diff_info_| array at (x,y). - DiffInfo GetDiffInfo(int x, int y) { - DiffInfo* diff_info = differ_->diff_info_.get(); + bool GetDiffInfo(int x, int y) { + bool* diff_info = differ_->diff_info_.get(); return diff_info[(y * GetDiffInfoWidth()) + x]; } @@ -134,8 +134,8 @@ class DifferTest : public testing::Test { return differ_->diff_info_size_; } - void SetDiffInfo(int x, int y, const DiffInfo& value) { - DiffInfo* diff_info = differ_->diff_info_.get(); + void SetDiffInfo(int x, int y, bool value) { + bool* diff_info = differ_->diff_info_.get(); diff_info[(y * GetDiffInfoWidth()) + x] = value; } @@ -143,7 +143,7 @@ class DifferTest : public testing::Test { void MarkBlocks(int x_origin, int y_origin, int width, int height) { for (int y = 0; y < height; y++) { for (int x = 0; x < width; x++) { - SetDiffInfo(x_origin + x, y_origin + y, 1); + SetDiffInfo(x_origin + x, y_origin + y, true); } } } @@ -240,7 +240,7 @@ TEST_F(DifferTest, MarkDirtyBlocks_All) { // Make sure each block is marked as dirty. for (int y = 0; y < GetDiffInfoHeight() - 1; y++) { for (int x = 0; x < GetDiffInfoWidth() - 1; x++) { - EXPECT_EQ(1, GetDiffInfo(x, y)) + EXPECT_TRUE(GetDiffInfo(x, y)) << "when x = " << x << ", and y = " << y; } } @@ -258,23 +258,23 @@ TEST_F(DifferTest, MarkDirtyBlocks_Sampling) { MarkDirtyBlocks(prev_.get(), curr_.get()); // Make sure corresponding blocks are updated. - EXPECT_EQ(0, GetDiffInfo(0, 0)); - EXPECT_EQ(0, GetDiffInfo(0, 1)); - EXPECT_EQ(1, GetDiffInfo(0, 2)); - EXPECT_EQ(1, GetDiffInfo(1, 0)); - EXPECT_EQ(0, GetDiffInfo(1, 1)); - EXPECT_EQ(0, GetDiffInfo(1, 2)); - EXPECT_EQ(0, GetDiffInfo(2, 0)); - EXPECT_EQ(1, GetDiffInfo(2, 1)); - EXPECT_EQ(0, GetDiffInfo(2, 2)); + EXPECT_FALSE(GetDiffInfo(0, 0)); + EXPECT_FALSE(GetDiffInfo(0, 1)); + EXPECT_TRUE(GetDiffInfo(0, 2)); + EXPECT_TRUE(GetDiffInfo(1, 0)); + EXPECT_FALSE(GetDiffInfo(1, 1)); + EXPECT_FALSE(GetDiffInfo(1, 2)); + EXPECT_FALSE(GetDiffInfo(2, 0)); + EXPECT_TRUE(GetDiffInfo(2, 1)); + EXPECT_FALSE(GetDiffInfo(2, 2)); } TEST_F(DifferTest, DiffBlock) { InitDiffer(kScreenWidth, kScreenHeight); // Verify no differences at start. - EXPECT_EQ(0, DiffBlock(0, 0)); - EXPECT_EQ(0, DiffBlock(1, 1)); + EXPECT_FALSE(DiffBlock(0, 0)); + EXPECT_FALSE(DiffBlock(1, 1)); // Write new data into the 4 corners of the middle block and verify that // neighboring blocks are not affected. @@ -283,15 +283,15 @@ TEST_F(DifferTest, DiffBlock) { WriteBlockPixel(curr_.get(), 1, 1, 0, max, 0xffffff); WriteBlockPixel(curr_.get(), 1, 1, max, 0, 0xffffff); WriteBlockPixel(curr_.get(), 1, 1, max, max, 0xffffff); - EXPECT_EQ(0, DiffBlock(0, 0)); - EXPECT_EQ(0, DiffBlock(0, 1)); - EXPECT_EQ(0, DiffBlock(0, 2)); - EXPECT_EQ(0, DiffBlock(1, 0)); - EXPECT_EQ(1, DiffBlock(1, 1)); // Only this block should change. - EXPECT_EQ(0, DiffBlock(1, 2)); - EXPECT_EQ(0, DiffBlock(2, 0)); - EXPECT_EQ(0, DiffBlock(2, 1)); - EXPECT_EQ(0, DiffBlock(2, 2)); + EXPECT_FALSE(DiffBlock(0, 0)); + EXPECT_FALSE(DiffBlock(0, 1)); + EXPECT_FALSE(DiffBlock(0, 2)); + EXPECT_FALSE(DiffBlock(1, 0)); + EXPECT_TRUE(DiffBlock(1, 1)); // Only this block should change. + EXPECT_FALSE(DiffBlock(1, 2)); + EXPECT_FALSE(DiffBlock(2, 0)); + EXPECT_FALSE(DiffBlock(2, 1)); + EXPECT_FALSE(DiffBlock(2, 2)); } TEST_F(DifferTest, Partial_Setup) { @@ -328,7 +328,7 @@ TEST_F(DifferTest, Partial_FirstPixel) { // Make sure each block is marked as dirty. for (int y = 0; y < GetDiffInfoHeight() - 1; y++) { for (int x = 0; x < GetDiffInfoWidth() - 1; x++) { - EXPECT_EQ(1, GetDiffInfo(x, y)) + EXPECT_TRUE(GetDiffInfo(x, y)) << "when x = " << x << ", and y = " << y; } } @@ -351,18 +351,18 @@ TEST_F(DifferTest, Partial_BorderPixel) { // Make sure last (partial) block in each row/column is marked as dirty. int x_last = GetDiffInfoWidth() - 2; for (int y = 0; y < GetDiffInfoHeight() - 1; y++) { - EXPECT_EQ(1, GetDiffInfo(x_last, y)) + EXPECT_TRUE(GetDiffInfo(x_last, y)) << "when x = " << x_last << ", and y = " << y; } int y_last = GetDiffInfoHeight() - 2; for (int x = 0; x < GetDiffInfoWidth() - 1; x++) { - EXPECT_EQ(1, GetDiffInfo(x, y_last)) + EXPECT_TRUE(GetDiffInfo(x, y_last)) << "when x = " << x << ", and y = " << y_last; } // All other blocks are clean. for (int y = 0; y < GetDiffInfoHeight() - 2; y++) { for (int x = 0; x < GetDiffInfoWidth() - 2; x++) { - EXPECT_EQ(0, GetDiffInfo(x, y)) << "when x = " << x << ", and y = " << y; + EXPECT_FALSE(GetDiffInfo(x, y)) << "when x = " << x << ", and y = " << y; } } }