Fix setting of cicp values in avifgainmaputil combine and convert. (#3110)

CICP must be set *before* calling ReadImage() so that the RGB->YUV
conversion uses the right matrix coefficient for PNG/JPEG files.
Also explicitly set the default matrix coefficients when not specified.

For Avif files, we overwrite the CICP after reading the file in ReadImage()
(keeps the previous behavior).

Fix --cicp-input flag not being used in tonemap command.

Fix memory leak when in 'combine' command when using with a file that already has a gain map.
Fix memory leak in 'tonemap' and 'swapbase' command with RGB pixels.

Fixes #3102 and #2869

Author: maryla-uc

CHANGELOG.md

@@ -15,6 +15,8 @@ The changes are relative to the previous release, unless the baseline is specifi
 * Update libxml2.cmd/LocalLibXml2.cmake: v2.15.2
 * Update libyuv.cmd/LocalLibyuv.cmake: 6067afde5 (1922)
 * Support long path names in Windows
+* Fix cicp management and memory leaks in avifgainmaputil
+  https://github.com/AOMediaCodec/libavif/issues/3102.
 
 ### Removed since 1.4.0
 

apps/avifgainmaputil/combine_command.cc

@@ -3,6 +3,7 @@
 
 #include "combine_command.h"
 
+#include <cassert>
 #include <cmath>
 
 #include "avif/avif_cxx.h"
@@ -88,29 +89,21 @@ avifResult CombineCommand::Run() {
   if (base_image == nullptr || alternate_image == nullptr) {
     return AVIF_RESULT_OUT_OF_MEMORY;
   }
-  avifResult result =
-      ReadImage(base_image.get(), arg_base_filename_, pixel_format,
-                arg_image_read_.depth, arg_image_read_.ignore_profile);
-  if (result != AVIF_RESULT_OK) {
-    std::cout << "Failed to read base image: " << avifResultToString(result)
-              << "\n";
-    return result;
-  }
   if (arg_base_cicp_.provenance() == argparse::Provenance::SPECIFIED) {
     base_image->colorPrimaries = arg_base_cicp_.value().color_primaries;
     base_image->transferCharacteristics =
         arg_base_cicp_.value().transfer_characteristics;
     base_image->matrixCoefficients = arg_base_cicp_.value().matrix_coefficients;
   }
-
-  result =
-      ReadImage(alternate_image.get(), arg_alternate_filename_, pixel_format,
-                arg_image_read_.depth, arg_image_read_.ignore_profile);
+  avifResult result = ReadImage(
+      base_image.get(), arg_base_filename_, pixel_format, arg_image_read_.depth,
+      arg_image_read_.ignore_profile, /*ignore_gain_map=*/true);
   if (result != AVIF_RESULT_OK) {
-    std::cout << "Failed to read alternate image: "
-              << avifResultToString(result) << "\n";
+    std::cout << "Failed to read base image: " << avifResultToString(result)
+              << "\n";
     return result;
   }
+
   if (arg_alternate_cicp_.provenance() == argparse::Provenance::SPECIFIED) {
     alternate_image->colorPrimaries =
         arg_alternate_cicp_.value().color_primaries;
@@ -119,6 +112,14 @@ avifResult CombineCommand::Run() {
     alternate_image->matrixCoefficients =
         arg_alternate_cicp_.value().matrix_coefficients;
   }
+  result =
+      ReadImage(alternate_image.get(), arg_alternate_filename_, pixel_format,
+                arg_image_read_.depth, arg_image_read_.ignore_profile);
+  if (result != AVIF_RESULT_OK) {
+    std::cout << "Failed to read alternate image: "
+              << avifResultToString(result) << "\n";
+    return result;
+  }
 
   const uint32_t downscaling = std::max<int>(1, arg_downscaling_);
   const uint32_t rounding = downscaling / 2;
@@ -129,6 +130,10 @@ avifResult CombineCommand::Run() {
   std::cout << "Creating a gain map of size " << gain_map_width << " x "
             << gain_map_height << "\n";
 
+  // Because base_image is read with ignore_gain_map=true, there is no
+  // preexisting gain map. Otherwise, overwriting the pointer would cause a
+  // memory leak.
+  assert(base_image->gainMap == nullptr);
   base_image->gainMap = avifGainMapCreate();
   base_image->gainMap->image =
       avifImageCreate(gain_map_width, gain_map_height, arg_gain_map_depth_,

apps/avifgainmaputil/convert_command.cc

@@ -52,32 +52,19 @@ avifResult ConvertCommand::Run() {
     return AVIF_RESULT_OUT_OF_MEMORY;
   }
 
-  const avifAppFileFormat file_format = avifReadImage(
-      arg_input_filename_.value().c_str(),
-      AVIF_APP_FILE_FORMAT_UNKNOWN /* guess format */, pixel_format,
-      arg_image_read_.depth, AVIF_CHROMA_DOWNSAMPLING_AUTOMATIC,
-      arg_image_read_.ignore_profile,
-      /*ignoreExif=*/false,
-      /*ignoreXMP=*/false,
-      /*ignoreGainMap=*/false, AVIF_DEFAULT_IMAGE_SIZE_LIMIT, image.get(),
-      /*outDepth=*/nullptr,
-      /*sourceTiming=*/nullptr,
-      /*frameIter=*/nullptr);
-  if (file_format == AVIF_APP_FILE_FORMAT_UNKNOWN) {
-    std::cout << "Failed to decode image: " << arg_input_filename_;
-    return AVIF_RESULT_INVALID_ARGUMENT;
-  }
   if (arg_cicp_.provenance() == argparse::Provenance::SPECIFIED) {
     image->colorPrimaries = arg_cicp_.value().color_primaries;
     image->transferCharacteristics = arg_cicp_.value().transfer_characteristics;
     image->matrixCoefficients = arg_cicp_.value().matrix_coefficients;
-  } else if (image->icc.size == 0 &&
-             image->colorPrimaries == AVIF_COLOR_PRIMARIES_UNSPECIFIED &&
-             image->transferCharacteristics ==
-                 AVIF_COLOR_PRIMARIES_UNSPECIFIED) {
-    // If there is no ICC and no CICP, assume sRGB by default.
-    image->colorPrimaries = AVIF_COLOR_PRIMARIES_SRGB;
-    image->transferCharacteristics = AVIF_TRANSFER_CHARACTERISTICS_SRGB;
+  }
+
+  avifResult result =
+      ReadImage(image.get(), arg_input_filename_.value(), pixel_format,
+                arg_image_read_.depth, arg_image_read_.ignore_profile,
+                /*ignore_gain_map*/ false);
+  if (result != AVIF_RESULT_OK) {
+    std::cout << "Failed to decode image: " << arg_input_filename_;
+    return result;
   }
 
   if (image->gainMap && image->gainMap->altICC.size == 0) {
@@ -110,8 +97,7 @@ avifResult ConvertCommand::Run() {
     if (new_base == nullptr) {
       return AVIF_RESULT_OUT_OF_MEMORY;
     }
-    const avifResult result =
-        ChangeBase(*image, depth, image->yuvFormat, new_base.get());
+    result = ChangeBase(*image, depth, image->yuvFormat, new_base.get());
     if (result != AVIF_RESULT_OK) {
       return result;
     }
@@ -126,10 +112,9 @@ avifResult ConvertCommand::Run() {
   encoder->qualityAlpha = arg_image_encode_.quality_alpha;
   encoder->qualityGainMap = arg_gain_map_quality_;
   encoder->speed = arg_image_encode_.speed;
-  const avifResult result =
-      WriteAvifGrid(image.get(), arg_image_encode_.grid.value().grid_cols,
-                    arg_image_encode_.grid.value().grid_rows, encoder.get(),
-                    arg_output_filename_);
+  result = WriteAvifGrid(image.get(), arg_image_encode_.grid.value().grid_cols,
+                         arg_image_encode_.grid.value().grid_rows,
+                         encoder.get(), arg_output_filename_);
   if (result != AVIF_RESULT_OK) {
     std::cout << "Failed to encode image: " << avifResultToString(result)
               << " (" << encoder->diag.error << ")\n";

apps/avifgainmaputil/imageio.cc

@@ -155,7 +155,7 @@ avifResult WriteAvifGrid(const avifImage* image, int grid_cols, int grid_rows,
 
 avifResult ReadImage(avifImage* image, const std::string& input_filename,
                      avifPixelFormat requested_format, uint32_t requested_depth,
-                     bool ignore_profile) {
+                     bool ignore_profile, bool ignore_gain_map) {
   avifAppFileFormat input_format = avifGuessFileFormat(input_filename.c_str());
   if (input_format == AVIF_APP_FILE_FORMAT_UNKNOWN) {
     std::cerr << "Cannot determine input format: " << input_filename;
@@ -165,10 +165,21 @@ avifResult ReadImage(avifImage* image, const std::string& input_filename,
     if (decoder == nullptr) {
       return AVIF_RESULT_OUT_OF_MEMORY;
     }
+    if (!ignore_gain_map) {
+      decoder->imageContentToDecode |= AVIF_IMAGE_CONTENT_GAIN_MAP;
+    }
     avifResult result = ReadAvif(decoder.get(), input_filename, ignore_profile);
     if (result != AVIF_RESULT_OK) {
       return result;
     }
+    if (ignore_gain_map && decoder->image->gainMap) {
+      avifGainMapDestroy(decoder->image->gainMap);
+      decoder->image->gainMap = nullptr;
+    }
+    const avifColorPrimaries in_primaries = image->colorPrimaries;
+    const avifTransferCharacteristics in_transfer =
+        image->transferCharacteristics;
+    const avifMatrixCoefficients in_matrix = image->matrixCoefficients;
     if (decoder->image->imageOwnsYUVPlanes &&
         (decoder->image->alphaPlane == nullptr ||
          decoder->image->imageOwnsAlphaPlane)) {
@@ -179,26 +190,37 @@ avifResult ReadImage(avifImage* image, const std::string& input_filename,
         return result;
       }
     }
+    if (in_primaries != AVIF_COLOR_PRIMARIES_UNSPECIFIED ||
+        in_transfer != AVIF_TRANSFER_CHARACTERISTICS_UNSPECIFIED ||
+        in_matrix != AVIF_MATRIX_COEFFICIENTS_UNSPECIFIED) {
+      image->colorPrimaries = in_primaries;
+      image->transferCharacteristics = in_transfer;
+      image->matrixCoefficients = in_matrix;
+    }
   } else {
     const avifAppFileFormat file_format = avifReadImage(
         input_filename.c_str(), AVIF_APP_FILE_FORMAT_UNKNOWN /* guess format */,
         requested_format, static_cast<int>(requested_depth),
         AVIF_CHROMA_DOWNSAMPLING_AUTOMATIC, ignore_profile,
-        /*ignoreExif=*/false, /*ignoreXMP=*/false,
-        /*ignoreGainMap=*/true, AVIF_DEFAULT_IMAGE_SIZE_LIMIT, image,
+        /*ignoreExif=*/false, /*ignoreXMP=*/false, ignore_gain_map,
+        AVIF_DEFAULT_IMAGE_SIZE_LIMIT, image,
         /*outDepth=*/nullptr,
         /*sourceTiming=*/nullptr, /*frameIter=*/nullptr);
     if (file_format == AVIF_APP_FILE_FORMAT_UNKNOWN) {
       std::cout << "Failed to decode image: " << input_filename;
       return AVIF_RESULT_INVALID_ARGUMENT;
     }
-    if (image->icc.size == 0) {
-      // Assume sRGB by default.
-      if (image->colorPrimaries == AVIF_COLOR_PRIMARIES_UNSPECIFIED &&
-          image->transferCharacteristics == AVIF_COLOR_PRIMARIES_UNSPECIFIED) {
-        image->colorPrimaries = AVIF_COLOR_PRIMARIES_SRGB;
-        image->transferCharacteristics = AVIF_TRANSFER_CHARACTERISTICS_SRGB;
-      }
+    // Assume sRGB by default.
+    if (image->icc.size == 0 &&
+        image->colorPrimaries == AVIF_COLOR_PRIMARIES_UNSPECIFIED &&
+        image->transferCharacteristics == AVIF_COLOR_PRIMARIES_UNSPECIFIED) {
+      image->colorPrimaries = AVIF_COLOR_PRIMARIES_SRGB;
+      image->transferCharacteristics = AVIF_TRANSFER_CHARACTERISTICS_SRGB;
+    }
+    if (image->matrixCoefficients == AVIF_MATRIX_COEFFICIENTS_UNSPECIFIED) {
+      // Explicitly set the default matrix coefficient, see
+      // avifCalcYUVCoefficients().
+      image->matrixCoefficients = AVIF_MATRIX_COEFFICIENTS_BT601;
     }
   }
   return AVIF_RESULT_OK;

apps/avifgainmaputil/imageio.h

@@ -19,7 +19,7 @@ avifResult WriteImage(const avifImage* image, int grid_cols, int grid_rows,
 // Reads an image in any of the supported formats. Ignores any gain map.
 avifResult ReadImage(avifImage* image, const std::string& input_filename,
                      avifPixelFormat requested_format, uint32_t requested_depth,
-                     bool ignore_profile);
+                     bool ignore_profile, bool ignore_gain_map = true);
 
 // Reads an image in avif format given a pre-configured encoder.
 avifResult WriteAvif(const avifImage* image, avifEncoder* encoder,

apps/avifgainmaputil/swapbase_command.cc

@@ -54,6 +54,7 @@ avifResult ChangeBase(const avifImage& image, int depth,
   }
 
   avifRGBImage swapped_rgb;
+  RGBImageCleanup rgb_cleanup(&swapped_rgb);
   avifRGBImageSetDefaults(&swapped_rgb, swapped);
 
   avifContentLightLevelInformationBox clli = image.gainMap->altCLLI;

apps/avifgainmaputil/tonemap_command.cc

@@ -69,6 +69,13 @@ avifResult TonemapCommand::Run() {
   if (result != AVIF_RESULT_OK) {
     return result;
   }
+  if (arg_input_cicp_.provenance() == argparse::Provenance::SPECIFIED) {
+    decoder->image->colorPrimaries = arg_input_cicp_.value().color_primaries;
+    decoder->image->transferCharacteristics =
+        arg_input_cicp_.value().transfer_characteristics;
+    decoder->image->matrixCoefficients =
+        arg_input_cicp_.value().matrix_coefficients;
+  }
 
   avifImage* image = decoder->image;
   if (image->gainMap == nullptr || image->gainMap->image == nullptr) {
@@ -115,10 +122,9 @@ avifResult TonemapCommand::Run() {
             image->gainMap->altMatrixCoefficients};
   }
   if (cicp.color_primaries == AVIF_COLOR_PRIMARIES_UNSPECIFIED) {
-    // TODO(maryla): for now avifImageApplyGainMap always uses the primaries of
-    // the base image, but it should take into account the metadata's
-    // useBaseColorSpace property.
-    cicp.color_primaries = image->colorPrimaries;
+    cicp.color_primaries = image->gainMap->useBaseColorSpace
+                               ? image->colorPrimaries
+                               : image->gainMap->altColorPrimaries;
   }
   if (cicp.transfer_characteristics ==
       AVIF_TRANSFER_CHARACTERISTICS_UNSPECIFIED) {
@@ -181,7 +187,13 @@ avifResult TonemapCommand::Run() {
   if (tone_mapped == nullptr) {
     return AVIF_RESULT_OUT_OF_MEMORY;
   }
+  tone_mapped->colorPrimaries = cicp.color_primaries;
+  tone_mapped->transferCharacteristics = cicp.transfer_characteristics;
+  tone_mapped->matrixCoefficients = cicp.matrix_coefficients;
+  tone_mapped->clli = clli_box;
+
   avifRGBImage tone_mapped_rgb;
+  RGBImageCleanup rgb_cleanup(&tone_mapped_rgb);
   avifRGBImageSetDefaults(&tone_mapped_rgb, tone_mapped.get());
   avifDiagnostics diag;
   result = avifImageApplyGainMap(
@@ -200,11 +212,6 @@ avifResult TonemapCommand::Run() {
     return result;
   }
 
-  tone_mapped->clli = clli_box;
-  tone_mapped->transferCharacteristics = cicp.transfer_characteristics;
-  tone_mapped->colorPrimaries = cicp.color_primaries;
-  tone_mapped->matrixCoefficients = cicp.matrix_coefficients;
-
   return WriteImage(tone_mapped.get(), arg_image_encode_.grid.value().grid_cols,
                     arg_image_encode_.grid.value().grid_rows,
                     arg_output_filename_, arg_image_encode_.quality,

tests/CMakeLists.txt

@@ -267,10 +267,9 @@ if(AVIF_BUILD_APPS)
     add_cmd_test(test_cmd_stdin ${CMAKE_CURRENT_SOURCE_DIR}/data)
     add_cmd_test(test_cmd_targetsize ${CMAKE_CURRENT_SOURCE_DIR}/data)
     add_cmd_test(test_cmd_transform ${CMAKE_CURRENT_SOURCE_DIR}/data)
+    add_cmd_test(test_cmd_avifgainmaputil ${CMAKE_CURRENT_SOURCE_DIR}/data)
 
-    if(AVIF_ENABLE_JPEG_GAIN_MAP_CONVERSION)
-        add_cmd_test(test_cmd_avifgainmaputil ${CMAKE_CURRENT_SOURCE_DIR}/data)
-    else()
+    if(NOT AVIF_ENABLE_JPEG_GAIN_MAP_CONVERSION)
         set_tests_properties(test_cmd_stdin PROPERTIES DISABLED True)
     endif()
 
@@ -361,6 +360,7 @@ if(AVIF_CODEC_AVM_ENABLED)
                 avifrangetest
                 aviftunetest
                 avify4mtest
+                test_cmd_avifgainmaputil
                 PROPERTIES DISABLED True
             )
 

tests/data/README.md

@@ -823,6 +823,18 @@ after changing `kUpdateTestImages` to true in the `avifgainmaptest.cc`.
 
 SDR image with a gain map to allow tone mapping to HDR. The gain map's width and height are halved compared to the base image.
 
+### File [seine_sdr_gainmap_srgb_icc.avif](seine_sdr_gainmap_srgb_icc.avif)
+
+![](seine_sdr_gainmap_srgb_icc.avif)
+
+License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
+
+Source : created by running the following command with avifenc compiled with libxml enabled:
+
+```bash
+avifenc seine_sdr_gainmap_srgb.jpg seine_sdr_gainmap_srgb_icc.avif --qcolor 90 --qgain-map 90
+```
+
 ## Files colors*_hdr_*.avif and colors*_sdr_srgb.avif
 
 ![](colors_wcg_hdr_rec2020.avif)