From 73a91d8c913fa3f2cdf4861b8113cd22221fcc34 Mon Sep 17 00:00:00 2001 From: Yannis Guyon Date: Fri, 22 Nov 2024 16:46:33 +0100 Subject: [PATCH] Move oddness check to CropRectRequiresUpsampling() Remove yuvFormat arg from avifCropRectFromCleanApertureBox(). Make avifCropRectConvertCleanApertureBox() behave exactly as before. Fix comments. Update avifclaptest. Print warnings in avifdec. --- apps/avifdec.c | 18 ++++++++++++ apps/avifenc.c | 4 +-- apps/shared/avifutil.c | 5 ++-- include/avif/avif.h | 7 ++--- src/avif.c | 56 ++++++++++++++++++++----------------- src/read.c | 5 +--- tests/gtest/avifclaptest.cc | 18 ++++++------ 7 files changed, 64 insertions(+), 49 deletions(-) diff --git a/apps/avifdec.c b/apps/avifdec.c index 1a6006d206..2aba20eae9 100644 --- a/apps/avifdec.c +++ b/apps/avifdec.c @@ -347,6 +347,24 @@ int main(int argc, char * argv[]) printf("Image details:\n"); avifImageDump(decoder->image, 0, 0, decoder->progressiveState); + if (decoder->image->transformFlags & AVIF_TRANSFORM_CLAP) { + avifCropRect cropRect; + if (avifCropRectFromCleanApertureBox(&cropRect, + &decoder->image->clap, + decoder->image->width, + decoder->image->height, + &decoder->diag)) { + if (cropRect.x != 0 || cropRect.y != 0 || cropRect.width != decoder->image->width || + cropRect.height != decoder->image->height) { + // TODO: Implement, see https://github.com/AOMediaCodec/libavif/issues/2427 + fprintf(stderr, "Warning: Clean Aperture values were ignored, the output image was NOT cropped\n"); + } + } else { + // Should happen only if AVIF_STRICT_CLAP_VALID is disabled. + fprintf(stderr, "Warning: Invalid Clean Aperture values\n"); + } + } + if (ignoreICC && (decoder->image->icc.size > 0)) { printf("[--ignore-icc] Discarding ICC profile.\n"); // This cannot fail. diff --git a/apps/avifenc.c b/apps/avifenc.c index c6b5c442d5..ccb0af993e 100644 --- a/apps/avifenc.c +++ b/apps/avifenc.c @@ -2290,10 +2290,9 @@ int main(int argc, char * argv[]) // Validate clap avifCropRect cropRect; - avifBool upsampleBeforeCropping; avifDiagnostics diag; avifDiagnosticsClearError(&diag); - if (!avifCropRectFromCleanApertureBox(&cropRect, &upsampleBeforeCropping, &image->clap, image->width, image->height, image->yuvFormat, &diag)) { + if (!avifCropRectFromCleanApertureBox(&cropRect, &image->clap, image->width, image->height, &diag)) { fprintf(stderr, "ERROR: Invalid clap: width:[%d / %d], height:[%d / %d], horizOff:[%d / %d], vertOff:[%d / %d] - %s\n", (int32_t)image->clap.widthN, @@ -2307,6 +2306,7 @@ int main(int argc, char * argv[]) diag.error); goto cleanup; } + // avifCropRectRequiresUpsampling() does not need to be called for clap validation. } if (irotAngle != 0xff) { image->transformFlags |= AVIF_TRANSFORM_IROT; diff --git a/apps/shared/avifutil.c b/apps/shared/avifutil.c index 521e07f9b3..78b740675e 100644 --- a/apps/shared/avifutil.c +++ b/apps/shared/avifutil.c @@ -102,15 +102,14 @@ static void avifImageDumpInternal(const avifImage * avif, uint32_t gridCols, uin avifBool upsampleBeforeCropping; avifDiagnostics diag; avifDiagnosticsClearError(&diag); - avifBool validClap = - avifCropRectFromCleanApertureBox(&cropRect, &upsampleBeforeCropping, &avif->clap, avif->width, avif->height, avif->yuvFormat, &diag); + avifBool validClap = avifCropRectFromCleanApertureBox(&cropRect, &avif->clap, avif->width, avif->height, &diag); if (validClap) { printf(" * Valid, derived crop rect: X: %d, Y: %d, W: %d, H: %d%s\n", cropRect.x, cropRect.y, cropRect.width, cropRect.height, - upsampleBeforeCropping ? " (upsample before cropping)" : ""); + avifCropRectRequiresUpsampling(&cropRect, avif->yuvFormat) ? " (upsample before cropping)" : ""); } else { printf(" * Invalid: %s\n", diag.error); } diff --git a/include/avif/avif.h b/include/avif/avif.h index 1a0f2a99bd..54be521f89 100644 --- a/include/avif/avif.h +++ b/include/avif/avif.h @@ -546,14 +546,10 @@ typedef struct avifCropRect // These will return AVIF_FALSE if the resultant values violate any standards, and if so, the output // values are not guaranteed to be complete or correct and should not be used. -// If upsampleBeforeCropping is true, the image must be upsampled from 4:2:0 or 4:2:2 to 4:4:4 before -// Clean Aperture values are applied. AVIF_NODISCARD AVIF_API avifBool avifCropRectFromCleanApertureBox(avifCropRect * cropRect, - avifBool * upsampleBeforeCropping, const avifCleanApertureBox * clap, uint32_t imageW, uint32_t imageH, - avifPixelFormat yuvFormat, avifDiagnostics * diag); AVIF_NODISCARD AVIF_API avifBool avifCleanApertureBoxConvertCropRect(avifCleanApertureBox * clap, const avifCropRect * cropRect, @@ -561,6 +557,9 @@ AVIF_NODISCARD AVIF_API avifBool avifCleanApertureBoxConvertCropRect(avifCleanAp uint32_t imageH, avifPixelFormat yuvFormat, avifDiagnostics * diag); +// If this function returns true, the image must be upsampled from 4:2:0 or 4:2:2 to 4:4:4 before +// Clean Aperture values are applied. +AVIF_NODISCARD AVIF_API avifBool avifCropRectRequiresUpsampling(const avifCropRect * cropRect, avifPixelFormat yuvFormat); // Deprecated. Use avifCropRectFromCleanApertureBox() instead. AVIF_NODISCARD AVIF_API avifBool diff --git a/src/avif.c b/src/avif.c index 4bc269f1f5..f034c691bd 100644 --- a/src/avif.c +++ b/src/avif.c @@ -750,11 +750,9 @@ static avifBool avifCropRectIsValid(const avifCropRect * cropRect, uint32_t imag } avifBool avifCropRectFromCleanApertureBox(avifCropRect * cropRect, - avifBool * upsampleBeforeCropping, const avifCleanApertureBox * clap, uint32_t imageW, uint32_t imageH, - avifPixelFormat yuvFormat, avifDiagnostics * diag) { avifDiagnosticsClearError(diag); @@ -764,16 +762,6 @@ avifBool avifCropRectFromCleanApertureBox(avifCropRect * cropRect, // positive or negative. For cleanApertureWidth and cleanApertureHeight, // N shall be positive and D shall be strictly positive. - // ISO/IEC 23000-22:2024, 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: - // - cleanApertureWidth and cleanApertureHeight shall be integers; - // - If any of the following conditions hold true, the image is first implicitly upsampled to 4:4:4: - // - chroma is subsampled horizontally (i.e., 4:2:2 and 4:2:0) and cleanApertureWidth is odd - // - chroma is subsampled horizontally (i.e., 4:2:2 and 4:2:0) and left-most pixel is on an odd position - // - chroma is subsampled vertically (i.e., 4:2:0) and cleanApertureHeight is odd - // - chroma is subsampled vertically (i.e., 4:2:0) and topmost line is on an odd position - const int32_t widthN = (int32_t)clap->widthN; const int32_t widthD = (int32_t)clap->widthD; const int32_t heightN = (int32_t)clap->heightN; @@ -792,9 +780,6 @@ avifBool avifCropRectFromCleanApertureBox(avifCropRect * cropRect, } // 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: // - cleanApertureWidth and cleanApertureHeight shall be integers; // - The leftmost pixel and the topmost line of the clean aperture as // defined in ISO/IEC 14496-12:2020, Section 12.1.4.1 shall be integers; @@ -871,14 +856,21 @@ avifBool avifCropRectFromCleanApertureBox(avifCropRect * cropRect, cropRect->y = (uint32_t)(cropY.n / cropY.d); cropRect->width = (uint32_t)clapW; cropRect->height = (uint32_t)clapH; - if (!avifCropRectIsValid(cropRect, imageW, imageH, diag)) { - return AVIF_FALSE; - } + return avifCropRectIsValid(cropRect, imageW, imageH, diag); +} - *upsampleBeforeCropping = ((yuvFormat == AVIF_PIXEL_FORMAT_YUV420 || yuvFormat == AVIF_PIXEL_FORMAT_YUV422) && - (cropRect->width % 2 || cropRect->x % 2)) || - (yuvFormat == AVIF_PIXEL_FORMAT_YUV420 && (cropRect->height % 2 || cropRect->y % 2)); - return AVIF_TRUE; +avifBool avifCropRectRequiresUpsampling(const avifCropRect * cropRect, avifPixelFormat yuvFormat) +{ + // ISO/IEC 23000-22:2024 FDIS, Section 7.3.6.7: + // - If any of the following conditions hold true, the image is first implicitly upsampled to 4:4:4: + // - chroma is subsampled horizontally (i.e., 4:2:2 and 4:2:0) and cleanApertureWidth is odd + // - chroma is subsampled horizontally (i.e., 4:2:2 and 4:2:0) and left-most pixel is on an odd position + // - chroma is subsampled vertically (i.e., 4:2:0) and cleanApertureHeight is odd + // - chroma is subsampled vertically (i.e., 4:2:0) and topmost line is on an odd position + + // AV1 supports odd dimensions with chroma subsampling in those directions, so only look for x and y. + return ((yuvFormat == AVIF_PIXEL_FORMAT_YUV420 || yuvFormat == AVIF_PIXEL_FORMAT_YUV422) && (cropRect->x % 2)) || + (yuvFormat == AVIF_PIXEL_FORMAT_YUV420 && (cropRect->y % 2)); } avifBool avifCropRectConvertCleanApertureBox(avifCropRect * cropRect, @@ -888,10 +880,22 @@ avifBool avifCropRectConvertCleanApertureBox(avifCropRect * cropRect, avifPixelFormat yuvFormat, avifDiagnostics * diag) { - // Keep the same pre-deprecation behavior. - avifBool upsampleBeforeCropping; - return avifCropRectFromCleanApertureBox(cropRect, &upsampleBeforeCropping, clap, imageW, imageH, yuvFormat, diag) && - !upsampleBeforeCropping; + if (avifCropRectFromCleanApertureBox(cropRect, clap, imageW, imageH, diag)) { + // Keep the same pre-deprecation behavior. + + // ISO/IEC 23000-22:2019/Amd. 2:2021, Section 7.3.6.7: + // - 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 (avifCropRectRequiresUpsampling(cropRect, yuvFormat)) { + avifDiagnosticsPrintf(diag, "[Strict] crop rect X and Y offsets must be even due to this image's YUV subsampling"); + return AVIF_FALSE; + } + return AVIF_TRUE; + } + return AVIF_FALSE; } avifBool avifCleanApertureBoxConvertCropRect(avifCleanApertureBox * clap, diff --git a/src/read.c b/src/read.c index 752c94590a..72046a0cc5 100644 --- a/src/read.c +++ b/src/read.c @@ -1277,12 +1277,9 @@ static avifResult avifDecoderItemValidateProperties(const avifDecoderItem * item } avifCropRect cropRect; - avifBool upsampleBeforeCropping; const uint32_t imageW = ispeProp->u.ispe.width; const uint32_t imageH = ispeProp->u.ispe.height; - const avifPixelFormat configFormat = avifCodecConfigurationBoxGetFormat(&configProp->u.av1C); - const avifBool validClap = - avifCropRectFromCleanApertureBox(&cropRect, &upsampleBeforeCropping, &clapProp->u.clap, imageW, imageH, configFormat, diag); + const avifBool validClap = avifCropRectFromCleanApertureBox(&cropRect, &clapProp->u.clap, imageW, imageH, diag); if (!validClap) { return AVIF_RESULT_BMFF_PARSE_FAILED; } diff --git a/tests/gtest/avifclaptest.cc b/tests/gtest/avifclaptest.cc index 130134b7c2..a1d01599b1 100644 --- a/tests/gtest/avifclaptest.cc +++ b/tests/gtest/avifclaptest.cc @@ -86,11 +86,9 @@ INSTANTIATE_TEST_SUITE_P(Parameterized, InvalidClapPropertyTest, TEST_P(InvalidClapPropertyTest, ValidateClapProperty) { const InvalidClapPropertyParam& param = GetParam(); avifCropRect crop_rect; - avifBool upsampleBeforeCropping; avifDiagnostics diag; EXPECT_FALSE(avifCropRectFromCleanApertureBox( - &crop_rect, &upsampleBeforeCropping, ¶m.clap, param.width, - param.height, param.yuv_format, &diag)); + &crop_rect, ¶m.clap, param.width, param.height, &diag)); } struct ValidClapPropertyParam { @@ -134,7 +132,7 @@ constexpr ValidClapPropertyParam kValidClapPropertyTestParams[] = { AVIF_PIXEL_FORMAT_YUV420, {99, 1, 99, 1, static_cast(-1), 2, static_cast(-1), 2}, {0, 0, 99, 99}, - true}, + false}, // pcX = -1/2 + (100 - 1)/2 = 49 // pcY = -1/2 + (100 - 1)/2 = 49 // leftmost = 49 - (99 - 1)/2 = 0 @@ -156,23 +154,23 @@ INSTANTIATE_TEST_SUITE_P(Parameterized, ValidClapPropertyTest, TEST_P(ValidClapPropertyTest, ValidateClapProperty) { const ValidClapPropertyParam& param = GetParam(); avifCropRect crop_rect; - avifBool upsampleBeforeCropping; avifDiagnostics diag; - EXPECT_TRUE(avifCropRectFromCleanApertureBox( - &crop_rect, &upsampleBeforeCropping, ¶m.clap, param.width, - param.height, param.yuv_format, &diag)) + ASSERT_TRUE(avifCropRectFromCleanApertureBox( + &crop_rect, ¶m.clap, param.width, param.height, &diag)) << diag.error; + const avifBool upsample_before_cropping = + avifCropRectRequiresUpsampling(&crop_rect, param.yuv_format); 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); EXPECT_EQ(crop_rect.height, param.expected_crop_rect.height); - EXPECT_EQ(upsampleBeforeCropping, param.expected_upsample_before_cropping); + EXPECT_EQ(upsample_before_cropping, param.expected_upsample_before_cropping); // Deprecated function coverage. EXPECT_EQ(avifCropRectConvertCleanApertureBox(&crop_rect, ¶m.clap, param.width, param.height, param.yuv_format, &diag), - !upsampleBeforeCropping); + !upsample_before_cropping); } } // namespace