diff --git a/AUTHORS b/AUTHORS index b272adfd2..42959c1d8 100644 --- a/AUTHORS +++ b/AUTHORS @@ -5,3 +5,4 @@ Google Inc. Ho Cheung Ivan Pavlotskiy +Sida Zhu diff --git a/source/scale.cc b/source/scale.cc index 29424ae71..6dc768918 100644 --- a/source/scale.cc +++ b/source/scale.cc @@ -1966,12 +1966,19 @@ int ScalePlane(const uint8_t* src, int dy = 0; int y = 0; // When scaling down, use the center 2 rows to filter. - // When scaling up, last row of destination uses the last 2 source rows. if (dst_height <= src_height) { dy = FixedDiv(src_height, dst_height); y = CENTERSTART(dy, -32768); // Subtract 0.5 (32768) to center filter. } else if (src_height > 1 && dst_height > 1) { - dy = FixedDiv1(src_height, dst_height); + // Point sampling uses centered source/destination stepping, matching + // ScaleSlope. Bilinear scaling maps the source endpoints. + if (filtering == kFilterNone) { + dy = FixedDiv(src_height, dst_height); + y = CENTERSTART(dy, 0); + } else { + // When scaling up, last row of destination uses the last 2 source rows. + dy = FixedDiv1(src_height, dst_height); + } } // Arbitrary scale vertically, but unscaled horizontally. ScalePlaneVertical(src_height, dst_width, dst_height, src_stride, @@ -2074,15 +2081,19 @@ int ScalePlane_16(const uint16_t* src, int dy = 0; int y = 0; // When scaling down, use the center 2 rows to filter. - // When scaling up, last row of destination uses the last 2 source rows. if (dst_height <= src_height) { dy = FixedDiv(src_height, dst_height); y = CENTERSTART(dy, -32768); // Subtract 0.5 (32768) to center filter. - // When scaling up, ensure the last row of destination uses the last - // source. Avoid divide by zero for dst_height but will do no scaling - // later. } else if (src_height > 1 && dst_height > 1) { - dy = FixedDiv1(src_height, dst_height); + // Point sampling uses centered source/destination stepping, matching + // ScaleSlope. Bilinear scaling maps the source endpoints. + if (filtering == kFilterNone) { + dy = FixedDiv(src_height, dst_height); + y = CENTERSTART(dy, 0); + } else { + // When scaling up, last row of destination uses the last 2 source rows. + dy = FixedDiv1(src_height, dst_height); + } } // Arbitrary scale vertically, but unscaled horizontally. ScalePlaneVertical_16(src_height, dst_width, dst_height, src_stride, diff --git a/source/scale_common.cc b/source/scale_common.cc index d0b0edd95..325e72319 100644 --- a/source/scale_common.cc +++ b/source/scale_common.cc @@ -1474,8 +1474,18 @@ void ScalePlaneVertical(int src_height, void (*InterpolateRow)(uint8_t* dst_argb, const uint8_t* src_argb, ptrdiff_t src_stride, int dst_width, int source_y_fraction) = InterpolateRow_C; - const int64_t max_y = - (src_height > 1) ? (((int64_t)src_height - 1) << 16) - 1 : 0; + const bool interpolate = filtering == kFilterBilinear; + int64_t max_y = 0; + if (src_height > 1) { + max_y = ((int64_t)src_height - 1) << 16; + if (interpolate) { + --max_y; + } + } + // Without vertical interpolation, InterpolateRow() copies one row. A zero + // stride keeps it from forming a second source row pointer past the end when + // y is clamped to the last row. + const ptrdiff_t interpolate_stride = interpolate ? src_stride : 0; int64_t y64 = y; int j; assert(bpp >= 1 && bpp <= 4); @@ -1530,9 +1540,9 @@ void ScalePlaneVertical(int src_height, y64 = max_y; } yi = (int)(y64 >> 16); - yf = filtering ? (int)((y64 >> 8) & 255) : 0; - InterpolateRow(dst_argb, src_argb + yi * (ptrdiff_t)src_stride, src_stride, - dst_width_bytes, yf); + yf = interpolate ? (int)((y64 >> 8) & 255) : 0; + InterpolateRow(dst_argb, src_argb + yi * (ptrdiff_t)src_stride, + interpolate_stride, dst_width_bytes, yf); dst_argb += dst_stride; y64 += dy; } @@ -1555,8 +1565,18 @@ void ScalePlaneVertical_16(int src_height, void (*InterpolateRow)(uint16_t* dst_argb, const uint16_t* src_argb, ptrdiff_t src_stride, int dst_width, int source_y_fraction) = InterpolateRow_16_C; - const int64_t max_y = - (src_height > 1) ? (((int64_t)src_height - 1) << 16) - 1 : 0; + const bool interpolate = filtering == kFilterBilinear; + int64_t max_y = 0; + if (src_height > 1) { + max_y = ((int64_t)src_height - 1) << 16; + if (interpolate) { + --max_y; + } + } + // Without vertical interpolation, InterpolateRow() copies one row. A zero + // stride keeps it from forming a second source row pointer past the end when + // y is clamped to the last row. + const ptrdiff_t interpolate_stride = interpolate ? src_stride : 0; int64_t y64 = y; int j; assert(wpp >= 1 && wpp <= 2); @@ -1600,9 +1620,9 @@ void ScalePlaneVertical_16(int src_height, y64 = max_y; } yi = (int)(y64 >> 16); - yf = filtering ? (int)((y64 >> 8) & 255) : 0; - InterpolateRow(dst_argb, src_argb + yi * (ptrdiff_t)src_stride, src_stride, - dst_width_words, yf); + yf = interpolate ? (int)((y64 >> 8) & 255) : 0; + InterpolateRow(dst_argb, src_argb + yi * (ptrdiff_t)src_stride, + interpolate_stride, dst_width_words, yf); dst_argb += dst_stride; y64 += dy; } diff --git a/unit_test/scale_plane_test.cc b/unit_test/scale_plane_test.cc index b04dda10c..ab521ebcf 100644 --- a/unit_test/scale_plane_test.cc +++ b/unit_test/scale_plane_test.cc @@ -570,6 +570,126 @@ TEST_F(LibYUVScaleTest, PlaneTest1_16_Box) { free_aligned_buffer_page_end(orig_pixels_alloc); } +TEST_F(LibYUVScaleTest, ScalePlaneVerticalPointUp) { + const int kWidth = 32; + const uint8_t kSentinel = 0xFF; + const uint8_t expected[] = {10, 10, 20, 30, 30}; + + // An extra sentinel row exposes accidental interpolation past the last valid + // row. 3->5 is the uneven ratio that used to drop the last source row. + align_buffer_page_end(src, kWidth * 4); + align_buffer_page_end(dst, kWidth * 5); + memset(src, kSentinel, kWidth * 4); + memset(dst, kSentinel, kWidth * 5); + memset(src, 10, kWidth); + memset(src + kWidth, 20, kWidth); + memset(src + 2 * kWidth, 30, kWidth); + + ASSERT_EQ(0, ScalePlane(src, kWidth, kWidth, 3, dst, kWidth, kWidth, 5, + kFilterNone)); + for (int y = 0; y < 5; ++y) { + for (int x = 0; x < kWidth; ++x) { + EXPECT_EQ(expected[y], dst[y * kWidth + x]); + } + } + + free_aligned_buffer_page_end(dst); + free_aligned_buffer_page_end(src); +} + +TEST_F(LibYUVScaleTest, ScalePlaneVerticalPointUp_16) { + const int kWidth = 32; + const uint16_t kSentinel = 0xFFFF; + const uint16_t expected[] = {10, 10, 20, 30, 30}; + + align_buffer_page_end(src_alloc, kWidth * 4 * 2); + align_buffer_page_end(dst_alloc, kWidth * 5 * 2); + uint16_t* src = reinterpret_cast(src_alloc); + uint16_t* dst = reinterpret_cast(dst_alloc); + for (int i = 0; i < kWidth * 4; ++i) { + src[i] = kSentinel; + } + for (int i = 0; i < kWidth * 5; ++i) { + dst[i] = kSentinel; + } + for (int x = 0; x < kWidth; ++x) { + src[x] = 10; + src[kWidth + x] = 20; + src[2 * kWidth + x] = 30; + } + + ASSERT_EQ(0, ScalePlane_16(src, kWidth, kWidth, 3, dst, kWidth, kWidth, 5, + kFilterNone)); + for (int y = 0; y < 5; ++y) { + for (int x = 0; x < kWidth; ++x) { + EXPECT_EQ(expected[y], dst[y * kWidth + x]); + } + } + + free_aligned_buffer_page_end(dst_alloc); + free_aligned_buffer_page_end(src_alloc); +} + +TEST_F(LibYUVScaleTest, ScalePlaneVerticalBilinearUp) { + const int kWidth = 32; + const uint8_t kSentinel = 0xFF; + // Same-width 2x upsampling still uses FixedDiv1. With y = 0 the 8.8 + // fractions are 0, 85, 170, 255. InterpolateRow_C: + // (a * (256 - yf) + b * yf + 128) >> 8. + // Nearest 2x would duplicate rows as {10, 10, 200, 200}. + const uint8_t expected[] = {10, 73, 136, 199}; + + align_buffer_page_end(src, kWidth * 3); + align_buffer_page_end(dst, kWidth * 4); + memset(src, kSentinel, kWidth * 3); + memset(dst, kSentinel, kWidth * 4); + memset(src, 10, kWidth); + memset(src + kWidth, 200, kWidth); + + ASSERT_EQ(0, ScalePlane(src, kWidth, kWidth, 2, dst, kWidth, kWidth, 4, + kFilterBilinear)); + for (int y = 0; y < 4; ++y) { + for (int x = 0; x < kWidth; ++x) { + EXPECT_EQ(expected[y], dst[y * kWidth + x]); + } + } + + free_aligned_buffer_page_end(dst); + free_aligned_buffer_page_end(src); +} + +TEST_F(LibYUVScaleTest, ScalePlaneVerticalBilinearUp_16) { + const int kWidth = 32; + const uint16_t kSentinel = 0xFFFF; + const uint16_t expected[] = {10, 73, 136, 199}; + + align_buffer_page_end(src_alloc, kWidth * 3 * 2); + align_buffer_page_end(dst_alloc, kWidth * 4 * 2); + uint16_t* src = reinterpret_cast(src_alloc); + uint16_t* dst = reinterpret_cast(dst_alloc); + for (int i = 0; i < kWidth * 3; ++i) { + src[i] = kSentinel; + } + for (int i = 0; i < kWidth * 4; ++i) { + dst[i] = kSentinel; + } + for (int x = 0; x < kWidth; ++x) { + src[x] = 10; + src[kWidth + x] = 200; + } + + ASSERT_EQ(0, ScalePlane_16(src, kWidth, kWidth, 2, dst, kWidth, kWidth, 4, + kFilterBilinear)); + for (int y = 0; y < 4; ++y) { + for (int x = 0; x < kWidth; ++x) { + EXPECT_EQ(expected[y], dst[y * kWidth + x]); + } + } + + free_aligned_buffer_page_end(dst_alloc); + free_aligned_buffer_page_end(src_alloc); +} + // POC: int * int overflow in ScalePlaneVertical (scale_common.cc). // // `yi * src_stride` is evaluated as int * int. When the product exceeds