Ignore gain maps with unsupported version (#2380)

Ignore gain maps with unsupported version. Allow extra bytes after the gain map metadata of writer_version is larger than the supported version. Issue #2369
AOMediaCodec · Aug 9, 2024 · d142df6 · d142df6
1 parent 58acc88
commit d142df6
Show file tree

Hide file tree

Showing 9 changed files with 218 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ The changes are relative to the previous release, unless the baseline is specifi
 * Renamed AVIF_ENABLE_EXPERIMENTAL_METAV1 to AVIF_ENABLE_EXPERIMENTAL_MINI and
   updated the experimental reduced header feature to the latest specification
   draft.
+* Ignore gain maps with unsupported metadata.
 
 ## [1.1.1] - 2024-07-30
 

diff --git a/include/avif/avif.h b/include/avif/avif.h
@@ -1312,16 +1312,16 @@ typedef struct avifDecoder
     // Version 1.1.0 ends here. Add any new members after this line.
 
 #if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP)
-    // This is true when avifDecoderParse() detects a gain map.
-    avifBool gainMapPresent;
-    // Enable decoding the gain map image if present (defaults to AVIF_FALSE).
-    // (see also enableParsingGainMapMetadata below).
-    // gainMapPresent is still set if the presence of a gain map is detected, regardless
+    // 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).
-    // gainMapPresent is still set if the presence of a gain map is detected, regardless
-    // of this setting.
+    // 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.
@@ -1330,6 +1330,11 @@ typedef struct avifDecoder
     // 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.
+    avifBool gainMapPresent;
 #endif
 } avifDecoder;
 

