Skip to content

Commit 80545a9

Browse files
authored
fix(oiiotool): -i:ch=... didn't fix up alpha and z channels (#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>
1 parent fb26ed8 commit 80545a9

3 files changed

Lines changed: 25 additions & 11 deletions

File tree

src/libOpenImageIO/imagebufalgo_channels.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ ImageBufAlgo::channels(ImageBuf& dst, const ImageBuf& src, int nchannels,
7676
return false;
7777
}
7878

79-
// If channelorder is NULL, it will be interpreted as
79+
// If channelorder is empty, it will be interpreted as
8080
// {0, 1, ..., nchannels-1}.
81-
int* local_channelorder = NULL;
81+
int* local_channelorder = nullptr;
8282
if (channelorder.empty()) {
8383
local_channelorder = OIIO_ALLOCA(int, nchannels);
8484
for (int c = 0; c < nchannels; ++c)

src/oiiotool/imagerec.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,8 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set)
293293
std::vector<std::string> newchannelnames;
294294
std::vector<int> channel_set_channels;
295295
std::vector<float> channel_set_values;
296+
int new_alpha_channel = -1;
297+
int new_z_channel = -1;
296298
int chbegin = 0, chend = -1;
297299
if (channel_set.size()) {
298300
decode_channel_set(ib->nativespec(), channel_set,
@@ -306,6 +308,10 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set)
306308
&& channel_set_channels[c]
307309
!= channel_set_channels[c - 1] + 1)
308310
post_channel_set_action = true; // non-consecutive chans
311+
if (channel_set_channels[c] == ib->spec().alpha_channel)
312+
new_alpha_channel = c;
313+
if (channel_set_channels[c] == ib->spec().z_channel)
314+
new_z_channel = c;
309315
}
310316
if (ib->deep())
311317
post_channel_set_action = true;
@@ -349,6 +355,11 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set)
349355
}
350356
if (!ok)
351357
errorfmt("{}", ib->geterror());
358+
if (channel_set.size()) {
359+
// Adjust the spec to reflect the new channel set
360+
ib->specmod().alpha_channel = new_alpha_channel;
361+
ib->specmod().z_channel = new_z_channel;
362+
}
352363

353364
allok &= ok;
354365
// Remove any existing SHA-1 hash from the spec.

src/tiff.imageio/tiffoutput.cpp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -801,21 +801,24 @@ TIFFOutput::open(const std::string& name, const ImageSpec& userspec,
801801
TIFFSetField(m_tif, TIFFTAG_PREDICTOR, m_predictor);
802802

803803
// ExtraSamples tag
804-
if ((m_spec.alpha_channel >= 0 || m_spec.nchannels > 3)
804+
if (((m_spec.alpha_channel >= 0 && m_spec.alpha_channel < m_spec.nchannels)
805+
|| m_spec.nchannels > 3)
805806
&& m_photometric != PHOTOMETRIC_SEPARATED
806807
&& m_spec.get_int_attribute("tiff:write_extrasamples", 1)) {
807808
bool unass = m_spec.get_int_attribute("oiio:UnassociatedAlpha", 0);
808809
int defaultchans = m_spec.nchannels >= 3 ? 3 : 1;
809810
short e = m_spec.nchannels - defaultchans;
810-
std::vector<unsigned short> extra(e);
811-
for (int c = 0; c < e; ++c) {
812-
if (m_spec.alpha_channel == (c + defaultchans))
813-
extra[c] = unass ? EXTRASAMPLE_UNASSALPHA
814-
: EXTRASAMPLE_ASSOCALPHA;
815-
else
816-
extra[c] = EXTRASAMPLE_UNSPECIFIED;
811+
if (e > 0) {
812+
std::vector<unsigned short> extra(e);
813+
for (int c = 0; c < e; ++c) {
814+
if (m_spec.alpha_channel == (c + defaultchans))
815+
extra[c] = unass ? EXTRASAMPLE_UNASSALPHA
816+
: EXTRASAMPLE_ASSOCALPHA;
817+
else
818+
extra[c] = EXTRASAMPLE_UNSPECIFIED;
819+
}
820+
TIFFSetField(m_tif, TIFFTAG_EXTRASAMPLES, e, extra.data());
817821
}
818-
TIFFSetField(m_tif, TIFFTAG_EXTRASAMPLES, e, &extra[0]);
819822
}
820823

821824
ParamValue* param;

0 commit comments

Comments
 (0)