Skip to content

Commit

Permalink
Improve Python error reporting for ImageOutput and IB.set_pixels (#2127)
Browse files Browse the repository at this point in the history 
The underlying decoding of pixel value arrays -- used by some of the
ImageOutput methods and ImageBuf.set_pixels -- threw exceptions when
the pixel array didn't conform to expectations. This led to very
confusing behavior, and crashes if the python program didn't carefully
catch them. Instead of throwing exceptions, bubble up an error message
for the usual OIIO error reporting, so wrongly-shaped pixel data arrays
are treated similarly to other kinds of error.

Also, detect with a better error message cases where user tries to write
scanlines to a tiled file or vice versa -- it was handled well in C++,
but the Python would start by parsing the pixel array, then have a
confusing error that it was the wrong shape, instead of the more clear
error message about using tiles instead of scanlines.
  • Loading branch information
@lgritz
lgritz authored Jan 9, 2019
1 parent 8b9b6db commit 304a03c
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 24 deletions.
5 changes: 3 additions & 2 deletions src/python/py_imagebuf.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,9 @@ ImageBuf_set_pixels_buffer(ImageBuf& self, ROI roi, py::buffer& buffer)
}
oiio_bufinfo buf(buffer.request(), roi.nchannels(), roi.width(),
roi.height(), roi.depth(), self.spec().depth > 1 ? 3 : 2);
if (!buf.data) {
self.error("set_pixels unspecified error decoding the Python buffer");
if (!buf.data || buf.error.size()) {
self.errorf("set_pixels error: %s",
buf.error.size() ? buf.error.c_str() : "unspecified");
return false; // failed sanity checks
}
if (!buf.data || buf.size != size) {
Expand Down
42 changes: 33 additions & 9 deletions src/python/py_imageoutput.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,14 @@ bool
ImageOutput_write_scanline(ImageOutput& self, int y, int z, py::buffer& buffer)
{
const ImageSpec& spec(self.spec());
if (spec.tile_width != 0) {
self.errorf("Cannot write scanlines to a filed file.");
return false;
}
oiio_bufinfo buf(buffer.request(), spec.nchannels, spec.width, 1, 1, 1);
if (!buf.data) {
self.error("Could not decode python buffer");
if (!buf.data || buf.error.size()) {
self.errorf("Pixel data array error: %s",
buf.error.size() ? buf.error.c_str() : "unspecified");
return false; // failed sanity checks
}
if (static_cast<int>(buf.size)
Expand All @@ -80,10 +85,15 @@ ImageOutput_write_scanlines(ImageOutput& self, int ybegin, int yend, int z,
py::buffer& buffer)
{
const ImageSpec& spec(self.spec());
if (spec.tile_width != 0) {
self.errorf("Cannot write scanlines to a filed file.");
return false;
}
oiio_bufinfo buf(buffer.request(), spec.nchannels, spec.width,
yend - ybegin, 1, 2);
if (!buf.data) {
self.error("Could not decode python buffer");
if (!buf.data || buf.error.size()) {
self.errorf("Pixel data array error: %s",
buf.error.size() ? buf.error.c_str() : "unspecified");
return false; // failed sanity checks
}
if (static_cast<int>(buf.size)
Expand All @@ -103,11 +113,16 @@ ImageOutput_write_tile(ImageOutput& self, int x, int y, int z,
py::buffer& buffer)
{
const ImageSpec& spec(self.spec());
if (spec.tile_width == 0) {
self.errorf("Cannot write tiles to a scanline file.");
return false;
}
oiio_bufinfo buf(buffer.request(), spec.nchannels, spec.tile_width,
spec.tile_height, spec.tile_depth,
spec.tile_depth > 1 ? 3 : 2);
if (!buf.data) {
self.error("Could not decode python buffer");
if (!buf.data || buf.error.size()) {
self.errorf("Pixel data array error: %s",
buf.error.size() ? buf.error.c_str() : "unspecified");
return false; // failed sanity checks
}
if (buf.size < self.spec().tile_pixels() * self.spec().nchannels) {
Expand All @@ -126,10 +141,15 @@ ImageOutput_write_tiles(ImageOutput& self, int xbegin, int xend, int ybegin,
int yend, int zbegin, int zend, py::buffer& buffer)
{
const ImageSpec& spec(self.spec());
if (spec.tile_width == 0) {
self.errorf("Cannot write tiles to a scanline file.");
return false;
}
oiio_bufinfo buf(buffer.request(), spec.nchannels, xend - xbegin,
yend - ybegin, zend - zbegin, spec.tile_depth > 1 ? 3 : 2);
if (!buf.data) {
self.error("Could not decode python buffer");
if (!buf.data || buf.error.size()) {
self.errorf("Pixel data array error: %s",
buf.error.size() ? buf.error.c_str() : "unspecified");
return false; // failed sanity checks
}
if (static_cast<int>(buf.size) < (xend - xbegin) * (yend - ybegin)
Expand All @@ -152,8 +172,12 @@ ImageOutput_write_image(ImageOutput& self, py::buffer& buffer)
const ImageSpec& spec(self.spec());
oiio_bufinfo buf(buffer.request(), spec.nchannels, spec.width, spec.height,
spec.depth, spec.depth > 1 ? 3 : 2);
if (!buf.data || buf.size < spec.image_pixels() * spec.nchannels)
if (!buf.data || buf.size < spec.image_pixels() * spec.nchannels
|| buf.error.size()) {
self.errorf("Pixel data array error: %s",
buf.error.size() ? buf.error.c_str() : "unspecified");
return false; // failed sanity checks
}
py::gil_scoped_release gil;
return self.write_image(buf.format, buf.data, buf.xstride, buf.ystride,
buf.zstride);
Expand Down
26 changes: 13 additions & 13 deletions src/python/py_oiio.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,10 @@ oiio_bufinfo::oiio_bufinfo(const py::buffer_info& pybuf, int nchans, int width,
|| pybuf.size
!= int64_t(width) * int64_t(height) * int64_t(depth * nchans)) {
format = TypeUnknown; // Something went wrong
throw std::length_error(Strutil::sprintf(
error = Strutil::sprintf(
"buffer is wrong size (expected %dx%dx%dx%d, got total %d)", depth,
height, width, nchans, pybuf.size));
height, width, nchans, pybuf.size);
return;
}
size = pybuf.size;
if (pixeldims == 3) {
Expand All @@ -116,7 +117,7 @@ oiio_bufinfo::oiio_bufinfo(const py::buffer_info& pybuf, int nchans, int width,
zstride = pybuf.strides[0];
} else {
format = TypeUnknown; // No idea what's going on -- error
throw std::invalid_argument("Bad pixeldims in oiio_bufinfo ctr");
error = "Bad dimensions of pixel data";
}
} else if (pixeldims == 2) {
// Reading an 2D image rectangle
Expand All @@ -136,19 +137,19 @@ oiio_bufinfo::oiio_bufinfo(const py::buffer_info& pybuf, int nchans, int width,
xstride = pybuf.strides[0] * nchans;
} else {
format = TypeUnknown; // error
throw std::invalid_argument(Strutil::sprintf(
error = Strutil::sprintf(
"Can't figure out array shape (pixeldims=%d, pydim=%d)",
pixeldims, pybuf.ndim));
pixeldims, pybuf.ndim);
}
} else if (pybuf.ndim == 1
&& pybuf.shape[0] == height * width * nchans) {
// all pixels & channels smushed together
// just rely on autostride
} else {
format = TypeUnknown; // No idea what's going on -- error
throw std::invalid_argument(Strutil::sprintf(
error = Strutil::sprintf(
"Can't figure out array shape (pixeldims=%d, pydim=%d)",
pixeldims, pybuf.ndim));
pixeldims, pybuf.ndim);
}
} else if (pixeldims == 1) {
// Reading a 1D scanline span
Expand All @@ -161,20 +162,19 @@ oiio_bufinfo::oiio_bufinfo(const py::buffer_info& pybuf, int nchans, int width,
xstride = pybuf.strides[0] * nchans;
} else {
format = TypeUnknown; // No idea what's going on -- error
throw std::invalid_argument(Strutil::sprintf(
error = Strutil::sprintf(
"Can't figure out array shape (pixeldims=%d, pydim=%d)",
pixeldims, pybuf.ndim));
pixeldims, pybuf.ndim);
}
} else {
throw std::invalid_argument(Strutil::sprintf(
error = Strutil::sprintf(
"Can't figure out array shape (pixeldims=%d, pydim=%d)", pixeldims,
pybuf.ndim));
pybuf.ndim);
}

if (nchans > 1 && size_t(pybuf.strides.back()) != format.size()) {
format = TypeUnknown; // can't handle noncontig channels
throw std::invalid_argument(
"Can't handle numpy array with noncontiguous channels");
error = "Can't handle numpy array with noncontiguous channels";
}
if (format != TypeUnknown)
data = pybuf.ptr;
Expand Down
1 change: 1 addition & 0 deletions src/python/py_oiio.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ struct oiio_bufinfo {
void* data = nullptr;
stride_t xstride = AutoStride, ystride = AutoStride, zstride = AutoStride;
size_t size = 0;
std::string error;

oiio_bufinfo(const py::buffer_info& pybuf, int nchans, int width,
int height, int depth, int pixeldims);
Expand Down

0 comments on commit 304a03c

Please sign in to comment.