From 04e3a0720da567361d2bb109a021d434cdaefbad Mon Sep 17 00:00:00 2001 From: Vincent Rabaud Date: Wed, 5 Mar 2025 11:03:02 +0100 Subject: [PATCH 1/2] Do not allow for monochrome + identity matrix This fixes #2639 --- src/write.c | 7 ++ tests/gtest/aviflosslesstest.cc | 195 +++++++++++++++++++------------- 2 files changed, 126 insertions(+), 76 deletions(-) diff --git a/src/write.c b/src/write.c index 32cf133121..812ada371c 100644 --- a/src/write.c +++ b/src/write.c @@ -1657,6 +1657,13 @@ static avifResult avifValidateGrid(uint32_t gridCols, return AVIF_RESULT_INVALID_IMAGE_GRID; } + // AV1 (Version 1.0.0 with Errata 1), Section 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. + if (cellImage->matrixCoefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY && cellImage->yuvFormat != AVIF_PIXEL_FORMAT_YUV444) { + avifDiagnosticsPrintf(diag, "subsampling must be 0 with identity matrix coefficients"); + return AVIF_RESULT_INVALID_ARGUMENT; + } + if (!cellImage->yuvPlanes[AVIF_CHAN_Y]) { return AVIF_RESULT_NO_CONTENT; } diff --git a/tests/gtest/aviflosslesstest.cc b/tests/gtest/aviflosslesstest.cc index b5cd313bfb..9f58766440 100644 --- a/tests/gtest/aviflosslesstest.cc +++ b/tests/gtest/aviflosslesstest.cc @@ -1,6 +1,8 @@ // Copyright 2023 Google LLC // SPDX-License-Identifier: BSD-2-Clause +#include + #include "avif/avif.h" #include "aviftest_helpers.h" #include "avifutil.h" @@ -12,86 +14,127 @@ namespace { // Used to pass the data folder path to the GoogleTest suites. const char* data_path = nullptr; +// Read image losslessly, using identiy MC. +ImagePtr ReadImageLossless(const std::string& path, + avifPixelFormat requested_format, + avifBool ignore_icc) { + ImagePtr image(avifImageCreateEmpty()); + image->matrixCoefficients = AVIF_MATRIX_COEFFICIENTS_IDENTITY; + if (!image || + avifReadImage(path.c_str(), requested_format, /*requested_depth=*/0, + /*chroma_downsampling=*/AVIF_CHROMA_DOWNSAMPLING_AUTOMATIC, + ignore_icc, /*ignore_exif=*/false, /*ignore_xmp=*/false, + /*allow_changing_cicp=*/false, /*ignore_gain_map=*/false, + AVIF_DEFAULT_IMAGE_SIZE_LIMIT, image.get(), + /*outDepth=*/nullptr, /*sourceTiming=*/nullptr, + /*frameIter=*/nullptr) == AVIF_APP_FILE_FORMAT_UNKNOWN) { + return nullptr; + } + return image; +} + +class LosslessTest + : public testing::TestWithParam> {}; + // Tests encode/decode round trips under different matrix coefficients. -TEST(BasicTest, EncodeDecodeMatrixCoefficients) { - 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); - - for (auto matrix_coefficient : - {AVIF_MATRIX_COEFFICIENTS_IDENTITY, AVIF_MATRIX_COEFFICIENTS_YCGCO, - AVIF_MATRIX_COEFFICIENTS_YCGCO_RE, - AVIF_MATRIX_COEFFICIENTS_YCGCO_RO}) { - // 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_NONE, - /*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 (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; - } - 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()); - - // 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 || - matrix_coefficient == AVIF_MATRIX_COEFFICIENTS_YCGCO_RE) { - 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); - } - } +TEST_P(LosslessTest, EncodeDecodeMatrixCoefficients) { + const std::string& file_name = std::get<0>(GetParam()); + const avifMatrixCoefficients matrix_coefficients = + static_cast(std::get<1>(GetParam())); + const avifPixelFormat pixel_format = + static_cast(std::get<2>(GetParam())); + // Ignore ICC when going from RGB to gray. + const avifBool ignore_icc = + pixel_format == AVIF_PIXEL_FORMAT_YUV400 ? AVIF_TRUE : AVIF_FALSE; + + // Read a ground truth image but ask for certain matrix coefficients. + const std::string file_path = std::string(data_path) + file_name; + ImagePtr image(avifImageCreateEmpty()); + ASSERT_NE(image, nullptr); + image->matrixCoefficients = matrix_coefficients; + const avifAppFileFormat file_format = avifReadImage( + file_path.c_str(), + /*requestedFormat=*/pixel_format, + /*requestedDepth=*/0, + /*chromaDownsampling=*/AVIF_CHROMA_DOWNSAMPLING_AUTOMATIC, + /*ignoreColorProfile=*/ignore_icc, /*ignoreExif=*/false, + /*ignoreXMP=*/false, /*allowChangingCicp=*/true, + /*ignoreGainMap=*/true, AVIF_DEFAULT_IMAGE_SIZE_LIMIT, image.get(), + /*outDepth=*/nullptr, /*sourceTiming=*/nullptr, + /*frameIter=*/nullptr); + if (matrix_coefficients == 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); + return; + } + 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); + + if (matrix_coefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY && + pixel_format != AVIF_PIXEL_FORMAT_NONE && + pixel_format != AVIF_PIXEL_FORMAT_YUV444) { + // The AV1 spec does not allow identity with subsampling. + ASSERT_EQ(result, AVIF_RESULT_INVALID_ARGUMENT); + return; + } + 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); + + // What we read should be what we encoded + ASSERT_TRUE(testutil::AreImagesEqual(*image, *decoded)); + + // Convert to a temporary PNG and read back, to have default matrix + // coefficients. + const std::string tmp_path = testing::TempDir() + "decoded_default.png"; + ASSERT_TRUE(testutil::WriteImage(decoded.get(), tmp_path.c_str())); + const ImagePtr decoded_lossless = + ReadImageLossless(tmp_path, pixel_format, ignore_icc); + + // Verify that the ground truth and decoded images are the same. + const ImagePtr ground_truth_lossless = + ReadImageLossless(file_path, pixel_format, ignore_icc); + const bool are_images_equal = + testutil::AreImagesEqual(*ground_truth_lossless, *decoded_lossless); + + if (matrix_coefficients == AVIF_MATRIX_COEFFICIENTS_YCGCO) { + // AVIF_MATRIX_COEFFICIENTS_YCGCO is not a lossless transform. + ASSERT_FALSE(are_images_equal); + } else if (pixel_format == AVIF_PIXEL_FORMAT_YUV400) { + // For gray images, information is lost. + ASSERT_FALSE(are_images_equal); + } else { + ASSERT_TRUE(are_images_equal); } } +INSTANTIATE_TEST_SUITE_P( + LosslessTestInstantiation, LosslessTest, + testing::Combine( + testing::Values("paris_icc_exif_xmp.png", "paris_exif_xmp_icc.jpg"), + testing::Values(static_cast(AVIF_MATRIX_COEFFICIENTS_IDENTITY), + static_cast(AVIF_MATRIX_COEFFICIENTS_YCGCO), + static_cast(AVIF_MATRIX_COEFFICIENTS_YCGCO_RE), + static_cast(AVIF_MATRIX_COEFFICIENTS_YCGCO_RO)), + testing::Values(static_cast(AVIF_PIXEL_FORMAT_NONE), + static_cast(AVIF_PIXEL_FORMAT_YUV400)))); + } // namespace } // namespace avif From 82e54e56529719cf357b8f303bfe377bf3314e14 Mon Sep 17 00:00:00 2001 From: Vincent Rabaud Date: Thu, 6 Mar 2025 17:29:07 +0100 Subject: [PATCH 2/2] Fixes according to comments. --- CHANGELOG.md | 2 ++ src/reformat.c | 2 +- src/write.c | 2 +- tests/gtest/aviflosslesstest.cc | 27 +++++++++++++++++++++++---- 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 806aeb5408..f8c6b01463 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ The changes are relative to the previous release, unless the baseline is specifi * Fix local libargparse dependency patch step on macOS 10.15 and earlier. * Patch local libyuv dependency for compatibility with gcc 10. * Use stricter C99 syntax to avoid related compilation issues. +* Forbid encoding with AVIF_MATRIX_COEFFICIENTS_IDENTITY and + AVIF_PIXEL_FORMAT_YUV400 to be AV1 spec compatible ## [1.2.0] - 2025-02-25 diff --git a/src/reformat.c b/src/reformat.c index 919829be74..ecba09e5b1 100644 --- a/src/reformat.c +++ b/src/reformat.c @@ -115,11 +115,11 @@ avifBool avifGetYUVColorSpaceInfo(const avifImage * image, avifYUVColorSpaceInfo return AVIF_FALSE; } + // Removing 400 here would break backward behavior but would respect the spec. if ((image->matrixCoefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY) && (image->yuvFormat != AVIF_PIXEL_FORMAT_YUV444) && (image->yuvFormat != AVIF_PIXEL_FORMAT_YUV400)) { return AVIF_FALSE; } - avifGetPixelFormatInfo(image->yuvFormat, &info->formatInfo); avifCalcYUVCoefficients(image, &info->kr, &info->kg, &info->kb); diff --git a/src/write.c b/src/write.c index 812ada371c..502140ff5b 100644 --- a/src/write.c +++ b/src/write.c @@ -1660,7 +1660,7 @@ static avifResult avifValidateGrid(uint32_t gridCols, // AV1 (Version 1.0.0 with Errata 1), Section 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. if (cellImage->matrixCoefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY && cellImage->yuvFormat != AVIF_PIXEL_FORMAT_YUV444) { - avifDiagnosticsPrintf(diag, "subsampling must be 0 with identity matrix coefficients"); + avifDiagnosticsPrintf(diag, "subsampling must be 0 (4:4:4) with identity matrix coefficients"); return AVIF_RESULT_INVALID_ARGUMENT; } diff --git a/tests/gtest/aviflosslesstest.cc b/tests/gtest/aviflosslesstest.cc index 9f58766440..75296e6318 100644 --- a/tests/gtest/aviflosslesstest.cc +++ b/tests/gtest/aviflosslesstest.cc @@ -68,6 +68,14 @@ TEST_P(LosslessTest, EncodeDecodeMatrixCoefficients) { ASSERT_EQ(file_format, AVIF_APP_FILE_FORMAT_UNKNOWN); return; } + if (matrix_coefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY && + image->yuvFormat == AVIF_PIXEL_FORMAT_YUV420) { + // 420 cannot be converted from RGB to YUV with + // AVIF_MATRIX_COEFFICIENTS_IDENTITY due to a decision taken in + // avifGetYUVColorSpaceInfo. + ASSERT_EQ(file_format, AVIF_APP_FILE_FORMAT_UNKNOWN); + return; + } ASSERT_NE(file_format, AVIF_APP_FILE_FORMAT_UNKNOWN); // Encode. @@ -75,13 +83,11 @@ TEST_P(LosslessTest, EncodeDecodeMatrixCoefficients) { 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); if (matrix_coefficients == AVIF_MATRIX_COEFFICIENTS_IDENTITY && - pixel_format != AVIF_PIXEL_FORMAT_NONE && - pixel_format != AVIF_PIXEL_FORMAT_YUV444) { + image->yuvFormat != AVIF_PIXEL_FORMAT_YUV444) { // The AV1 spec does not allow identity with subsampling. ASSERT_EQ(result, AVIF_RESULT_INVALID_ARGUMENT); return; @@ -102,14 +108,24 @@ TEST_P(LosslessTest, EncodeDecodeMatrixCoefficients) { // Convert to a temporary PNG and read back, to have default matrix // coefficients. - const std::string tmp_path = testing::TempDir() + "decoded_default.png"; + const std::string tmp_path = + testing::TempDir() + "decoded_EncodeDecodeMatrixCoefficients.png"; ASSERT_TRUE(testutil::WriteImage(decoded.get(), tmp_path.c_str())); const ImagePtr decoded_lossless = ReadImageLossless(tmp_path, pixel_format, ignore_icc); + if (image->yuvFormat == AVIF_PIXEL_FORMAT_YUV420) { + // 420 cannot be converted from RGB to YUV with + // AVIF_MATRIX_COEFFICIENTS_IDENTITY due to a decision taken in + // avifGetYUVColorSpaceInfo. + ASSERT_EQ(decoded_lossless, nullptr); + return; + } + ASSERT_NE(decoded_lossless, nullptr); // Verify that the ground truth and decoded images are the same. const ImagePtr ground_truth_lossless = ReadImageLossless(file_path, pixel_format, ignore_icc); + ASSERT_NE(ground_truth_lossless, nullptr); const bool are_images_equal = testutil::AreImagesEqual(*ground_truth_lossless, *decoded_lossless); @@ -117,6 +133,7 @@ TEST_P(LosslessTest, EncodeDecodeMatrixCoefficients) { // AVIF_MATRIX_COEFFICIENTS_YCGCO is not a lossless transform. ASSERT_FALSE(are_images_equal); } else if (pixel_format == AVIF_PIXEL_FORMAT_YUV400) { + ASSERT_EQ(matrix_coefficients, AVIF_MATRIX_COEFFICIENTS_YCGCO_RE); // For gray images, information is lost. ASSERT_FALSE(are_images_equal); } else { @@ -133,6 +150,8 @@ INSTANTIATE_TEST_SUITE_P( static_cast(AVIF_MATRIX_COEFFICIENTS_YCGCO_RE), static_cast(AVIF_MATRIX_COEFFICIENTS_YCGCO_RO)), testing::Values(static_cast(AVIF_PIXEL_FORMAT_NONE), + static_cast(AVIF_PIXEL_FORMAT_YUV444), + static_cast(AVIF_PIXEL_FORMAT_YUV420), static_cast(AVIF_PIXEL_FORMAT_YUV400)))); } // namespace