Don't require crop width,height be even for 4:2:0

These requirements were removed in ISO/IEC 23000-22:2019/Amd. 2:2021 from Section 7.3.6.7. This is one reason most of the clap properties seen by Chrome were considered invalid. Bug: b/308458941 (cherry picked from commit 9342e58)
wantehchang · Nov 15, 2023 · a03f20c · a03f20c
1 parent d1c26fa
commit a03f20c
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+* Update avifCropRectConvertCleanApertureBox() to the revised requirements in
+  ISO/IEC 23000-22:2019/Amd. 2:2021 Section 7.3.6.7.
+
 ## [1.0.1] - 2023-08-29
 
 ### Changed

diff --git a/src/avif.c b/src/avif.c
@@ -624,16 +624,15 @@ static avifBool overflowsInt32(int64_t x)
 static avifBool avifCropRectIsValid(const avifCropRect * cropRect, uint32_t imageW, uint32_t imageH, avifPixelFormat yuvFormat, avifDiagnostics * diag)
 
 {
-    // ISO/IEC 23000-22:2019/DAM 2:2021, Section 7.3.6.7:
+    // ISO/IEC 23000-22:2019/Amd. 2:2021, Section 7.3.6.7:
     //   The clean aperture property is restricted according to the chroma
     //   sampling format of the input image (4:4:4, 4:2:2:, 4:2:0, or 4:0:0) as
     //   follows:
-    //   - when the image is 4:0:0 (monochrome) or 4:4:4, the horizontal and
-    //     vertical cropped offsets and widths shall be integers;
-    //   - when the image is 4:2:2 the horizontal cropped offset and width
-    //     shall be even numbers and the vertical values shall be integers;
-    //   - when the image is 4:2:0 both the horizontal and vertical cropped
-    //     offsets and widths shall be even numbers.
+    //   ...
+    //   - If chroma is subsampled horizontally (i.e., 4:2:2 and 4:2:0), the
+    //     leftmost pixel of the clean aperture shall be even numbers;
+    //   - If chroma is subsampled vertically (i.e., 4:2:0), the topmost line
+    //     of the clean aperture shall be even numbers.
 
     if ((cropRect->width == 0) || (cropRect->height == 0)) {
         avifDiagnosticsPrintf(diag, "[Strict] crop rect width and height must be nonzero");
@@ -646,14 +645,14 @@ static avifBool avifCropRectIsValid(const avifCropRect * cropRect, uint32_t imag
     }
 
     if ((yuvFormat == AVIF_PIXEL_FORMAT_YUV420) || (yuvFormat == AVIF_PIXEL_FORMAT_YUV422)) {
-        if (((cropRect->x % 2) != 0) || ((cropRect->width % 2) != 0)) {
-            avifDiagnosticsPrintf(diag, "[Strict] crop rect X offset and width must both be even due to this image's YUV subsampling");
+        if ((cropRect->x % 2) != 0) {
+            avifDiagnosticsPrintf(diag, "[Strict] crop rect X offset must be even due to this image's YUV subsampling");
             return AVIF_FALSE;
         }
     }
     if (yuvFormat == AVIF_PIXEL_FORMAT_YUV420) {
-        if (((cropRect->y % 2) != 0) || ((cropRect->height % 2) != 0)) {
-            avifDiagnosticsPrintf(diag, "[Strict] crop rect Y offset and height must both be even due to this image's YUV subsampling");
+        if ((cropRect->y % 2) != 0) {
+            avifDiagnosticsPrintf(diag, "[Strict] crop rect Y offset must be even due to this image's YUV subsampling");
             return AVIF_FALSE;
         }
     }

diff --git a/tests/gtest/avifclaptest.cc b/tests/gtest/avifclaptest.cc
@@ -65,6 +65,15 @@ constexpr InvalidClapPropertyParam kInvalidClapPropertyTestParams[] = {
      722,
      AVIF_PIXEL_FORMAT_YUV420,
      {330, 1, 385, 1, static_cast<uint32_t>(-308), 1, 103, 1}},
+    // pcX = -1/2 + (99 - 1)/2 = 48.5
+    // pcY = -1/2 + (99 - 1)/2 = 48.5
+    // leftmost = 48.5 - (99 - 1)/2 = -0.5 (not an integer)
+    // topmost = 48.5 - (99 - 1)/2 = -0.5 (not an integer)
+    {99,
+     99,
+     AVIF_PIXEL_FORMAT_YUV420,
+     {99, 1, 99, 1, static_cast<uint32_t>(-1), 2, static_cast<uint32_t>(-1),
+      2}},
 };
 
 using InvalidClapPropertyTest =
@@ -112,6 +121,15 @@ constexpr ValidClapPropertyParam kValidClapPropertyTestParams[] = {
      {60, 1, 80, 1, static_cast<uint32_t>(-30), 1, static_cast<uint32_t>(-40),
       1},
      {0, 0, 60, 80}},
+    // pcX = -1/2 + (100 - 1)/2 = 49
+    // pcY = -1/2 + (100 - 1)/2 = 49
+    // leftmost = 49 - (99 - 1)/2 = 0
+    // topmost = 49 - (99 - 1)/2 = 0
+    {100,
+     100,
+     AVIF_PIXEL_FORMAT_YUV420,
+     {99, 1, 99, 1, static_cast<uint32_t>(-1), 2, static_cast<uint32_t>(-1), 2},
+     {0, 0, 99, 99}},
 };
 
 using ValidClapPropertyTest = ::testing::TestWithParam<ValidClapPropertyParam>;
@@ -126,7 +144,8 @@ TEST_P(ValidClapPropertyTest, ValidateClapProperty) {
   avifDiagnostics diag;
   EXPECT_TRUE(avifCropRectConvertCleanApertureBox(&crop_rect, &param.clap,
                                                   param.width, param.height,
-                                                  param.yuv_format, &diag));
+                                                  param.yuv_format, &diag))
+      << diag.error;
   EXPECT_EQ(crop_rect.x, param.expected_crop_rect.x);
   EXPECT_EQ(crop_rect.y, param.expected_crop_rect.y);
   EXPECT_EQ(crop_rect.width, param.expected_crop_rect.width);