Skip to content

fix(softimage): support channel packets with differing bit depths #5309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lgritz wants to merge 1 commit into
base: main
Choose a base branch
from
+141 −84
Open

fix(softimage): support channel packets with differing bit depths #5309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
204 changes: 120 additions & 84 deletions src/softimage.imageio/softimageinput.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// SPDX-License-Identifier: Apache-2.0
// https://github.com/AcademySoftwareFoundation/OpenImageIO

#include <cstring>

#include "softimage_pvt.h"

OIIO_PLUGIN_NAMESPACE_BEGIN
Expand All @@ -10,6 +12,40 @@ using namespace softimage_pvt;



namespace {

// Decode n (1 or 2) on-disk big-endian bytes into a native integer.
inline uint16_t
assemble_be(const uint8_t* bytes, size_t n)
{
return n == 2 ? uint16_t((uint16_t(bytes[0]) << 8) | bytes[1]) : bytes[0];
}


// Widen 8-bit to 16-bit via exact bit replication (v*257, since
// 255*257==65535); the only possible promotion, since packet depths are
// only ever 8 or 16 bits.
inline uint16_t
promote_depth(uint16_t value, size_t packetBytes, size_t storageBytes)
{
return packetBytes == storageBytes ? value : uint16_t(value * 257);
}


// Store storageBytes of value at dst in native machine byte order.
inline void
store_native(uint8_t* dst, uint16_t value, size_t storageBytes)
{
if (storageBytes == 1)
dst[0] = uint8_t(value);
else
memcpy(dst, &value, sizeof(uint16_t));
}

} // namespace



class SoftimageInput final : public ImageInput {
public:
SoftimageInput() { init(); }
Expand Down Expand Up @@ -51,8 +87,16 @@ class SoftimageInput final : public ImageInput {
std::string m_filename;
std::vector<fpos_t> m_scanline_markers;
// Maps absolute channel index (0=R,1=G,2=B,3=A) to sequential output
// offset within the scanline buffer. Initialized to -1 (unused).
// channel number in the ImageSpec. Initialized to -1 (unused).
int m_channel_map[4];
// Byte offset of each absolute channel within a pixel. Valid only
// where m_channel_map is >= 0.
size_t m_channel_byte_offset[4];
// Uniform storage size (bytes) for all channels: the widest packet
// depth in the file. Narrower channels are promoted to it on read.
size_t m_storage_bytes;
// Native bytes per pixel (nchannels * m_storage_bytes).
size_t m_pixel_bytes;
};


Expand Down Expand Up @@ -80,6 +124,9 @@ SoftimageInput::init()
m_channel_packets.clear();
m_scanline_markers.clear();
std::fill(m_channel_map, m_channel_map + 4, -1);
std::fill(m_channel_byte_offset, m_channel_byte_offset + 4, size_t(0));
m_storage_bytes = 0;
m_pixel_bytes = 0;
}


Expand Down Expand Up @@ -145,30 +192,39 @@ SoftimageInput::open(const std::string& name, ImageSpec& spec)
}
} while (curPacket.chained);

// Build channel map: absolute RGBA index -> sequential output offset
int nchannels = 0;
{
for (auto& cp : m_channel_packets)
for (int ch : cp.channels())
if (m_channel_map[ch] == -1)
m_channel_map[ch] = nchannels++;
// Build channel map: absolute RGBA index -> sequential output channel.
// Packets may have different bit depths; rather than expose that as
// per-channel formats (a legacy format isn't worth the resulting
// non-uniform, potentially misaligned pixel layout), we always store
// a single uniform format -- the widest depth present -- and promote
// narrower channels (see promote_depth()) as they're read.
int nchannels = 0;
TypeDesc widest = TypeDesc::UINT8;
for (auto& cp : m_channel_packets) {
if (cp.size == 16)
widest = TypeDesc::UINT16;
for (int ch : cp.channels())
if (m_channel_map[ch] == -1)
m_channel_map[ch] = nchannels++;
}

// Get the depth per pixel per channel
TypeDesc chanType = TypeDesc::UINT8;
if (curPacket.size == 16)
chanType = TypeDesc::UINT16;

// Set the details in the ImageSpec
m_spec = ImageSpec(m_pic_header.width, m_pic_header.height, nchannels,
chanType);
widest);