diff --git a/src/read.c b/src/read.c
@@ -1953,53 +1953,35 @@ static avifBool avifParseImageGridBox(avifImageGrid * grid,
 
 #if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP)
 
-static avifBool avifParseToneMappedImageBox(avifGainMapMetadata * metadata, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
+static avifBool avifParseGainMapMetadata(avifGainMapMetadata * metadata, avifROStream * s)
 {
-    BEGIN_STREAM(s, raw, rawLen, diag, "Box[tmap]");
-
-    uint8_t version;
-    AVIF_CHECK(avifROStreamRead(&s, &version, 1)); // unsigned int(8) version = 0;
-    if (version != 0) {
-        avifDiagnosticsPrintf(diag, "Box[tmap] has unsupported version [%u]", version);
-        return AVIF_FALSE;
-    }
-
-    uint16_t minimumVersion;
-    AVIF_CHECK(avifROStreamReadU16(&s, &minimumVersion)); // unsigned int(16) minimum_version;
-    if (minimumVersion != 0) {
-        avifDiagnosticsPrintf(diag, "Box[tmap] has unsupported minimum version [%u]", minimumVersion);
-        return AVIF_FALSE;
-    }
-    uint16_t writerVersion;
-    AVIF_CHECK(avifROStreamReadU16(&s, &writerVersion)); // unsigned int(16) writer_version;
-
     uint32_t isMultichannel;
-    AVIF_CHECK(avifROStreamReadBits(&s, &isMultichannel, 1)); // unsigned int(1) is_multichannel;
+    AVIF_CHECK(avifROStreamReadBits(s, &isMultichannel, 1)); // unsigned int(1) is_multichannel;
     const uint8_t channelCount = isMultichannel ? 3 : 1;
 
     uint32_t useBaseColorSpace;
-    AVIF_CHECK(avifROStreamReadBits(&s, &useBaseColorSpace, 1)); // unsigned int(1) use_base_colour_space;
+    AVIF_CHECK(avifROStreamReadBits(s, &useBaseColorSpace, 1)); // unsigned int(1) use_base_colour_space;
     metadata->useBaseColorSpace = useBaseColorSpace ? AVIF_TRUE : AVIF_FALSE;
 
     uint32_t reserved;
-    AVIF_CHECK(avifROStreamReadBits(&s, &reserved, 6)); // unsigned int(6) reserved;
+    AVIF_CHECK(avifROStreamReadBits(s, &reserved, 6)); // unsigned int(6) reserved;
 
-    AVIF_CHECK(avifROStreamReadU32(&s, &metadata->baseHdrHeadroomN));      // unsigned int(32) base_hdr_headroom_numerator;
-    AVIF_CHECK(avifROStreamReadU32(&s, &metadata->baseHdrHeadroomD));      // unsigned int(32) base_hdr_headroom_denominator;
-    AVIF_CHECK(avifROStreamReadU32(&s, &metadata->alternateHdrHeadroomN)); // unsigned int(32) alternate_hdr_headroom_numerator;
-    AVIF_CHECK(avifROStreamReadU32(&s, &metadata->alternateHdrHeadroomD)); // unsigned int(32) alternate_hdr_headroom_denominator;
+    AVIF_CHECK(avifROStreamReadU32(s, &metadata->baseHdrHeadroomN));      // unsigned int(32) base_hdr_headroom_numerator;
+    AVIF_CHECK(avifROStreamReadU32(s, &metadata->baseHdrHeadroomD));      // unsigned int(32) base_hdr_headroom_denominator;
+    AVIF_CHECK(avifROStreamReadU32(s, &metadata->alternateHdrHeadroomN)); // unsigned int(32) alternate_hdr_headroom_numerator;
+    AVIF_CHECK(avifROStreamReadU32(s, &metadata->alternateHdrHeadroomD)); // unsigned int(32) alternate_hdr_headroom_denominator;
 
     for (int c = 0; c < channelCount; ++c) {
-        AVIF_CHECK(avifROStreamReadU32(&s, (uint32_t *)&metadata->gainMapMinN[c])); // int(32) gain_map_min_numerator;
-        AVIF_CHECK(avifROStreamReadU32(&s, &metadata->gainMapMinD[c]));             // unsigned int(32) gain_map_min_denominator;
-        AVIF_CHECK(avifROStreamReadU32(&s, (uint32_t *)&metadata->gainMapMaxN[c])); // int(32) gain_map_max_numerator;
-        AVIF_CHECK(avifROStreamReadU32(&s, &metadata->gainMapMaxD[c]));             // unsigned int(32) gain_map_max_denominator;
-        AVIF_CHECK(avifROStreamReadU32(&s, &metadata->gainMapGammaN[c]));           // unsigned int(32) gamma_numerator;
-        AVIF_CHECK(avifROStreamReadU32(&s, &metadata->gainMapGammaD[c]));           // unsigned int(32) gamma_denominator;
-        AVIF_CHECK(avifROStreamReadU32(&s, (uint32_t *)&metadata->baseOffsetN[c])); // int(32) base_offset_numerator;
-        AVIF_CHECK(avifROStreamReadU32(&s, &metadata->baseOffsetD[c]));             // unsigned int(32) base_offset_denominator;
-        AVIF_CHECK(avifROStreamReadU32(&s, (uint32_t *)&metadata->alternateOffsetN[c])); // int(32) alternate_offset_numerator;
-        AVIF_CHECK(avifROStreamReadU32(&s, &metadata->alternateOffsetD[c])); // unsigned int(32) alternate_offset_denominator;
+        AVIF_CHECK(avifROStreamReadU32(s, (uint32_t *)&metadata->gainMapMinN[c])); // int(32) gain_map_min_numerator;
+        AVIF_CHECK(avifROStreamReadU32(s, &metadata->gainMapMinD[c]));             // unsigned int(32) gain_map_min_denominator;
+        AVIF_CHECK(avifROStreamReadU32(s, (uint32_t *)&metadata->gainMapMaxN[c])); // int(32) gain_map_max_numerator;
+        AVIF_CHECK(avifROStreamReadU32(s, &metadata->gainMapMaxD[c]));             // unsigned int(32) gain_map_max_denominator;
+        AVIF_CHECK(avifROStreamReadU32(s, &metadata->gainMapGammaN[c]));           // unsigned int(32) gamma_numerator;
+        AVIF_CHECK(avifROStreamReadU32(s, &metadata->gainMapGammaD[c]));           // unsigned int(32) gamma_denominator;
+        AVIF_CHECK(avifROStreamReadU32(s, (uint32_t *)&metadata->baseOffsetN[c])); // int(32) base_offset_numerator;
+        AVIF_CHECK(avifROStreamReadU32(s, &metadata->baseOffsetD[c]));             // unsigned int(32) base_offset_denominator;
+        AVIF_CHECK(avifROStreamReadU32(s, (uint32_t *)&metadata->alternateOffsetN[c])); // int(32) alternate_offset_numerator;
+        AVIF_CHECK(avifROStreamReadU32(s, &metadata->alternateOffsetD[c])); // unsigned int(32) alternate_offset_denominator;
     }
 
     // Fill the remaining values by copying those from the first channel.
@@ -2015,8 +1997,38 @@ static avifBool avifParseToneMappedImageBox(avifGainMapMetadata * metadata, cons
         metadata->alternateOffsetN[c] = metadata->alternateOffsetN[0];
         metadata->alternateOffsetD[c] = metadata->alternateOffsetD[0];
     }
+    return AVIF_TRUE;
+}
 
-    return avifROStreamRemainingBytes(&s) == 0;
+// If the gain map's version or minimum_version tag is not supported, returns AVIF_RESULT_NOT_IMPLEMENTED.
+static avifResult avifParseToneMappedImageBox(avifGainMapMetadata * metadata, const uint8_t * raw, size_t rawLen, avifDiagnostics * diag)
+{
+    BEGIN_STREAM(s, raw, rawLen, diag, "Box[tmap]");
+
+    uint8_t version;
+    AVIF_CHECKERR(avifROStreamRead(&s, &version, 1), AVIF_RESULT_INVALID_TONE_MAPPED_IMAGE); // unsigned int(8) version = 0;
+    if (version != 0) {
+        avifDiagnosticsPrintf(diag, "Box[tmap] has unsupported version [%u]", version);
+        return AVIF_RESULT_NOT_IMPLEMENTED;
+    }
+
+    uint16_t minimumVersion;
+    AVIF_CHECKERR(avifROStreamReadU16(&s, &minimumVersion), AVIF_RESULT_INVALID_TONE_MAPPED_IMAGE); // unsigned int(16) minimum_version;
+    const uint16_t supportedMetadataVersion = 0;
+    if (minimumVersion > supportedMetadataVersion) {
+        avifDiagnosticsPrintf(diag, "Box[tmap] has unsupported minimum version [%u]", minimumVersion);
+        return AVIF_RESULT_NOT_IMPLEMENTED;
+    }
+    uint16_t writerVersion;
+    AVIF_CHECKERR(avifROStreamReadU16(&s, &writerVersion), AVIF_RESULT_INVALID_TONE_MAPPED_IMAGE); // unsigned int(16) writer_version;
+    AVIF_CHECKERR(writerVersion >= minimumVersion, AVIF_RESULT_INVALID_TONE_MAPPED_IMAGE);
+
+    AVIF_CHECKERR(avifParseGainMapMetadata(metadata, &s), AVIF_RESULT_INVALID_TONE_MAPPED_IMAGE);
+
+    if (writerVersion <= supportedMetadataVersion) {
+        AVIF_CHECKERR(avifROStreamRemainingBytes(&s) == 0, AVIF_RESULT_INVALID_TONE_MAPPED_IMAGE);
+    }
+    return AVIF_RESULT_OK;
 }
 #endif // AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP
 
@@ -5384,8 +5396,18 @@ avifResult avifDecoderReset(avifDecoder * decoder)
                 decoder->image->gainMap = avifGainMapCreate();
                 AVIF_CHECKERR(decoder->image->gainMap, AVIF_RESULT_OUT_OF_MEMORY);
             }
-            AVIF_CHECKERR(avifParseToneMappedImageBox(&decoder->image->gainMap->metadata, tmapData.data, tmapData.size, data->diag),
-                          AVIF_RESULT_INVALID_TONE_MAPPED_IMAGE);
+            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);
+            }
         }
 #endif // AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP
 

