api: first draft for removing nativespec from ImageBuf

src/include/OpenImageIO/imagebuf.h

@@ -1064,19 +1064,25 @@ class OIIO_API ImageBuf {
     /// knowledge of its pixel memory layout.  USE WITH EXTREME CAUTION.
     ImageSpec& specmod();
 
-    /// Return a read-only (const) reference to the "native" image spec
-    /// (that describes the file, which may be slightly different than
-    /// the spec of the ImageBuf, particularly if the IB is backed by an
-    /// ImageCache that is imposing some particular data format or tile
-    /// size).
+    /// Return the "native" channel format that describes the file,
+    /// which may be slightly different than the spec of the ImageBuf,
+    /// particularly if the IB is backed by an ImageCache that is imposing
+    /// some particular data format or tile size.
     ///
     /// This may differ from `spec()` --- for example, if a data format
     /// conversion was requested, if the buffer is backed by an ImageCache
     /// which stores the pixels internally in a different data format than
     /// that of the file, or if the file had differing per-channel data
     /// formats (ImageBuf must contain a single data format for all
     /// channels).
-    const ImageSpec& nativespec() const;
+    TypeDesc file_format() const;
+    /// Same as the above for channel specific formats.
+    std::vector<TypeDesc> file_channelformats() const;
+
+    /// DEPRECATED old API. This will now return spec() by default.
+    /// We recommend switching to the new API.
+    /// TODO: uncomment for backwards compatibility once everything else is in place.
+    // const ImageSpec& nativespec() const;
 
     /// Does this ImageBuf have an associated thumbnail?
     bool has_thumbnail() const;

src/iv/ivimage.cpp

@@ -119,7 +119,7 @@ std::string
 IvImage::longinfo() const
 {
     if (m_longinfo.empty()) {
-        const ImageSpec& m_spec(nativespec());
+        const ImageSpec& m_spec(spec());
         m_longinfo += "<table>";
         //        m_longinfo += html_table_row (Strutil::fmt::format("<b>{}</b>", m_name.c_str()).c_str(),
         //                                std::string());

src/libOpenImageIO/imagebuf.cpp

@@ -269,11 +269,39 @@ class ImageBufImpl {
         validate_spec();
         return m_spec;
     }
-    const ImageSpec& nativespec() const
+
+    TypeDesc file_format() const
     {
-        validate_spec();
-        return m_nativespec;
+        //! TODO: remove m_nativespec and store only the "native format"
+        return m_nativespec.format;
+    }
+
+    TypeDesc& file_format()
+    {
+        //! TODO: remove m_nativespec and store only the "native format"
+        return m_nativespec.format;
+    }
+
+    std::vector<TypeDesc> file_channelformats() const
+    {
+        //! TODO: remove m_nativespec and store only the "native channels format"
+        return m_nativespec.channelformats;
+    }
+
+    std::vector<TypeDesc>& file_channelformats()
+    {
+        //! TODO: remove m_nativespec and store only the "native channels format"
+        return m_nativespec.channelformats;
     }
+
+    /// DEPRECTATED
+    /// TODO: uncomment for backwards compatibility once everything else is in place.
+    // const ImageSpec& nativespec() const
+    // {
+    //     validate_spec();
+    //     return m_nativespec;
+    // }
+
     ImageSpec& specmod()
     {
         validate_spec();
@@ -338,6 +366,7 @@ class ImageBufImpl {
     int m_nmiplevels;               ///< # of MIP levels in the current subimage
     mutable int m_threads;          ///< thread policy for this image
     ImageSpec m_spec;               ///< Describes the image (size, etc)
+    //! TODO: remove m_nativespec
     ImageSpec m_nativespec;         ///< Describes the true native image
     std::unique_ptr<char[]> m_pixels;  ///< Pixel data, if local and we own it
     char* m_localpixels;               ///< Pointer to local pixels
@@ -384,7 +413,7 @@ class ImageBufImpl {
             return m_write_format[channel];
         if (m_write_format.size() == 1)
             return m_write_format[0];
-        return m_nativespec.format;
+        return file_format();
     }
 
     void lock() const { m_mutex.lock(); }
@@ -784,7 +813,7 @@ void
 ImageBufImpl::clear()
 {
     if (m_imagecache && !m_name.empty()
-        && (storage() == ImageBuf::IMAGECACHE || m_rioproxy)) {
+        && (cachedpixels() || m_rioproxy)) {
         // If we were backed by an ImageCache, invalidate any IC entries we
         // might have made. Also do so if we were using an IOProxy, because
         // the proxy may not survive long after the ImageBuf is destroyed.
@@ -914,7 +943,9 @@ ImageBufImpl::reset(string_view filename, const ImageSpec& spec,
     m_current_miplevel = 0;
     if (buforigin || bufspan.size()) {
         m_spec           = spec;
+        //! TODO: remove m_nativespec
         m_nativespec     = nativespec ? *nativespec : spec;
+
         m_channel_stride = stride_t(spec.format.size());
         m_xstride        = xstride;
         m_ystride        = ystride;
@@ -1077,7 +1108,9 @@ ImageBufImpl::init_spec(string_view filename, int subimage, int miplevel,
                                      TypeString, &fmt);
         m_fileformat = ustring(fmt);
 
+        // Read native image spec from the ImageCache
         m_imagecache->get_imagespec(m_name, m_nativespec, subimage);
+        // Copy and override the spec with cache internal data size
         m_spec = m_nativespec;
         m_imagecache->get_cache_dimensions(m_name, m_spec, subimage, miplevel);
 
@@ -1221,9 +1254,12 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend,
 
     m_current_subimage = subimage;
     m_current_miplevel = miplevel;
-    if (chend < 0 || chend > nativespec().nchannels)
-        chend = nativespec().nchannels;
-    bool use_channel_subset = (chbegin != 0 || chend != nativespec().nchannels);
+
+    //! NOTE: m_imagecache can be null, make sure there is a fallback solution
+    const ImageSpec& nativespec = *m_imagecache->imagespec(m_name, m_current_subimage);
+    if (chend < 0 || chend > nativespec.nchannels)
+        chend = nativespec.nchannels;
+    bool use_channel_subset = (chbegin != 0 || chend != nativespec.nchannels);
 
     if (m_spec.deep) {
         Timer timer;
@@ -1278,7 +1314,7 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend,
     } else {
         // No cache should take the "forced read now" route.
         force             = true;
-        m_cachedpixeltype = m_nativespec.format;
+        m_cachedpixeltype = file_format();
     }
 
     if (use_channel_subset) {
@@ -1287,32 +1323,34 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend,
         m_spec.nchannels = chend - chbegin;
         m_spec.channelnames.resize(m_spec.nchannels);
         for (int c = 0; c < m_spec.nchannels; ++c)
-            m_spec.channelnames[c] = m_nativespec.channelnames[c + chbegin];
-        if (m_nativespec.channelformats.size()) {
+            m_spec.channelnames[c] = nativespec.channelnames[c + chbegin];
+        const std::vector<TypeDesc>& cformats = file_channelformats();
+        if (cformats.size()) {
             m_spec.channelformats.resize(m_spec.nchannels);
             for (int c = 0; c < m_spec.nchannels; ++c)
                 m_spec.channelformats[c]
-                    = m_nativespec.channelformats[c + chbegin];
+                    = cformats[c + chbegin];
         }
     }
 
     if (convert != TypeDesc::UNKNOWN)
         m_spec.format = convert;
     else
-        m_spec.format = m_nativespec.format;
+        m_spec.format = file_format();
     realloc();
     // N.B. realloc sets m_bufspan
 
     // If forcing a full read, make sure the spec reflects the nativespec's
     // tile sizes, rather than that imposed by the ImageCache.
+    /// TODO: what to do here again ? getting rid of nativespec will lose that info..
     m_spec.tile_width  = m_nativespec.tile_width;
     m_spec.tile_height = m_nativespec.tile_height;
     m_spec.tile_depth  = m_nativespec.tile_depth;
 
     if (force || !m_imagecache || m_rioproxy
         || (convert != TypeDesc::UNKNOWN && convert != m_cachedpixeltype
             && convert.size() >= m_cachedpixeltype.size()
-            && convert.size() >= m_nativespec.format.size())) {
+            && convert.size() >= file_format().size())) {
         // A specific conversion type was requested which is not the cached
         // type and whose bit depth is as much or more than the cached type.
         // Bypass the cache and read directly so that there is no possible
@@ -1660,8 +1698,8 @@ ImageBuf::write(string_view _filename, TypeDesc dtype, string_view _fileformat,
     } else {
         // No override on the ImageBuf, nor on this call to write(), so
         // we just use what is known from the imagespec.
-        newspec.set_format(nativespec().format);
-        newspec.channelformats = nativespec().channelformats;
+        newspec.set_format(file_format());
+        newspec.channelformats = file_channelformats();
     }
 
     if (m_impl->m_wioproxy) {
@@ -1712,13 +1750,17 @@ ImageBufImpl::copy_metadata(const ImageBufImpl& src)
     m_spec.full_width  = srcspec.full_width;
     m_spec.full_height = srcspec.full_height;
     m_spec.full_depth  = srcspec.full_depth;
-    if (src.storage() == ImageBuf::IMAGECACHE) {
+    if (src.cachedpixels()) {
         // If we're copying metadata from a cached image, be sure to
         // get the file's tile size, not the cache's tile size.
-        m_spec.tile_width  = src.nativespec().tile_width;
-        m_spec.tile_height = src.nativespec().tile_height;
-        m_spec.tile_depth  = src.nativespec().tile_depth;
-    } else {
+        //! TOCHECK: is it actually correct to do as follows ?
+        const ImageSpec& nativespec = *m_imagecache->imagespec(m_name);
+        m_spec.tile_width  = nativespec.tile_width;
+        m_spec.tile_height = nativespec.tile_height;
+        m_spec.tile_depth  = nativespec.tile_depth;
+    }
+    else
+    {
         m_spec.tile_width  = srcspec.tile_width;
         m_spec.tile_height = srcspec.tile_height;
         m_spec.tile_depth  = srcspec.tile_depth;
@@ -1752,14 +1794,31 @@ ImageBuf::specmod()
 
 
 
-const ImageSpec&
-ImageBuf::nativespec() const
+TypeDesc
+ImageBuf::file_format() const
 {
-    return m_impl->nativespec();
+    return m_impl->file_format();
 }
 
 
 
+std::vector<TypeDesc>
+ImageBuf::file_channelformats() const
+{
+    return m_impl->file_channelformats();
+}
+
+
+
+// DEPRECATED
+// const ImageSpec&
+// ImageBuf::nativespec() const
+// {
+//     return m_impl->nativespec();
+// }
+
+
+
 bool
 ImageBufImpl::has_thumbnail(DoLock do_lock) const
 {
@@ -2167,12 +2226,12 @@ ImageBuf::copy(const ImageBuf& src, TypeDesc format)
         return true;
     }
     if (src.deep()) {
-        m_impl->reset(src.name(), src.spec(), &src.nativespec());
+        m_impl->reset(src.name(), src.spec());
         m_impl->m_deepdata = src.m_impl->m_deepdata;
         return true;
     }
     if (format.basetype == TypeDesc::UNKNOWN || src.deep())
-        m_impl->reset(src.name(), src.spec(), &src.nativespec());
+        m_impl->reset(src.name(), src.spec());
     else {
         ImageSpec newspec(src.spec());
         newspec.set_format(format);

src/libOpenImageIO/imagebuf_test.cpp

@@ -423,7 +423,6 @@ test_read_channel_subset()
            true /*force*/, TypeDesc::FLOAT);
     std::cout << " After reading channels [2,5), we have:\n";
     print(B);
-    OIIO_CHECK_EQUAL(B.nativespec().nchannels, 6);
     OIIO_CHECK_EQUAL(B.spec().nchannels, 3);
     OIIO_CHECK_EQUAL(B.spec().format, TypeDesc::FLOAT);
     OIIO_CHECK_EQUAL(B.spec().channelnames[0], "B");

src/libOpenImageIO/maketexture.cpp

@@ -1124,8 +1124,8 @@ make_texture_impl(ImageBufAlgo::MakeTextureMode mode, const ImageBuf* input,
     }
 
     // The cache might mess with the apparent data format, so make sure
-    // it's the nativespec that we consult for data format of the file.
-    TypeDesc out_dataformat = src->nativespec().format;
+    // it's the original file format that we consult.
+    TypeDesc out_dataformat = src->file_format();
 
     if (configspec.format != TypeDesc::UNKNOWN)
         out_dataformat = configspec.format;

src/oiiotool/expressions.cpp

@@ -239,7 +239,7 @@ Oiiotool::express_parse_atom(const string_view expr, string_view& s,
             read(img);
             ParamValue tmpparam;
             if (metadata == "nativeformat") {
-                result = img->nativespec(0, 0)->format.c_str();
+                result = img->nativespec(0)->format.c_str();
             } else if (auto p = img->spec(0, 0)->find_attribute(metadata,
                                                                 tmpparam)) {
                 std::string val = ImageSpec::metadata_val(*p);
@@ -310,7 +310,7 @@ Oiiotool::express_parse_atom(const string_view expr, string_view& s,
                 // OiioTool::print_stats(out, *this, (*img)());
 
                 std::string err;
-                if (!pvt::print_stats(out, "", (*img)(), (*img)().nativespec(),
+                if (!pvt::print_stats(out, "", (*img)(), (*img)().spec(),
                                       ROI(), err))
                     errorfmt("stats", "unable to compute: {}", err);
 

src/oiiotool/imagerec.cpp

@@ -247,6 +247,19 @@ ImageRec::read_nativespec()
 
 
 
+const ImageSpec*
+ImageRec::nativespec(int subimg) const
+{
+    if (subimg < subimages())
+    {
+        ustring uname(name());
+        return m_imagecache->imagespec(uname, subimg);
+    }
+    return nullptr;
+}
+
+
+
 bool
 ImageRec::read(ReadPolicy readpolicy, string_view channel_set)
 {
@@ -282,8 +295,8 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set)
             // relying on the cache to read their frames on demand rather
             // than reading the whole movie up front, even though each frame
             // individually would be well below the threshold.
-            ImageSpec spec;
-            m_imagecache->get_imagespec(uname, spec, s);
+            const ImageSpec& nativespec = *m_imagecache->imagespec(uname, s);
+            ImageSpec spec = nativespec;
             m_imagecache->get_cache_dimensions(uname, spec, s, m);
             imagesize_t imgbytes = spec.image_bytes();
             bool forceread       = (s == 0 && m == 0
@@ -299,7 +312,7 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set)
             int new_z_channel     = -1;
             int chbegin = 0, chend = -1;
             if (channel_set.size()) {
-                decode_channel_set(ib->nativespec(), channel_set,
+                decode_channel_set(nativespec, channel_set,
                                    newchannelnames, channel_set_channels,
                                    channel_set_values, eh);
                 for (size_t c = 0, e = channel_set_channels.size(); c < e;
@@ -332,11 +345,11 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set)
             TypeDesc convert = TypeDesc::FLOAT;
             if (m_input_dataformat != TypeDesc::UNKNOWN) {
                 convert = m_input_dataformat;
-                if (m_input_dataformat != ib->nativespec().format)
+                if (m_input_dataformat != nativespec.format)
                     m_subimages[s].m_was_direct_read = false;
                 forceread = true;
             } else if (readpolicy & ReadNative)
-                convert = ib->nativespec().format;
+                convert = nativespec.format;
             if (!forceread && convert != TypeDesc::UINT8
                 && convert != TypeDesc::UINT16 && convert != TypeDesc::HALF
                 && convert != TypeDesc::FLOAT) {
@@ -377,7 +390,7 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set)
             m_subimages[s].m_specs[m]     = ib->spec();
             // For ImageRec purposes, we need to restore a few of the
             // native settings.
-            const ImageSpec& nativespec(ib->nativespec());
+
             // m_subimages[s].m_specs[m].format = nativespec.format;
             m_subimages[s].m_specs[m].tile_width  = nativespec.tile_width;
             m_subimages[s].m_specs[m].tile_height = nativespec.tile_height;