From 9be2966e6bcd04ca9b801e9fe4f0d202cfb9fbed Mon Sep 17 00:00:00 2001 From: maryla-uc Date: Thu, 22 Aug 2024 14:54:47 +0200 Subject: [PATCH] Fix fuzzer following gain map API changes (#2405) And a little bit of gain map reading cleanup in read.c (assume enableParsingGainMapMetadata is true in avifDecoderFindGainMapItem) --- src/read.c | 88 ++++++++----------- .../avif_fuzztest_enc_dec_experimental.cc | 14 ++- tests/gtest/avif_fuzztest_helpers.cc | 10 ++- tests/gtest/avif_fuzztest_helpers.h | 12 +-- 4 files changed, 57 insertions(+), 67 deletions(-) diff --git a/src/read.c b/src/read.c index 297c7aa789..612c1c2a0a 100644 --- a/src/read.c +++ b/src/read.c @@ -5027,7 +5027,8 @@ static avifResult avifDecoderDataFindToneMappedImageItem(const avifDecoderData * // Finds a 'tmap' (tone mapped image item) box associated with the given 'colorItem', // then finds the associated gain map image. -// If found, fills 'toneMappedImageItem', 'gainMapItem' and 'gainMapCodecType'. +// If found, fills 'toneMappedImageItem', 'gainMapItem' and 'gainMapCodecType', and +// allocates and fills metadata in decoder->image->gainMap. // Otherwise, sets 'toneMappedImageItem' and 'gainMapItem' to NULL. // Returns AVIF_RESULT_OK if no errors were encountered (whether or not a gain map was found). // Assumes that there is a single tmap item, and not, e.g., a grid of tmap items. @@ -5064,51 +5065,45 @@ static avifResult avifDecoderFindGainMapItem(const avifDecoder * decoder, &data->tileInfos[AVIF_ITEM_GAIN_MAP].grid, gainMapCodecType)); - if (decoder->enableDecodingGainMap || decoder->enableParsingGainMapMetadata) { - decoder->image->gainMap = avifGainMapCreate(); - AVIF_CHECKERR(decoder->image->gainMap, AVIF_RESULT_OUT_OF_MEMORY); - } - - if (decoder->enableParsingGainMapMetadata) { - avifGainMap * const gainMap = decoder->image->gainMap; - - AVIF_CHECKRES(avifReadColorProperties(decoder->io, - &toneMappedImageItemTmp->properties, - &gainMap->altICC, - &gainMap->altColorPrimaries, - &gainMap->altTransferCharacteristics, - &gainMap->altMatrixCoefficients, - &gainMap->altYUVRange, - /*cicpSet=*/NULL)); - - const avifProperty * clliProp = avifPropertyArrayFind(&toneMappedImageItemTmp->properties, "clli"); - if (clliProp) { - gainMap->altCLLI = clliProp->u.clli; - } + decoder->image->gainMap = avifGainMapCreate(); + AVIF_CHECKERR(decoder->image->gainMap, AVIF_RESULT_OUT_OF_MEMORY); - const avifProperty * pixiProp = avifPropertyArrayFind(&toneMappedImageItemTmp->properties, "pixi"); - if (pixiProp) { - gainMap->altPlaneCount = pixiProp->u.pixi.planeCount; - gainMap->altDepth = pixiProp->u.pixi.planeDepths[0]; - } + avifGainMap * const gainMap = decoder->image->gainMap; + AVIF_CHECKRES(avifReadColorProperties(decoder->io, + &toneMappedImageItemTmp->properties, + &gainMap->altICC, + &gainMap->altColorPrimaries, + &gainMap->altTransferCharacteristics, + &gainMap->altMatrixCoefficients, + &gainMap->altYUVRange, + /*cicpSet=*/NULL)); + + const avifProperty * clliProp = avifPropertyArrayFind(&toneMappedImageItemTmp->properties, "clli"); + if (clliProp) { + gainMap->altCLLI = clliProp->u.clli; + } - if (avifPropertyArrayFind(&toneMappedImageItemTmp->properties, "pasp") || - avifPropertyArrayFind(&toneMappedImageItemTmp->properties, "clap") || - avifPropertyArrayFind(&toneMappedImageItemTmp->properties, "irot") || - avifPropertyArrayFind(&toneMappedImageItemTmp->properties, "imir")) { - // libavif requires the bitstream contain the same pasp, clap, irot, imir - // properties for both the base and gain map image items used as input to - // the tone-mapped derived image item. libavif also requires the tone-mapped - // derived image item itself not be associated with these properties. This is - // enforced at encoding. Other patterns are rejected at decoding. - avifDiagnosticsPrintf(data->diag, - "Box[tmap] 'pasp', 'clap', 'irot' and 'imir' properties must be associated with base and gain map items instead of 'tmap'"); - return AVIF_RESULT_INVALID_TONE_MAPPED_IMAGE; - } + const avifProperty * pixiProp = avifPropertyArrayFind(&toneMappedImageItemTmp->properties, "pixi"); + if (pixiProp) { + gainMap->altPlaneCount = pixiProp->u.pixi.planeCount; + gainMap->altDepth = pixiProp->u.pixi.planeDepths[0]; + } + + if (avifPropertyArrayFind(&toneMappedImageItemTmp->properties, "pasp") || + avifPropertyArrayFind(&toneMappedImageItemTmp->properties, "clap") || + avifPropertyArrayFind(&toneMappedImageItemTmp->properties, "irot") || + avifPropertyArrayFind(&toneMappedImageItemTmp->properties, "imir")) { + // libavif requires the bitstream contain the same pasp, clap, irot, imir + // properties for both the base and gain map image items used as input to + // the tone-mapped derived image item. libavif also requires the tone-mapped + // derived image item itself not be associated with these properties. This is + // enforced at encoding. Other patterns are rejected at decoding. + avifDiagnosticsPrintf(data->diag, + "Box[tmap] 'pasp', 'clap', 'irot' and 'imir' properties must be associated with base and gain map items instead of 'tmap'"); + return AVIF_RESULT_INVALID_TONE_MAPPED_IMAGE; } if (decoder->enableDecodingGainMap) { - avifGainMap * const gainMap = decoder->image->gainMap; gainMap->image = avifImageCreateEmpty(); AVIF_CHECKERR(gainMap->image, AVIF_RESULT_OUT_OF_MEMORY); @@ -5487,18 +5482,13 @@ avifResult avifDecoderReset(avifDecoder * decoder) // Read the gain map's metadata. avifROData tmapData; AVIF_CHECKRES(avifDecoderItemRead(toneMappedImageItem, decoder->io, &tmapData, 0, 0, data->diag)); - if (decoder->image->gainMap == NULL) { - decoder->image->gainMap = avifGainMapCreate(); - AVIF_CHECKERR(decoder->image->gainMap, AVIF_RESULT_OUT_OF_MEMORY); - } + AVIF_ASSERT_OR_RETURN(decoder->image->gainMap != NULL); const avifResult tmapParsingRes = avifParseToneMappedImageBox(&decoder->image->gainMap->metadata, tmapData.data, tmapData.size, data->diag); - if (tmapParsingRes != AVIF_RESULT_OK) { - avifGainMapDestroy(decoder->image->gainMap); - decoder->image->gainMap = NULL; - } if (tmapParsingRes == AVIF_RESULT_NOT_IMPLEMENTED) { // Unsupported gain map version. Simply ignore the gain map. + avifGainMapDestroy(decoder->image->gainMap); + decoder->image->gainMap = NULL; } else { AVIF_CHECKRES(tmapParsingRes); decoder->gainMapPresent = AVIF_TRUE; diff --git a/tests/gtest/avif_fuzztest_enc_dec_experimental.cc b/tests/gtest/avif_fuzztest_enc_dec_experimental.cc index 4a54339e1a..a1a8646840 100644 --- a/tests/gtest/avif_fuzztest_enc_dec_experimental.cc +++ b/tests/gtest/avif_fuzztest_enc_dec_experimental.cc @@ -63,14 +63,12 @@ void EncodeDecodeValid(ImagePtr image, EncoderPtr encoder, DecoderPtr decoder) { EXPECT_EQ(decoded_image->depth, image->depth); EXPECT_EQ(decoded_image->yuvFormat, image->yuvFormat); - EXPECT_EQ(decoder->gainMapPresent, - image->gainMap != nullptr && image->gainMap->image != nullptr); - ASSERT_EQ(decoded_image->gainMap != nullptr, - decoder->gainMapPresent && (decoder->enableDecodingGainMap || - decoder->enableParsingGainMapMetadata)); - ASSERT_EQ(decoded_image->gainMap != nullptr && - decoded_image->gainMap->image != nullptr, - decoder->gainMapPresent && decoder->enableDecodingGainMap); + if (decoder->enableParsingGainMapMetadata) { + EXPECT_EQ(decoder->gainMapPresent, image->gainMap != nullptr); + } else { + EXPECT_FALSE(decoder->gainMapPresent); + } + ASSERT_EQ(decoded_image->gainMap != nullptr, decoder->gainMapPresent); if (decoder->gainMapPresent && decoder->enableDecodingGainMap) { ASSERT_NE(decoded_image->gainMap, nullptr); ASSERT_NE(decoded_image->gainMap->image, nullptr); diff --git a/tests/gtest/avif_fuzztest_helpers.cc b/tests/gtest/avif_fuzztest_helpers.cc index e3022e2743..aa78da2c92 100644 --- a/tests/gtest/avif_fuzztest_helpers.cc +++ b/tests/gtest/avif_fuzztest_helpers.cc @@ -158,10 +158,12 @@ ImagePtr AvifImageToUniquePtr(avifImage* image) { return ImagePtr(image); } #if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) DecoderPtr AddGainMapOptionsToDecoder(DecoderPtr decoder, - bool enable_parsing_gain_map_metadata, - bool enable_decoding_gain_map) { - decoder->enableParsingGainMapMetadata = enable_parsing_gain_map_metadata; - decoder->enableDecodingGainMap = enable_decoding_gain_map; + GainMapDecodeMode gain_map_decode_mode) { + decoder->enableParsingGainMapMetadata = + (gain_map_decode_mode == GainMapDecodeMode::kMetadataOnly || + gain_map_decode_mode == GainMapDecodeMode::kDecode); + decoder->enableDecodingGainMap = + (gain_map_decode_mode == GainMapDecodeMode::kDecode); // Do not fuzz 'ignoreColorAndAlpha' since most tests assume that if the // file/buffer is successfully decoded, then the main image was decoded, which // is no longer the case when this option is on. diff --git a/tests/gtest/avif_fuzztest_helpers.h b/tests/gtest/avif_fuzztest_helpers.h index 117893c826..d83d080a35 100644 --- a/tests/gtest/avif_fuzztest_helpers.h +++ b/tests/gtest/avif_fuzztest_helpers.h @@ -50,9 +50,9 @@ DecoderPtr CreateAvifDecoder(avifCodecChoice codec_choice, int max_threads, uint32_t image_count_limit, avifStrictFlags strict_flags); #if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) +enum class GainMapDecodeMode { kDontDecode, kMetadataOnly, kDecode }; DecoderPtr AddGainMapOptionsToDecoder(DecoderPtr decoder, - bool enable_parsing_gain_map_metadata, - bool enable_decoding_gain_map); + GainMapDecodeMode gain_map_decode_mode); #endif //------------------------------------------------------------------------------ @@ -226,10 +226,10 @@ inline auto ArbitraryBaseAvifDecoder() { // options fuzzed, with the exception of 'ignoreColorAndAlpha' (because it would // break most tests' assumptions). inline auto ArbitraryAvifDecoderWithGainMapOptions() { - return fuzztest::Map( - AddGainMapOptionsToDecoder, ArbitraryBaseAvifDecoder(), - /*enable_parsing_gain_map_metadata=*/fuzztest::Arbitrary(), - /*enable_decoding_gain_map=*/fuzztest::Arbitrary()); + return fuzztest::Map(AddGainMapOptionsToDecoder, ArbitraryBaseAvifDecoder(), + fuzztest::ElementOf({GainMapDecodeMode::kDontDecode, + GainMapDecodeMode::kMetadataOnly, + GainMapDecodeMode::kDecode})); } // Generator for an arbitrary DecoderPtr.