commit a8d6d1b098f08677e5a0c587e1bfb88d552b656f
author Wan-Teh Chang <wtc@google.com> Fri Jan 27 17:05:11 2023 -0800
committer Wan-Teh Chang <wtc@google.com> Mon Jan 30 09:01:21 2023 -0800
tree ae0bd51c0e4180da7d31178c88cda317ca667475
parent 1d8a92a87adcd6a5581edf4c9fef652ff91d0774

Address review comments

src/write.c

diff --git a/src/write.c b/src/write.c
index 7de26e4..e2494f5 100644
--- a/src/write.c
+++ b/src/write.c
@@ -1137,6 +1137,7 @@
         // Another frame in an image sequence, or layer in a layered image
 
         const avifImage * imageMetadata = encoder->data->imageMetadata;
+        // Image metadata that are copied to the av1C and nclx boxes are not allowed to change.
         // If the first image in the sequence had an alpha plane (even if fully opaque), all
         // subsequent images must have alpha as well.
         if ((imageMetadata->depth != firstCell->depth) || (imageMetadata->yuvFormat != firstCell->yuvFormat) ||

tests/gtest/avifchangesettingtest.cc

diff --git a/tests/gtest/avifchangesettingtest.cc b/tests/gtest/avifchangesettingtest.cc
index cf60537..b8a9439 100644
--- a/tests/gtest/avifchangesettingtest.cc
+++ b/tests/gtest/avifchangesettingtest.cc
@@ -18,10 +18,10 @@
     GTEST_SKIP() << "Codec unavailable, skip test.";
   }
 
-  const uint32_t image_size = 512;
-  testutil::AvifImagePtr image =
-      testutil::CreateImage(image_size, image_size, 8, AVIF_PIXEL_FORMAT_YUV420,
-                            AVIF_PLANES_YUV, AVIF_RANGE_FULL);
+  constexpr uint32_t kImageSize = 512;
+  testutil::AvifImagePtr image = testutil::CreateImage(
+      kImageSize, kImageSize, /*depth=*/8, AVIF_PIXEL_FORMAT_YUV420,
+      AVIF_PLANES_YUV, AVIF_RANGE_FULL);
   ASSERT_NE(image, nullptr);
   testutil::FillImageGradient(image.get());
 
@@ -117,10 +117,10 @@
     GTEST_SKIP() << "Codec unavailable, skip test.";
   }
 
-  const uint32_t image_size = 512;
-  testutil::AvifImagePtr image =
-      testutil::CreateImage(image_size, image_size, 8, AVIF_PIXEL_FORMAT_YUV420,
-                            AVIF_PLANES_YUV, AVIF_RANGE_FULL);
+  constexpr uint32_t kImageSize = 512;
+  testutil::AvifImagePtr image = testutil::CreateImage(
+      kImageSize, kImageSize, /*depth=*/8, AVIF_PIXEL_FORMAT_YUV420,
+      AVIF_PLANES_YUV, AVIF_RANGE_FULL);
   ASSERT_NE(image, nullptr);
   testutil::FillImageGradient(image.get());
 
@@ -154,20 +154,14 @@
             AVIF_RESULT_CANNOT_CHANGE_SETTING);
 }
 
