Inconsistent handling of identity matrix + monochrome b/w library & avifenc

[jzern]

Tested at v1.1.1-219-gc3a92347.

$ avifenc -y 400 --lossless --cicp 2/2/0 input -o output
WARNING: matrixCoefficients may not be set to identity (0) when encoding 4:0:0. Resetting MC to defaults (6).
...

This warning in avifenc.c comes from 035b550 to fix #1481.

reformat.c has:

    if ((image->matrixCoefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY) && (image->yuvFormat != AVIF_PIXEL_FORMAT_YUV444) &&
        (image->yuvFormat != AVIF_PIXEL_FORMAT_YUV400)) {
        return AVIF_FALSE;
    }

which will allow the identity matrix with monochrome. This check comes from 424b0f9 to address #1453.

---

Technically this is considered an invalid bitstream. libaom was recently updated to avoid creating these files (e107085626 ensure subsampling is 0 w/AOM_CICP_MC_IDENTITY).

From the spec (Version 1.0.0 with Errata 1):

6.4.2. Color config semantics
...
If matrix_coefficients is equal to MC_IDENTITY, it is a requirement of bitstream conformance that subsampling_x is equal
to 0 and subsampling_y is equal to 0.

There are no tests that cover this case, but setting strict_std_compliance = 1 in codec_dav1d.c would cause the decode to fail:

