Correctly set alt image metadata in various places. (#1820)

Update test images. Print alt image metadata where relevant.
AOMediaCodec · Dec 4, 2023 · 3bcc782 · 3bcc782
1 parent c2fb27e
commit 3bcc782
Show file tree

Hide file tree

Showing 14 changed files with 97 additions and 54 deletions.
diff --git a/apps/avifenc.c b/apps/avifenc.c
@@ -2285,6 +2285,11 @@ MAIN()
         // image didn't provide any CICP. Explicitly signal SRGB CP/TC here, as 2/2/x will be
         // interpreted as SRGB anyway.
         image->colorPrimaries = AVIF_COLOR_PRIMARIES_SRGB;
+#if defined(AVIF_ENABLE_EXPERIMENTAL_JPEG_GAIN_MAP_CONVERSION)
+        if (image->gainMap && image->gainMap->image) {
+            image->gainMap->altColorPrimaries = AVIF_COLOR_PRIMARIES_SRGB;
+        }
+#endif
         image->transferCharacteristics = AVIF_TRANSFER_CHARACTERISTICS_SRGB;
     }
 

diff --git a/apps/avifgainmaputil/convert_command.cc b/apps/avifgainmaputil/convert_command.cc
@@ -64,6 +64,7 @@ avifResult ConvertCommand::Run() {
     // If there is no ICC and no CICP, assume sRGB by default.
     image->colorPrimaries = AVIF_COLOR_PRIMARIES_SRGB;
     image->transferCharacteristics = AVIF_TRANSFER_CHARACTERISTICS_SRGB;
+    image->gainMap->altColorPrimaries = AVIF_COLOR_PRIMARIES_SRGB;
   }
 
   if (image->gainMap == nullptr || image->gainMap->image == nullptr) {
@@ -77,9 +78,12 @@ avifResult ConvertCommand::Run() {
     if (depth == 0) {
       depth = image->gainMap->metadata.alternateHdrHeadroomN == 0 ? 8 : 10;
     }
-    ImagePtr new_base(
-        avifImageCreate(image->width, image->height, depth, image->yuvFormat));
-    const avifResult result = ChangeBase(*image, new_base.get());
+    ImagePtr new_base(avifImageCreateEmpty());
+    if (new_base == nullptr) {
+      return AVIF_RESULT_OUT_OF_MEMORY;
+    }
+    const avifResult result =
+        ChangeBase(*image, depth, image->yuvFormat, new_base.get());
     if (result != AVIF_RESULT_OK) {
       return result;
     }

diff --git a/apps/avifgainmaputil/swapbase_command.cc b/apps/avifgainmaputil/swapbase_command.cc
@@ -8,11 +8,20 @@
 
 namespace avif {
 
-avifResult ChangeBase(avifImage& image, avifImage* swapped) {
+avifResult ChangeBase(const avifImage& image, int depth,
+                      avifPixelFormat yuvFormat, avifImage* swapped) {
   if (image.gainMap == nullptr || image.gainMap->image == nullptr) {
     return AVIF_RESULT_INVALID_ARGUMENT;
   }
 
+  // Copy all metadata (no planes).
+  avifResult result = avifImageCopy(swapped, &image, /*planes=*/0);
+  if (result != AVIF_RESULT_OK) {
+    return result;
+  }
+  swapped->depth = depth;
+  swapped->yuvFormat = yuvFormat;
+
   const float headroom =
       static_cast<double>(image.gainMap->metadata.alternateHdrHeadroomN) /
       image.gainMap->metadata.alternateHdrHeadroomD;
@@ -48,9 +57,9 @@ avifResult ChangeBase(avifImage& image, avifImage* swapped) {
       !tone_mapping_to_sdr && clli.maxCLL == 0 && clli.maxPALL == 0;
 
   avifDiagnostics diag;
-  avifResult result = avifImageApplyGainMap(
-      &image, image.gainMap, headroom, swapped->transferCharacteristics,
-      &swapped_rgb, (compute_clli ? &clli : nullptr), &diag);
+  result = avifImageApplyGainMap(&image, image.gainMap, headroom,
+                                 swapped->transferCharacteristics, &swapped_rgb,
+                                 (compute_clli ? &clli : nullptr), &diag);
   if (result != AVIF_RESULT_OK) {
     std::cout << "Failed to tone map image: " << avifResultToString(result)
               << " (" << diag.error << ")\n";
@@ -64,10 +73,13 @@ avifResult ChangeBase(avifImage& image, avifImage* swapped) {
   }
 
   swapped->clli = clli;
-  swapped->gainMap = image.gainMap;
-  // 'swapped' has taken ownership of the gain map, so remove ownership
-  // from the old image to prevent a double free.
-  image.gainMap = nullptr;
+  // Copy the gain map's planes.
+  result = avifImageCopy(swapped->gainMap->image, image.gainMap->image,
+                         AVIF_PLANES_YUV);
+  if (result != AVIF_RESULT_OK) {
+    return result;
+  }
+
   // Fill in the information on the alternate image
   result =
       avifRWDataSet(&swapped->gainMap->altICC, image.icc.data, image.icc.size);
@@ -94,10 +106,6 @@ avifResult ChangeBase(avifImage& image, avifImage* swapped) {
     std::swap(metadata.baseOffsetD, metadata.alternateOffsetD);
   }
 
-  // Steal metadata.
-  std::swap(swapped->xmp, image.xmp);
-  std::swap(swapped->exif, image.exif);
-
   return AVIF_RESULT_OK;
 }
 
@@ -129,28 +137,35 @@ avifResult SwapBaseCommand::Run() {
     return result;
   }
 
-  if (decoder->image->gainMap == nullptr ||
-      decoder->image->gainMap->image == nullptr) {
+  const avifImage* image = decoder->image;
+  if (image->gainMap == nullptr || image->gainMap->image == nullptr) {
     std::cerr << "Input image " << arg_input_filename_
               << " does not contain a gain map\n";
     return AVIF_RESULT_INVALID_ARGUMENT;
   }
 
   int depth = arg_image_read_.depth;
   if (depth == 0) {
-    depth = decoder->image->gainMap->metadata.alternateHdrHeadroomN == 0
-                ? 8
-                : std::max(decoder->image->depth,
-                           decoder->image->gainMap->image->depth);
+    depth = image->gainMap->altDepth;
+  }
+  if (depth == 0) {
+    // Default to the max depth between the base image and the gain map/
+    depth = std::max(image->depth, image->gainMap->image->depth);
   }
+
   avifPixelFormat pixel_format =
       (avifPixelFormat)arg_image_read_.pixel_format.value();
   if (pixel_format == AVIF_PIXEL_FORMAT_NONE) {
-    pixel_format = AVIF_PIXEL_FORMAT_YUV444;
+    pixel_format = (image->gainMap->altPlaneCount == 1)
+                       ? AVIF_PIXEL_FORMAT_YUV420
+                       : AVIF_PIXEL_FORMAT_YUV444;
+  }
+
+  ImagePtr new_base(avifImageCreateEmpty());
+  if (new_base == nullptr) {
+    return AVIF_RESULT_OUT_OF_MEMORY;
   }
-  ImagePtr new_base(avifImageCreate(
-      decoder->image->width, decoder->image->height, depth, pixel_format));
-  result = ChangeBase(*decoder->image, new_base.get());
+  result = ChangeBase(*image, depth, pixel_format, new_base.get());
   if (result != AVIF_RESULT_OK) {
     return result;
   }

diff --git a/apps/avifgainmaputil/swapbase_command.h b/apps/avifgainmaputil/swapbase_command.h
@@ -10,9 +10,9 @@
 namespace avif {
 
 // Given an 'image' with a gain map, tone maps it to get the "alternate" image,
-// and saves it to 'output'. Also steals the gain map of 'image' to give it to
-// 'output'. To avoid unnecessary copies, the input 'image' is modified.
-avifResult ChangeBase(avifImage& image, avifImage* output);
+// and saves it to 'output'.
+avifResult ChangeBase(const avifImage& image, int depth,
+                      avifPixelFormat yuvFormat, avifImage* output);
 
 class SwapBaseCommand : public ProgramCommand {
  public:

diff --git a/apps/shared/avifjpeg.c b/apps/shared/avifjpeg.c
@@ -1143,6 +1143,9 @@ static avifBool avifJPEGReadInternal(FILE * f,
             gainMap->altColorPrimaries = avif->colorPrimaries;
             gainMap->altTransferCharacteristics = AVIF_TRANSFER_CHARACTERISTICS_PQ;
             gainMap->altMatrixCoefficients = avif->matrixCoefficients;
+            gainMap->altDepth = 8;
+            gainMap->altPlaneCount =
+                (avif->yuvFormat == AVIF_PIXEL_FORMAT_YUV400 && gainMap->image->yuvFormat == AVIF_PIXEL_FORMAT_YUV400) ? 1 : 3;
             avif->gainMap = gainMap;
         } else {
             avifGainMapDestroy(gainMap);

diff --git a/apps/shared/avifutil.c b/apps/shared/avifutil.c
@@ -132,21 +132,28 @@ static void avifImageDumpInternal(const avifImage * avif,
 
 #if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP)
     printf(" * Gain map       : ");
-    avifImage * gainMap = avif->gainMap ? avif->gainMap->image : NULL;
-    if (gainMap != NULL) {
+    avifImage * gainMapImage = avif->gainMap ? avif->gainMap->image : NULL;
+    if (gainMapImage != NULL) {
         printf("%ux%u pixels, %u bit, %s, %s Range, Matrix Coeffs. %u, Base Image is %s\n",
-               gainMap->width,
-               gainMap->height,
-               gainMap->depth,
-               avifPixelFormatToString(gainMap->yuvFormat),
-               (gainMap->yuvRange == AVIF_RANGE_FULL) ? "Full" : "Limited",
-               gainMap->matrixCoefficients,
+               gainMapImage->width,
+               gainMapImage->height,
+               gainMapImage->depth,
+               avifPixelFormatToString(gainMapImage->yuvFormat),
+               (gainMapImage->yuvRange == AVIF_RANGE_FULL) ? "Full" : "Limited",
+               gainMapImage->matrixCoefficients,
                (avif->gainMap->metadata.baseHdrHeadroomN == 0) ? "SDR" : "HDR");
-        printf("    * Color Primaries: %u\n", gainMap->colorPrimaries);
-        printf("    * Transfer Char. : %u\n", gainMap->transferCharacteristics);
-        printf("    * Matrix Coeffs. : %u\n", gainMap->matrixCoefficients);
-        if (gainMap->clli.maxCLL > 0 || gainMap->clli.maxPALL > 0) {
-            printf("    * CLLI           : %hu, %hu\n", gainMap->clli.maxCLL, gainMap->clli.maxPALL);
+        printf(" * Alternate image:\n");
+        printf("    * Color Primaries: %u\n", avif->gainMap->altColorPrimaries);
+        printf("    * Transfer Char. : %u\n", avif->gainMap->altTransferCharacteristics);
+        printf("    * Matrix Coeffs. : %u\n", avif->gainMap->altMatrixCoefficients);
+        if (avif->gainMap->altDepth) {
+            printf("    * Bit Depth      : %u\n", avif->gainMap->altDepth);
+        }
+        if (avif->gainMap->altPlaneCount) {
+            printf("    * Planes         : %u\n", avif->gainMap->altPlaneCount);
+        }
+        if (gainMapImage->clli.maxCLL > 0 || gainMapImage->clli.maxPALL > 0) {
+            printf("    * CLLI           : %hu, %hu\n", gainMapImage->clli.maxCLL, gainMapImage->clli.maxPALL);
         }
         printf("\n");
     } else if (gainMapPresent) {

diff --git a/tests/data/README.md b/tests/data/README.md
@@ -505,7 +505,7 @@ SDR image with a gain map to allow tone mapping to HDR.
 ![](seine_sdr_gainmap_big_srgb.avif)
 
 Source : modified version of `seine_sdr_gainmap_srgb.avif` with an upscaled gain map, generated using libavif's API.
-See `CreateTestImages` in `avifgainmaptest.cc`.
+See `CreateTestImages` in `avifgainmaptest.cc` (set kUpdateTestImages to update images).
 
 SDR image with a gain map to allow tone mapping to HDR. The gain map's width and height are doubled compared to the base image.
 This is an atypical image just for testing. Typically, the gain map would be either the same size or smaller as the base image.
@@ -517,7 +517,7 @@ This is an atypical image just for testing. Typically, the gain map would be eit
 License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
 
 Source : created from `seine_hdr_srgb.avif` (for the base image) and `seine_sdr_gainmap_srgb.avif` (for the gain map) with libavif's API.
-See `CreateTestImages` in `avifgainmaptest.cc`.
+See `CreateTestImages` in `avifgainmaptest.cc` (set kUpdateTestImages to update images).
 
 HDR image with a gain map to allow tone mapping to SDR.
 
@@ -528,7 +528,7 @@ HDR image with a gain map to allow tone mapping to SDR.
 License: [same as libavif](https://github.com/AOMediaCodec/libavif/blob/main/LICENSE)
 
 Source : modified version of `seine_hdr_gainmap_srgb.avif` with a downscaled gain map, generated using libavif's API.
-See `CreateTestImages` in `avifgainmaptest.cc`.
+See `CreateTestImages` in `avifgainmaptest.cc` (set kUpdateTestImages to update images).
 
 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.
 

diff --git a/tests/data/goldens/paris_exif_xmp_gainmap_bigendian.jpg.avif.xml b/tests/data/goldens/paris_exif_xmp_gainmap_bigendian.jpg.avif.xml
@@ -8,7 +8,7 @@
 <BrandEntry AlternateBrand="miaf"/>
 <BrandEntry AlternateBrand="MA1B"/>
 </FileTypeBox>
-<MetaBox Size="605" Type="meta" Version="0" Flags="0" Specification="p12" Container="file moov trak moof traf udta" >
+<MetaBox Size="606" Type="meta" Version="0" Flags="0" Specification="p12" Container="file moov trak moof traf udta" >
 <HandlerBox Size="40" Type="hdlr" Version="0" Flags="0" Specification="p12" Container="mdia meta minf" hdlrType="pict" Name="libavif" reserved1="0" reserved2="data:application/octet-string,000000000000000000000000">
 </HandlerBox>
 <PrimaryItemBox Size="14" Type="pitm" Version="0" Flags="0" Specification="p12" Container="meta" item_ID="1">
@@ -54,7 +54,7 @@
 <ItemReferenceBoxEntry ItemID="1"/>
 </ItemReferenceBox>
 </ItemReferenceBox>
-<ItemPropertiesBox Size="202" Type="iprp" Specification="iff" Container="meta" >
+<ItemPropertiesBox Size="203" Type="iprp" Specification="iff" Container="meta" >
 <ItemPropertyContainerBox Size="159" Type="ipco" Specification="iff" Container="iprp" >
 <ImageSpatialExtentsPropertyBox Size="20" Type="ispe" Version="0" Flags="0" Specification="iff" Container="ipco" image_width="403" image_height="302">
 </ImageSpatialExtentsPropertyBox>
@@ -69,7 +69,7 @@
 </AV1ConfigurationBox>
 <ColourInformationBox Size="19" Type="colr" Specification="iff" Container="video_sample_entry ipco encv resv" colour_type="nclx" colour_primaries="1" transfer_characteristics="13" matrix_coefficients="6" full_range_flag="1">
 </ColourInformationBox>
-<ColourInformationBox Size="19" Type="colr" Specification="iff" Container="video_sample_entry ipco encv resv" colour_type="nclx" colour_primaries="2" transfer_characteristics="16" matrix_coefficients="6" full_range_flag="1">
+<ColourInformationBox Size="19" Type="colr" Specification="iff" Container="video_sample_entry ipco encv resv" colour_type="nclx" colour_primaries="1" transfer_characteristics="16" matrix_coefficients="6" full_range_flag="1">
 </ColourInformationBox>
 <ImageSpatialExtentsPropertyBox Size="20" Type="ispe" Version="0" Flags="0" Specification="iff" Container="ipco" image_width="512" image_height="384">
 </ImageSpatialExtentsPropertyBox>
@@ -83,15 +83,16 @@
 <ColourInformationBox Size="19" Type="colr" Specification="iff" Container="video_sample_entry ipco encv resv" colour_type="nclx" colour_primaries="2" transfer_characteristics="2" matrix_coefficients="6" full_range_flag="1">
 </ColourInformationBox>
 </ItemPropertyContainerBox>
-<ItemPropertyAssociationBox Size="35" Type="ipma" Version="0" Flags="0" Specification="iff" Container="iprp" entry_count="3">
+<ItemPropertyAssociationBox Size="36" Type="ipma" Version="0" Flags="0" Specification="iff" Container="iprp" entry_count="3">
 <AssociationEntry item_ID="1" association_count="4">
 <Property index="1" essential="0"/>
 <Property index="2" essential="0"/>
 <Property index="3" essential="1"/>
 <Property index="4" essential="0"/>
 </AssociationEntry>
-<AssociationEntry item_ID="2" association_count="2">
+<AssociationEntry item_ID="2" association_count="3">
 <Property index="1" essential="0"/>
+<Property index="2" essential="0"/>
 <Property index="5" essential="0"/>
 </AssociationEntry>
 <AssociationEntry item_ID="3" association_count="4">

diff --git a/tests/data/goldens/paris_exif_xmp_gainmap_littleendian.jpg.avif.xml b/tests/data/goldens/paris_exif_xmp_gainmap_littleendian.jpg.avif.xml
@@ -8,7 +8,7 @@
 <BrandEntry AlternateBrand="miaf"/>
 <BrandEntry AlternateBrand="MA1B"/>
 </FileTypeBox>
-<MetaBox Size="605" Type="meta" Version="0" Flags="0" Specification="p12" Container="file moov trak moof traf udta" >
+<MetaBox Size="606" Type="meta" Version="0" Flags="0" Specification="p12" Container="file moov trak moof traf udta" >
 <HandlerBox Size="40" Type="hdlr" Version="0" Flags="0" Specification="p12" Container="mdia meta minf" hdlrType="pict" Name="libavif" reserved1="0" reserved2="data:application/octet-string,000000000000000000000000">
 </HandlerBox>
 <PrimaryItemBox Size="14" Type="pitm" Version="0" Flags="0" Specification="p12" Container="meta" item_ID="1">
@@ -54,7 +54,7 @@
 <ItemReferenceBoxEntry ItemID="1"/>
 </ItemReferenceBox>
 </ItemReferenceBox>
-<ItemPropertiesBox Size="202" Type="iprp" Specification="iff" Container="meta" >
+<ItemPropertiesBox Size="203" Type="iprp" Specification="iff" Container="meta" >
 <ItemPropertyContainerBox Size="159" Type="ipco" Specification="iff" Container="iprp" >
 <ImageSpatialExtentsPropertyBox Size="20" Type="ispe" Version="0" Flags="0" Specification="iff" Container="ipco" image_width="403" image_height="302">
 </ImageSpatialExtentsPropertyBox>
@@ -69,7 +69,7 @@
 </AV1ConfigurationBox>
 <ColourInformationBox Size="19" Type="colr" Specification="iff" Container="video_sample_entry ipco encv resv" colour_type="nclx" colour_primaries="1" transfer_characteristics="13" matrix_coefficients="6" full_range_flag="1">
 </ColourInformationBox>
-<ColourInformationBox Size="19" Type="colr" Specification="iff" Container="video_sample_entry ipco encv resv" colour_type="nclx" colour_primaries="2" transfer_characteristics="16" matrix_coefficients="6" full_range_flag="1">
+<ColourInformationBox Size="19" Type="colr" Specification="iff" Container="video_sample_entry ipco encv resv" colour_type="nclx" colour_primaries="1" transfer_characteristics="16" matrix_coefficients="6" full_range_flag="1">
 </ColourInformationBox>
 <ImageSpatialExtentsPropertyBox Size="20" Type="ispe" Version="0" Flags="0" Specification="iff" Container="ipco" image_width="512" image_height="384">
 </ImageSpatialExtentsPropertyBox>
@@ -83,15 +83,16 @@
 <ColourInformationBox Size="19" Type="colr" Specification="iff" Container="video_sample_entry ipco encv resv" colour_type="nclx" colour_primaries="2" transfer_characteristics="2" matrix_coefficients="6" full_range_flag="1">
 </ColourInformationBox>
 </ItemPropertyContainerBox>
-<ItemPropertyAssociationBox Size="35" Type="ipma" Version="0" Flags="0" Specification="iff" Container="iprp" entry_count="3">
+<ItemPropertyAssociationBox Size="36" Type="ipma" Version="0" Flags="0" Specification="iff" Container="iprp" entry_count="3">
 <AssociationEntry item_ID="1" association_count="4">
 <Property index="1" essential="0"/>
 <Property index="2" essential="0"/>
 <Property index="3" essential="1"/>
 <Property index="4" essential="0"/>
 </AssociationEntry>
-<AssociationEntry item_ID="2" association_count="2">
+<AssociationEntry item_ID="2" association_count="3">
 <Property index="1" essential="0"/>
+<Property index="2" essential="0"/>
 <Property index="5" essential="0"/>
 </AssociationEntry>
 <AssociationEntry item_ID="3" association_count="4">

diff --git a/tests/data/seine_hdr_gainmap_small_srgb.avif b/tests/data/seine_hdr_gainmap_small_srgb.avif
diff --git a/tests/data/seine_hdr_gainmap_srgb.avif b/tests/data/seine_hdr_gainmap_srgb.avif
diff --git a/tests/data/seine_sdr_gainmap_big_srgb.avif b/tests/data/seine_sdr_gainmap_big_srgb.avif
diff --git a/tests/data/seine_sdr_gainmap_srgb.avif b/tests/data/seine_sdr_gainmap_srgb.avif
diff --git a/tests/gtest/avifgainmaptest.cc b/tests/gtest/avifgainmaptest.cc
@@ -932,6 +932,13 @@ TEST(GainMapTest, CreateTestImages) {
     hdr_image->gainMap = sdr_with_gainmap->gainMap;
     sdr_with_gainmap->gainMap = nullptr;
     SwapBaseAndAlternate(hdr_image->gainMap->metadata);
+    hdr_image->gainMap->altColorPrimaries = sdr_with_gainmap->colorPrimaries;
+    hdr_image->gainMap->altTransferCharacteristics =
+        sdr_with_gainmap->transferCharacteristics;
+    hdr_image->gainMap->altMatrixCoefficients =
+        sdr_with_gainmap->matrixCoefficients;
+    hdr_image->gainMap->altDepth = sdr_with_gainmap->depth;
+    hdr_image->gainMap->altPlaneCount = 3;
 
     const testutil::AvifRwData encoded =
         testutil::Encode(hdr_image.get(), /*speed=*/9, /*quality=*/90);