feat: CICP metadata support for PNG (#4746)

This PR addresses the primary concern of #4678 -- implement support for
reading and writing the `cICP` chunk for PNGs.

CICP (Coding Independent Code Points) is a means for using a tuple of
integers to communicate characteristics of a variety of, traditionally,
video encodings. The tuple is a series of four values that map to
integer enumerations (codified in [ITU-T
H.273](https://www.itu.int/rec/T-REC-H.273-202407-I/en)) representing a
certain ColourPrimaries, TransferCharacteristics, and
MatrixCoefficients, as well as a VideoFullRangeFlag.

In particular, CICP is used for describing HDR encodings to browsers and
image viewers capable of displaying the image as intended. For example,
if one were to write P3-D65 PQ-encoded RGB values to a PNG, it will only
look "correct" if there's an appropriate `cICP` chunk describing the
primaries (14) and transfer function (16); without such metadata,
PQ-encoded images will appear significantly darker and lower in
contrast.

Internally, CICP metadata is stored in a `int[4]` type `CICP` ImageSpec
attribute.

This PR adds the following:
- An oiiotool `--cicp` flag for setting, modifying, or removing CICP
metadata for the top image
- Methods for reading and writing PNG `cICP` chunk metadata <-->
ImageSpec `CICP`
  - Tests


I've included tests. I've also embedded CICP metadata in the existing
16-bit test png in the testsuite.

But to see what this is all about with your own eyes, you can quickly
convert a scene-linear AP0-encoded EXR to a PQ-encoded P3D65 HDR PNG
with the following command:

`$ oiiotool -i input_linap0.exr --ociodisplay:from=ACES2065-1
"ST2084-P3-D65 - Display" "ACES 1.1 - HDR Video (1000 nits & P3 lim)"
--cicp "12,16,0,1" -o output_p3d65pq.png`

If you compare in a capable image viewer on a capable display the image
produced with the above command compared to another one that lacks CICP
metadata, you should see a pretty significant difference:

`$ oiiotool -i output_p3d65pq.png --cicp "" -o output_no_cicp.png`

("Preview" on a ~5 year old Macbook should suffice).

---------

Signed-off-by: Zach Lewis <[email protected]>

Author: zachlewis

src/doc/oiiotool.rst

@@ -4690,6 +4690,15 @@ will be printed with the command `oiiotool --colorconfiginfo`.
 
     This was added to OpenImageIO 2.5.
 
+.. option:: --cicp <pri>,<trc>,<mtx>,<vfr>
+    
+    The `--cicp` command adds, modifies, or removes a `"CICP"` attribute 
+    belonging to the top image, stored as an array of four integers.  
+    The integers represent, in order, the color primaries, transfer 
+    function, color matrix (for YUV colorspaces), and 
+    video-full-range-flag.
+
+    This was added to OpenImageIO 3.1.
 
 |
 

src/doc/stdmetadata.rst

@@ -164,6 +164,17 @@ Color information
     window that are not overlapping the pixel data window.  If not supplied,
     the default is black (0 in all channels).
 
+.. option:: "CICP" : int[4]
+
+    The CICP color space information, as defined by
+    `ITU-T H.273 <https://www.itu.int/rec/T-REC-H.273>`_.  This is an array
+    of four integers, with the following meanings:
+
+    - `[0]` : color primaries
+    - `[1]` : transfer characteristics
+    - `[2]` : matrix coefficients
+    - `[3]` : full range flag
+
 .. option:: "ICCProfile" : uint8[]
             "ICCProfile:...various..." : ...various types...
 

src/include/OpenImageIO/imageio.h

@@ -1087,6 +1087,9 @@ class OIIO_API ImageInput {
     /// - `"exif"` :
     ///       Can this format store Exif camera data?
     ///
+    /// - `"cicp"` :
+    ///       Does this format support embedding CICP metadata?
+    ///
     /// - `"ioproxy"` :
     ///       Does this format reader support reading from an `IOProxy`?
     ///
@@ -2527,6 +2530,9 @@ class OIIO_API ImageOutput {
     ///        Does this format allow 0x0 sized images, i.e. an image file
     ///        with metadata only and no pixels?
     ///
+    ///  - `"cicp"` :
+    ///        Does this format support embedding CICP metadata?
+    ///
     /// This list of queries may be extended in future releases. Since this
     /// can be done simply by recognizing new query strings, and does not
     /// require any new API entry points, addition of support for new

src/libOpenImageIO/formatspec.cpp

@@ -1295,6 +1295,8 @@ void
 ImageSpec::set_colorspace(string_view colorspace)
 {
     ColorConfig::default_colorconfig().set_colorspace(*this, colorspace);
+    // Invalidate potentially contradictory metadata
+    erase_attribute("CICP");
 }
 
 

src/oiiotool/oiiotool.cpp

@@ -2248,6 +2248,60 @@ icc_read(Oiiotool& ot, cspan<const char*> argv)
 }
 
 
+// Set, modify, or remove the top image's CICP (ITU-T H.273) metadata.
+class OpSetCICP final : public OiiotoolOp {
+public:
+    OpSetCICP(Oiiotool& ot, string_view opname, cspan<const char*> argv)
+        : OiiotoolOp(ot, opname, argv, 1)
+    {
+        inplace(true);  // This action operates in-place
+        cicp = args(1);
+    }
+    OpSetCICP(Oiiotool& ot, string_view opname, int argc, const char* argv[])
+        : OpSetCICP(ot, opname, { argv, span_size_t(argc) })
+    {
+    }
+    bool setup() override
+    {
+        ir(0)->metadata_modified(true);
+        return true;
+    }
+    bool impl(span<ImageBuf*> img) override
+    {
+        // Because this is an in-place operation, img[0] is the same as
+        // img[1].
+        if (cicp.empty()) {
+            img[0]->specmod().erase_attribute("CICP");
+            return true;
+        }
+        std::vector<int> vals { 0, 0, 0, 1 };
+        auto p = img[0]->spec().find_attribute("CICP",
+                                               TypeDesc(TypeDesc::INT, 4));
+        if (p) {
+            const int* existing = static_cast<const int*>(p->data());
+            for (int i = 0; i < 4; ++i)
+                vals[i] = existing[i];
+        }
+        Strutil::extract_from_list_string<int>(vals, cicp);
+        img[0]->specmod().attribute("CICP", TypeDesc(TypeDesc::INT, 4),
+                                    vals.data());
+        return true;
+    }
+
+private:
+    string_view cicp;
+};
+
+
+// --cicp
+static void
+action_cicp(Oiiotool& ot, cspan<const char*> argv)
+{
+    OpSetCICP op(ot, "cicp", argv);
+    op();
+}
+
+
 
 // --colorconfig
 static void
@@ -7176,6 +7230,9 @@ Oiiotool::getargs(int argc, char* argv[])
     ap.arg("--iccread %s:FILENAME")
       .help("Add the contents of the file to the top image as its ICC profile")
       .OTACTION(icc_read);
+    ap.arg("--cicp %s:CICP")
+       .help("Set or modifiy CICP metadata for supporting output formats (e.g., \"12,16,0,1\")") //; selectively persist existing values if not specified (e.g., \",,,0\")")
+       .OTACTION(action_cicp);
     // clang-format on
 
     if (ap.parse_args(argc, (const char**)argv) < 0) {

src/png.imageio/png_pvt.h

@@ -40,7 +40,7 @@ For further information see the following mailing list threads:
 OIIO_PLUGIN_NAMESPACE_BEGIN
 
 #define ICC_PROFILE_ATTR "ICCProfile"
-
+#define CICP_ATTR "CICP"
 
 namespace PNG_pvt {
 
@@ -224,7 +224,7 @@ read_info(png_structp& sp, png_infop& ip, int& bit_depth, int& color_type,
     int srgb_intent;
     double gamma = 0.0;
     if (png_get_sRGB(sp, ip, &srgb_intent)) {
-        spec.set_colorspace("srgb_rec709_scene");
+        spec.attribute("oiio:ColorSpace", "srgb_rec709_scene");
     } else if (png_get_gAMA(sp, ip, &gamma) && gamma > 0.0) {
         // Round gamma to the nearest hundredth to prevent stupid
         // precision choices and make it easier for apps to make
@@ -235,7 +235,7 @@ read_info(png_structp& sp, png_infop& ip, int& bit_depth, int& color_type,
         set_colorspace_rec709_gamma(spec, g);
     } else {
         // If there's no info at all, assume sRGB.
-        set_colorspace(spec, "srgb_rec709_scene");
+        spec.attribute("oiio:ColorSpace", "srgb_rec709_scene");
     }
 
     if (png_get_valid(sp, ip, PNG_INFO_iCCP)) {
@@ -326,6 +326,16 @@ read_info(png_structp& sp, png_infop& ip, int& bit_depth, int& color_type,
 
     interlace_type = png_get_interlace_type(sp, ip);
 
+#ifdef PNG_cICP_SUPPORTED
+    {
+        png_byte pri = 0, trc = 0, mtx = 0, vfr = 0;
+        if (png_get_cICP(sp, ip, &pri, &trc, &mtx, &vfr)) {
+            int cicp[4] = { pri, trc, mtx, vfr };
+            spec.attribute(CICP_ATTR, TypeDesc(TypeDesc::INT, 4), cicp);
+        }
+    }
+#endif
+
 #ifdef PNG_eXIf_SUPPORTED
     // Recent version of PNG and libpng (>= 1.6.32, I think) have direct
     // support for Exif chunks. Older versions don't support it, and I'm not
@@ -713,6 +723,21 @@ write_info(png_structp& sp, png_infop& ip, int& color_type, ImageSpec& spec,
                      (png_uint_32)(yres * scale), unittype);
     }
 
+#ifdef PNG_cICP_SUPPORTED
+    const ParamValue* p = spec.find_attribute(CICP_ATTR,
+                                              TypeDesc(TypeDesc::INT, 4));
+    if (p) {
+        const int* int_vals = static_cast<const int*>(p->data());
+        png_byte vals[4];
+        for (int i = 0; i < 4; ++i)
+            vals[i] = static_cast<png_byte>(int_vals[i]);
+        if (setjmp(png_jmpbuf(sp)))  // NOLINT(cert-err52-cpp)
+            return "Could not set PNG cICP chunk";
+        // libpng will only write the chunk if the third byte is 0
+        png_set_cICP(sp, ip, vals[0], vals[1], (png_byte)0, vals[3]);
+    }
+#endif
+
 #ifdef PNG_eXIf_SUPPORTED
     std::vector<char> exifBlob;
     encode_exif(spec, exifBlob, endian::big);

src/png.imageio/pnginput.cpp

@@ -18,7 +18,14 @@ class PNGInput final : public ImageInput {
     const char* format_name(void) const override { return "png"; }
     int supports(string_view feature) const override
     {
-        return (feature == "ioproxy" || feature == "exif");
+        return (feature == "ioproxy"
+#ifdef PNG_eXIf_SUPPORTED
+                || feature == "exif"
+#endif
+#ifdef PNG_cICP_SUPPORTED
+                || feature == "cicp"
+#endif
+        );
     }
     bool valid_file(Filesystem::IOProxy* ioproxy) const override;
     bool open(const std::string& name, ImageSpec& newspec) override;

src/png.imageio/pngoutput.cpp

@@ -24,6 +24,9 @@ class PNGOutput final : public ImageOutput {
         return (feature == "alpha" || feature == "ioproxy"
 #ifdef PNG_eXIf_SUPPORTED
                 || feature == "exif"
+#endif
+#ifdef PNG_cICP_SUPPORTED
+                || feature == "cicp"
 #endif
         );
     }

testsuite/png/ref/out-libpng15.txt

@@ -26,6 +26,10 @@ Reading exif.png
 exif.png             :   64 x   64, 3 channel, uint8 png
     SHA-1: 7CB41FEA50720B48BE0C145E1473982B23E9AB77
     channel list: R, G, B
+    Exif:ExifVersion: "0230"
+    Exif:FlashPixVersion: "0100"
+    Exif:FocalLength: 45.7 (45.7 mm)
+    Exif:WhiteBalance: 0 (auto)
     oiio:ColorSpace: "srgb_rec709_scene"
 alphagamma:
    1 x    1, 4 channel, float png
@@ -86,5 +90,17 @@ gimp_gradient:
     Monochrome: No
 smallalpha.png       :    1 x    1, 4 channel, uint8 png
     Pixel (0, 0): 240 108 119 1 (0.94117653 0.42352945 0.4666667 0.003921569)
+cicp:
+  16 x   16, 4 channel, float png
+    channel list: R, G, B, A
+    oiio:ColorSpace: "srgb_rec709_scene"
+removed_cicp:
+  16 x   16, 4 channel, float png
+    channel list: R, G, B, A
+    oiio:ColorSpace: "srgb_rec709_scene"
+remove_cicp_via_set_colorspace:
+  16 x   16, 4 channel, float png
+    channel list: R, G, B, A
+    oiio:ColorSpace: "g22_rec709_display"
 Comparing "test16.png" and "ref/test16.png"
 PASS

testsuite/png/ref/out.txt

@@ -90,5 +90,18 @@ gimp_gradient:
     Monochrome: No
 smallalpha.png       :    1 x    1, 4 channel, uint8 png
     Pixel (0, 0): 240 108 119 1 (0.94117653 0.42352945 0.4666667 0.003921569)
+cicp:
+  16 x   16, 4 channel, float png
+    channel list: R, G, B, A
+    CICP: 1, 13, 0, 1
+    oiio:ColorSpace: "srgb_rec709_scene"
+removed_cicp:
+  16 x   16, 4 channel, float png
+    channel list: R, G, B, A
+    oiio:ColorSpace: "srgb_rec709_scene"
+remove_cicp_via_set_colorspace:
+  16 x   16, 4 channel, float png
+    channel list: R, G, B, A
+    oiio:ColorSpace: "g22_rec709_display"
 Comparing "test16.png" and "ref/test16.png"
 PASS