diff --git a/tests/data/README.md b/tests/data/README.md
@@ -501,6 +501,7 @@ exiftool "-icc_profile<=p3.icc" paris_exif_xmp_icc_gainmap_bigendian.jpg
 License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
 
 Source: generated with a modified libavif at https://github.com/maryla-uc/libavif/tree/weirdgainmaps
+by running `./tests/avifgainmaptest --gtest_filter=GainMapTest.CreateGainMapImages ../tests/data/` 
 
 Contains a 4x3 color grid, a 4x3 alpha grid, and a 2x2 gain map grid.
 
@@ -511,6 +512,7 @@ Contains a 4x3 color grid, a 4x3 alpha grid, and a 2x2 gain map grid.
 License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
 
 Source: generated with a modified libavif at https://github.com/maryla-uc/libavif/tree/weirdgainmaps
+by running `./tests/avifgainmaptest --gtest_filter=GainMapTest.CreateGainMapImages ../tests/data/` 
 
 Contains a single color image, single alpha image, and a 2x2 gain map grid.
 
@@ -521,9 +523,59 @@ Contains a single color image, single alpha image, and a 2x2 gain map grid.
 License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
 
 Source: generated with a modified libavif at https://github.com/maryla-uc/libavif/tree/weirdgainmaps
+by running `./tests/avifgainmaptest --gtest_filter=GainMapTest.CreateGainMapImages ../tests/data/` 
 
 Contains a 4x3 color grid, a 4x3 alpha grid, and a single gain map image.
 
