diff --git a/src/doc/oiiotool.rst b/src/doc/oiiotool.rst index e32eaa0dc9..8498277d9c 100644 --- a/src/doc/oiiotool.rst +++ b/src/doc/oiiotool.rst @@ -916,8 +916,8 @@ output each one to a different file, with names `sub0001.tif`, .. option:: --cache - Set the underlying ImageCache size (in MB). See Section - :ref:`sec-imagecache-api`. + Causes images to be read through an ImageCache and set the underlying + cache size (in MB). See Section :ref:`sec-imagecache-api`. .. option:: --oiioattrib @@ -1264,29 +1264,30 @@ These are all non-positional flags that affect how all images are read in the .. option:: --native - Normally, all images read by :program:`oiiotool` are read into an - ImageBuf backed by an underlying ImageCache, and are automatically + Normally, all images read by :program:`oiiotool` are automatically converted to `float` pixels for internal storage (because any subsequent image processing is usually much faster and more accurate when done on - floating-point values). - - This option causes (1) input images to be stored internally in their - native pixel data type rather than converted to float, and (2) to bypass - the ImageCache (reading directly into an ImageBuf) if the pixel data - type is not one of the types that is supported internally to ImageCache - (`UINT8`, `uint16`, `half`, and `float`). - - images whose pixels are comprised of data types that are not natively - representable exactly in the ImageCache to bypass the ImageCache and be - read directly into an ImageBuf. - - The typical use case for this is when you know you are dealing with - unusual pixel data types that might lose precision if converted to - `float` (for example, if you have images with `uint32` or `double` - pixels). Another use case is if you are using :program:`oiiotool` merely - for file format or data format conversion, with no actual image - processing math performed on the pixel values -- in that case, you might - save time and memory by bypassing the conversion to `float`. + floating-point values), and also if the `--cache` option is used, the + reading and storage of images will be mediated through an ImageCache. + + The `--native` option causes input images to be stored internally in their + native pixel data type of th file rather than converted to float. Also, + even if the `--cache` option is used, reads will bypass the ImageCache + (reading directly into an ImageBuf) if the pixel data type is not one of + the types that is supported internally by ImageCache (`UINT8`, `uint16`, + `half`, and `float`). + + There are three uses cases where `--native` might be very helpful: (a) If + you are using :program:`oiiotool` merely for file format or data format + conversion, with no actual image processing math performed on the pixel + values -- in that case, you might save time and memory by avoiding the + conversion to `float`. (b) If you are reading exceptionally large images + that have smaller data types than `float` (for example, `uint8` pixels), + and the only way to make the images fit in memory are to store them as + uint8 rather than converting to float (which takes 4 times as much space + in memory). (c) If you know the file has unusual pixel data types that + might lose precision if converted to `float` (for example, if you have + images with `uint32` or `double` pixels). .. option:: --autotile diff --git a/src/include/OpenImageIO/imagebuf.h b/src/include/OpenImageIO/imagebuf.h index 632035fc20..5d5e5b82f1 100644 --- a/src/include/OpenImageIO/imagebuf.h +++ b/src/include/OpenImageIO/imagebuf.h @@ -61,7 +61,7 @@ enum class InitializePixels { No = 0, Yes = 1 }; /// the details of memory layout and data representation (translating /// to/from float automatically). /// -/// ImageBuf makes an important simplification: all channels are just one +/// ImageBuf makes an important simplification: all channels are the same /// data type. For example, if an image file on disk has a mix of `half` and /// `float` channels, the in-memory ImageBuf representation will be entirely /// `float` (for mixed data types, it will try to pick one that can best @@ -76,21 +76,17 @@ enum class InitializePixels { No = 0, Yes = 1 }; /// other than the first subimage of the file, or forcing the read to /// translate into a different data format than appears in the file). /// -/// ImageBuf data coming from disk files is backed by ImageCache. That is, -/// especially for tiled files, specific regions of the image will only -/// be read if and when they are needed, and if there are many large -/// ImageBuf's, memory holding pixels not recently accesssed will be -/// automatically freed. Thus, performance of ImageBuf on very large images -/// (or if there are many ImageBuf's simultaneously in use) can be sensitive -/// to choices of the ImageCache parameters such as "autotile". It may be -/// wise for maximum performance to explicitly `read()` (with `force=true`) -/// small images into memory rather than using the ImageCache, in cases -/// where your application has no need for the ImageCache features that -/// limit memory footprint (such as if you know for sure that your app will -/// only read a small number of images, of reasonable size, and will need -/// to access all the pixels of all the images it reads). +/// ImageBuf data coming from disk files may optionally be backed by +/// ImageCache, by explicitly passing an `ImageCache*` to the ImageBuf +/// constructor or `reset()` method (pass `ImageCache::create()` to get a +/// pointer to the default global ImageCache), or by having previously set the +/// global OIIO attribute `"imagebuf:use_imagecache"` to a nonzero value. When +/// an ImageBuf is backed by ImageCache in this way, specific regions of the +/// image will only be read if and when they are needed, and if there are many +/// large ImageBuf's, memory holding pixels not recently accessed will be +/// automatically freed if the cache size limit is reached. /// -/// Writeable ImageBufs are always stored entirely in memory, and do not use +/// Writable ImageBufs are always stored entirely in memory, and do not use /// the ImageCache or any other clever schemes to limit memory. If you have /// enough simultaneous writeable large ImageBuf's, you can run out of RAM. /// Note that if an ImageBuf starts as readable (backed by ImageCache), any @@ -178,11 +174,8 @@ class OIIO_API ImageBuf { /// The subimage and MIP level to read (defaults to the /// first subimage of the file, highest-res MIP level). /// @param imagecache - /// Optionally, a particular ImageCache to use. If nullptr, - /// the default global/shared image cache will be used. If - /// a custom ImageCache (not the global/shared one), it is - /// important that the IC should not be destroyed while the - /// ImageBuf is still alive. + /// Optionally, an ImageCache to use, if possible, rather + /// than reading the entire image file into memory. /// @param config /// Optionally, a pointer to an ImageSpec whose metadata /// contains configuration hints that set options related @@ -393,7 +386,8 @@ class OIIO_API ImageBuf { /// `LOCALPIXELS` storage buffer). Otherwise, it is up to /// the implementation whether to immediately read or have /// the image backed by an ImageCache (storage - /// `IMAGECACHE`.) + /// `IMAGECACHE`, if the ImageBuf was originall constructed + /// or reset with an ImageCache specified). /// @param convert /// If set to a specific type (not`UNKNOWN`), the ImageBuf /// memory will be allocated for that type specifically and @@ -1026,7 +1020,8 @@ class OIIO_API ImageBuf { /// image being in RAM somewhere? bool cachedpixels() const; - /// A pointer to the underlying ImageCache. + /// A pointer to the underlying ImageCache, or nullptr if this ImageBuf + /// is not backed by an ImageCache. ImageCache* imagecache() const; /// Return the address where pixel `(x,y,z)`, channel `ch`, is stored in diff --git a/src/include/OpenImageIO/imageio.h b/src/include/OpenImageIO/imageio.h index 9c96fe773c..a6d5a22d8f 100644 --- a/src/include/OpenImageIO/imageio.h +++ b/src/include/OpenImageIO/imageio.h @@ -2994,6 +2994,11 @@ OIIO_API std::string geterror(bool clear = true); /// to neither retrieve errors themselves nor have them printed in this /// manner can disable the behavior by setting this attribute to 0. /// +/// - `imagebuf:use_imagecache` (0) +/// +/// If nonzero, an `ImageBuf` that references a file but is not given an +/// ImageCache will read the image through the default ImageCache. +/// OIIO_API bool attribute(string_view name, TypeDesc type, const void* val); /// Shortcut attribute() for setting a single integer. @@ -3059,6 +3064,21 @@ inline bool attribute (string_view name, string_view val) { /// full paths), and all the directories that OpenImageIO will search for /// fonts. (Added in OpenImageIO 2.5) /// +/// - int64_t IB_local_mem_current +/// - int64_t IB_local_mem_peak +/// +/// Current and peak size (in bytes) of how much memory was consumed by +/// ImageBufs that owned their own allcoated local pixel buffers. (Added in +/// OpenImageIO 2.5.) +/// +/// - float IB_total_open_time +/// - float IB_total_image_read_time +/// +/// Total amount of time (in seconds) that ImageBufs spent opening +/// (including reading header information) and reading pixel data from files +/// that they opened and read themselves (that is, excluding I/O from IBs +/// that were backed by ImageCach. (Added in OpenImageIO 2.5.) +/// /// - `string opencolorio_version` /// /// Returns the version (such as "2.2.0") of OpenColorIO that is used by diff --git a/src/libOpenImageIO/imagebuf.cpp b/src/libOpenImageIO/imagebuf.cpp index 7125fecb4b..e60fc9f149 100644 --- a/src/libOpenImageIO/imagebuf.cpp +++ b/src/libOpenImageIO/imagebuf.cpp @@ -28,7 +28,14 @@ OIIO_NAMESPACE_BEGIN OIIO_STRONG_PARAM_TYPE(DoLock, bool); -static atomic_ll IB_local_mem_current; +namespace pvt { +int imagebuf_print_uncaught_errors(1); +int imagebuf_use_imagecache(0); +atomic_ll IB_local_mem_current; +atomic_ll IB_local_mem_peak; +std::atomic IB_total_open_time(0.0f); +std::atomic IB_total_image_read_time(0.0f); +} // namespace pvt @@ -291,11 +298,11 @@ class ImageBufImpl { stride_t m_zstride; stride_t m_channel_stride; bool m_contiguous; - ImageCache* m_imagecache; ///< ImageCache to use - TypeDesc m_cachedpixeltype; ///< Data type stored in the cache - DeepData m_deepdata; ///< Deep data - size_t m_allocated_size; ///< How much memory we've allocated - std::vector m_blackpixel; ///< Pixel-sized zero bytes + ImageCache* m_imagecache = nullptr; ///< ImageCache to use + TypeDesc m_cachedpixeltype; ///< Data type stored in the cache + DeepData m_deepdata; ///< Deep data + size_t m_allocated_size; ///< How much memory we've allocated + std::vector m_blackpixel; ///< Pixel-sized zero bytes std::vector m_write_format; /// Pixel data format to use for write() int m_write_tile_width; int m_write_tile_height; @@ -389,23 +396,9 @@ ImageBufImpl::ImageBufImpl(string_view filename, int subimage, int miplevel, } m_spec_valid = true; } else if (filename.length() > 0) { + // filename being nonempty means this ImageBuf refers to a file. OIIO_DASSERT(buffer == nullptr); - // Invalidate the image in cache. Do so unconditionally if there's - // a chance that configuration hints may have changed. - invalidate(m_name, config || m_configspec); - // If a filename was given, read the spec and set it up as an - // ImageCache-backed image. Reallocate later if an explicit read() - // is called to force read into a local buffer. - if (config) - m_configspec.reset(new ImageSpec(*config)); - m_rioproxy = ioproxy; - if (m_rioproxy) { - add_configspec(); - m_configspec->attribute("oiio:ioproxy", TypeDesc::PTR, &m_rioproxy); - } - read(subimage, miplevel); - // FIXME: investigate if the above read is really necessary, or if - // it can be eliminated and done fully lazily. + reset(filename, subimage, miplevel, imagecache, config, ioproxy); } else { OIIO_DASSERT(buffer == nullptr); } @@ -618,14 +611,15 @@ ImageBufImpl::new_pixels(size_t size, const void* data) size = 0; } m_allocated_size = size; - IB_local_mem_current += m_allocated_size; + pvt::IB_local_mem_current += m_allocated_size; + atomic_max(pvt::IB_local_mem_peak, (long long)pvt::IB_local_mem_current); if (data && size) memcpy(m_pixels.get(), data, size); m_localpixels = m_pixels.get(); m_storage = size ? ImageBuf::LOCALBUFFER : ImageBuf::UNINITIALIZED; if (pvt::oiio_print_debug > 1) OIIO::debugfmt("IB allocated {} MB, global IB memory now {} MB\n", - size >> 20, IB_local_mem_current >> 20); + size >> 20, pvt::IB_local_mem_current >> 20); eval_contiguous(); return m_localpixels; } @@ -637,8 +631,9 @@ ImageBufImpl::free_pixels() if (m_allocated_size) { if (pvt::oiio_print_debug > 1) OIIO::debugfmt("IB freed {} MB, global IB memory now {} MB\n", - m_allocated_size >> 20, IB_local_mem_current >> 20); - IB_local_mem_current -= m_allocated_size; + m_allocated_size >> 20, + pvt::IB_local_mem_current >> 20); + pvt::IB_local_mem_current -= m_allocated_size; m_allocated_size = 0; } m_pixels.reset(); @@ -768,11 +763,14 @@ ImageBufImpl::reset(string_view filename, int subimage, int miplevel, { clear(); m_name = ustring(filename); - invalidate(m_name, false); + if (m_imagecache || pvt::imagebuf_use_imagecache) { + // Invalidate the image in cache. Do so unconditionally if there's a + // chance that configuration hints may have changed. + invalidate(m_name, config || m_configspec); + } m_current_subimage = subimage; m_current_miplevel = miplevel; - if (imagecache) - m_imagecache = imagecache; + m_imagecache = imagecache; if (config) m_configspec.reset(new ImageSpec(*config)); m_rioproxy = ioproxy; @@ -782,10 +780,10 @@ ImageBufImpl::reset(string_view filename, int subimage, int miplevel, } if (m_name.length() > 0) { - // If a filename was given, read the spec and set it up as an - // ImageCache-backed image. Reallocate later if an explicit read() - // is called to force read into a local buffer. + // filename non-empty means this ImageBuf refers to a file. read(subimage, miplevel); + // FIXME: investigate if the above read is really necessary, or if + // it can be eliminated and done fully lazily. } } @@ -825,12 +823,17 @@ ImageBufImpl::reset(string_view filename, const ImageSpec& spec, m_current_subimage = 0; m_current_miplevel = 0; if (buffer) { - m_spec = spec; - m_xstride = xstride; - m_ystride = ystride; - m_zstride = zstride; + m_spec = spec; + m_nativespec = nativespec ? *nativespec : spec; + m_channel_stride = stride_t(spec.format.size()); + m_xstride = xstride; + m_ystride = ystride; + m_zstride = zstride; ImageSpec::auto_stride(m_xstride, m_ystride, m_zstride, m_spec.format, m_spec.nchannels, m_spec.width, m_spec.height); + m_blackpixel.resize(round_to_multiple(spec.pixel_bytes(), + OIIO_SIMD_MAX_SIZE_BYTES), + 0); m_localpixels = (char*)buffer; m_storage = ImageBuf::APPBUFFER; m_pixels_valid = true; @@ -936,81 +939,135 @@ ImageBufImpl::init_spec(string_view filename, int subimage, int miplevel, m_name = filename; - // Make sure we have access to an imagecache. Also invalidate any cache - // info for the file just in case it has changed on disk. - if (!m_imagecache) - m_imagecache = ImageCache::create(true /* shared cache */); - - m_pixels_valid = false; - m_nsubimages = 0; - m_nmiplevels = 0; - static ustring s_subimages("subimages"), s_miplevels("miplevels"); - static ustring s_fileformat("fileformat"); - if (m_configspec) { // Pass configuration options to cache - // Invalidate the file in the cache, and add with replacement - // because it might have a different config than last time. - m_imagecache->invalidate(m_name, true); - m_imagecache->add_file(m_name, nullptr, m_configspec.get(), - /*replace=*/true); - } else { - // If no configspec, just do a regular soft invalidate - invalidate(m_name, false); - } - m_imagecache->get_image_info(m_name, subimage, miplevel, s_subimages, - TypeInt, &m_nsubimages); - m_imagecache->get_image_info(m_name, subimage, miplevel, s_miplevels, - TypeInt, &m_nmiplevels); - const char* fmt = NULL; - m_imagecache->get_image_info(m_name, subimage, miplevel, s_fileformat, - TypeString, &fmt); - m_fileformat = ustring(fmt); - m_imagecache->get_imagespec(m_name, m_spec, subimage, miplevel); - m_imagecache->get_imagespec(m_name, m_nativespec, subimage, miplevel, true); - m_xstride = m_spec.pixel_bytes(); - m_ystride = m_spec.scanline_bytes(); - m_zstride = clamped_mult64(m_ystride, (imagesize_t)m_spec.height); - m_channel_stride = m_spec.format.size(); - m_blackpixel.resize(round_to_multiple(m_xstride, OIIO_SIMD_MAX_SIZE_BYTES), - 0); - // ^^^ NB make it big enough for SIMD - - // Go ahead and read any thumbnail that exists. Is that bad? - if (m_spec["thumbnail_width"].get() - && m_spec["thumbnail_height"].get()) { - m_thumbnail.reset(new ImageBuf); - m_imagecache->get_thumbnail(m_name, *m_thumbnail, subimage); - m_has_thumbnail = true; - } - - // Subtlety: m_nativespec will have the true formats of the file, but - // we rig m_spec to reflect what it will look like in the cache. - // This may make m_spec appear to change if there's a subsequent read() - // that forces a full read into local memory, but what else can we do? - // It causes havoc for it to suddenly change in the other direction - // when the file is lazily read. - int peltype = TypeDesc::UNKNOWN; - m_imagecache->get_image_info(m_name, subimage, miplevel, - ustring("cachedpixeltype"), TypeInt, &peltype); - if (peltype != TypeDesc::UNKNOWN) { - m_spec.format = (TypeDesc::BASETYPE)peltype; - m_spec.channelformats.clear(); - } - - if (m_nsubimages) { - m_badfile = false; - m_pixelaspect = m_spec.get_float_attribute("pixelaspectratio", 1.0f); - m_current_subimage = subimage; - m_current_miplevel = miplevel; - m_spec_valid = true; + // If we weren't given an imagecache but "imagebuf:use_imagecache" + // attribute was set, use a shared IC. + if (m_imagecache == nullptr && pvt::imagebuf_use_imagecache) + m_imagecache = ImageCache::create(true); + + if (m_imagecache) { + m_pixels_valid = false; + m_nsubimages = 0; + m_nmiplevels = 0; + static ustring s_subimages("subimages"), s_miplevels("miplevels"); + static ustring s_fileformat("fileformat"); + if (m_configspec) { // Pass configuration options to cache + // Invalidate the file in the cache, and add with replacement + // because it might have a different config than last time. + m_imagecache->invalidate(m_name, true); + m_imagecache->add_file(m_name, nullptr, m_configspec.get(), + /*replace=*/true); + } else { + // If no configspec, just do a regular soft invalidate + invalidate(m_name, false); + } + m_imagecache->get_image_info(m_name, subimage, miplevel, s_subimages, + TypeInt, &m_nsubimages); + m_imagecache->get_image_info(m_name, subimage, miplevel, s_miplevels, + TypeInt, &m_nmiplevels); + const char* fmt = NULL; + m_imagecache->get_image_info(m_name, subimage, miplevel, s_fileformat, + TypeString, &fmt); + m_fileformat = ustring(fmt); + m_imagecache->get_imagespec(m_name, m_spec, subimage, miplevel); + m_imagecache->get_imagespec(m_name, m_nativespec, subimage, miplevel, + true); + m_xstride = m_spec.pixel_bytes(); + m_ystride = m_spec.scanline_bytes(); + m_zstride = clamped_mult64(m_ystride, (imagesize_t)m_spec.height); + m_channel_stride = m_spec.format.size(); + m_blackpixel.resize(round_to_multiple(m_xstride, + OIIO_SIMD_MAX_SIZE_BYTES), + 0); + // ^^^ NB make it big enough for SIMD + + // Go ahead and read any thumbnail that exists. Is that bad? + if (m_spec["thumbnail_width"].get() + && m_spec["thumbnail_height"].get()) { + m_thumbnail.reset(new ImageBuf); + m_imagecache->get_thumbnail(m_name, *m_thumbnail, subimage); + m_has_thumbnail = true; + } + + // Subtlety: m_nativespec will have the true formats of the file, but + // we rig m_spec to reflect what it will look like in the cache. + // This may make m_spec appear to change if there's a subsequent read() + // that forces a full read into local memory, but what else can we do? + // It causes havoc for it to suddenly change in the other direction + // when the file is lazily read. + int peltype = TypeDesc::UNKNOWN; + m_imagecache->get_image_info(m_name, subimage, miplevel, + ustring("cachedpixeltype"), TypeInt, + &peltype); + if (peltype != TypeDesc::UNKNOWN) { + m_spec.format = (TypeDesc::BASETYPE)peltype; + m_spec.channelformats.clear(); + } + + if (m_nsubimages) { + m_badfile = false; + m_pixelaspect = m_spec.get_float_attribute("pixelaspectratio", + 1.0f); + m_current_subimage = subimage; + m_current_miplevel = miplevel; + m_spec_valid = true; + } else { + m_badfile = true; + m_current_subimage = -1; + m_current_miplevel = -1; + m_err = m_imagecache->geterror(); + m_spec_valid = false; + // std::cerr << "ImageBuf ERROR: " << m_err << "\n"; + } } else { + // + // No imagecache supplied, we will use ImageInput directly + // + Timer timer; m_badfile = true; + m_pixels_valid = false; + m_spec_valid = false; + m_nsubimages = 0; + m_nmiplevels = 0; + m_badfile = false; m_current_subimage = -1; m_current_miplevel = -1; - m_err = m_imagecache->geterror(); - m_spec_valid = false; - // std::cerr << "ImageBuf ERROR: " << m_err << "\n"; - } + auto input = ImageInput::open(filename, m_configspec.get(), m_rioproxy); + if (!input) { + m_err = OIIO::geterror(); + atomic_fetch_add(pvt::IB_total_open_time, float(timer())); + return false; + } + m_spec = input->spec(subimage, miplevel); + if (input->has_error()) { + m_err = input->geterror(); + atomic_fetch_add(pvt::IB_total_open_time, float(timer())); + return false; + } + m_badfile = false; + m_spec_valid = true; + m_fileformat = ustring(input->format_name()); + m_nativespec = m_spec; + m_xstride = m_spec.pixel_bytes(); + m_ystride = m_spec.scanline_bytes(); + m_zstride = clamped_mult64(m_ystride, (imagesize_t)m_spec.height); + m_channel_stride = m_spec.format.size(); + m_blackpixel.resize( + round_to_multiple(m_xstride, OIIO_SIMD_MAX_SIZE_BYTES)); + // ^^^ NB make it big enough for SIMD + + // Go ahead and read any thumbnail that exists. Is that bad? + if (m_spec["thumbnail_width"].get() + && m_spec["thumbnail_height"].get()) { + m_thumbnail.reset(new ImageBuf); + m_has_thumbnail = input->get_thumbnail(*m_thumbnail.get(), + subimage); + } + m_current_subimage = subimage; + m_current_miplevel = miplevel; + m_pixelaspect = m_spec.get_float_attribute("pixelaspectratio", 1.0f); + atomic_fetch_add(pvt::IB_total_open_time, float(timer())); + } return !m_badfile; } @@ -1056,6 +1113,7 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, bool use_channel_subset = (chbegin != 0 || chend != nativespec().nchannels); if (m_spec.deep) { + Timer timer; auto input = ImageInput::open(m_name.string(), m_configspec.get(), m_rioproxy); if (!input) { @@ -1063,41 +1121,51 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, return false; } input->threads(threads()); // Pass on our thread policy - if (!input->read_native_deep_image(subimage, miplevel, m_deepdata)) { + bool ok = input->read_native_deep_image(subimage, miplevel, m_deepdata); + if (ok) { + m_spec = m_nativespec; // Deep images always use native data + m_pixels_valid = true; + m_storage = ImageBuf::LOCALBUFFER; + } else { error(input->geterror()); - return false; } - m_spec = m_nativespec; // Deep images always use native data - m_pixels_valid = true; - m_storage = ImageBuf::LOCALBUFFER; - return true; + atomic_fetch_add(pvt::IB_total_image_read_time, float(timer())); + return ok; } m_pixelaspect = m_spec.get_float_attribute("pixelaspectratio", 1.0f); - // If we don't already have "local" pixels, and we aren't asking to - // convert the pixels to a specific (and different) type, then take an - // early out by relying on the cache. - int peltype = TypeDesc::UNKNOWN; - m_imagecache->get_image_info(m_name, subimage, miplevel, - ustring("cachedpixeltype"), TypeInt, &peltype); - m_cachedpixeltype = TypeDesc((TypeDesc::BASETYPE)peltype); - if (!m_localpixels && !force && !use_channel_subset - && (convert == m_cachedpixeltype || convert == TypeDesc::UNKNOWN)) { - m_spec.format = m_cachedpixeltype; - m_xstride = m_spec.pixel_bytes(); - m_ystride = m_spec.scanline_bytes(); - m_zstride = clamped_mult64(m_ystride, (imagesize_t)m_spec.height); - m_blackpixel.resize(round_to_multiple(m_xstride, - OIIO_SIMD_MAX_SIZE_BYTES), - 0); - // NB make it big enough for SSE - m_pixels_valid = true; - m_storage = ImageBuf::IMAGECACHE; + if (m_imagecache) { + // If we don't already have "local" pixels, and we aren't asking to + // convert the pixels to a specific (and different) type, then take an + // early out by relying on the cache. + int peltype = TypeDesc::UNKNOWN; + m_imagecache->get_image_info(m_name, subimage, miplevel, + ustring("cachedpixeltype"), TypeInt, + &peltype); + m_cachedpixeltype = TypeDesc((TypeDesc::BASETYPE)peltype); + if (!m_localpixels && !force && !use_channel_subset + && (convert == m_cachedpixeltype || convert == TypeDesc::UNKNOWN)) { + m_spec.format = m_cachedpixeltype; + m_xstride = m_spec.pixel_bytes(); + m_ystride = m_spec.scanline_bytes(); + m_zstride = clamped_mult64(m_ystride, (imagesize_t)m_spec.height); + m_blackpixel.resize(round_to_multiple(m_xstride, + OIIO_SIMD_MAX_SIZE_BYTES), + 0); + // NB make it big enough for SSE + m_pixels_valid = true; + m_storage = ImageBuf::IMAGECACHE; #ifndef NDEBUG - // std::cerr << "read was not necessary -- using cache\n"; + // std::cerr << "read was not necessary -- using cache\n"; #endif - return true; + return true; + } + + } else { + // No cache should take the "forced read now" route. + force = true; + m_cachedpixeltype = m_nativespec.format; } if (use_channel_subset) { @@ -1127,7 +1195,7 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, m_spec.tile_height = m_nativespec.tile_height; m_spec.tile_depth = m_nativespec.tile_depth; - if (force || m_rioproxy + if (force || !m_imagecache || m_rioproxy || (convert != TypeDesc::UNKNOWN && convert != m_cachedpixeltype && convert.size() >= m_cachedpixeltype.size() && convert.size() >= m_nativespec.format.size())) { @@ -1137,8 +1205,9 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, // loss of range or precision resulting from going through the // cache. Or the caller requested a forced read, for that case we // also do a direct read now. - if (!m_configspec - || !m_configspec->find_attribute("oiio:UnassociatedAlpha")) { + if (m_imagecache + && (!m_configspec + || !m_configspec->find_attribute("oiio:UnassociatedAlpha"))) { int unassoc = 0; if (m_imagecache->getattribute("unassociatedalpha", unassoc)) { // Since IB needs to act as if it's backed by an ImageCache, @@ -1150,6 +1219,7 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, m_configspec->attribute("oiio:UnassociatedAlpha", unassoc); } } + Timer timer; auto in = ImageInput::open(m_name.string(), m_configspec.get(), m_rioproxy); if (in) { @@ -1169,6 +1239,7 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, m_pixels_valid = false; error(OIIO::geterror()); } + atomic_fetch_add(pvt::IB_total_image_read_time, float(timer())); // Since we have read in the entire image now, if we are using an // IOProxy, we invalidate any cache entry to avoid lifetime issues // related to the IOProxy. This helps to eliminate trouble emerging @@ -1186,13 +1257,14 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, // // the user thinks the forced immediate read was the // // last it'll be needed. But the cache entry still // // has a pointer to it! Oh no! - if (m_rioproxy) + if (m_imagecache && m_rioproxy) m_imagecache->invalidate(m_name); return m_pixels_valid; } // All other cases, no loss of precision is expected, so even a forced // read should go through the image cache. + OIIO_ASSERT(m_imagecache); if (m_imagecache->get_pixels(m_name, subimage, miplevel, m_spec.x, m_spec.x + m_spec.width, m_spec.y, m_spec.y + m_spec.height, m_spec.z, @@ -2825,7 +2897,9 @@ ImageBuf::WrapMode_from_string(string_view name) void ImageBuf::IteratorBase::release_tile() { - m_ib->imagecache()->release_tile(m_tile); + auto ic = m_ib->imagecache(); + OIIO_DASSERT(ic); + ic->release_tile(m_tile); } @@ -2836,6 +2910,7 @@ ImageBufImpl::retile(int x, int y, int z, ImageCache::Tile*& tile, int& tilexend, bool& haderror, bool exists, ImageBuf::WrapMode wrap) const { + OIIO_DASSERT(m_imagecache); if (!exists) { // Special case -- (x,y,z) describes a location outside the data // window. Use the wrap mode to possibly give a meaningful data diff --git a/src/libOpenImageIO/imageio.cpp b/src/libOpenImageIO/imageio.cpp index d6aa474593..e929867e62 100644 --- a/src/libOpenImageIO/imageio.cpp +++ b/src/libOpenImageIO/imageio.cpp @@ -52,7 +52,6 @@ int dds_bc5normal(0); int limit_channels(1024); int limit_imagesize_MB(std::min(32 * 1024, int(Sysutil::physical_memory() >> 20))); -int imagebuf_print_uncaught_errors(1); ustring font_searchpath(Sysutil::getenv("OPENIMAGEIO_FONTS")); ustring plugin_searchpath(OIIO_DEFAULT_PLUGIN_SEARCHPATH); std::string format_list; // comma-separated list of all formats @@ -349,6 +348,10 @@ attribute(string_view name, TypeDesc type, const void* val) imagebuf_print_uncaught_errors = *(const int*)val; return true; } + if (name == "imagebuf:use_imagecache" && type == TypeInt) { + imagebuf_use_imagecache = *(const int*)val; + return true; + } if (name == "use_tbb" && type == TypeInt) { oiio_use_tbb = *(const int*)val; return true; @@ -481,6 +484,10 @@ getattribute(string_view name, TypeDesc type, void* val) *(int*)val = imagebuf_print_uncaught_errors; return true; } + if (name == "imagebuf:use_imagecache" && type == TypeInt) { + *(int*)val = imagebuf_use_imagecache; + return true; + } if (name == "use_tbb" && type == TypeInt) { *(int*)val = oiio_use_tbb; return true; @@ -539,6 +546,22 @@ getattribute(string_view name, TypeDesc type, void* val) *(int*)val = OIIO::pvt::opencv_version; return true; } + if (name == "IB_local_mem_current" && type == TypeInt64) { + *(long long*)val = IB_local_mem_current; + return true; + } + if (name == "IB_local_mem_peak" && type == TypeInt64) { + *(long long*)val = IB_local_mem_peak; + return true; + } + if (name == "IB_total_open_time" && type == TypeFloat) { + *(float*)val = IB_total_open_time; + return true; + } + if (name == "IB_total_image_read_time" && type == TypeFloat) { + *(float*)val = IB_total_image_read_time; + return true; + } return false; } diff --git a/src/libOpenImageIO/imageio_pvt.h.in b/src/libOpenImageIO/imageio_pvt.h.in index 051b839020..c90a0cf060 100644 --- a/src/libOpenImageIO/imageio_pvt.h.in +++ b/src/libOpenImageIO/imageio_pvt.h.in @@ -45,6 +45,11 @@ extern int limit_channels; extern int limit_imagesize_MB; extern int opencv_version; extern int imagebuf_print_uncaught_errors; +extern int imagebuf_use_imagecache; +extern atomic_ll IB_local_mem_current; +extern atomic_ll IB_local_mem_peak; +extern std::atomic IB_total_open_time; +extern std::atomic IB_total_image_read_time; extern OIIO_UTIL_API int oiio_use_tbb; // This lives in libOpenImageIO_Util OIIO_API const std::vector& font_dirs(); OIIO_API const std::vector& font_file_list(); diff --git a/src/oiiotool/oiiotool.cpp b/src/oiiotool/oiiotool.cpp index 562c086796..141acad625 100644 --- a/src/oiiotool/oiiotool.cpp +++ b/src/oiiotool/oiiotool.cpp @@ -582,7 +582,12 @@ set_cachesize(Oiiotool& ot, cspan argv) { OIIO_DASSERT(argv.size() == 2); ot.cachesize = Strutil::stoi(argv[1]); - ot.imagecache->attribute("max_memory_MB", float(ot.cachesize)); + if (ot.cachesize) { + OIIO::attribute("imagebuf:use_imagecache", 1); + ot.imagecache->attribute("max_memory_MB", float(ot.cachesize)); + } else { + OIIO::attribute("imagebuf:use_imagecache", 0); + } } @@ -7422,30 +7427,38 @@ main(int argc, char* argv[]) if (ot.runstats) { double total_time = ot.total_runtime(); double unaccounted = total_time; - std::cout << "\n"; - int threads = -1; - OIIO::getattribute("threads", threads); - std::cout << "Threads: " << threads << "\n"; - std::cout << "oiiotool runtime statistics:\n"; - std::cout << " Total time: " - << Strutil::timeintervalformat(total_time, 2) << "\n"; - static const char* timeformat = " %-12s : %5.2f\n"; + print("\n"); + print("Threads: {}\n", OIIO::get_int_attribute("threads")); + print("oiiotool runtime statistics:\n"); + print(" Total time: {}\n", Strutil::timeintervalformat(total_time, 2)); for (auto& func : ot.function_times) { double t = func.second; if (t > 0.0) { - Strutil::printf(timeformat, func.first, t); + Strutil::print(" {:<12} : {:5.2f}\n", func.first, t); unaccounted -= t; } } - if (unaccounted > 0.0) { - Strutil::printf(timeformat, "unaccounted", unaccounted); - } + if (unaccounted > 0.0) + Strutil::print(" {:<12} : {:5.2f}\n", "unaccounted", + unaccounted); ot.check_peak_memory(); - std::cout << " Peak memory: " << Strutil::memformat(ot.peak_memory) - << "\n"; - std::cout << " Current memory: " - << Strutil::memformat(Sysutil::memory_used()) << "\n"; - std::cout << "\n" << ot.imagecache->getstats(2) << "\n"; + print(" Peak memory: {}\n", Strutil::memformat(ot.peak_memory)); + print(" Current memory: {}\n", + Strutil::memformat(Sysutil::memory_used())); + { + int64_t current = 0, peak = 0; + OIIO::getattribute("IB_local_mem_current", TypeInt64, ¤t); + OIIO::getattribute("IB_local_mem_peak", TypeInt64, &peak); + print("\nImageBuf local memory: current {}, peak {}\n", + Strutil::memformat(current), Strutil::memformat(peak)); + float opentime = OIIO::get_float_attribute("IB_total_open_time"); + float readtime = OIIO::get_float_attribute( + "IB_total_image_read_time"); + print("ImageBuf direct read time: {}, open time {}\n", + Strutil::timeintervalformat(readtime, 2), + Strutil::timeintervalformat(opentime, 2)); + } + print("\n{}\n", ot.imagecache->getstats(2)); } // Release references of images that might hold onto a shared diff --git a/testsuite/python-imagebuf/ref/out-alt-python3.txt b/testsuite/python-imagebuf/ref/out-alt-python3.txt index 2afb9dc573..1502d3d222 100644 --- a/testsuite/python-imagebuf/ref/out-alt-python3.txt +++ b/testsuite/python-imagebuf/ref/out-alt-python3.txt @@ -30,8 +30,6 @@ Constructing from a bare numpy array: from 4D, shape is float 0 2 0 2 0 2 0 4 Testing read of ../common/textures/grid.tx: -subimage: 0 / 1 -miplevel: 0 / 11 channels: 4 name: ../common/textures/grid.tx file_format_name: tiff diff --git a/testsuite/python-imagebuf/ref/out-alt.txt b/testsuite/python-imagebuf/ref/out-alt.txt index 5af4b9bcb0..78d39262b3 100644 --- a/testsuite/python-imagebuf/ref/out-alt.txt +++ b/testsuite/python-imagebuf/ref/out-alt.txt @@ -30,8 +30,6 @@ Constructing from a bare numpy array: from 4D, shape is float 0 2 0 2 0 2 0 4 Testing read of ../common/textures/grid.tx: -subimage: 0 / 1 -miplevel: 0 / 11 channels: 4 name: ../common/textures/grid.tx file_format_name: tiff diff --git a/testsuite/python-imagebuf/ref/out-python3.txt b/testsuite/python-imagebuf/ref/out-python3.txt index 2afb9dc573..1502d3d222 100644 --- a/testsuite/python-imagebuf/ref/out-python3.txt +++ b/testsuite/python-imagebuf/ref/out-python3.txt @@ -30,8 +30,6 @@ Constructing from a bare numpy array: from 4D, shape is float 0 2 0 2 0 2 0 4 Testing read of ../common/textures/grid.tx: -subimage: 0 / 1 -miplevel: 0 / 11 channels: 4 name: ../common/textures/grid.tx file_format_name: tiff diff --git a/testsuite/python-imagebuf/ref/out.txt b/testsuite/python-imagebuf/ref/out.txt index 5af4b9bcb0..78d39262b3 100644 --- a/testsuite/python-imagebuf/ref/out.txt +++ b/testsuite/python-imagebuf/ref/out.txt @@ -30,8 +30,6 @@ Constructing from a bare numpy array: from 4D, shape is float 0 2 0 2 0 2 0 4 Testing read of ../common/textures/grid.tx: -subimage: 0 / 1 -miplevel: 0 / 11 channels: 4 name: ../common/textures/grid.tx file_format_name: tiff diff --git a/testsuite/python-imagebuf/src/test_imagebuf.py b/testsuite/python-imagebuf/src/test_imagebuf.py index e172f917d0..a915384785 100755 --- a/testsuite/python-imagebuf/src/test_imagebuf.py +++ b/testsuite/python-imagebuf/src/test_imagebuf.py @@ -191,8 +191,10 @@ def ftupstr(tup) : b = oiio.ImageBuf ("../common/textures/grid.tx") b.init_spec ("../common/textures/grid.tx") b.read () - print ("subimage:", b.subimage, " / ", b.nsubimages) - print ("miplevel:", b.miplevel, " / ", b.nmiplevels) + if b.nsubimages != 0: + print ("subimage:", b.subimage, " / ", b.nsubimages) + if b.nmiplevels != 0: + print ("miplevel:", b.miplevel, " / ", b.nmiplevels) print ("channels:", b.nchannels) print ("name:", b.name) print ("file_format_name:", b.file_format_name)