-TEST(ChangeSettingTest, UnchangeableImageMetadata) {
-  const uint32_t image_size = 512;
-  // The first image is full range.
-  testutil::AvifImagePtr image1 =
-      testutil::CreateImage(image_size, image_size, 8, AVIF_PIXEL_FORMAT_YUV420,
-                            AVIF_PLANES_YUV, AVIF_RANGE_FULL);
-  ASSERT_NE(image1, nullptr);
-  testutil::FillImageGradient(image1.get());
-  // The second image is limited range.
-  testutil::AvifImagePtr image2 =
-      testutil::CreateImage(image_size, image_size, 8, AVIF_PIXEL_FORMAT_YUV420,
-                            AVIF_PLANES_YUV, AVIF_RANGE_LIMITED);
-  ASSERT_NE(image2, nullptr);
-  testutil::FillImageGradient(image2.get());
+TEST(ChangeSettingTest, UnchangeableImageColorRange) {
+  constexpr uint32_t kImageSize = 512;
+  testutil::AvifImagePtr image = testutil::CreateImage(
+      kImageSize, kImageSize, /*depth=*/8, AVIF_PIXEL_FORMAT_YUV420,
+      AVIF_PLANES_YUV, AVIF_RANGE_FULL);
+  ASSERT_NE(image, nullptr);
+  const uint32_t yuva[] = {128, 128, 128, 255};
+  testutil::FillImagePlain(image.get(), yuva);
 
   // Encode
   testutil::AvifEncoderPtr encoder(avifEncoderCreate(), avifEncoderDestroy);
@@ -176,25 +170,52 @@
   encoder->speed = AVIF_SPEED_FASTEST;
   encoder->timescale = 1;
   ASSERT_EQ(encoder->repetitionCount, AVIF_REPETITION_COUNT_INFINITE);
-  encoder->minQuantizer = 63;
-  encoder->maxQuantizer = 63;
+  encoder->quality = AVIF_QUALITY_WORST;
 
-  ASSERT_EQ(avifEncoderAddImage(encoder.get(), image1.get(), 1,
+  ASSERT_EQ(avifEncoderAddImage(encoder.get(), image.get(), 1,
                                 AVIF_ADD_IMAGE_FLAG_NONE),
             AVIF_RESULT_OK);
 
-  ASSERT_EQ(avifEncoderAddImage(encoder.get(), image1.get(), 1,
+  ASSERT_EQ(avifEncoderAddImage(encoder.get(), image.get(), 1,
                                 AVIF_ADD_IMAGE_FLAG_NONE),
             AVIF_RESULT_OK);
 
-  ASSERT_EQ(image1->yuvChromaSamplePosition,
-            AVIF_CHROMA_SAMPLE_POSITION_UNKNOWN);
-  image1->yuvChromaSamplePosition = AVIF_CHROMA_SAMPLE_POSITION_VERTICAL;
-  ASSERT_EQ(avifEncoderAddImage(encoder.get(), image1.get(), 1,
+  image->yuvRange = AVIF_RANGE_LIMITED;
+  ASSERT_EQ(avifEncoderAddImage(encoder.get(), image.get(), 1,
                                 AVIF_ADD_IMAGE_FLAG_NONE),
             AVIF_RESULT_INCOMPATIBLE_IMAGE);
+}
 
-  ASSERT_EQ(avifEncoderAddImage(encoder.get(), image2.get(), 1,
+TEST(ChangeSettingTest, UnchangeableImageChromaSamplePosition) {
+  constexpr uint32_t kImageSize = 512;
+  testutil::AvifImagePtr image = testutil::CreateImage(
+      kImageSize, kImageSize, /*depth=*/8, AVIF_PIXEL_FORMAT_YUV420,
+      AVIF_PLANES_YUV, AVIF_RANGE_FULL);
+  ASSERT_NE(image, nullptr);
+  const uint32_t yuva[] = {128, 128, 128, 255};
+  testutil::FillImagePlain(image.get(), yuva);
+
+  // Encode
+  testutil::AvifEncoderPtr encoder(avifEncoderCreate(), avifEncoderDestroy);
+  ASSERT_NE(encoder, nullptr);
+  encoder->codecChoice = AVIF_CODEC_CHOICE_AOM;
+  encoder->speed = AVIF_SPEED_FASTEST;
+  encoder->timescale = 1;
+  ASSERT_EQ(encoder->repetitionCount, AVIF_REPETITION_COUNT_INFINITE);
+  encoder->quality = AVIF_QUALITY_WORST;
+
+  ASSERT_EQ(avifEncoderAddImage(encoder.get(), image.get(), 1,
+                                AVIF_ADD_IMAGE_FLAG_NONE),
+            AVIF_RESULT_OK);
+
+  ASSERT_EQ(avifEncoderAddImage(encoder.get(), image.get(), 1,
+                                AVIF_ADD_IMAGE_FLAG_NONE),
+            AVIF_RESULT_OK);
+
+  ASSERT_EQ(image->yuvChromaSamplePosition,
+            AVIF_CHROMA_SAMPLE_POSITION_UNKNOWN);
+  image->yuvChromaSamplePosition = AVIF_CHROMA_SAMPLE_POSITION_VERTICAL;
+  ASSERT_EQ(avifEncoderAddImage(encoder.get(), image.get(), 1,
                                 AVIF_ADD_IMAGE_FLAG_NONE),
             AVIF_RESULT_INCOMPATIBLE_IMAGE);
 }