fix(oiiotool): -i:ch=... didn't fix up alpha and z channels (Academ…

…ySoftwareFoundation#4373)

Fix a bug in oiiotool wherein `-i:ch=...` (input only a subset of
channels when reading from a file) was not adjusting the spec's
alpha_channel and z_channel -- that is, they still contained the OLD
channel numbers of those things, which not only could be wrong due to
the reordering, but they might be out of range of the new number of
channels.

Also a related fix in the TIFF reader, to fix a possible dereference of
an empty vector. This was being triggered by that out-of-range
alpha_channel. Maybe that can't happen anymore with the above bug fix,
but it still feels like the safer way to proceed.

This was all found by the sanitizers in CI -- and only recently, despite
this code having this bug for a long time. So yay for sanitizer CI!

Signed-off-by: Larry Gritz <lg@larrygritz.com>

src/libOpenImageIO/imagebufalgo_channels.cpp

@@ -76,9 +76,9 @@ ImageBufAlgo::channels(ImageBuf& dst, const ImageBuf& src, int nchannels,
         return false;
     }
 
-    // If channelorder is NULL, it will be interpreted as
+    // If channelorder is empty, it will be interpreted as
     // {0, 1, ..., nchannels-1}.
-    int* local_channelorder = NULL;
+    int* local_channelorder = nullptr;
     if (channelorder.empty()) {
         local_channelorder = OIIO_ALLOCA(int, nchannels);
         for (int c = 0; c < nchannels; ++c)

src/oiiotool/imagerec.cpp

@@ -293,6 +293,8 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set)
             std::vector<std::string> newchannelnames;
             std::vector<int> channel_set_channels;
             std::vector<float> channel_set_values;
+            int new_alpha_channel = -1;
+            int new_z_channel     = -1;
             int chbegin = 0, chend = -1;
             if (channel_set.size()) {
                 decode_channel_set(ib->nativespec(), channel_set,
@@ -306,6 +308,10 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set)
                              && channel_set_channels[c]
                                     != channel_set_channels[c - 1] + 1)
                         post_channel_set_action = true;  // non-consecutive chans
+                    if (channel_set_channels[c] == ib->spec().alpha_channel)
+                        new_alpha_channel = c;
+                    if (channel_set_channels[c] == ib->spec().z_channel)
+                        new_z_channel = c;
                 }
                 if (ib->deep())
                     post_channel_set_action = true;
@@ -349,6 +355,11 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set)
             }
             if (!ok)
                 errorfmt("{}", ib->geterror());
+            if (channel_set.size()) {
+                // Adjust the spec to reflect the new channel set
+                ib->specmod().alpha_channel = new_alpha_channel;
+                ib->specmod().z_channel     = new_z_channel;
+            }
 
             allok &= ok;
             // Remove any existing SHA-1 hash from the spec.

src/tiff.imageio/tiffoutput.cpp

@@ -802,21 +802,24 @@ TIFFOutput::open(const std::string& name, const ImageSpec& userspec,
         TIFFSetField(m_tif, TIFFTAG_PREDICTOR, m_predictor);
 
     // ExtraSamples tag
-    if ((m_spec.alpha_channel >= 0 || m_spec.nchannels > 3)
+    if (((m_spec.alpha_channel >= 0 && m_spec.alpha_channel < m_spec.nchannels)
+         || m_spec.nchannels > 3)
         && m_photometric != PHOTOMETRIC_SEPARATED
         && m_spec.get_int_attribute("tiff:write_extrasamples", 1)) {
         bool unass = m_spec.get_int_attribute("oiio:UnassociatedAlpha", 0);
         int defaultchans = m_spec.nchannels >= 3 ? 3 : 1;
         short e          = m_spec.nchannels - defaultchans;
-        std::vector<unsigned short> extra(e);
-        for (int c = 0; c < e; ++c) {
-            if (m_spec.alpha_channel == (c + defaultchans))
-                extra[c] = unass ? EXTRASAMPLE_UNASSALPHA
-                                 : EXTRASAMPLE_ASSOCALPHA;
-            else
-                extra[c] = EXTRASAMPLE_UNSPECIFIED;
+        if (e > 0) {
+            std::vector<unsigned short> extra(e);
+            for (int c = 0; c < e; ++c) {
+                if (m_spec.alpha_channel == (c + defaultchans))
+                    extra[c] = unass ? EXTRASAMPLE_UNASSALPHA
+                                     : EXTRASAMPLE_ASSOCALPHA;
+                else
+                    extra[c] = EXTRASAMPLE_UNSPECIFIED;
+            }
+            TIFFSetField(m_tif, TIFFTAG_EXTRASAMPLES, e, extra.data());
         }
-        TIFFSetField(m_tif, TIFFTAG_EXTRASAMPLES, e, &extra[0]);
     }
 
     ParamValue* param;