Hi,

First off thanks for this amazing library. We use it extensively in Natron (open-source compositing software) (https://github.com/MrKepzie/Natron/tree/workshop via OpenFX: https://github.com/MrKepzie/openfx-io ) for most of the image reading/writing.

We have received some complaints by our users that the reading of multi-layered EXR files was freakishly slow compared to other similar applications. I have narrowed it down to the ImageCache of OIIO. We make use of the ImageCache::get_pixels function. The problem is that if the image is not present into the cache, the OIIO cache internaly will create a new Tile and calls ImageInput::read_image which in turns calls
ImageInput::read_scanlines(ybegin,yend,z,0,m_spec.nchannels,format,data,xstride,ystride)

So basically regardless of what we requested via the get_pixels function with chbegin and chend, the Tile will always decode all available layers in the image. Needless to say that for EXR files rendered out with many passes it takes a while.

Could this be improved to only read the channels that were requested by ImageCache::get_pixels in the first place ?

Our temporary workaround is to use the ImageInput::read_scanlines API directly, but we suffer from the lack of caching because if the user of Natron requests more than 1 layer to display, it will open as many ImageInput on our side that the the user requested, which is not efficient at all.

While using ThreadSanitizer to check Duke's implementation I stumbled upon this data race.

WARNING: ThreadSanitizer: data race (pid=17556)
  Atomic write of size 4 at 0x7ff153d4cdc0 by thread T2:
    #0 __tsan_atomic32_exchange ??:0 (exe+0x0000001376a7)
    #1 OpenImageIO::v1_3::spin_mutex::try_lock() /home/clitte/git/oiio/src/include/thread.h:570 (libOpenImageIO.so.1.3+0x0000003d482a)
    #2 OpenImageIO::v1_3::spin_mutex::lock() /home/clitte/git/oiio/src/include/thread.h:534 (libOpenImageIO.so.1.3+0x0000003d4731)
    #3 OpenImageIO::v1_3::spin_rw_mutex::read_lock() /home/clitte/git/oiio/src/include/thread.h:633 (libOpenImageIO.so.1.3+0x000000b62ff7)
    #4 read_lock_guard /home/clitte/git/oiio/src/include/thread.h:669 (libOpenImageIO.so.1.3+0x000000b62f96)
    #5 read_lock_guard /home/clitte/git/oiio/src/include/thread.h:669 (libOpenImageIO.so.1.3+0x000000b58430)
    #6 OpenImageIO::v1_3::ustring::make_unique(char const*) /home/clitte/git/oiio/src/libutil/ustring.cpp:179 (libOpenImageIO.so.1.3+0x000000b56457)
    #7 ustring /home/clitte/git/oiio/src/include/ustring.h:166 (libOpenImageIO.so.1.3+0x000000306ea6)
    #8 ustring /home/clitte/git/oiio/src/include/ustring.h:167 (libOpenImageIO.so.1.3+0x000000306e10)
    #9 ustring /home/clitte/git/oiio/src/include/ustring.h:186 (libOpenImageIO.so.1.3+0x000000306c67)
    #10 ustring /home/clitte/git/oiio/src/include/ustring.h:186 (libOpenImageIO.so.1.3+0x000000306bd0)
    #11 OpenImageIO::v1_3::ParamValue::init(std::string, OpenImageIO::v1_3::TypeDesc, int, void const*, bool) /home/clitte/git/oiio/src/include/paramlist.h:104 (libOpenImageIO.so.1.3+0x0000002e8399)
    #12 OpenImageIO::v1_3::ImageSpec::attribute(std::string const&, OpenImageIO::v1_3::TypeDesc, void const*) /home/clitte/git/oiio/src/libOpenImageIO/formatspec.cpp:355 (libOpenImageIO.so.1.3+0x0000002d962f)
    #13 OpenImageIO::v1_3::ImageSpec::attribute(std::string const&, char const*) /home/clitte/git/oiio/src/include/imageio.h:306 (libOpenImageIO.so.1.3+0x0000002c2643)
    #14 OpenImageIO::v1_3::JpgInput::open(std::string const&, OpenImageIO::v1_3::ImageSpec&) /home/clitte/git/oiio/src/jpeg.imageio/jpeginput.cpp:212 (libOpenImageIO.so.1.3+0x000000d76af6)
    #15 OpenImageIOReader /home/clitte/git/duke/src/duke/imageio_plugins/openimageio/OpenImageIO.cpp:135 (exe+0x0000001603b6)
    #16 OpenImageIOReader /home/clitte/git/duke/src/duke/imageio_plugins/openimageio/OpenImageIO.cpp:175 (exe+0x00000015fee0)
    #17 duke::OpenImageIODescriptor::getReaderFromFile(char const*) const /home/clitte/git/duke/src/duke/imageio_plugins/openimageio/OpenImageIO.cpp:223 (exe+0x00000015fdba)
    #18 duke::(anonymous namespace)::tryReader(char const*, duke::IIODescriptor const*, std::function<void (duke::RawPackedFrame&&, void const*)> const&) /home/clitte/git/duke/src/duke/engine/ImageLoadUtils.cpp:45 (exe+0x0000002e7f80)
    #19 duke::(anonymous namespace)::load(char const*, char const*, std::function<void (duke::RawPackedFrame&&, void const*)> const&) /home/clitte/git/duke/src/duke/engine/ImageLoadUtils.cpp:55 (exe+0x0000002e6d71)
    #20 duke::load(char const*, char const*, std::function<void (duke::RawPackedFrame&&, void const*)> const&, std::string&) /home/clitte/git/duke/src/duke/engine/ImageLoadUtils.cpp:66 (exe+0x0000002e69a3)
    #21 duke::LoadedImageCache::workerStep(std::pair<duke::IMediaStream const*, unsigned long>&, std::string&, std::string&) /home/clitte/git/duke/src/duke/engine/cache/LoadedImageCache.cpp:142 (exe+0x00000024d04c)
    #22 duke::LoadedImageCache::workerFunction() /home/clitte/git/duke/src/duke/engine/cache/LoadedImageCache.cpp:109 (exe+0x00000024c82f)
    #23 void std::_Mem_fn<void (duke::LoadedImageCache::*)()>::operator()<, void>(duke::LoadedImageCache*) const /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/functional:601 (exe+0x00000026ca43)
    #24 void std::_Bind_simple<std::_Mem_fn<void (duke::LoadedImageCache::*)()> (duke::LoadedImageCache*)>::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/functional:1732 (exe+0x00000026c8d0)
    #25 std::_Bind_simple<std::_Mem_fn<void (duke::LoadedImageCache::*)()> (duke::LoadedImageCache*)>::operator()() /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/functional:1720 (exe+0x00000026c840)
    #26 std::thread::_Impl<std::_Bind_simple<std::_Mem_fn<void (duke::LoadedImageCache::*)()> (duke::LoadedImageCache*)> >::_M_run() /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/thread:115 (exe+0x00000026c7e9)
    #27 <null> <null>:0 (libstdc++.so.6+0x0000000b1d2f)

  Previous write of size 4 at 0x7ff153d4cdc0 by thread T1:
    #0 OpenImageIO::v1_3::spin_mutex::unlock() /home/clitte/git/oiio/src/include/thread.h:553 (libOpenImageIO.so.1.3+0x0000003d4618)
    #1 OpenImageIO::v1_3::spin_rw_mutex::read_lock() /home/clitte/git/oiio/src/include/thread.h:637 (libOpenImageIO.so.1.3+0x000000b63015)
    #2 read_lock_guard /home/clitte/git/oiio/src/include/thread.h:669 (libOpenImageIO.so.1.3+0x000000b62f96)
    #3 read_lock_guard /home/clitte/git/oiio/src/include/thread.h:669 (libOpenImageIO.so.1.3+0x000000b58430)
    #4 OpenImageIO::v1_3::ustring::make_unique(char const*) /home/clitte/git/oiio/src/libutil/ustring.cpp:179 (libOpenImageIO.so.1.3+0x000000b56457)
    #5 ustring /home/clitte/git/oiio/src/include/ustring.h:166 (libOpenImageIO.so.1.3+0x000000306ea6)
    #6 ustring /home/clitte/git/oiio/src/include/ustring.h:167 (libOpenImageIO.so.1.3+0x000000306e10)
    #7 ustring /home/clitte/git/oiio/src/include/ustring.h:186 (libOpenImageIO.so.1.3+0x000000306c67)
    #8 ustring /home/clitte/git/oiio/src/include/ustring.h:186 (libOpenImageIO.so.1.3+0x000000306bd0)
    #9 OpenImageIO::v1_3::ParamValue::init(std::string, OpenImageIO::v1_3::TypeDesc, int, void const*, bool) /home/clitte/git/oiio/src/include/paramlist.h:104 (libOpenImageIO.so.1.3+0x0000002e8399)
    #10 OpenImageIO::v1_3::ImageSpec::attribute(std::string const&, OpenImageIO::v1_3::TypeDesc, void const*) /home/clitte/git/oiio/src/libOpenImageIO/formatspec.cpp:355 (libOpenImageIO.so.1.3+0x0000002d962f)
    #11 OpenImageIO::v1_3::ImageSpec::attribute(std::string const&, char const*) /home/clitte/git/oiio/src/include/imageio.h:306 (libOpenImageIO.so.1.3+0x0000002c2643)
    #12 OpenImageIO::v1_3::JpgInput::open(std::string const&, OpenImageIO::v1_3::ImageSpec&) /home/clitte/git/oiio/src/jpeg.imageio/jpeginput.cpp:212 (libOpenImageIO.so.1.3+0x000000d76af6)
    #13 OpenImageIOReader /home/clitte/git/duke/src/duke/imageio_plugins/openimageio/OpenImageIO.cpp:135 (exe+0x0000001603b6)
    #14 OpenImageIOReader /home/clitte/git/duke/src/duke/imageio_plugins/openimageio/OpenImageIO.cpp:175 (exe+0x00000015fee0)
    #15 duke::OpenImageIODescriptor::getReaderFromFile(char const*) const /home/clitte/git/duke/src/duke/imageio_plugins/openimageio/OpenImageIO.cpp:223 (exe+0x00000015fdba)
    #16 duke::(anonymous namespace)::tryReader(char const*, duke::IIODescriptor const*, std::function<void (duke::RawPackedFrame&&, void const*)> const&) /home/clitte/git/duke/src/duke/engine/ImageLoadUtils.cpp:45 (exe+0x0000002e7f80)
    #17 duke::(anonymous namespace)::load(char const*, char const*, std::function<void (duke::RawPackedFrame&&, void const*)> const&) /home/clitte/git/duke/src/duke/engine/ImageLoadUtils.cpp:55 (exe+0x0000002e6d71)
    #18 duke::load(char const*, char const*, std::function<void (duke::RawPackedFrame&&, void const*)> const&, std::string&) /home/clitte/git/duke/src/duke/engine/ImageLoadUtils.cpp:66 (exe+0x0000002e69a3)
    #19 duke::LoadedImageCache::workerStep(std::pair<duke::IMediaStream const*, unsigned long>&, std::string&, std::string&) /home/clitte/git/duke/src/duke/engine/cache/LoadedImageCache.cpp:142 (exe+0x00000024d04c)
    #20 duke::LoadedImageCache::workerFunction() /home/clitte/git/duke/src/duke/engine/cache/LoadedImageCache.cpp:109 (exe+0x00000024c82f)
    #21 void std::_Mem_fn<void (duke::LoadedImageCache::*)()>::operator()<, void>(duke::LoadedImageCache*) const /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/functional:601 (exe+0x00000026ca43)
    #22 void std::_Bind_simple<std::_Mem_fn<void (duke::LoadedImageCache::*)()> (duke::LoadedImageCache*)>::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/functional:1732 (exe+0x00000026c8d0)
    #23 std::_Bind_simple<std::_Mem_fn<void (duke::LoadedImageCache::*)()> (duke::LoadedImageCache*)>::operator()() /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/functional:1720 (exe+0x00000026c840)
    #24 std::thread::_Impl<std::_Bind_simple<std::_Mem_fn<void (duke::LoadedImageCache::*)()> (duke::LoadedImageCache*)> >::_M_run() /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/thread:115 (exe+0x00000026c7e9)
    #25 <null> <null>:0 (libstdc++.so.6+0x0000000b1d2f)

  Thread T2 (tid=17560, running) created by main thread at:
    #0 pthread_create ??:0 (exe+0x000000123772)
    #1 <null> <null>:0 (libstdc++.so.6+0x0000000b1f7e)
    #2 std::thread::thread<void (duke::LoadedImageCache::*)(), duke::LoadedImageCache*>(void (duke::LoadedImageCache::*&&)(), duke::LoadedImageCache*&&) /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/thread:138 (exe+0x000000268ab0)
    #3 void __gnu_cxx::new_allocator<std::thread>::construct<std::thread, void (duke::LoadedImageCache::*)(), duke::LoadedImageCache*>(std::thread*, void (duke::LoadedImageCache::*&&)(), duke::LoadedImageCache*&&) /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/ext/new_allocator.h:120 (exe+0x0000002689f5)
    #4 _ZNSt16allocator_traitsISaISt6threadEE12_S_constructIS0_JMN4duke16LoadedImageCacheEFvvEPS5_EEENSt9enable_ifIXsr18__construct_helperIT_DpT0_EE5valueEvE4typeERS1_PSA_DpOSB_ /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/bits/alloc_traits.h:254 (exe+0x0000002688fd)
    #5 _ZNSt16allocator_traitsISaISt6threadEE9constructIS0_JMN4duke16LoadedImageCacheEFvvEPS5_EEEDTcl12_S_constructfp_fp0_spclsr3stdE7forwardIT0_Efp1_EEERS1_PT_DpOS9_ /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/bits/alloc_traits.h:393 (exe+0x000000266acd)
    #6 void std::vector<std::thread, std::allocator<std::thread> >::_M_emplace_back_aux<void (duke::LoadedImageCache::*)(), duke::LoadedImageCache*>(void (duke::LoadedImageCache::*&&)(), duke::LoadedImageCache*&&) /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/bits/vector.tcc:409 (exe+0x000000266d0e)
    #7 void std::vector<std::thread, std::allocator<std::thread> >::emplace_back<void (duke::LoadedImageCache::*)(), duke::LoadedImageCache*>(void (duke::LoadedImageCache::*&&)(), duke::LoadedImageCache*&&) /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/bits/vector.tcc:101 (exe+0x00000024f497)
    #8 duke::LoadedImageCache::startWorkers() /home/clitte/git/duke/src/duke/engine/cache/LoadedImageCache.cpp:93 (exe+0x00000024ba46)
    #9 duke::LoadedImageCache::load(duke::Timeline const&) /home/clitte/git/duke/src/duke/engine/cache/LoadedImageCache.cpp:32 (exe+0x00000024bbbb)
    #10 duke::LoadedTextureCache::load(duke::Timeline const&) /home/clitte/git/duke/src/duke/engine/cache/LoadedTextureCache.cpp:20 (exe+0x00000020d995)
    #11 duke::Player::load(duke::Timeline const&, FrameDuration const&) /home/clitte/git/duke/src/duke/engine/Player.cpp:11 (exe+0x000000277019)
    #12 duke::DukeMainWindow::load(duke::Timeline const&, FrameDuration const&, duke::FitMode, int) /home/clitte/git/duke/src/duke/engine/DukeMainWindow.cpp:64 (exe+0x0000001e9269)
    #13 DukeApplication /home/clitte/git/duke/src/duke/engine/DukeApplication.cpp:136 (exe+0x0000001be2ef)
    #14 main /home/clitte/git/duke/src/duke/main.cpp:37 (exe+0x000000168d34)

  Thread T1 (tid=17559, running) created by main thread at:
    #0 pthread_create ??:0 (exe+0x000000123772)
    #1 <null> <null>:0 (libstdc++.so.6+0x0000000b1f7e)
    #2 std::thread::thread<void (duke::LoadedImageCache::*)(), duke::LoadedImageCache*>(void (duke::LoadedImageCache::*&&)(), duke::LoadedImageCache*&&) /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/thread:138 (exe+0x000000268ab0)
    #3 void __gnu_cxx::new_allocator<std::thread>::construct<std::thread, void (duke::LoadedImageCache::*)(), duke::LoadedImageCache*>(std::thread*, void (duke::LoadedImageCache::*&&)(), duke::LoadedImageCache*&&) /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/ext/new_allocator.h:120 (exe+0x0000002689f5)
    #4 _ZNSt16allocator_traitsISaISt6threadEE12_S_constructIS0_JMN4duke16LoadedImageCacheEFvvEPS5_EEENSt9enable_ifIXsr18__construct_helperIT_DpT0_EE5valueEvE4typeERS1_PSA_DpOSB_ /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/bits/alloc_traits.h:254 (exe+0x0000002688fd)
    #5 _ZNSt16allocator_traitsISaISt6threadEE9constructIS0_JMN4duke16LoadedImageCacheEFvvEPS5_EEEDTcl12_S_constructfp_fp0_spclsr3stdE7forwardIT0_Efp1_EEERS1_PT_DpOS9_ /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/bits/alloc_traits.h:393 (exe+0x000000266acd)
    #6 void std::vector<std::thread, std::allocator<std::thread> >::_M_emplace_back_aux<void (duke::LoadedImageCache::*)(), duke::LoadedImageCache*>(void (duke::LoadedImageCache::*&&)(), duke::LoadedImageCache*&&) /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/bits/vector.tcc:409 (exe+0x000000266d0e)
    #7 void std::vector<std::thread, std::allocator<std::thread> >::emplace_back<void (duke::LoadedImageCache::*)(), duke::LoadedImageCache*>(void (duke::LoadedImageCache::*&&)(), duke::LoadedImageCache*&&) /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.2/../../../../include/c++/4.8.2/bits/vector.tcc:101 (exe+0x00000024f497)
    #8 duke::LoadedImageCache::startWorkers() /home/clitte/git/duke/src/duke/engine/cache/LoadedImageCache.cpp:93 (exe+0x00000024ba46)
    #9 duke::LoadedImageCache::load(duke::Timeline const&) /home/clitte/git/duke/src/duke/engine/cache/LoadedImageCache.cpp:32 (exe+0x00000024bbbb)
    #10 duke::LoadedTextureCache::load(duke::Timeline const&) /home/clitte/git/duke/src/duke/engine/cache/LoadedTextureCache.cpp:20 (exe+0x00000020d995)
    #11 duke::Player::load(duke::Timeline const&, FrameDuration const&) /home/clitte/git/duke/src/duke/engine/Player.cpp:11 (exe+0x000000277019)
    #12 duke::DukeMainWindow::load(duke::Timeline const&, FrameDuration const&, duke::FitMode, int) /home/clitte/git/duke/src/duke/engine/DukeMainWindow.cpp:64 (exe+0x0000001e9269)
    #13 DukeApplication /home/clitte/git/duke/src/duke/engine/DukeApplication.cpp:136 (exe+0x0000001be2ef)
    #14 main /home/clitte/git/duke/src/duke/main.cpp:37 (exe+0x000000168d34)

SUMMARY: ThreadSanitizer: data race ??:0 __tsan_atomic32_exchange

Description

This adds a minimal C API called "coiio" currently that is built as an additional shared library, the implementation of which is under src/coiio and gated behind option BUILD_COIIO (defaults to ON in this PR). I've also added a test under testsuite/coiio that exercises it by loading a tiff, adding a couple of attributes to the header, writing it out as an exr, then reading the exr back and getting the attribute values.

Notable things are right now I've only wrapped TypeDesc::TypeDesc(string_view) as TypeDesc_from_string in order to generate TypeDescs because it was quick. I imagine we'll ultimately want to declare prebuilt extern TypeDesc objects as a replacement for the static constexpr ones for commonly used types in C++.

One slight wrinkle is in all the *_geterror() functions. In C++ they return a copy of an internally owned std::string. I've worked around this for now by caching the string in a thread_local and returning a pointer from that to C. I'm not completely up to date on the semantics of thread_local (does it even work in Apple's libc++ yet?) so not sure if this is that great.

An alternative would be to add a new function, e.g. const char* ImageInput::geterrorc() that would just call m_errmsg.c_str(), then ImageInput::geterror() could call that and make a copy of the string to return, while the C API would just call geterrorc, with an explicit warning that the pointer is only valid until the ImageInput is mutated again. Obviously if ImageInput were to be mutated from another thread then all bets are off and thus I don't think this idea works.

Yet another way, and an idiom you see often in C APIs would be to provide a couple more functions:

int ImageInput::geterrorlength() { return m_errmsg.size(); }
void ImageInput::geterrormsg(char* msg) { strcpy(msg, m_errmsg.c_str(), m_errmsg.size()); }

and have the caller manage the memory. In the multithreaded case this also doesn't work, but another way of doing this is:

void ImageInput::geterrormsg(char* msg, int buffer_len) { 
    lock_guard lock(m_mutex);
    strcpy(msg, m_errmsg.c_str(), min(buffer_len, m_errmsg.size()));
}

Plenty of opportunity for bikeshedding here too. The name is just what I called the library in my rust crate, so can be whatever you prefer. I would suggest making it different enough from OpenImageIO so that you don't mistake the headers in include statements though, e.g.:

#include <OpenImageIO/imagebuf.h>
#include <COpenImageIO/imageio.h> //< oops, but probably won't lead to subtle bugs so may be ok?

Tests

New test is testsuite/coiio. I also copied over run_app() from OSL to the testsuite to allow building the test binary as part of the test.

Checklist:

• [x] I have read the contribution guidelines.
• [ ] If this is more extensive than a small change to existing code, I have previously submitted a Contributor License Agreement (individual, and if there is any way my employers might think my programming belongs to them, then also corporate).
• [x] I have updated the documentation, if applicable.
• [x] I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
• [x] My code follows the prevailing code style of this project.

TODO checklist:

• [x] Add licence blurb to each file
• [x] 3 lines between functions
• [x] extra newline before closing namespace
• [x] rename files to c-imageio.h (or imageio-c.h?)
• [x] fix doc comments to refer to underlying C++ instead of copying full doc comments
• [x] Resolve geterror() situation

Hi, just a question can images (tiff, PSD and others) with more than 4 Channels be read into a memory buffer and written back using OpenImageIO? In the printing industry images with CMYK plus a number of spot color channels are quite common. Photoshop files must only be read, but TIFF must be written.

Is also BigTIFF supported?

Regards

Stefan

Hey,

This PR is to add support for converting from ImageBuf to IplImage. I noticed there was the code to go one way, but not the other. If there's any bad code smell, let me know. I'm primarily a Python programmer, so there's a few things that I'm use to being able to do in Python that I can't seem to do in C++. It could be a limitation of the programmer (most likely), or the language.

If this successfully goes through, then I'll work on implementing OpenCV's Mat next.

I have tested this on a simple image that I grabbed from the net, so it works, but may break under more testing. I want to clean up the code more before I try throwing more image types at this.

One thing that isn't supported right now is the Half type because I'm not sure how is the best way to handle that. Suggestions welcome!

Make atomic_test and spinlock_test run for many more iterations, and time their results.

This allows us to rigorously compare the speed of our atomics and spin locks. Also make their output a little neater by locking around the status printouts, and eliminate the #if's that provide safety for Boost < 1.35, which is no longer supported.

Allow ImageCache/TextureSystem to store partial channel sets in cache.

ImageCache's original design would read all channels into a tile, because (1) most files have only a few channels, (2) most uses (for example, texture) tend to use most or all of the channels in the file, in close temporal proximity if not in the same lookup, (3) most file formats don't have a way to read a channel subset that's any more efficient than reading all of them.

However, for the rare case that there are MANY channels, only a few of which are needed (for example, looking up 3 channels of textures from a 40 channel file), this is very wasteful of texture space, and MAY (depending on the format) also be wasteful of I/O.

So this patch:

• Modifies ImageCache so that a tile can store any contiguous subset of channels. This means the TileID stores a chbegin/chend range, the hashes consider this, and certain ImageCache public API calls now have varieties that take chbegin/chend parameters.
• ImageCache will create cache tiles that store exactly the channel range requested. The default is all channels in the tile. Use subsets wisely -- if you have a 4-channel file and you separately look up channel ranges [0,1) and [0,3), it will redundantly read and store the overlapping channels!
• TextureSystem is modified to ask for all channels when there are only a few channels in the file, but when there are more than max_tile_channels (a new settable attribute of the TS) channels in the file, it will ask the IC for only the channels it needs for the texture lookup. The max_tile_channels is set by default to 5, which means for the vast majority of files (R, RGB, RGBA, RGBAZ) it will read all channels and behave as before. But if you have a mega-channel file, it will try to read and cache only the subset of channels you requested with the texture call. If you don't want the new behavior, just set max_tile_channels to something bigger.

The bottom line is that if your app uses IC with the old API calls, or if you use TS with texture files having 5 or fewer channels (which is almost always the case), you should see no change in behavior or performance. But for those of you who for some reason deal with files of many many channels, accessing a small subset of them via texture or IC should result much more efficient cache behavior because the IC won't be caching channels you aren't accessing.

Hi,

I've been working on this for a few days, and just wanted to get some feedback. Raw format reader powered by LibRaw. Handles most RAW formats. I've given some attributes to control the most common settings (Exposure, ColorSpace primaries, Demosaic algorithm). Only thing missing at the moment is the EXIF metadata, but It shouldn't be too difficult to add that in.

Thanks, Mark

Following up on a conversation with @lgritz , I'd like to request the other direction of what #1895 is requesting.

The Need

In USD and Hydra, we’ve been relying on OIIO for marquee image services, and it’s far and away the most compelling choice. However, recently we’ve grown a need to be able to decode formats (USD, images, volumes) from in-memory buffers, which may have been mmapped from files or created or transmitted. There are two different scenarios:

1. We’re providing an archive format that, in a single (uncompressed zip) file, contains USD files, textures, and anything else a scene requires. To be able to consume the scene without unpacking it, we want to access data out of it by mmapping with offsets into the zip file.
2. People can create “asset resolvers” that deliver data to USD directly over the network, from dbs, etc, and would like to void the step of writing that data out to disk.

So...

We'd like to be able to hand OIIO a buffer-pointer, plus length, plus "identifier" that corresponds to a filename and has an extension that indicate format; that buffer represents an actual serialized image. When we're using OIIO as primarily an image reader, the problem is reasonably straightforward - as Larry suggested, we should be able to initialize an ImageInput from these inputs. This is sufficient for our more immediate needs.

The more involved case is when a client wants to use OIIO as a texture manager, using ImageCache/TextureSystem . In that case, we need to acknowledge that our buffer is a resource that needs to be managed, so we will need a way to allow OIIO to do that. On the USD-side, we'll have a resource object whose lifetime manages the buffer's lifetime, but we'd need a way to adapt that for OIIO's consumption.

Thanks!

Based on private mail with Terrance Mali..., the author of OpenDCP, he brought up the fact that Jasper does not support profile 3 or 4 of the jpeg2000 spec as well as supporting codestream lengths or single tiles, making it unsuitable for DCDM creation.

The solution I put forward is to rewrite the jpeg2000 plugin to use libopenjpeg instead.

After commit e81036b6559 (#2531), I can no longer build on Windows using VS2015 because I get build errors of the form:

oiio\src\oiiotool\oiiotool.cpp(5244): error C2664: 'OpenImageIO_v2_2::ArgParse::Arg &OpenImageIO_v2_2::ArgParse::Arg::action(int (__cdecl *)(int,const char **))': cannot convert argument 1 from 'void (__cdecl *)(OpenImageIO_v2_2::span<const char *const ,-1>)' to 'OpenImageIO_v2_2::ArgParse::ArgAction &&'
oiio\src\oiiotool\oiiotool.cpp(5244): note: Reason: cannot convert from 'overloaded-function' to 'OpenImageIO_v2_2::ArgParse::ArgAction'
oiio\src\oiiotool\oiiotool.cpp(5244): note: No constructor could take the source type, or constructor overload resolution was ambiguous

Looking at the code, I can't see how this could be considered ambiguous and this should work, so I suspect a compiler bug. And in fact, if I build with VS2019 I get no error.

I'm hoping very soon we'll be building OIIO with a newer compiler than VS2015, so in my particular case I don't think I need this fixed in OIIO. However, currently the minimum visual studio required, according to the following is 2015:

src/include/OpenImageIO/platform.h
# if _MSC_VER < 1900
 # error "This version of OIIO is meant to work only with Visual Studio 2015 or later"
# endif

So I think either we'd want to raise the minimum VS (I haven't tested 2017, but I do know 2019 works) or implement a temporary work-around? There's also the possibility I'm messing something up, so of course would be good if someone else could do a sanity check?

Some background: TIFF allows for pixel values to be encoded in some alternative color spaces, like YCbCr, CIELAB, etc., and this is indicated by the TIFF PhotometricInterpretation tag. Since in practice we almost never encounter these encodings in the wild, and they require a lot of custom code to handle (for example, YCbCr also comes along with the ability to subsample the chroma, and options for different placements of the subsampled positions, yuck), for most of these cases we just lean on libtiff's ability to convert the whole thing to RGBA for us, and so in that case, we return to the user the pixels as RGBA, and set the "tiff:ColorSpace" metadata just as an advisory to indicate how it had been encoded in the file. But we don't support writing any of those. (One exception is that we do allow reading and writing of CMYK files.)

OK, but now what about a case where the app already has the data in one of these spaces (such as YCrCb; and let's ignore the whole subsampling miasma and assume we don't want to support that), and all we want to do is write that data to a TIFF file -- without any kind of data conversion -- but to set the TIFF tags properly to indicate that the data in the file is what we know it to be?

A related special case of this is: what if we have a TIFF file that has YCbCr data but neglected to mark it as such (I'm staring daggers at a certain widely used compositing package right now), and to fix this, we want to use OIIO to just copy the file and set the right tags?

It turns out that in the status quo, it is surprisingly difficult to do this. There is just nothing in our existing TIFF output code that will write any of those photometric tags for those spaces, and no way to communicate that this is what we desire.

Whew. Now here's what this patch does:

• If the "oiio:ColorSpace" attribute (our usual way to indicate presumed color space of the pixel data) says that the pixels are already in one of those color spaces (specifically, "YCbCr", "CIELAB", "ICCLAB", "ITULAB", "LOGL", "LOGLUV"), then we set the PhotometricInterpretation tag as such in the TIFF file that we write.

• We still DO NOT interpret that as a request to convert pixel data from any other space into this one. In other words, the TIFF output does not convert color spaces (except for RGB <-> CMYK); this merely lets us output a file that correctly says the data are what the app thinks they are.

• A bunch of minor refactoring was necessary because of some unobvious coupling and order dependencies. For example, the sgilog compression techniques MUST be used with the LOGLUV color space, and vice versa. You have to know how both should be set before checking that they are compatible, and if they are not, adjust something that it's not too late to change.

So the punchline at the end is that if you had a TIFF file that has YCbCr encoded pixels, but for some reason it's marked as ordinary RGB, you can fix it as follows:

oiiotool ycbcr_disguised_as_rgb.tif --iscolorspace YCbCb -o real_ycbcr.tif

• Better Typedesc testing of 'tostring' with fmt style formatting commands, and fix a bug that uncovered.

• Refine 'make CODECOV=1' behavior

• Beef up python-imagecache test to test getpixels, more attribute, invalidate.

• Better testing of python IB construction from numpy array.

• testtex: Use correct guard to avoid compiling unused code for coverage analysis testtex: get rid of unused 3d warp code.

• Add texture-threadtimes test

• Eliminate redundant code in exroutput.c by implementing write_scanline using write_scanlines.

Description

Stores any EXIF data from the given ImageSpec into the PNG eXIf chunk.

Tests

Checklist:

• [x] I have read the contribution guidelines.
• [x] ~~If this is more extensive than a small change to existing code, I have previously submitted a Contributor License Agreement (individual, and if there is any way my employers might think my programming belongs to them, then also corporate).~~ Change is small enough to not require a CLA
• [x] ~~I have updated the documentation, if applicable.~~ Not applicable I think
• [x] I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
• [x] My code follows the prevailing code style of this project.

Describe the bug I loaded two images from the same path. The file was changed between the two operations but since the file's modification time stayed the same (in seconds) ImageCache didn't invalidate it:

If the change takes a longer time such that the modification time of the file, which has seconds resolution, differs for the second image load, buf2 will load from the correct, updated file and that would be the expected behaviour always.

To Reproduce Steps to reproduce the behavior:

std::string filePath = ...;
// Save an image to filePath
ImageBuf buf1(filePath);
// Save another image to filePath
ImageBuf buf2(filePath);

Expected behavior I expect buf2 to hold the update image which is the case if the modification time is updated (in seconds) for the second operation.

Evidence This is intentional behavior of the ImageCache as we can see it here: https://github.com/OpenImageIO/oiio/blob/003a1f15aeb4a7ee7e31c0c21c27eafc9b981f57/src/libtexture/imagecache.cpp#L3395 I don't think ignoring the changes of a file should depend on the resolution of the file time, at least not if the resolution is seconds.

Platform information:

• OIIO branch/version: 2.3.7.2
• OS: macOS 12.6.2 (21G320)
• C++ compiler: Apple clang version 14.0.0 (clang-1400.0.29.202)
• Any non-default build flags when you build OIIO: none

Describe the bug I am trying to reencode an animated GIF from an IOMemReader to an IOVecOutput. All the code for copying the images and creating the output works except that I cannot set the delay (FPS, FramesPerSecond) in the output. I've reviewed the code, and cannot see how I'm supposed to be able to set the delay without a change to the OIIO library.

Let me walk you through the library code's logic. In gif.h the routine GifWriteLzwImage encodes the output using the argument delay. GifWriteLzwImage is called from gif.h's GifWriteFrame routine which also gets delay from an argument. GifWriteFrame is called by gifoutput.cpp's GIFOutput::finish_subimage which passes GIFOutput::m_delay. GIFOutput::m_delay only gets set in a single place, GIFOutput::open(const std::string&, int, const ImageSpec*). This open routine is where I believe the issue to be. m_delay is set using the attribute "FramesPerSecond" in m_spec. You'll notice that open gets an ImageSpec as an argument. The passed in ImageSpec is only ever used to initialize the m_subimagespecs array. The m_subimagespecs array is never used for anything, and can be removed. It's my belief that m_spec, which is used to create m_delay, is never initializable from outside the library. From outside the library you're able to control the specs argument, but that's never actually used. So I believe the fix is to use specs to initialize m_spec, and then get the "FramesPerSecond" attribute.

To Reproduce Steps to reproduce the behavior:

1. Run code to take https://media.tenor.com/QWAo2echlVEAAAAC/cute.gif and reencode the animated GIF with the same 0.1 second delay. That's a "FramesPerSecond" attribute of 100/10.

Expected behavior It should be possible to use the OpenImageIO library to write an animated GIF with a non-default delay between frames.

Evidence No matter what I do, the animated GIFs I write with OpenImageIO always encode with the default 1 seconds delay. Hopefully my code review is evidence enough, but I'd appreciate any alternate way of viewing the fix. Maybe there's another way to set m_delay or m_spec that I'm unaware of.

Platform information:

• OIIO branch/version: master branch commit 1449b465000bf7c83e983cf870ca7b1f2966d446
• OS: CentOS Stream 9
• C++ compiler: gcc version 11.3.1 20221121
• Any non-default build flags when you build OIIO: None

IF YOU ALREADY HAVE A CODE FIX: Here's the fix that I've tested. If there's an alternate solution, please let me know. Thank you.

diff --git a/src/gif.imageio/gifoutput.cpp b/src/gif.imageio/gifoutput.cpp
index e033f221..14a8c0e4 100644
--- a/src/gif.imageio/gifoutput.cpp
+++ b/src/gif.imageio/gifoutput.cpp
@@ -83,7 +83,6 @@ private:
     int m_subimage;  // Current subimage index
     int m_nsubimages;
     bool m_pending_write;                    // Do we have an image buffered?
-    std::vector<ImageSpec> m_subimagespecs;  // Saved subimage specs
     GifWriter<Filesystem::IOProxy> m_gifwriter;
     std::vector<uint8_t> m_canvas;  // Image canvas, accumulating output
     int m_delay;
@@ -154,7 +153,9 @@ GIFOutput::open(const std::string& name, int subimages, const ImageSpec* specs)
     m_filename   = name;
     m_subimage   = 0;
     m_nsubimages = subimages;
-    m_subimagespecs.assign(specs, specs + subimages);
+    if (specs != nullptr) {
+        m_spec = *specs;
+    }
     float fps = m_spec.get_float_attribute("FramesPerSecond", 1.0f);
     m_delay   = (fps == 0.0f ? 0 : (int)(100.0f / fps));

Describe the bug

1. https://github.com/OpenImageIO/oiio/blob/master/src/build-scripts/build_ninja.bash#L13 performs a check to see if a .zip file exists. If it doesn't a .tar.gz file is downloaded. Either way a .tar.gz file is extracted. The check should be for the existance of a .tar.gz file, not a .zip file.
2. The path of the file downloaded https://github.com/ninja-build/ninja/archive/v1.10.2.tar.gz, no longer aligned with GitHub's release file structure. The download is now available at https://github.com/ninja-build/ninja/archive/refs/tags/v1.10.2.tar.gz. "refs/tags" needs to be added.

To Reproduce Steps to reproduce the behavior:

1. Run src/build-scripts/build_ninja.bash
2. Or run src/build-scripts/build_ninja.bash with oiio/ext/downloads/ninja-1.10.2.zip already present. You can download it from https://github.com/ninja-build/ninja/archive/refs/tags/v1.10.2.zip

Expected behavior If the .zip file is present, but the .tar.gz file is not, the script will attempt to untar the nonexistent file and fail. If neither file is present, the script will attempt to download the .tar.gz file file from the wrong location and fail.

Evidence

+ LOCAL_DEPS_DIR=/home/cent/git/external/oiio/ext
+ DOWNLOADS_DIR=/home/cent/git/external/oiio/ext/downloads
+ NINJA_REPO=https://github.com/ninja-build/ninja
+ NINJA_VERSION=1.10.2
+ NINJA_BRANCH=v1.10.2
+ NINJA_INSTALL_DIR=/home/cent/git/external/oiio/ext/dist/bin
+ '[' '!' -f /home/cent/git/external/oiio/ext/downloads/ninja-1.10.2.zip ']'
+ curl --location https://github.com/ninja-build/ninja/archive/v1.10.2.tar.gz -o /home/cent/git/external/oiio/ext/downloads/ninja-v1.10.2.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0Warning: Failed to create the file
Warning: /home/cent/git/external/oiio/ext/downloads/ninja-v1.10.2.tar.gz: No
Warning: such file or directory
  0  208k    0   512    0     0    773      0  0:04:36 --:--:--  0:04:36  2327
curl: (23) Failure writing output to destination

Platform information:

• OIIO branch/version: commit 1449b465000bf7c83e983cf870ca7b1f2966d446 (HEAD -> master, origin/master, origin/HEAD)
• OS: CentOS Stream release 9
• C++ compiler: g++ 11.3.1 20221121
• Any non-default build flags when you build OIIO: No. Not getting that far.