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
AOMediaCodec · Nov 10, 2023 · 9342e58 · 9342e58
1 parent 4dd98ef
commit 9342e58
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -55,6 +55,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * API calls now return AVIF_RESULT_OUT_OF_MEMORY instead of aborting on memory
   allocation failure.
 * avifenc: Change the default value of the --jobs option from 1 to "all".
+* 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
 

diff --git a/src/avif.c b/src/avif.c
@@ -653,16 +653,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");
@@ -675,14 +674,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);