Fix broken TIFF reads of multi-subimage non-spectral files #2692
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For a few rare edge cases of photometric interpretation, we fall back on libtiff's ability to auto-translate to an RGBA image for us. It only does this for the whole image at once, so we do it for the first scanline we read (storing the whole image in a buffer, skipping the conversion for subsequent scanlines -- and it can tell it's the "first" scanline because the buffer isn't yet allocated). The bug was that for these cases (such as a photometric YCbCr), if the file was *multi-subimage*, we were not clearing the rgba buffer, so upon reading the second subimage, it thought its buffer was already filled, so it wasn't actually doing the read and convert, just using whatever was already living in the buffer. Oops. Also, I noticed that we should clear m_use_rgba_interface for each subimage -- it was previously just setting it when it found those edge cases. We never saw a symptom from this, but I realized it would be problematic for a file where the first subimage was YCbCr and the second subimage was regular RGB, it wouldn't know to fall back to our usual code path. Signed-off-by: Larry Gritz <lg@larrygritz.com>
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Sep 3, 2020
…ftwareFoundation#2692) For a few rare edge cases of photometric interpretation, we fall back on libtiff's ability to auto-translate to an RGBA image for us. It only does this for the whole image at once, so we do it for the first scanline we read (storing the whole image in a buffer, skipping the conversion for subsequent scanlines -- and it can tell it's the "first" scanline because the buffer isn't yet allocated). The bug was that for these cases (such as a photometric YCbCr), if the file was *multi-subimage*, we were not clearing the rgba buffer, so upon reading the second subimage, it thought its buffer was already filled, so it wasn't actually doing the read and convert, just using whatever was already living in the buffer. Oops. Also, I noticed that we should clear m_use_rgba_interface for each subimage -- it was previously just setting it when it found those edge cases. We never saw a symptom from this, but I realized it would be problematic for a file where the first subimage was YCbCr and the second subimage was regular RGB, it wouldn't know to fall back to our usual code path. Signed-off-by: Larry Gritz <lg@larrygritz.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For a few rare edge cases of photometric interpretation, we fall back
on libtiff's ability to auto-translate to an RGBA image for us. It
only does this for the whole image at once, so we do it for the first
scanline we read (storing the whole image in a buffer, skipping the
conversion for subsequent scanlines -- and it can tell it's the
"first" scanline because the buffer isn't yet allocated).
The bug was that for these cases (such as a photometric YCbCr), if the
file was multi-subimage, we were not clearing the rgba buffer, so
upon reading the second subimage, it thought its buffer was already
filled, so it wasn't actually doing the read and convert, just using
whatever was already living in the buffer. Oops.
Also, I noticed that we should clear m_use_rgba_interface for each
subimage -- it was previously just setting it when it found those edge
cases. We never saw a symptom from this, but I realized it would be
problematic for a file where the first subimage was YCbCr and the
second subimage was regular RGB, it wouldn't know to fall back to our
usual code path.
Signed-off-by: Larry Gritz lg@larrygritz.com