+### File [unsupported_gainmap_version.avif](unsupported_gainmap_version.avif)
+
+![](unsupported_gainmap_version.avif)
+
+License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
+
+Source: generated with a modified libavif at https://github.com/maryla-uc/libavif/tree/weirdgainmaps
+by running `./tests/avifgainmaptest --gtest_filter=GainMapTest.CreateGainMapImages ../tests/data/` 
+
+Contains a gain map with the `version` field set to 99 in the tmap box.
+`minimum_version` and `writer_version` are 0.
+
+### File [unsupported_gainmap_minimum_version.avif](unsupported_gainmap_minimum_version.avif)
+
+![](unsupported_gainmap_minimum_version.avif)
+
+License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
+
+Source: generated with a modified libavif at https://github.com/maryla-uc/libavif/tree/weirdgainmaps
+by running `./tests/avifgainmaptest --gtest_filter=GainMapTest.CreateGainMapImages ../tests/data/` 
+
+Contains a gain map with the `minimum_version` field set to 99 in the tmap box.
+`version` and `writer_version` are 0.
+
+### File [unsupported_gainmap_version.avif](unsupported_gainmap_version.avif)
+
+![](unsupported_gainmap_version.avif)
+
+License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
+
+Source: generated with a modified libavif at https://github.com/maryla-uc/libavif/tree/weirdgainmaps
+by running `./tests/avifgainmaptest --gtest_filter=GainMapTest.CreateGainMapImages ../tests/data/` 
+
+Contains a gain map with the `writer_version` field set to 99 in the tmap box,
+and some extra unexpected bytes at the end of the gain map metadata.
+`version` and `minimum_version` are 0.
+
+### File [supported_gainmap_writer_version_with_extra_bytes.avif](supported_gainmap_writer_version_with_extra_bytes.avif)
+
+![](supported_gainmap_writer_version_with_extra_bytes.avif)
+
+License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
+
+Source: generated with a modified libavif at https://github.com/maryla-uc/libavif/tree/weirdgainmaps
+by running `./tests/avifgainmaptest --gtest_filter=GainMapTest.CreateGainMapImages ../tests/data/` 
+
+Contains a gain map with some extra unexpected bytes at the end of the gain map metadata.
+Contains `version`, `minimum_version` and `writer_version` are 0.
+
 ### File [seine_hdr_srgb.avif](seine_hdr_srgb.avif)
 
 ![](seine_hdr_srgb.avif)

diff --git a/tests/data/supported_gainmap_writer_version_with_extra_bytes.avif b/tests/data/supported_gainmap_writer_version_with_extra_bytes.avif
diff --git a/tests/data/unsupported_gainmap_minimum_version.avif b/tests/data/unsupported_gainmap_minimum_version.avif
diff --git a/tests/data/unsupported_gainmap_version.avif b/tests/data/unsupported_gainmap_version.avif
diff --git a/tests/data/unsupported_gainmap_writer_version_with_extra_bytes.avif b/tests/data/unsupported_gainmap_writer_version_with_extra_bytes.avif
diff --git a/tests/gtest/avifgainmaptest.cc b/tests/gtest/avifgainmaptest.cc
@@ -864,6 +864,98 @@ TEST(GainMapTest, DecodeColorNoGridGainMapGrid) {
   EXPECT_EQ(decoded->gainMap->metadata.baseHdrHeadroomD, 2u);
 }
 
