From 3e4b9c88761aab919cbf98377447b7315dd834b3 Mon Sep 17 00:00:00 2001 From: maryla-uc Date: Wed, 21 Aug 2024 11:05:52 +0200 Subject: [PATCH] Change gain map parsing API. (#2399) Now gainMapPresent is only populated if enableParsingGainMapMetadata is true. The combination of enableDecodingGainMap=true and enableParsingGainMapMetadata=false is no longer allowed (returns AVIF_RESULT_INVALID_ARGUMENT). --- CHANGELOG.md | 5 ++ .../avifgainmaputil/extractgainmap_command.cc | 1 + include/avif/avif.h | 18 ++--- src/read.c | 76 +++++++++---------- tests/gtest/avifgainmaptest.cc | 45 ++++------- 5 files changed, 60 insertions(+), 85 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a51c5a582b..e69541cb23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,11 @@ The changes are relative to the previous release, unless the baseline is specifi draft. * Ignore gain maps with unsupported metadata. Handle gain maps with writer_version > 0 correctly. + The combination of settings enableParsingGainMapMetadata=false with + enableDecodingGainMap=true is no longer allowed and returns an invalid argument + error. + The `gainMapPresent` field is now only populated if enableParsingGainMapMetadata + is true. ## [1.1.1] - 2024-07-30 diff --git a/apps/avifgainmaputil/extractgainmap_command.cc b/apps/avifgainmaputil/extractgainmap_command.cc index 2df46ba9a8..a4c834c16e 100644 --- a/apps/avifgainmaputil/extractgainmap_command.cc +++ b/apps/avifgainmaputil/extractgainmap_command.cc @@ -21,6 +21,7 @@ avifResult ExtractGainMapCommand::Run() { if (decoder == NULL) { return AVIF_RESULT_OUT_OF_MEMORY; } + decoder->enableParsingGainMapMetadata = true; decoder->enableDecodingGainMap = true; decoder->ignoreColorAndAlpha = true; diff --git a/include/avif/avif.h b/include/avif/avif.h index 1b8267f268..57e2e9bcfe 100644 --- a/include/avif/avif.h +++ b/include/avif/avif.h @@ -1312,28 +1312,20 @@ typedef struct avifDecoder // Version 1.1.0 ends here. Add any new members after this line. #if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) - // Enable decoding the gain map image if present (defaults to AVIF_FALSE) - // (see also 'enableParsingGainMapMetadata' below). - // If 'enableParsingGainMapMetadata' is also true, the gain map is only decoded if - // the gan map's metadata is a supported version. - // 'gainMapPresent' is still set if the presence of a gain map is detected, regardless - // of this setting. - avifBool enableDecodingGainMap; // Enable parsing the gain map metadata if present (defaults to AVIF_FALSE). - // This setting can affect the value of 'gainMapPresent', see the description of - // 'gainMapPresent' below. // Gain map metadata is read during avifDecoderParse(). Like Exif and XMP, this data // can be (unfortunately) packed at the end of the file, which will cause // avifDecoderParse() to return AVIF_RESULT_WAITING_ON_IO until it finds it. // If you don't actually use this data, it's best to leave this to AVIF_FALSE (default). avifBool enableParsingGainMapMetadata; + // Enable decoding the gain map image if present (defaults to AVIF_FALSE). + // If set to true, enableParsingGainMapMetadata must also be true. + avifBool enableDecodingGainMap; // Do not decode the color/alpha planes of the main image. // Can be useful to decode the gain map image only. avifBool ignoreColorAndAlpha; - // If 'enableParsingGainMapMetadata' is false: gainMapPresent is true when avifDecoderParse() - // detects a gain map. - // If 'enableParsingGainMapMetadata' is true: gainMapPresent is true when avifDecoderParse() - // detects a gain map whose metadata is a supported version. + // True when avifDecoderParse() detects a supported gain map. + // Requires enableParsingGainMapMetadata to be set to true. avifBool gainMapPresent; #endif } avifDecoder; diff --git a/src/read.c b/src/read.c index 91538c76cd..cf4752b0ce 100644 --- a/src/read.c +++ b/src/read.c @@ -4534,6 +4534,12 @@ avifResult avifDecoderParse(avifDecoder * decoder) if (!decoder->io || !decoder->io->read) { return AVIF_RESULT_IO_NOT_SET; } +#if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) + if (decoder->enableDecodingGainMap && !decoder->enableParsingGainMapMetadata) { + avifDiagnosticsPrintf(&decoder->diag, "If enableDecodingGainMap is true, enableParsingGainMapMetadata must also be true"); + return AVIF_RESULT_INVALID_ARGUMENT; + } +#endif // Cleanup anything lingering in the decoder avifDecoderCleanup(decoder); @@ -5406,46 +5412,36 @@ avifResult avifDecoderReset(avifDecoder * decoder) } #if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) - avifDecoderItem * toneMappedImageItem; - const avifResult findGainMapResult = avifDecoderFindGainMapItem(decoder, - mainItems[AVIF_ITEM_COLOR], - &toneMappedImageItem, - &mainItems[AVIF_ITEM_GAIN_MAP], - &codecType[AVIF_ITEM_GAIN_MAP]); - if (!decoder->enableDecodingGainMap) { - // When ignoring the gain map, we still report whether one is present or not, - // but do not fail if there was any error with the gain map. - if (findGainMapResult != AVIF_RESULT_OK) { - // Only ignore reproducible errors (caused by the bitstream and not by the environment). - AVIF_CHECKERR(findGainMapResult != AVIF_RESULT_OUT_OF_MEMORY, findGainMapResult); - // Clear diagnostic message. - avifDiagnosticsClearError(data->diag); - } - decoder->gainMapPresent = (mainItems[AVIF_ITEM_GAIN_MAP] != NULL); - // We also ignore the actual item and don't decode it. - mainItems[AVIF_ITEM_GAIN_MAP] = NULL; - } else { - AVIF_CHECKRES(findGainMapResult); - } - if (toneMappedImageItem != NULL && decoder->enableParsingGainMapMetadata) { - // 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); - } - const avifResult tmapParsingRes = - avifParseToneMappedImageBox(&decoder->image->gainMap->metadata, tmapData.data, tmapData.size, data->diag); - if (tmapParsingRes == AVIF_RESULT_NOT_IMPLEMENTED) { - // Forget about the gain map. - toneMappedImageItem = NULL; - mainItems[AVIF_ITEM_GAIN_MAP] = NULL; - avifGainMapDestroy(decoder->image->gainMap); - decoder->image->gainMap = NULL; - decoder->gainMapPresent = AVIF_FALSE; - } else { - AVIF_CHECKRES(tmapParsingRes); + if (decoder->enableParsingGainMapMetadata) { + avifDecoderItem * toneMappedImageItem; + avifDecoderItem * gainMapItem; + avifCodecType gainMapCodecType; + AVIF_CHECKRES( + avifDecoderFindGainMapItem(decoder, mainItems[AVIF_ITEM_COLOR], &toneMappedImageItem, &gainMapItem, &gainMapCodecType)); + if (toneMappedImageItem != NULL) { + // 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); + } + 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. + } else { + AVIF_CHECKRES(tmapParsingRes); + decoder->gainMapPresent = AVIF_TRUE; + if (decoder->enableDecodingGainMap) { + mainItems[AVIF_ITEM_GAIN_MAP] = gainMapItem; + codecType[AVIF_ITEM_GAIN_MAP] = gainMapCodecType; + } + } } } #endif // AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP diff --git a/tests/gtest/avifgainmaptest.cc b/tests/gtest/avifgainmaptest.cc index 75c94660b5..d79537b81c 100644 --- a/tests/gtest/avifgainmaptest.cc +++ b/tests/gtest/avifgainmaptest.cc @@ -576,9 +576,9 @@ TEST(GainMapTest, IgnoreGainMap) { // Verify that the input and decoded images are close. EXPECT_GT(testutil::GetPsnr(*image, *decoded), 40.0); - // Verify that the gain map was detected... - EXPECT_TRUE(decoder->gainMapPresent); - // ... but not decoded because enableDecodingGainMap is false by default. + // Verify that the gain map is not detected. + EXPECT_FALSE(decoder->gainMapPresent); + // And not decoded because enableDecodingGainMap is false by default. EXPECT_EQ(decoded->gainMap, nullptr); } @@ -624,7 +624,7 @@ TEST(GainMapTest, IgnoreGainMapButReadMetadata) { image->gainMap->altMatrixCoefficients); } -TEST(GainMapTest, DecodeGainMapButIgnoreMetadata) { +TEST(GainMapTest, DecodeGainMapTrueParseMetadataFalse) { ImagePtr image = CreateTestImageWithGainMap(/*base_rendition_is_hdr=*/false); ASSERT_NE(image, nullptr); @@ -643,27 +643,11 @@ TEST(GainMapTest, DecodeGainMapButIgnoreMetadata) { decoder->enableDecodingGainMap = AVIF_TRUE; result = avifDecoderReadMemory(decoder.get(), decoded.get(), encoded.data, encoded.size); - ASSERT_EQ(result, AVIF_RESULT_OK) + // Verify we get an error because the combination of + // enableDecodingGainMap=false and enableParsingGainMapMetadata=true + // is not allowed. + ASSERT_EQ(result, AVIF_RESULT_INVALID_ARGUMENT) << avifResultToString(result) << " " << decoder->diag.error; - - // Verify that the gain map was detected... - EXPECT_TRUE(decoder->gainMapPresent); - ASSERT_NE(decoded->gainMap, nullptr); - ASSERT_NE(decoded->gainMap->image, nullptr); - EXPECT_GT(testutil::GetPsnr(*image->gainMap->image, *decoded->gainMap->image), - 40.0); - - // Check that the gain map metadata was not populated. - CheckGainMapMetadataMatches(decoded->gainMap->metadata, - avifGainMapMetadata()); - EXPECT_EQ(decoded->gainMap->altDepth, 0); - EXPECT_EQ(decoded->gainMap->altPlaneCount, 0); - EXPECT_EQ(decoded->gainMap->altColorPrimaries, - AVIF_COLOR_PRIMARIES_UNSPECIFIED); - EXPECT_EQ(decoded->gainMap->altTransferCharacteristics, - AVIF_TRANSFER_CHARACTERISTICS_UNSPECIFIED); - EXPECT_EQ(decoded->gainMap->altMatrixCoefficients, - AVIF_MATRIX_COEFFICIENTS_UNSPECIFIED); } TEST(GainMapTest, IgnoreColorAndAlpha) { @@ -884,9 +868,8 @@ TEST(GainMapTest, DecodeUnsupportedVersion) { decoder->enableParsingGainMapMetadata = false; ASSERT_EQ(avifDecoderReadFile(decoder.get(), decoded.get(), path.c_str()), AVIF_RESULT_OK); - // Gain map marked as present because the metadata was not parsed, so we - // don't know it's not supported. - EXPECT_EQ(decoder->gainMapPresent, true); + // Gain map not found since enableParsingGainMapMetadata is false. + EXPECT_EQ(decoder->gainMapPresent, false); ASSERT_EQ(decoded->gainMap, nullptr); ASSERT_EQ(avifDecoderReset(decoder.get()), AVIF_RESULT_OK); @@ -901,12 +884,10 @@ TEST(GainMapTest, DecodeUnsupportedVersion) { ASSERT_EQ(avifDecoderReset(decoder.get()), AVIF_RESULT_OK); decoder->enableDecodingGainMap = true; decoder->enableParsingGainMapMetadata = false; + // Invalid enableDecodingGainMap=true and enableParsingGainMapMetadata + // combination. ASSERT_EQ(avifDecoderReadFile(decoder.get(), decoded.get(), path.c_str()), - AVIF_RESULT_OK); - // Gain map marked as present because the metadata was not parsed, so we - // don't know it's not supported. - EXPECT_EQ(decoder->gainMapPresent, true); - ASSERT_NE(decoded->gainMap, nullptr); + AVIF_RESULT_INVALID_ARGUMENT); ASSERT_EQ(avifDecoderReset(decoder.get()), AVIF_RESULT_OK); decoder->enableDecodingGainMap = true;