deps: libheif 1.21 support

[lgritz]

Starting with 1.21, libheif seems to change behavior: When no CICP metadata is present, libheif now returns 2,2,2 (all unspecified) on read. OIIO convention, though, is to not set the attribute if valid CICP data is not in the file.

[jessey-git]

Doing a local diff between this new ref output and the others shows what look like reasonable changes except this oiio:ColorSpace value here. Given the file is called colorspace_hlg.avif and the other reference outputs say oiio:ColorSpace: "hlg_rec2020_display", do we have to worry about this?

[lgritz]

So... I wondered about this as well.

Without my changing any heif related code in OIIO, it started failing for me locally on all branches, on my Mac laptop when Homebrew updated the libheif version from 1.20 to 1.21. So I'm going on the assumption that this is one of those version-to-version changes in behavior that we just have to absorb by providing more reference outputs for the new version. (libheif seems especially prone to frequent changes like this, I believe there are more reference output variations in testsuite/heif than any other test.)

I admit that I did not try to square this with changes in libheif internals to understand exactly what changed and validate that it's correct. Did they fix a bug? Introduce a new bug? Or just have a neutral change in behavior that is an equally plausible thing to report.

@zachlewis @brechtvl You're the additional CICP and color experts here. Do you have opinions or concerns?

[zachlewis]

Hmm... interesting.

In briefly glancing at the libheif changes that affect our reader:

• It looks like behavior changed if an image is holding both ICC and NCLX metadata
  https://github.com/strukturag/libheif/blame/master/libheif/api/libheif/heif_color.h#L238-L241

• It looks like behavior may have changed re: reading NCLX profiles in the bitstream
  (with a TODO to improve the behavior in the future)
  https://github.com/strukturag/libheif/blame/master/libheif/api/libheif/heif_color.h#L224-L225

I haven't looked at our test images yet, but if the only differences between cicp_pq.avif and colorspace_hlg.avif really are the actual color space encodings and the CICP values stored in the nclx colr box (9-16-9-1 versus 9-18-9-1), this is could be an upstream error...

I'd be interested to know what the heif reader does when it encounters an ICCv4 profile carrying CICP metadata...

[brechtvl]

When no CICP metadata is present, libheif now returns 2,2,2 (all unspecified) on read. The HLG case fails because it's using --attrib oiio:ColorSpace rather than --iscolorspace, so the CICP value does not get overridden.

Not sure if this is something that should be fixed in libheif, but the easy solution on the OIIO side would be to just ignore such unspecified CICP values.

diff --git a/src/heif.imageio/heifinput.cpp b/src/heif.imageio/heifinput.cpp
index 349bcdb1d..9e0230a86 100644
--- a/src/heif.imageio/heifinput.cpp
+++ b/src/heif.imageio/heifinput.cpp
@@ -287,7 +287,14 @@ HeifInput::seek_subimage(int subimage, int miplevel)
             m_ihandle.get_raw_image_handle(), &nclx);

         if (nclx) {
-            if (err.code == heif_error_Ok) {
+            // When CICP metadata is not present in the file, libheif returns
+            // unspecified since v1.21. Ignore it then.
+            if (err.code == heif_error_Ok
+                && !(nclx->color_primaries == heif_color_primaries_unspecified
+                     && nclx->transfer_characteristics
+                            == heif_transfer_characteristic_unspecified
+                     && nclx->matrix_coefficients
+                            == heif_matrix_coefficients_unspecified)) {
                 const int cicp[4] = { int(nclx->color_primaries),
                                       int(nclx->transfer_characteristics),
                                       int(nclx->matrix_coefficients),

[lgritz]

I think I like this idea Brecht. If our convention is to not set CICP metadata if it's not present in the file, this preserves that behavior for the new libheif.

I will try that out.

[lgritz]

Updated with your suggestion. It eliminates the need for the extra reference output, so I removed it.

[lgritz]

I updated the PR's title and description after incorporating Brecht's suggestion for the correct solution.