+TEST(GainMapTest, DecodeUnsupportedVersion) {
+  // The two test files should produce the same results:
+  // One has an unsupported 'version' field, the other an unsupported
+  // 'minimum_version' field, but the behavior of these two fiels is the same.
+  for (const std::string image : {"unsupported_gainmap_version.avif",
+                                  "unsupported_gainmap_minimum_version.avif"}) {
+    SCOPED_TRACE(image);
+    const std::string path = std::string(data_path) + image;
+    ImagePtr decoded(avifImageCreateEmpty());
+    ASSERT_NE(decoded, nullptr);
+    DecoderPtr decoder(avifDecoderCreate());
+    ASSERT_NE(decoder, nullptr);
+
+    // Parse with various enableDecodingGainMap and enableParsingGainMapMetadata
+    // settings.
+
+    decoder->enableDecodingGainMap = false;
+    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);
+    ASSERT_EQ(decoded->gainMap, nullptr);
+
+    ASSERT_EQ(avifDecoderReset(decoder.get()), AVIF_RESULT_OK);
+    decoder->enableDecodingGainMap = false;
+    decoder->enableParsingGainMapMetadata = true;
+    ASSERT_EQ(avifDecoderReadFile(decoder.get(), decoded.get(), path.c_str()),
+              AVIF_RESULT_OK);
+    // Gain map marked as not present because the metadata is not supported.
+    EXPECT_EQ(decoder->gainMapPresent, false);
+    ASSERT_EQ(decoded->gainMap, nullptr);
+
+    ASSERT_EQ(avifDecoderReset(decoder.get()), AVIF_RESULT_OK);
+    decoder->enableDecodingGainMap = true;
+    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);
+    ASSERT_NE(decoded->gainMap, nullptr);
+
+    ASSERT_EQ(avifDecoderReset(decoder.get()), AVIF_RESULT_OK);
+    decoder->enableDecodingGainMap = true;
+    decoder->enableParsingGainMapMetadata = true;
+    ASSERT_EQ(avifDecoderReadFile(decoder.get(), decoded.get(), path.c_str()),
+              AVIF_RESULT_OK);
+    // Gain map marked as not present because the metadata is not supported.
+    EXPECT_EQ(decoder->gainMapPresent, false);
+    ASSERT_EQ(decoded->gainMap, nullptr);
+  }
+}
+
+TEST(GainMapTest, ExtraBytesAfterGainMapMetadataUnsupportedWriterVersion) {
+  const std::string path =
+      std::string(data_path) +
+      "unsupported_gainmap_writer_version_with_extra_bytes.avif";
+  ImagePtr decoded(avifImageCreateEmpty());
+  ASSERT_NE(decoded, nullptr);
+  DecoderPtr decoder(avifDecoderCreate());
+  ASSERT_NE(decoder, nullptr);
+
+  decoder->enableDecodingGainMap = false;
+  decoder->enableParsingGainMapMetadata = true;
+  ASSERT_EQ(avifDecoderReadFile(decoder.get(), decoded.get(), path.c_str()),
+            AVIF_RESULT_OK);
+  // Decodes successfully: there are extra bytes at the end of the gain map
+  // metadata but that's expected as the writer_version field is higher
+  // that supported.
+  EXPECT_EQ(decoder->gainMapPresent, true);
+  ASSERT_NE(decoded->gainMap, nullptr);
+}
+
+TEST(GainMapTest, ExtraBytesAfterGainMapMetadataSupporterWriterVersion) {
+  const std::string path =
+      std::string(data_path) +
+      "supported_gainmap_writer_version_with_extra_bytes.avif";
+  ImagePtr decoded(avifImageCreateEmpty());
+  ASSERT_NE(decoded, nullptr);
+  DecoderPtr decoder(avifDecoderCreate());
+  ASSERT_NE(decoder, nullptr);
+
+  decoder->enableDecodingGainMap = false;
+  decoder->enableParsingGainMapMetadata = true;
+  // Fails to decode: there are extra bytes at the end of the gain map metadata
+  // that shouldn't be there.
+  ASSERT_EQ(avifDecoderReadFile(decoder.get(), decoded.get(), path.c_str()),
+            AVIF_RESULT_INVALID_TONE_MAPPED_IMAGE);
+}
+
 #define EXPECT_FRACTION_NEAR(numerator, denominator, expected)     \
   EXPECT_NEAR(std::abs((double)numerator / denominator), expected, \
               expected * 0.001);