From 0e4343c0e095234d18553a1f2072be5e3279c43a Mon Sep 17 00:00:00 2001 From: Yannis Guyon Date: Tue, 10 Sep 2024 17:08:02 +0200 Subject: [PATCH 1/4] Allow odd clap dimensions and offsets The constraint in MIAF was replaced by an upsampling step before cropping. --- CHANGELOG.md | 3 + .../src/main/jni/libavif_jni.cc | 1 + apps/avifenc.c | 3 +- apps/shared/avifutil.c | 8 +- include/avif/avif.h | 23 ++++-- src/avif.c | 74 ++++++++++--------- src/read.c | 4 +- tests/gtest/avifclaptest.cc | 45 ++++++++--- 8 files changed, 105 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 487f641234..ddd197e045 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,9 @@ The changes are relative to the previous release, unless the baseline is specifi avifGainMapMetadataDouble structs. * Add avif(Un)SignedFraction structs and avifDoubleTo(Un)SignedFraction utility functions. +* Allow decoding subsampled images with odd Clean Aperture dimensions or offsets. +* Deprecate avifCropRectConvertCleanApertureBox(). Replace it with + avifCropRectFromCleanApertureBox(). ## [1.1.1] - 2024-07-30 diff --git a/android_jni/avifandroidjni/src/main/jni/libavif_jni.cc b/android_jni/avifandroidjni/src/main/jni/libavif_jni.cc index 66f6e902dd..9c75071a62 100644 --- a/android_jni/avifandroidjni/src/main/jni/libavif_jni.cc +++ b/android_jni/avifandroidjni/src/main/jni/libavif_jni.cc @@ -85,6 +85,7 @@ bool CreateDecoderAndParse(AvifDecoderWrapper* const decoder, avifDiagnostics diag; // If the image does not have a valid 'clap' property, then we simply display // the whole image. + // TODO: Use avifCropRectFromCleanApertureBox() instead. if (!(decoder->decoder->image->transformFlags & AVIF_TRANSFORM_CLAP) || !avifCropRectConvertCleanApertureBox( &decoder->crop, &decoder->decoder->image->clap, diff --git a/apps/avifenc.c b/apps/avifenc.c index b98820d0db..c6b5c442d5 100644 --- a/apps/avifenc.c +++ b/apps/avifenc.c @@ -2290,9 +2290,10 @@ int main(int argc, char * argv[]) // Validate clap avifCropRect cropRect; + avifBool upsampleBeforeCropping; avifDiagnostics diag; avifDiagnosticsClearError(&diag); - if (!avifCropRectConvertCleanApertureBox(&cropRect, &image->clap, image->width, image->height, image->yuvFormat, &diag)) { + if (!avifCropRectFromCleanApertureBox(&cropRect, &upsampleBeforeCropping, &image->clap, image->width, image->height, image->yuvFormat, &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, diff --git a/apps/shared/avifutil.c b/apps/shared/avifutil.c index fc36349e66..521e07f9b3 100644 --- a/apps/shared/avifutil.c +++ b/apps/shared/avifutil.c @@ -99,16 +99,18 @@ static void avifImageDumpInternal(const avifImage * avif, uint32_t gridCols, uin printf("\n"); avifCropRect cropRect; + avifBool upsampleBeforeCropping; avifDiagnostics diag; avifDiagnosticsClearError(&diag); avifBool validClap = - avifCropRectConvertCleanApertureBox(&cropRect, &avif->clap, avif->width, avif->height, avif->yuvFormat, &diag); + avifCropRectFromCleanApertureBox(&cropRect, &upsampleBeforeCropping, &avif->clap, avif->width, avif->height, avif->yuvFormat, &diag); if (validClap) { - printf(" * Valid, derived crop rect: X: %d, Y: %d, W: %d, H: %d\n", + printf(" * Valid, derived crop rect: X: %d, Y: %d, W: %d, H: %d%s\n", cropRect.x, cropRect.y, cropRect.width, - cropRect.height); + cropRect.height, + upsampleBeforeCropping ? " (upsample before cropping)" : ""); } else { printf(" * Invalid: %s\n", diag.error); } diff --git a/include/avif/avif.h b/include/avif/avif.h index a34db7e3e3..1a0f2a99bd 100644 --- a/include/avif/avif.h +++ b/include/avif/avif.h @@ -486,6 +486,8 @@ typedef struct avifPixelAspectRatioBox typedef struct avifCleanApertureBox { // 'clap' from ISO/IEC 14496-12:2022 12.1.4.3 + // Note that ISO/IEC 23000-22:2024 7.3.6.7 requires the decoded image to be upsampled to 4:4:4 before + // clean aperture is applied if a clean aperture size or offset is odd in a subsampled dimension. // a fractional number which defines the width of the clean aperture image uint32_t widthN; @@ -544,12 +546,15 @@ 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. -AVIF_NODISCARD AVIF_API avifBool avifCropRectConvertCleanApertureBox(avifCropRect * cropRect, - const avifCleanApertureBox * clap, - uint32_t imageW, - uint32_t imageH, - avifPixelFormat yuvFormat, - avifDiagnostics * diag); +// 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, uint32_t imageW, @@ -557,6 +562,10 @@ AVIF_NODISCARD AVIF_API avifBool avifCleanApertureBoxConvertCropRect(avifCleanAp avifPixelFormat yuvFormat, avifDiagnostics * diag); +// Deprecated. Use avifCropRectFromCleanApertureBox() instead. +AVIF_NODISCARD AVIF_API avifBool +avifCropRectConvertCleanApertureBox(avifCropRect *, const avifCleanApertureBox *, uint32_t, uint32_t, avifPixelFormat, avifDiagnostics *); + // --------------------------------------------------------------------------- // avifContentLightLevelInformationBox @@ -1116,7 +1125,7 @@ typedef enum avifStrictFlag AVIF_STRICT_PIXI_REQUIRED = (1 << 0), // This demands that the values surfaced in the clap box are valid, determined by attempting to - // convert the clap box to a crop rect using avifCropRectConvertCleanApertureBox(). If this + // convert the clap box to a crop rect using avifCropRectFromCleanApertureBox(). If this // function returns AVIF_FALSE and this strict flag is set, the decode will fail. AVIF_STRICT_CLAP_VALID = (1 << 1), diff --git a/src/avif.c b/src/avif.c index 93c46033e6..4bc269f1f5 100644 --- a/src/avif.c +++ b/src/avif.c @@ -735,19 +735,8 @@ static avifBool overflowsInt32(int64_t x) return (x < INT32_MIN) || (x > INT32_MAX); } -static avifBool avifCropRectIsValid(const avifCropRect * cropRect, uint32_t imageW, uint32_t imageH, avifPixelFormat yuvFormat, avifDiagnostics * diag) - +static avifBool avifCropRectIsValid(const avifCropRect * cropRect, uint32_t imageW, uint32_t imageH, avifDiagnostics * diag) { - // 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: - // ... - // - 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"); return AVIF_FALSE; @@ -757,28 +746,16 @@ static avifBool avifCropRectIsValid(const avifCropRect * cropRect, uint32_t imag avifDiagnosticsPrintf(diag, "[Strict] crop rect is out of the image's bounds"); return AVIF_FALSE; } - - if ((yuvFormat == AVIF_PIXEL_FORMAT_YUV420) || (yuvFormat == AVIF_PIXEL_FORMAT_YUV422)) { - 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) { - avifDiagnosticsPrintf(diag, "[Strict] crop rect Y offset must be even due to this image's YUV subsampling"); - return AVIF_FALSE; - } - } return AVIF_TRUE; } -avifBool avifCropRectConvertCleanApertureBox(avifCropRect * cropRect, - const avifCleanApertureBox * clap, - uint32_t imageW, - uint32_t imageH, - avifPixelFormat yuvFormat, - avifDiagnostics * diag) +avifBool avifCropRectFromCleanApertureBox(avifCropRect * cropRect, + avifBool * upsampleBeforeCropping, + const avifCleanApertureBox * clap, + uint32_t imageW, + uint32_t imageH, + avifPixelFormat yuvFormat, + avifDiagnostics * diag) { avifDiagnosticsClearError(diag); @@ -787,6 +764,16 @@ avifBool avifCropRectConvertCleanApertureBox(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; @@ -884,7 +871,27 @@ avifBool avifCropRectConvertCleanApertureBox(avifCropRect * cropRect, cropRect->y = (uint32_t)(cropY.n / cropY.d); cropRect->width = (uint32_t)clapW; cropRect->height = (uint32_t)clapH; - return avifCropRectIsValid(cropRect, imageW, imageH, yuvFormat, diag); + if (!avifCropRectIsValid(cropRect, imageW, imageH, diag)) { + return AVIF_FALSE; + } + + *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 avifCropRectConvertCleanApertureBox(avifCropRect * cropRect, + const avifCleanApertureBox * clap, + uint32_t imageW, + uint32_t imageH, + avifPixelFormat yuvFormat, + avifDiagnostics * diag) +{ + // Keep the same pre-deprecation behavior. + avifBool upsampleBeforeCropping; + return avifCropRectFromCleanApertureBox(cropRect, &upsampleBeforeCropping, clap, imageW, imageH, yuvFormat, diag) && + !upsampleBeforeCropping; } avifBool avifCleanApertureBoxConvertCropRect(avifCleanApertureBox * clap, @@ -894,9 +901,10 @@ avifBool avifCleanApertureBoxConvertCropRect(avifCleanApertureBox * clap, avifPixelFormat yuvFormat, avifDiagnostics * diag) { + (void)yuvFormat; avifDiagnosticsClearError(diag); - if (!avifCropRectIsValid(cropRect, imageW, imageH, yuvFormat, diag)) { + if (!avifCropRectIsValid(cropRect, imageW, imageH, diag)) { return AVIF_FALSE; } diff --git a/src/read.c b/src/read.c index 9891c151c7..752c94590a 100644 --- a/src/read.c +++ b/src/read.c @@ -1277,10 +1277,12 @@ 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); - avifBool validClap = avifCropRectConvertCleanApertureBox(&cropRect, &clapProp->u.clap, imageW, imageH, configFormat, diag); + const avifBool validClap = + avifCropRectFromCleanApertureBox(&cropRect, &upsampleBeforeCropping, &clapProp->u.clap, imageW, imageH, configFormat, diag); if (!validClap) { return AVIF_RESULT_BMFF_PARSE_FAILED; } diff --git a/tests/gtest/avifclaptest.cc b/tests/gtest/avifclaptest.cc index 77dfaf26e2..130134b7c2 100644 --- a/tests/gtest/avifclaptest.cc +++ b/tests/gtest/avifclaptest.cc @@ -82,14 +82,15 @@ using InvalidClapPropertyTest = INSTANTIATE_TEST_SUITE_P(Parameterized, InvalidClapPropertyTest, ::testing::ValuesIn(kInvalidClapPropertyTestParams)); -// Negative tests for the avifCropRectConvertCleanApertureBox() function. +// Negative tests for the avifCropRectFromCleanApertureBox() function. TEST_P(InvalidClapPropertyTest, ValidateClapProperty) { const InvalidClapPropertyParam& param = GetParam(); avifCropRect crop_rect; + avifBool upsampleBeforeCropping; avifDiagnostics diag; - EXPECT_FALSE(avifCropRectConvertCleanApertureBox(&crop_rect, ¶m.clap, - param.width, param.height, - param.yuv_format, &diag)); + EXPECT_FALSE(avifCropRectFromCleanApertureBox( + &crop_rect, &upsampleBeforeCropping, ¶m.clap, param.width, + param.height, param.yuv_format, &diag)); } struct ValidClapPropertyParam { @@ -99,6 +100,7 @@ struct ValidClapPropertyParam { avifCleanApertureBox clap; avifCropRect expected_crop_rect; + bool expected_upsample_before_cropping; }; constexpr ValidClapPropertyParam kValidClapPropertyTestParams[] = { @@ -110,7 +112,8 @@ constexpr ValidClapPropertyParam kValidClapPropertyTestParams[] = { 160, AVIF_PIXEL_FORMAT_YUV420, {96, 1, 132, 1, 0, 1, 0, 1}, - {12, 14, 96, 132}}, + {12, 14, 96, 132}, + false}, // pcX = -30 + (120 - 1)/2 = 29.5 // pcY = -40 + (160 - 1)/2 = 39.5 // leftmost = 29.5 - (60 - 1)/2 = 0 @@ -120,7 +123,8 @@ constexpr ValidClapPropertyParam kValidClapPropertyTestParams[] = { AVIF_PIXEL_FORMAT_YUV420, {60, 1, 80, 1, static_cast(-30), 1, static_cast(-40), 1}, - {0, 0, 60, 80}}, + {0, 0, 60, 80}, + false}, // pcX = -1/2 + (100 - 1)/2 = 49 // pcY = -1/2 + (100 - 1)/2 = 49 // leftmost = 49 - (99 - 1)/2 = 0 @@ -129,7 +133,18 @@ constexpr ValidClapPropertyParam kValidClapPropertyTestParams[] = { 100, AVIF_PIXEL_FORMAT_YUV420, {99, 1, 99, 1, static_cast(-1), 2, static_cast(-1), 2}, - {0, 0, 99, 99}}, + {0, 0, 99, 99}, + true}, + // 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, 1, 2, 1, 2}, + {1, 1, 99, 99}, + true}, }; using ValidClapPropertyTest = ::testing::TestWithParam; @@ -137,19 +152,27 @@ using ValidClapPropertyTest = ::testing::TestWithParam; INSTANTIATE_TEST_SUITE_P(Parameterized, ValidClapPropertyTest, ::testing::ValuesIn(kValidClapPropertyTestParams)); -// Positive tests for the avifCropRectConvertCleanApertureBox() function. +// Positive tests for the avifCropRectFromCleanApertureBox() function. TEST_P(ValidClapPropertyTest, ValidateClapProperty) { const ValidClapPropertyParam& param = GetParam(); avifCropRect crop_rect; + avifBool upsampleBeforeCropping; avifDiagnostics diag; - EXPECT_TRUE(avifCropRectConvertCleanApertureBox(&crop_rect, ¶m.clap, - param.width, param.height, - param.yuv_format, &diag)) + EXPECT_TRUE(avifCropRectFromCleanApertureBox( + &crop_rect, &upsampleBeforeCropping, ¶m.clap, param.width, + param.height, 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); EXPECT_EQ(crop_rect.height, param.expected_crop_rect.height); + EXPECT_EQ(upsampleBeforeCropping, param.expected_upsample_before_cropping); + + // Deprecated function coverage. + EXPECT_EQ(avifCropRectConvertCleanApertureBox(&crop_rect, ¶m.clap, + param.width, param.height, + param.yuv_format, &diag), + !upsampleBeforeCropping); } } // namespace From 73a91d8c913fa3f2cdf4861b8113cd22221fcc34 Mon Sep 17 00:00:00 2001 From: Yannis Guyon Date: Fri, 22 Nov 2024 16:46:33 +0100 Subject: [PATCH 2/4] 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 From 7766d13964de714d96629eaadb872cef8cea5dbe Mon Sep 17 00:00:00 2001 From: Yannis Guyon Date: Fri, 22 Nov 2024 16:49:49 +0100 Subject: [PATCH 3/4] Remove unused variable --- apps/shared/avifutil.c | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/shared/avifutil.c b/apps/shared/avifutil.c index 78b740675e..2c718dfba0 100644 --- a/apps/shared/avifutil.c +++ b/apps/shared/avifutil.c @@ -99,7 +99,6 @@ static void avifImageDumpInternal(const avifImage * avif, uint32_t gridCols, uin printf("\n"); avifCropRect cropRect; - avifBool upsampleBeforeCropping; avifDiagnostics diag; avifDiagnosticsClearError(&diag); avifBool validClap = avifCropRectFromCleanApertureBox(&cropRect, &avif->clap, avif->width, avif->height, &diag); From 858b0cd6ec8c3134f6e4f561bbcd9aebbca17cfc Mon Sep 17 00:00:00 2001 From: Yannis Guyon Date: Fri, 22 Nov 2024 16:53:14 +0100 Subject: [PATCH 4/4] Inline avifCodecConfigurationBoxGetFormat() Unused unless AVIF_ENABLE_EXPERIMENTAL_MINI is defined. --- src/read.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/read.c b/src/read.c index 72046a0cc5..f834e9ada9 100644 --- a/src/read.c +++ b/src/read.c @@ -367,19 +367,6 @@ static uint32_t avifCodecConfigurationBoxGetDepth(const avifCodecConfigurationBo return 8; } -// This is used as a hint to validating the clap box in avifDecoderItemValidateProperties. -static avifPixelFormat avifCodecConfigurationBoxGetFormat(const avifCodecConfigurationBox * av1C) -{ - if (av1C->monochrome) { - return AVIF_PIXEL_FORMAT_YUV400; - } else if (av1C->chromaSubsamplingY == 1) { - return AVIF_PIXEL_FORMAT_YUV420; - } else if (av1C->chromaSubsamplingX == 1) { - return AVIF_PIXEL_FORMAT_YUV422; - } - return AVIF_PIXEL_FORMAT_YUV444; -} - static const avifPropertyArray * avifSampleTableGetProperties(const avifSampleTable * sampleTable, avifCodecType codecType) { for (uint32_t i = 0; i < sampleTable->sampleDescriptions.count; ++i) { @@ -1231,9 +1218,19 @@ static avifResult avifDecoderItemValidateProperties(const avifDecoderItem * item if (item->miniBoxPixelFormat != AVIF_PIXEL_FORMAT_NONE) { // This is a MinimizedImageBox ('mini'). - if (item->miniBoxPixelFormat != avifCodecConfigurationBoxGetFormat(&configProp->u.av1C)) { + avifPixelFormat av1CPixelFormat; + if (configProp->u.av1C.monochrome) { + av1CPixelFormat = AVIF_PIXEL_FORMAT_YUV400; + } else if (configProp->u.av1C.chromaSubsamplingY == 1) { + av1CPixelFormat = AVIF_PIXEL_FORMAT_YUV420; + } else if (configProp->u.av1C.chromaSubsamplingX == 1) { + av1CPixelFormat = AVIF_PIXEL_FORMAT_YUV422; + } else { + av1CPixelFormat = AVIF_PIXEL_FORMAT_YUV444; + } + if (item->miniBoxPixelFormat != av1CPixelFormat) { if (!memcmp(configPropName, "av2C", 4) && item->miniBoxPixelFormat == AVIF_PIXEL_FORMAT_YUV400 && - avifCodecConfigurationBoxGetFormat(&configProp->u.av1C) == AVIF_PIXEL_FORMAT_YUV420) { + av1CPixelFormat == AVIF_PIXEL_FORMAT_YUV420) { // avm does not handle monochrome as of research-v8.0.0. // 4:2:0 is used instead. } else { @@ -1242,7 +1239,7 @@ static avifResult avifDecoderItemValidateProperties(const avifDecoderItem * item item->id, avifPixelFormatToString(item->miniBoxPixelFormat), configPropName, - avifPixelFormatToString(avifCodecConfigurationBoxGetFormat(&configProp->u.av1C))); + avifPixelFormatToString(av1CPixelFormat)); return AVIF_RESULT_BMFF_PARSE_FAILED; } }