if (!check_open(m_spec, { 0, 65535, 0, 65535, 0, 1, 0, 4 })) {
close();
return false;
}

m_spec.attribute("BitsPerSample", (int)curPacket.size);
// Precompute the uniform, naturally-aligned byte layout.
m_storage_bytes = widest.size();
m_pixel_bytes = nchannels * m_storage_bytes;
for (int ch = 0; ch < 4; ++ch)
if (m_channel_map[ch] >= 0)
m_channel_byte_offset[ch] = m_channel_map[ch] * m_storage_bytes;

m_spec.attribute("BitsPerSample", (int)(widest.size() * 8));

m_spec.attribute("softimage:compression", Strutil::join(encodings, ","));

Expand Down Expand Up @@ -326,22 +382,15 @@ SoftimageInput::read_pixels_uncompressed(
uint8_t* scanlineData = (uint8_t*)data;
for (size_t pixelX = 0; pixelX < m_pic_header.width; pixelX++) {
for (int channel : channels) {
for (size_t byte = 0; byte < pixelChannelSize; byte++) {
// Get which byte we should be placing this in depending on endianness
size_t curByte = byte;
if (littleendian())
curByte = ((pixelChannelSize)-1) - curByte;

//read the data into the correct place
if (fread(&scanlineData[(pixelX * pixelChannelSize
* m_spec.nchannels)
+ (m_channel_map[channel]
* pixelChannelSize)
+ curByte],
1, 1, m_fd)
!= 1)
return false;
}
uint8_t raw[2] = { 0, 0 };
if (fread(raw, 1, pixelChannelSize, m_fd) != pixelChannelSize)
return false;
uint16_t value
= promote_depth(assemble_be(raw, pixelChannelSize),
pixelChannelSize, m_storage_bytes);
store_native(&scanlineData[(pixelX * m_pixel_bytes)
+ m_channel_byte_offset[channel]],
value, m_storage_bytes);
}
}
} else {
Expand Down Expand Up @@ -392,26 +441,23 @@ SoftimageInput::read_pixels_pure_run_length(
if (fread(pixelData, 1, pixelSize, m_fd) != pixelSize)
return false;

// Now we've got the pixel value we need to push it into the data
// Decode and promote each channel's value once per run, then
// repeat it for each pixel in the run.
uint16_t chanValues[4];
for (size_t curChan = 0; curChan < channels.size(); curChan++)
chanValues[curChan] = promote_depth(
assemble_be(pixelData + curChan * pixelChannelSize,
pixelChannelSize),
pixelChannelSize, m_storage_bytes);

uint8_t* scanlineData = (uint8_t*)data;
for (size_t pixelX = linePixelCount;
pixelX < linePixelCount + curCount; pixelX++) {
for (size_t curChan = 0; curChan < channels.size(); curChan++) {
for (size_t byte = 0; byte < pixelChannelSize; byte++) {
// Get which byte we should be placing this in depending on endianness
size_t curByte = byte;
if (littleendian())
curByte = ((pixelChannelSize)-1) - curByte;

//put the data into the correct place
scanlineData[(pixelX * pixelChannelSize
* m_spec.nchannels)
+ (m_channel_map[channels[curChan]]
* pixelChannelSize)
+ curByte]
= pixelData[(curChan * pixelChannelSize) + curByte];
}
}
for (size_t curChan = 0; curChan < channels.size(); curChan++)
store_native(
&scanlineData[(pixelX * m_pixel_bytes)
+ m_channel_byte_offset[channels[curChan]]],
chanValues[curChan], m_storage_bytes);
}
} else {
// data pointer is null so we should just seek to the next scanline
Expand Down Expand Up @@ -464,22 +510,17 @@ SoftimageInput::read_pixels_mixed_run_length(
for (size_t pixelX = linePixelCount;
pixelX < linePixelCount + curCount; pixelX++) {
for (int channel : channels) {
for (size_t byte = 0; byte < pixelChannelSize; byte++) {
// Get which byte we should be placing this in depending on endianness
size_t curByte = byte;
if (littleendian())
curByte = ((pixelChannelSize)-1) - curByte;

//read the data into the correct place
if (fread(&scanlineData[(pixelX * pixelChannelSize
* m_spec.nchannels)
+ (m_channel_map[channel]
* pixelChannelSize)
+ curByte],
1, 1, m_fd)
!= 1)
return false;
}
uint8_t raw[2] = { 0, 0 };
if (fread(raw, 1, pixelChannelSize, m_fd)
!= pixelChannelSize)
return false;
uint16_t value
= promote_depth(assemble_be(raw, pixelChannelSize),
pixelChannelSize, m_storage_bytes);
store_native(
&scanlineData[(pixelX * m_pixel_bytes)
+ m_channel_byte_offset[channel]],
value, m_storage_bytes);
}
}
} else {
Expand Down Expand Up @@ -526,30 +567,25 @@ SoftimageInput::read_pixels_mixed_run_length(
if (fread(pixelData, 1, pixelSize, m_fd) != pixelSize)
return false;

// Now we've got the pixel value we need to push it into
// the data.
// Decode and promote each channel's value once per run,
// then repeat it for each pixel in the run.
uint16_t chanValues[4];
for (size_t curChan = 0; curChan < channels.size(); curChan++)
chanValues[curChan] = promote_depth(
assemble_be(pixelData + curChan * pixelChannelSize,
pixelChannelSize),
pixelChannelSize, m_storage_bytes);

uint8_t* scanlineData = (uint8_t*)data;
for (size_t pixelX = linePixelCount;
pixelX < linePixelCount + longCount; pixelX++) {
for (size_t curChan = 0; curChan < channels.size();
curChan++) {
for (size_t byte = 0; byte < pixelChannelSize; byte++) {
// Get which byte we should be placing this
// in depending on endianness.
size_t curByte = byte;
if (littleendian())
curByte = ((pixelChannelSize)-1) - curByte;

//put the data into the correct place
scanlineData[(pixelX * pixelChannelSize
* m_spec.nchannels)
+ (m_channel_map[channels[curChan]]
* pixelChannelSize)
+ curByte]
= pixelData[(curChan * pixelChannelSize)
+ curByte];
}
}
curChan++)
store_native(
&scanlineData
[(pixelX * m_pixel_bytes)
+ m_channel_byte_offset[channels[curChan]]],
chanValues[curChan], m_storage_bytes);
}
} else {
// data pointer is null so we should just seek to the
Expand Down
15 changes: 15 additions & 0 deletions testsuite/softimage/ref/out.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,18 @@ src/broken01.pic : 16 x 4, 1 channel, uint8 softimage
Stats FiniteCount: 64
Constant: No
Monochrome: Yes
src/mixed-bitdepth.pic : 2 x 1, 2 channel, uint16 softimage
SHA-1: 52D6B72AAD7084E2ECF1550D500A66C42F408ED1
channel list: R, G
BitsPerSample: 16
ImageDescription: "Mixed packet bit-depth overflow PoC"
softimage:compression: "none,none"
Stats Min: 4386 21845 (of 65535)
Stats Max: 13124 26214 (of 65535)
Stats Avg: 8755.00 24029.50 (of 65535)
Stats StdDev: 4369.00 2184.50 (of 65535)
Stats NanCount: 0 0
Stats InfCount: 0 0
Stats FiniteCount: 2 2
Constant: No
Monochrome: No
6 changes: 6 additions & 0 deletions testsuite/softimage/run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,9 @@
# Regression testing of error handling and corrupt files
command += info_command ("--stats src/broken01.pic",
info_program="iinfo", failureok=True)
# A file whose channel packets use different bit depths (here a 16-bit R
# packet and an 8-bit G packet): narrower channels get promoted to the
# widest depth present. This used to overrun the scanline buffer because a
# single bit depth was assumed for all channels.
command += info_command ("--stats src/mixed-bitdepth.pic",
info_program="iinfo", failureok=True)
Binary file added testsuite/softimage/src/mixed-bitdepth.pic
View file
Open in desktop
Binary file not shown.
Loading