diff --git a/src/include/OpenImageIO/span_util.h b/src/include/OpenImageIO/span_util.h index 4c2141beed..89063e7f90 100644 --- a/src/include/OpenImageIO/span_util.h +++ b/src/include/OpenImageIO/span_util.h @@ -69,6 +69,13 @@ make_span(T (&arg)[N]) // span from C array of known length return { arg }; } +template +inline constexpr span +make_span(T* data, oiio_span_size_type size) // span from ptr + size +{ + return { data, size }; +} + template inline constexpr cspan make_cspan(T (&arg)[N]) // cspan from C array of known length @@ -83,6 +90,13 @@ make_cspan(const T& arg) // cspan from a single value return { &arg, 1 }; } +template +inline constexpr cspan +make_cspan(const T* data, oiio_span_size_type size) // cspan from ptr + size +{ + return { data, size }; +} + /// Try to copy `n` items of type `T` from `src[srcoffset...]` to diff --git a/src/jpeg.imageio/jpeg_pvt.h b/src/jpeg.imageio/jpeg_pvt.h index d5c0d40dd7..3f4f4035da 100644 --- a/src/jpeg.imageio/jpeg_pvt.h +++ b/src/jpeg.imageio/jpeg_pvt.h @@ -36,7 +36,7 @@ extern "C" { OIIO_PLUGIN_NAMESPACE_BEGIN -#define MAX_DATA_BYTES_IN_MARKER 65519L +#define MAX_DATA_BYTES_IN_MARKER 65519UL #define ICC_HEADER_SIZE 14 #define ICC_PROFILE_ATTR "ICCProfile" diff --git a/src/jpeg.imageio/jpegoutput.cpp b/src/jpeg.imageio/jpegoutput.cpp index 789ee865af..896ce00177 100644 --- a/src/jpeg.imageio/jpegoutput.cpp +++ b/src/jpeg.imageio/jpegoutput.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include "jpeg_pvt.h" @@ -280,36 +281,33 @@ JpgOutput::open(const std::string& name, const ImageSpec& newspec, m_spec.set_format(TypeDesc::UINT8); // JPG is only 8 bit // Write ICC profile, if we have anything - const ParamValue* icc_profile_parameter = m_spec.find_attribute( - ICC_PROFILE_ATTR); - if (icc_profile_parameter != NULL) { - unsigned char* icc_profile - = (unsigned char*)icc_profile_parameter->data(); - unsigned int icc_profile_length = icc_profile_parameter->type().size(); - if (icc_profile && icc_profile_length) { + if (auto icc_profile_parameter = m_spec.find_attribute(ICC_PROFILE_ATTR)) { + cspan icc_profile((unsigned char*) + icc_profile_parameter->data(), + icc_profile_parameter->type().size()); + if (icc_profile.size() && icc_profile.data()) { /* Calculate the number of markers we'll need, rounding up of course */ - int num_markers = icc_profile_length / MAX_DATA_BYTES_IN_MARKER; - if ((unsigned int)(num_markers * MAX_DATA_BYTES_IN_MARKER) - != icc_profile_length) + size_t num_markers = icc_profile.size() / MAX_DATA_BYTES_IN_MARKER; + if (num_markers * MAX_DATA_BYTES_IN_MARKER + != std::size(icc_profile)) num_markers++; - int curr_marker = 1; /* per spec, count starts at 1*/ - size_t profile_size = MAX_DATA_BYTES_IN_MARKER + ICC_HEADER_SIZE; - std::vector profile(profile_size); + int curr_marker = 1; /* per spec, count starts at 1*/ + std::vector profile(MAX_DATA_BYTES_IN_MARKER + + ICC_HEADER_SIZE); + size_t icc_profile_length = icc_profile.size(); while (icc_profile_length > 0) { // length of profile to put in this marker - unsigned int length - = std::min(icc_profile_length, - (unsigned int)MAX_DATA_BYTES_IN_MARKER); + size_t length = std::min(icc_profile_length, + size_t(MAX_DATA_BYTES_IN_MARKER)); icc_profile_length -= length; // Write the JPEG marker header (APP2 code and marker length) strcpy((char*)profile.data(), "ICC_PROFILE"); // NOSONAR profile[11] = 0; profile[12] = curr_marker; profile[13] = (JOCTET)num_markers; - OIIO_ASSERT(profile_size >= ICC_HEADER_SIZE + length); - memcpy(profile.data() + ICC_HEADER_SIZE, - icc_profile + length * (curr_marker - 1), - length); //NOSONAR + OIIO_ASSERT(profile.size() >= ICC_HEADER_SIZE + length); + spancpy(make_span(profile), ICC_HEADER_SIZE, icc_profile, + length * (curr_marker - 1), length); jpeg_write_marker(&m_cinfo, JPEG_APP0 + 2, profile.data(), ICC_HEADER_SIZE + length); curr_marker++; diff --git a/src/libOpenImageIO/exif.cpp b/src/libOpenImageIO/exif.cpp index 4558c406c5..56ad1f2475 100644 --- a/src/libOpenImageIO/exif.cpp +++ b/src/libOpenImageIO/exif.cpp @@ -639,85 +639,117 @@ add_exif_item_to_spec(ImageSpec& spec, const char* name, int offset_adjustment = 0) { OIIO_ASSERT(dirp); - const uint8_t* dataptr = (const uint8_t*)pvt::dataptr(*dirp, buf, - offset_adjustment); - if (!dataptr) - return; TypeDesc type = tiff_datatype_to_typedesc(*dirp); + size_t count = dirp->tdir_count; if (dirp->tdir_type == TIFF_SHORT) { - std::vector d((const uint16_t*)dataptr, - (const uint16_t*)dataptr + dirp->tdir_count); - if (swab) - swap_endian(d.data(), d.size()); - spec.attribute(name, type, d.data()); + cspan dspan + = pvt::dataspan(*dirp, buf, offset_adjustment, count); + if (dspan.empty()) + return; + if (swab) { + // In the byte swap case, copy it into a vector because the + // upstream source isn't mutable. + std::vector dswab((const uint16_t*)dspan.begin(), + (const uint16_t*)dspan.end()); + swap_endian(dswab.data(), dswab.size()); + spec.attribute(name, type, dswab.data()); + } else { + spec.attribute(name, type, dspan.data()); + } return; } if (dirp->tdir_type == TIFF_LONG) { - std::vector d((const uint32_t*)dataptr, - (const uint32_t*)dataptr + dirp->tdir_count); - if (swab) - swap_endian(d.data(), d.size()); - spec.attribute(name, type, d.data()); + cspan dspan + = pvt::dataspan(*dirp, buf, offset_adjustment, count); + if (dspan.empty()) + return; + if (swab) { + // In the byte swap case, copy it into a vector because the + // upstream source isn't mutable. + std::vector dswab((const uint32_t*)dspan.begin(), + (const uint32_t*)dspan.end()); + swap_endian(dswab.data(), dswab.size()); + spec.attribute(name, type, dswab.data()); + } else { + spec.attribute(name, type, dspan.data()); + } return; } if (dirp->tdir_type == TIFF_RATIONAL) { - int n = dirp->tdir_count; // How many - float* f = OIIO_ALLOCA(float, n); - for (int i = 0; i < n; ++i) { + cspan dspan + = pvt::dataspan(*dirp, buf, offset_adjustment, 2 * count); + if (dspan.empty()) + return; + float* f = OIIO_ALLOCA(float, count); + for (size_t i = 0; i < count; ++i) { + // Because the values in the blob aren't 32-bit-aligned, memcpy + // them into ints to do the swapping. unsigned int num, den; - memcpy(&num, dataptr + (2 * i) * sizeof(unsigned int), - sizeof(unsigned int)); - memcpy(&den, dataptr + (2 * i + 1) * sizeof(unsigned int), + memcpy(&num, dspan.data() + (2 * i) * sizeof(unsigned int), + sizeof(unsigned int)); //NOSONAR + memcpy(&den, dspan.data() + (2 * i + 1) * sizeof(unsigned int), sizeof(unsigned int)); //NOSONAR if (swab) { - swap_endian(&num); - swap_endian(&den); + num = byteswap(num); + den = byteswap(den); } f[i] = (float)((double)num / (double)den); } if (dirp->tdir_count == 1) - spec.attribute(name, *f); + spec.attribute(name, f[0]); else - spec.attribute(name, TypeDesc(TypeDesc::FLOAT, n), f); + spec.attribute(name, TypeDesc(TypeDesc::FLOAT, int(count)), f); return; } if (dirp->tdir_type == TIFF_SRATIONAL) { - int n = dirp->tdir_count; // How many - float* f = OIIO_ALLOCA(float, n); - for (int i = 0; i < n; ++i) { + cspan dspan + = pvt::dataspan(*dirp, buf, offset_adjustment, 2 * count); + if (dspan.empty()) + return; + float* f = OIIO_ALLOCA(float, count); + for (size_t i = 0; i < count; ++i) { + // Because the values in the blob aren't 32-bit-aligned, memcpy + // them into ints to do the swapping. int num, den; - memcpy(&num, dataptr + (2 * i) * sizeof(int), sizeof(int)); - memcpy(&den, dataptr + (2 * i + 1) * sizeof(int), + memcpy(&num, dspan.data() + (2 * i) * sizeof(int), + sizeof(int)); //NOSONAR + memcpy(&den, dspan.data() + (2 * i + 1) * sizeof(int), sizeof(int)); //NOSONAR if (swab) { - swap_endian(&num); - swap_endian(&den); + num = byteswap(num); + den = byteswap(den); } f[i] = (float)((double)num / (double)den); } if (dirp->tdir_count == 1) - spec.attribute(name, *f); + spec.attribute(name, f[0]); else - spec.attribute(name, TypeDesc(TypeDesc::FLOAT, n), f); + spec.attribute(name, TypeDesc(TypeDesc::FLOAT, int(count)), f); return; } if (dirp->tdir_type == TIFF_ASCII) { - int len = tiff_data_size(*dirp); - const char* ptr = (const char*)dataptr; - while (len && ptr[len - 1] == 0) // Don't grab the terminating null - --len; - std::string str(ptr, len); + size_t len = tiff_data_size(*dirp); + cspan dspan = pvt::dataspan(*dirp, buf, + offset_adjustment, len); + if (dspan.empty()) + return; + // Don't grab the terminating null + while (dspan.size() && dspan.back() == 0) + dspan = dspan.subspan(0, dspan.size() - 1); + std::string str(dspan.begin(), dspan.end()); if (strlen(str.c_str()) < str.length()) // Stray \0 in the middle str = std::string(str.c_str()); spec.attribute(name, str); return; } - if (dirp->tdir_type == TIFF_BYTE && dirp->tdir_count == 1) { + if (dirp->tdir_type == TIFF_BYTE && count == 1) { // Not sure how to handle "bytes" generally, but certainly for just // one, add it as an int. - unsigned char d; - d = *dataptr; // byte stored in offset itself - spec.attribute(name, (int)d); + cspan dspan = pvt::dataspan(*dirp, buf, + offset_adjustment, count); + if (dspan.empty()) + return; + spec.attribute(name, (int)dspan[0]); return; } diff --git a/src/libOpenImageIO/exif.h b/src/libOpenImageIO/exif.h index 0973c9cd05..844d34924b 100644 --- a/src/libOpenImageIO/exif.h +++ b/src/libOpenImageIO/exif.h @@ -44,6 +44,30 @@ dataptr(const TIFFDirEntry& td, cspan data, int offset_adjustment) +// Return a span that bounds offset data within the dir entry. +// No matter what the type T, we still return a cspan because it's +// unaligned data somewhere in the middle of a byte array. We would have +// alignment errors if we try to access it as some other type if it's not +// properly aligned. +template +inline cspan +dataspan(const TIFFDirEntry& td, cspan data, int offset_adjustment, + size_t count) +{ + size_t len = tiff_data_size(td); + OIIO_DASSERT(len == sizeof(T) * count); + if (len <= 4) + return { (const uint8_t*)&td.tdir_offset, oiio_span_size_type(len) }; + else { + int offset = td.tdir_offset + offset_adjustment; + if (offset < 0 || size_t(offset) + len > std::size(data)) + return {}; // out of bounds! return empty span + return { data.data() + offset, oiio_span_size_type(len) }; + } +} + + + struct LabelIndex { int value; const char* label; diff --git a/src/libutil/paramlist_test.cpp b/src/libutil/paramlist_test.cpp index 670048235a..69538e0f0e 100644 --- a/src/libutil/paramlist_test.cpp +++ b/src/libutil/paramlist_test.cpp @@ -20,12 +20,12 @@ using namespace OIIO; // int or float, and also return a string representation. template static std::string -test_numeric(T* data, int num_elements, TypeDesc type) +test_numeric(cspan data, TypeDesc type, int num_elements = 1) { - ParamValue p("name", type, num_elements, data); + ParamValue p("name", type, num_elements, data.data()); int n = type.numelements() * num_elements; for (int i = 0; i < n; ++i) - OIIO_CHECK_EQUAL(p.get(i), ((const T*)data)[i]); + OIIO_CHECK_EQUAL(p.get(i), data[i]); if (std::numeric_limits::is_integer) { OIIO_CHECK_EQUAL(p.get_int(), int(data[0])); for (int i = 0; i < n; ++i) @@ -50,55 +50,55 @@ test_value_types() { int val = 42; - ret = test_numeric(&val, 1, TypeDesc::INT); + ret = test_numeric(make_cspan(val), TypeDesc::INT); OIIO_CHECK_EQUAL(ret, "42"); } { unsigned int val = 42; - ret = test_numeric(&val, 1, TypeDesc::UINT); + ret = test_numeric(make_cspan(val), TypeDesc::UINT); OIIO_CHECK_EQUAL(ret, "42"); } { short val = 42; - ret = test_numeric(&val, 1, TypeDesc::INT16); + ret = test_numeric(make_cspan(val), TypeDesc::INT16); OIIO_CHECK_EQUAL(ret, "42"); } { unsigned short val = 42; - ret = test_numeric(&val, 1, TypeDesc::UINT16); + ret = test_numeric(make_cspan(val), TypeDesc::UINT16); OIIO_CHECK_EQUAL(ret, "42"); } { char val = 42; - ret = test_numeric(&val, 1, TypeDesc::INT8); + ret = test_numeric(make_cspan(val), TypeDesc::INT8); OIIO_CHECK_EQUAL(ret, "42"); } { unsigned char val = 42; - ret = test_numeric(&val, 1, TypeDesc::UINT8); + ret = test_numeric(make_cspan(val), TypeDesc::UINT8); OIIO_CHECK_EQUAL(ret, "42"); } { float val = 2.25; - ret = test_numeric(&val, 1, TypeDesc::FLOAT); + ret = test_numeric(make_cspan(val), TypeDesc::FLOAT); OIIO_CHECK_EQUAL(ret, "2.25"); } { double val = 2.25; - ret = test_numeric(&val, 1, TypeDesc::DOUBLE); + ret = test_numeric(make_cspan(val), TypeDesc::DOUBLE); OIIO_CHECK_EQUAL(ret, "2.25"); } { half val = 2.25; - ret = test_numeric(&val, 1, TypeDesc::HALF); + ret = test_numeric(make_cspan(val), TypeDesc::HALF); OIIO_CHECK_EQUAL(ret, "2.25"); } @@ -127,36 +127,34 @@ test_value_types() { int imatrix[] = { 100, 200, 300, 400 }; - ret = test_numeric(&imatrix[0], 1, TypeInt); + ret = test_numeric(make_cspan(imatrix[0]), TypeInt); OIIO_CHECK_EQUAL(ret, "100"); - ret = test_numeric(imatrix, sizeof(imatrix) / sizeof(int), TypeInt); + ret = test_numeric(make_cspan(imatrix), TypeInt, 4); OIIO_CHECK_EQUAL(ret, "100, 200, 300, 400"); OIIO_CHECK_NE(ret, "100, 200, 300, 400,"); // Test it as an array as well - ret = test_numeric(&imatrix[0], 1, TypeDesc(TypeDesc::INT, 4)); + ret = test_numeric(make_cspan(imatrix), TypeDesc(TypeDesc::INT, 4)); OIIO_CHECK_EQUAL(ret, "100, 200, 300, 400"); } { float fmatrix[] = { 10.12f, 200.34f, 300.11f, 400.9f }; - ret = test_numeric(&fmatrix[0], 1, TypeFloat); + ret = test_numeric(make_cspan(fmatrix[0]), TypeFloat); OIIO_CHECK_EQUAL(ret, "10.12"); - ret = test_numeric(fmatrix, sizeof(fmatrix) / sizeof(float), TypeFloat); + ret = test_numeric(make_cspan(fmatrix), TypeFloat, 4); OIIO_CHECK_EQUAL(ret, "10.12, 200.34, 300.11, 400.9"); OIIO_CHECK_NE(ret, "10, 200, 300, 400"); OIIO_CHECK_NE(ret, "10.12, 200.34, 300.11, 400.9,"); - ret = test_numeric(&fmatrix[0], 1, TypeDesc(TypeDesc::FLOAT, 4)); + ret = test_numeric(make_cspan(fmatrix), TypeDesc(TypeDesc::FLOAT, 4)); OIIO_CHECK_EQUAL(ret, "10.12, 200.34, 300.11, 400.9"); } { unsigned long long ullmatrix[] = { 0xffffffffffffffffULL, 0xffffffffffffffffULL }; - ret = test_numeric(&ullmatrix[0], 1, TypeDesc::UINT64); + ret = test_numeric(make_cspan(ullmatrix[0]), TypeDesc::UINT64); OIIO_CHECK_EQUAL(ret, "18446744073709551615"); - ret = test_numeric(ullmatrix, - sizeof(ullmatrix) / sizeof(unsigned long long), - TypeDesc::UINT64); + ret = test_numeric(make_cspan(ullmatrix), TypeDesc::UINT64, 2); OIIO_CHECK_EQUAL(ret, "18446744073709551615, 18446744073709551615"); OIIO_CHECK_NE(ret, "-1, -1"); OIIO_CHECK_NE(ret, "18446744073709551615, 18446744073709551615,"); diff --git a/src/psd.imageio/psdinput.cpp b/src/psd.imageio/psdinput.cpp index 0a5b2875ad..085d775733 100644 --- a/src/psd.imageio/psdinput.cpp +++ b/src/psd.imageio/psdinput.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include // #include "jpeg_memory_src.h" @@ -367,14 +368,16 @@ class PSDInput final : public ImageInput { int alpha_channel, TypeDesc format) const; template - static void cmyk_to_rgb(int n, const T* cmyk, size_t cmyk_stride, T* rgb, - size_t rgb_stride) + static void cmyk_to_rgb(int n, cspan cmyk, size_t cmyk_stride, + span rgb, size_t rgb_stride) { - for (; n; --n, cmyk += cmyk_stride, rgb += rgb_stride) { - float C = convert_type(cmyk[0]); - float M = convert_type(cmyk[1]); - float Y = convert_type(cmyk[2]); - float K = convert_type(cmyk[3]); + DASSERT(size_t(n) * cmyk_stride <= std::size(cmyk)); + DASSERT(size_t(n) * rgb_stride <= std::size(rgb)); + for (int i = 0; i < n; ++i) { + float C = convert_type(cmyk[i * cmyk_stride + 0]); + float M = convert_type(cmyk[i * cmyk_stride + 1]); + float Y = convert_type(cmyk[i * cmyk_stride + 2]); + float K = convert_type(cmyk[i * cmyk_stride + 3]); #if 0 // WHY doesn't this work if it's cmyk? float R = (1.0f - C) * (1.0f - K); @@ -388,12 +391,13 @@ class PSDInput final : public ImageInput { float G = M * (K); float B = Y * (K); #endif - rgb[0] = convert_type(R); - rgb[1] = convert_type(G); - rgb[2] = convert_type(B); + rgb[i * rgb_stride + 0] = convert_type(R); + rgb[i * rgb_stride + 1] = convert_type(G); + rgb[i * rgb_stride + 2] = convert_type(B); if (cmyk_stride == 5 && rgb_stride == 4) { - rgb[3] = convert_type(cmyk[4]); + rgb[i * rgb_stride + 3] = convert_type( + cmyk[i * cmyk_stride + 4]); } } } @@ -783,32 +787,36 @@ PSDInput::read_native_scanline(int subimage, int miplevel, int y, int /*z*/, break; } } else if (m_header.color_mode == ColorMode_CMYK) { + oiio_span_size_type cmyklen = channel_count * spec.width; switch (bps) { case 4: { - std::unique_ptr cmyk( - new float[channel_count * spec.width]); + std::unique_ptr cmyk(new float[cmyklen]); interleave_row(cmyk.get(), channel_buffers, spec.width, channel_count); - cmyk_to_rgb(spec.width, cmyk.get(), channel_count, (float*)dst, + cmyk_to_rgb(spec.width, make_cspan(cmyk.get(), cmyklen), + channel_count, + make_span((float*)dst, spec.width * spec.nchannels), spec.nchannels); break; } case 2: { - std::unique_ptr cmyk( - new unsigned short[channel_count * spec.width]); + std::unique_ptr cmyk(new unsigned short[cmyklen]); interleave_row(cmyk.get(), channel_buffers, spec.width, channel_count); - cmyk_to_rgb(spec.width, cmyk.get(), channel_count, - (unsigned short*)dst, spec.nchannels); + cmyk_to_rgb(spec.width, make_cspan(cmyk.get(), cmyklen), + channel_count, + make_span((uint16_t*)dst, spec.width * spec.nchannels), + spec.nchannels); break; } default: { - std::unique_ptr cmyk( - new unsigned char[channel_count * spec.width]); + std::unique_ptr cmyk(new unsigned char[cmyklen]); interleave_row(cmyk.get(), channel_buffers, spec.width, channel_count); - cmyk_to_rgb(spec.width, cmyk.get(), channel_count, - (unsigned char*)dst, spec.nchannels); + cmyk_to_rgb(spec.width, make_cspan(cmyk.get(), cmyklen), + channel_count, + make_span((uint8_t*)dst, spec.width * spec.nchannels), + spec.nchannels); break; } }