diff --git a/src/codec_dav1d.c b/src/codec_dav1d.c
index 5aff5a12..957dc48f 100644
--- a/src/codec_dav1d.c
+++ b/src/codec_dav1d.c
@@ -72,6 +72,7 @@ static avifBool dav1dCodecGetNextImage(struct avifCodec * codec,
         dav1dSettings.frame_size_limit = (sizeof(size_t) < 8) ? AVIF_MIN(codec->imageSizeLimit, 8192 * 8192) : codec->imageSizeLimit;
         dav1dSettings.operating_point = codec->operatingPoint;
         dav1dSettings.all_layers = codec->allLayers;
+        dav1dSettings.strict_std_compliance = 1;
 
         if (dav1d_open(&codec->internal->dav1dContext, &dav1dSettings) != 0) {
             return AVIF_FALSE;

---

Both the encode and decode failure can be seen with this very quick addition to tests/gtest/aviflosslesstest.cc. Removing the libaom update will see the test pass unless dav1dSettings.strict_std_compliance = 1 is enabled. After the libaom update the encode will fail.

diff --git a/apps/shared/avifpng.c b/apps/shared/avifpng.c
index b1685c8d..8b8c86ed 100644
--- a/apps/shared/avifpng.c
+++ b/apps/shared/avifpng.c
@@ -611,7 +611,7 @@ avifBool avifPNGWrite(const char * outputFilename, const avifImage * avif, uint3
 
     png_set_IHDR(png, info, avif->width, avif->height, rgbDepth, colorType, PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
 
-    const avifBool hasIcc = avif->icc.data && (avif->icc.size > 0);
+    const avifBool hasIcc = AVIF_FALSE;  // avif->icc.data && (avif->icc.size > 0);
     if (hasIcc) {
         // If there is an ICC profile, the CICP values are irrelevant and only the ICC profile
         // is written. If we could extract the primaries/transfer curve from the ICC profile,
diff --git a/cmake/Modules/LocalAom.cmake b/cmake/Modules/LocalAom.cmake
index 9433c352..d012d2c2 100644
--- a/cmake/Modules/LocalAom.cmake
+++ b/cmake/Modules/LocalAom.cmake
@@ -1,4 +1,4 @@
-set(AVIF_AOM_GIT_TAG v3.12.0)
+set(AVIF_AOM_GIT_TAG e107085626c2336dce00198cd52eb604bc33e0a9)
 set(AVIF_AVM_GIT_TAG research-v9.0.0)
 
 if(AVIF_CODEC_AVM)
diff --git a/ext/aom.cmd b/ext/aom.cmd
index 3f3804e2..30f72f52 100755
--- a/ext/aom.cmd
+++ b/ext/aom.cmd
@@ -8,7 +8,7 @@
 : # If you're running this on Windows, be sure you've already run this (from your VC2019 install dir):
 : #     "C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Auxiliary\Build\vcvars64.bat"
 
-git clone -b v3.12.0 --depth 1 https://aomedia.googlesource.com/aom
+git clone -b e107085626c2336dce00198cd52eb604bc33e0a9 --depth 1 https://aomedia.googlesource.com/aom
 
 cd aom
 mkdir build.libavif
diff --git a/src/codec_dav1d.c b/src/codec_dav1d.c
index 5aff5a12..f8b8fad2 100644
--- a/src/codec_dav1d.c
+++ b/src/codec_dav1d.c
@@ -72,6 +72,7 @@ static avifBool dav1dCodecGetNextImage(struct avifCodec * codec,
         dav1dSettings.frame_size_limit = (sizeof(size_t) < 8) ? AVIF_MIN(codec->imageSizeLimit, 8192 * 8192) : codec->imageSizeLimit;
         dav1dSettings.operating_point = codec->operatingPoint;
         dav1dSettings.all_layers = codec->allLayers;
+        // dav1dSettings.strict_std_compliance = 1;
 
         if (dav1d_open(&codec->internal->dav1dContext, &dav1dSettings) != 0) {
             return AVIF_FALSE;
diff --git a/tests/gtest/aviflosslesstest.cc b/tests/gtest/aviflosslesstest.cc
index b5cd313b..81b417ce 100644
--- a/tests/gtest/aviflosslesstest.cc
+++ b/tests/gtest/aviflosslesstest.cc
@@ -92,6 +92,90 @@ TEST(BasicTest, EncodeDecodeMatrixCoefficients) {
   }
 }
 
+// Tests encode/decode round trips under different matrix coefficients.
+TEST(BasicTest, EncodeDecodeMatrixCoefficientsMonochrome) {
+  for (const auto& file_name :
+       {"paris_icc_exif_xmp.png", "paris_exif_xmp_icc.jpg"}) {
+    // Read a ground truth image with default matrix coefficients.
+    const std::string file_path = std::string(data_path) + file_name;
+    const ImagePtr ground_truth_image =
+        testutil::ReadImage(data_path, file_name, AVIF_PIXEL_FORMAT_YUV400);
+
+    for (auto matrix_coefficient : {
+               AVIF_MATRIX_COEFFICIENTS_IDENTITY
+         }) {
+      // Read a ground truth image but ask for certain matrix coefficients.
+      ImagePtr image(avifImageCreateEmpty());
+      ASSERT_NE(image, nullptr);
+      image->matrixCoefficients = (avifMatrixCoefficients)matrix_coefficient;
+      const avifAppFileFormat file_format = avifReadImage(
+          file_path.c_str(),
+          /*requestedFormat=*/AVIF_PIXEL_FORMAT_YUV400,
+          /*requestedDepth=*/0,
+          /*chromaDownsampling=*/AVIF_CHROMA_DOWNSAMPLING_AUTOMATIC,
+          /*ignoreColorProfile=*/false, /*ignoreExif=*/false,
+          /*ignoreXMP=*/false, /*allowChangingCicp=*/true,
+          /*ignoreGainMap=*/true, AVIF_DEFAULT_IMAGE_SIZE_LIMIT, image.get(),
+          /*outDepth=*/nullptr, /*sourceTiming=*/nullptr,
+          /*frameIter=*/nullptr);
+#if defined(AVIF_ENABLE_EXPERIMENTAL_YCGCO_R)
+      if (matrix_coefficient == AVIF_MATRIX_COEFFICIENTS_YCGCO_RO) {
+        // AVIF_MATRIX_COEFFICIENTS_YCGCO_RO does not work because the input
+        // depth is not odd.
+        ASSERT_EQ(file_format, AVIF_APP_FILE_FORMAT_UNKNOWN);
+        continue;
+      }
+#endif
+      ASSERT_NE(file_format, AVIF_APP_FILE_FORMAT_UNKNOWN);
+
+      // Encode.
+      EncoderPtr encoder(avifEncoderCreate());
+      ASSERT_NE(encoder, nullptr);
+      encoder->speed = AVIF_SPEED_FASTEST;
+      encoder->quality = AVIF_QUALITY_LOSSLESS;
+      encoder->qualityAlpha = AVIF_QUALITY_LOSSLESS;
+      testutil::AvifRwData encoded;
+      avifResult result =
+          avifEncoderWrite(encoder.get(), image.get(), &encoded);
+      ASSERT_EQ(result, AVIF_RESULT_OK) << avifResultToString(result);
+
+      // Decode to RAM.
+      ImagePtr decoded(avifImageCreateEmpty());
+      ASSERT_NE(decoded, nullptr);
+      DecoderPtr decoder(avifDecoderCreate());
+      ASSERT_NE(decoder, nullptr);
+      result = avifDecoderReadMemory(decoder.get(), decoded.get(), encoded.data,
+                                     encoded.size);
+      ASSERT_EQ(result, AVIF_RESULT_OK) << avifResultToString(result);
+
+      // Convert to a temporary PNG and read back, to have default matrix
+      // coefficients.
+      const std::string temp_dir = testing::TempDir();
+      const std::string temp_file = "decoded_default.png";
+      const std::string tmp_path = temp_dir + temp_file;
+      ASSERT_TRUE(testutil::WriteImage(decoded.get(), tmp_path.c_str()));
+      const ImagePtr decoded_default =
+          testutil::ReadImage(temp_dir.c_str(), temp_file.c_str(), AVIF_PIXEL_FORMAT_YUV400);
+
+      // Verify that the ground truth and decoded images are the same.
+      const bool are_images_equal =
+          testutil::AreImagesEqual(*ground_truth_image, *decoded_default);
+
+      if (matrix_coefficient == AVIF_MATRIX_COEFFICIENTS_IDENTITY
+#if defined(AVIF_ENABLE_EXPERIMENTAL_YCGCO_R)
+          || matrix_coefficient == AVIF_MATRIX_COEFFICIENTS_YCGCO_RE
+#endif
+      ) {
+        ASSERT_TRUE(are_images_equal);
+      } else {
+        // AVIF_MATRIX_COEFFICIENTS_YCGCO is not lossless because it does not
+        // expand the input bit range for YCgCo values.
+        ASSERT_FALSE(are_images_equal);
+      }
+    }
+  }
+}
+
 }  // namespace
 }  // namespace avif
 
diff --git a/tests/gtest/aviftest_helpers.cc b/tests/gtest/aviftest_helpers.cc
index 851e7c63..40739615 100644
--- a/tests/gtest/aviftest_helpers.cc
+++ b/tests/gtest/aviftest_helpers.cc
@@ -278,6 +278,7 @@ bool AreImagesEqual(const avifImage& image1, const avifImage& image2,
       row2 += row_bytes2;
     }
   }
+  return true;
   return AreByteSequencesEqual(image1.icc, image2.icc) &&
          AreByteSequencesEqual(image1.exif, image2.exif) &&
          AreByteSequencesEqual(image1.xmp, image2.xmp);
@@ -480,6 +481,7 @@ ImagePtr ReadImage(const char* folder_path, const char* file_name,
                    avifBool ignore_xmp, avifBool allow_changing_cicp,
                    avifBool ignore_gain_map) {
   ImagePtr image(avifImageCreateEmpty());
+  image->matrixCoefficients = AVIF_MATRIX_COEFFICIENTS_IDENTITY;
   if (!image ||
       avifReadImage((std::string(folder_path) + file_name).c_str(),
                     requested_format, requested_depth, chroma_downsampling,

---

There are a couple issues to resolve:

1. Adding test coverage.
2. Removing the ability to use the identity matrix with monochrome will prevent users from using avifImageRGBToYUV(), c.f. avifPrepareReformatState(). (Maybe this is OK as a user could fill out the Y plane directly.)

cc: @vrabaud @wantehchang

[vrabaud]

Having the check in avifPrepareReformatState makes avifRGBToYUV() fail for identity+400. This function is independent from AV1 so it should still work. I moved the check to avifValidateGrid which seems to be called before all encodings.