Fix same-width vertical point upsampling

ScalePlane's same-width vertical fast path used the endpoint-oriented
FixedDiv1 step for all upsampling. That stepping is appropriate for
bilinear interpolation, but not for point sampling, where source rows
should cover equally sized destination cells.

Use the same centered FixedDiv stepping as the generic point sampler.
Allow the point path to select the final source row while passing a
zero interpolation stride so row kernels cannot form a pointer beyond
it. Add 8-bit and 16-bit point regression coverage and verify that
bilinear upsampling remains unchanged.

Bug: 537818862
Change-Id: I7bee21b85913b31c146ecf7fe9f5f08608a8a17d
Reviewed-on: https://chromium-review.googlesource.com/c/libyuv/libyuv/+/8264449
Reviewed-by: Wan-Teh Chang <wtc@google.com>
Commit-Queue: Wan-Teh Chang <wtc@google.com>
Reviewed-by: Frank Barchard <fbarchard@google.com>

AUTHORS

@@ -5,3 +5,4 @@ Google Inc.
 
 Ho Cheung <uioptt24@gmail.com>
 Ivan Pavlotskiy <ivan.pavlotskiy@lgepartner.com>
+Sida Zhu <zhusida@bytedance.com>

source/scale.cc

@@ -1966,13 +1966,20 @@ 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) {
+      // 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,
                        dst_stride, src, dst, 0, y, dy, /*bpp=*/1, filtering);
@@ -2074,16 +2081,20 @@ 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) {
+      // 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,
                           dst_stride, src, dst, 0, y, dy, /*bpp=*/1, filtering);

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 =
+  const bool interpolate = filtering == kFilterBilinear;
-      (src_height > 1) ? (((int64_t)src_height - 1) << 16) - 1 : 0;
+  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;
+    yf = interpolate ? (int)((y64 >> 8) & 255) : 0;
-    InterpolateRow(dst_argb, src_argb + yi * (ptrdiff_t)src_stride, src_stride,
+    InterpolateRow(dst_argb, src_argb + yi * (ptrdiff_t)src_stride,
-                   dst_width_bytes, yf);
+                   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 =
+  const bool interpolate = filtering == kFilterBilinear;
-      (src_height > 1) ? (((int64_t)src_height - 1) << 16) - 1 : 0;
+  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;
+    yf = interpolate ? (int)((y64 >> 8) & 255) : 0;
-    InterpolateRow(dst_argb, src_argb + yi * (ptrdiff_t)src_stride, src_stride,
+    InterpolateRow(dst_argb, src_argb + yi * (ptrdiff_t)src_stride,
-                   dst_width_words, yf);
+                   interpolate_stride, dst_width_words, yf);
     dst_argb += dst_stride;
     y64 += dy;
   }

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<uint16_t*>(src_alloc);
+  uint16_t* dst = reinterpret_cast<uint16_t*>(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<uint16_t*>(src_alloc);
+  uint16_t* dst = reinterpret_cast<uint16_t*>(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