AcademySoftwareFoundation / OpenImageIO

Reading, writing, and processing images in a wide variety of file formats, using a format-agnostic API, aimed at VFX applications.
https://openimageio.readthedocs.org
Apache License 2.0
1.97k stars 597 forks source link

[BUG] Access violation using IBA::to_OpenCV() with an ImageCache-backed ImageBuf #3800

Closed jorgemb closed 1 year ago

jorgemb commented 1 year ago

Describe the bug I get an Exception 0xc0000005 encountered at address ***: Access violation reading location 0x00000000 while trying to convert a HEIC image to OpenCV format. Here is the PoC I'm working with:

#include <algorithm>
#include <filesystem>
#include <format>
#include <iostream>
#include <ranges>
#include <string>
#include <vector>

#include <OpenImageIO/imagebuf.h>
#include <OpenImageIO/imagebufalgo.h>
#include <OpenImageIO/imageio.h>
#include <opencv2/core.hpp>
#include <opencv2/highgui.hpp>

#include "lib.hpp"

namespace fs = std::filesystem;
namespace rng = std::ranges;
namespace vws = std::ranges::views;

auto main() -> int
{
  // Get image paths
  auto images = std::vector<fs::path> {};
  std::transform(fs::directory_iterator("img/"),
                 fs::directory_iterator(),
                 std::back_inserter(images),
                 [](auto const& entry) { return entry.path(); });

  for (auto const& image : images) {
    auto loaded_image = OIIO::ImageBuf(image.string());
    if (not loaded_image.initialized()) {
      std::cout << loaded_image.geterror() << '\n';
      continue;
    }

    auto cv_image = cv::Mat {};
    try {
      OIIO::ImageBufAlgo::to_OpenCV(cv_image, loaded_image, {}, 1);
      cv::imshow(image.filename().string(), cv_image);
    }catch(const std::exception& e){
      std::cout << "Error when converting: " << e.what() << "\n";
      return -1;
    }
  }

  cv::waitKey();
  return 0;
}

To Reproduce

  1. I'm using vcpkg on Windows with openimageio[libheif,opencv] as dependencies
  2. In the folder I have .heic images taken with an iPhone

Expected behavior I would expect to see the images shown via OpenCV highUI.

Evidence When debugging I get a Exception 0xc0000005 encountered at address ***: Access violation reading location 0x00000000 exception.

Platform information:

lgritz commented 1 year ago

Sorry this went so long without a response.

Do you run into this problem when reading any heic image? If only some of them trigger the exception, can you please supply one that can reproduce the problem?

jorgemb commented 1 year ago

Hi, no problem. Thanks for reaching out.

Yes, in general with heic images. I downloaded this test image (attached image1.zip) from here: https://github.com/tigranbs/test-heic-images.

I tried today with version 2.4.14.0#2 (using vcpkg as toolchain) and the error persists.

Here is the minimal code to reproduce:

#include <iostream>
#include <string>
#include <OpenImageIO/imagebuf.h>
#include <OpenImageIO/imagebufalgo.h>
#include <opencv2/core.hpp>
#include <opencv2/highgui.hpp>

using namespace std::string_literals;

auto main() -> int
{
  auto file_path = "image1.heic"s;

  auto loaded_image = OIIO::ImageBuf(file_path);
  if (not loaded_image.initialized()){
    std::cout << loaded_image.geterror() <<  'n';
    return -1;
  }

  auto cv_image = cv::Mat{};
  try{
    OIIO::ImageBufAlgo::to_OpenCV(cv_image, loaded_image, {}, 1);
  }catch(const std::exception& e){
    std::cout << "Error when converting: " << e.what() << '\n';
    return -1;
  }

  cv::imshow(file_path, cv_image);
  cv::waitKey();

  return 0;
}
lgritz commented 1 year ago

Thanks for the example. I'm unable to reproduce it on my end, it works fine. (In this case, I'm using OIIO master, on MacOS, with OpenCV 4.8.1).

The Access violation reading location 0x00000000 (that's a null pointer) has me suspicious that the cv::Mat never gets allocated because to_OpenCV is not doing anything at all.

I would like to suggest that you change the part of your program that calls to_OpenCV to include some error checking:

    bool ok = OIIO::ImageBufAlgo::to_OpenCV(cv_image, loaded_image, {}, 1);
    if (!ok){
        std::cout << "Error when converting: " << OIIO::geterror() << '\n';
        return -1;
    }

Does this print anything that might give us additional clues?

jorgemb commented 1 year ago

Ohh, that is strange 🤔 Funny thing is that the exception interrupts the program, it is not even caught by the try-catch statement. image

I'll try again under WSL and see if I get different results using GCC. This could mean it is an environment issue with my Windows 11 and msvc configuration.

I'm attaching the full PoC project with vcpkg and cmake configuration. Probably I have something wrong with how I'm importing the packages. test_heic.zip

I'll report back with my tests.

jorgemb commented 1 year ago

Got a Segmentation fault 👀 image

Built using gcc 11.4.0-1ubuntu1~22.04

lgritz commented 1 year ago

Can you build OIIO in debug mode, run this in gdb and get a stack trace that tells you the full call stack for where this is crashing?

jorgemb commented 1 year ago

Hi, sorry for taking so long, I had issues with my dev environment. I run it as you said and got the following stacktrace:

Thread 1 "test_heic" received signal SIGSEGV, Segmentation fault.
__memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:317
317     ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S: No such file or directory.
(gdb) backtrace
#0  __memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:317
#1  0x00005555559e42cb in OpenImageIO_v2_4::copy_image (nchannels=3, width=3992, height=2992, depth=1, src=0x0,
    pixelsize=3, src_xstride=3, src_ystride=11976, src_zstride=0, dst=0x7ffff4d1b040, dst_xstride=3,
    dst_ystride=11976, dst_zstride=0)
    at /home/jorge/vcpkg/buildtrees/openimageio/src/v2.4.14.0-e861b9d6fc.clean/src/libOpenImageIO/imageio.cpp:842
#2  0x00005555559e396f in OpenImageIO_v2_4::convert_image (nchannels=3, width=3992, height=2992, depth=1, src=0x0,
    src_type=..., src_xstride=3, src_ystride=11976, src_zstride=0, dst=0x7ffff4d1b040, dst_type=..., dst_xstride=3,
    dst_ystride=11976, dst_zstride=0)
    at /home/jorge/vcpkg/buildtrees/openimageio/src/v2.4.14.0-e861b9d6fc.clean/src/libOpenImageIO/imageio.cpp:743
#3  0x00005555559e3f11 in OpenImageIO_v2_4::parallel_convert_image (nchannels=3, width=3992, height=2992, depth=1,
    src=0x0, src_type=..., src_xstride=3, src_ystride=11976, src_zstride=0, dst=0x7ffff4d1b040, dst_type=...,
    dst_xstride=3, dst_ystride=11976, dst_zstride=0, nthreads=1)
    at /home/jorge/vcpkg/buildtrees/openimageio/src/v2.4.14.0-e861b9d6fc.clean/src/libOpenImageIO/imageio.cpp:797
#4  0x00005555558f89fd in OpenImageIO_v2_4::parallel_convert_image (nchannels=3, width=3992, height=2992, depth=1,
    src=0x0, src_type=..., src_xstride=3, src_ystride=11976, src_zstride=0, dst=0x7ffff4d1b040, dst_type=...,
    dst_xstride=3, dst_ystride=11976, dst_zstride=0, nthreads=1)
    at /home/jorge/vcpkg/buildtrees/openimageio/src/v2.4.14.0-e861b9d6fc.clean/src/include/OpenImageIO/imageio.h:3060
#5  0x00005555558ef851 in OpenImageIO_v2_4::ImageBufAlgo::to_OpenCV (dst=..., src=..., roi=..., nthreads=1)
    at /home/jorge/vcpkg/buildtrees/openimageio/src/v2.4.14.0-e861b9d6fc.clean/src/libOpenImageIO/imagebufalgo_opencv.cpp:346
#6  0x000055555567d68f in main () at /home/jorge/test_heic/source/main.cpp:22

I'm running it within WSL2 on Windows 11 using GCC 11.4.0

jorgemb commented 1 year ago

Ok, so I'm going through it step by step. When it reaches the copy_image function the src parameter is already NULL. Tracing back into the library I found this:

On imagebufalgo_opencv.cpp:

bool
ImageBufAlgo::to_OpenCV(cv::Mat& dst, const ImageBuf& src, ROI roi,
                        int nthreads)
{
   ... 
   bool converted   = parallel_convert_image(
        chans, roi.width(), roi.height(), 1,
        src.pixeladdr(roi.xbegin, roi.ybegin, roi.zbegin, roi.chbegin),
        spec.format, spec.pixel_bytes(), spec.scanline_bytes(), 0, dst.ptr(),
        dstSpecFormat, pixelsize, linestep, 0, -1, -1, nthreads);
   ...
}

By this moment the source is still valid, but src.pixeladdr(...) is returning nullptr. Because of this part:

On imagebuf.cpp

void*
ImageBufImpl::pixeladdr(int x, int y, int z, int ch)
{
    validate_pixels();
    if (cachedpixels())
        return nullptr;
    x -= m_spec.x;
    y -= m_spec.y;
    z -= m_spec.z;
    size_t p = y * m_ystride + x * m_xstride + z * m_zstride
               + ch * m_channel_stride;
    return &(m_localpixels[p]);
}

cachedpixels() returns false, making the whole thing return nullptr and then the copy_image eventually fails.+

Nonetheless, adding loaded_image.read() before the conversion doesn't solve the issue.

jorgemb commented 1 year ago

The issue was solved adding this before the conversion step:

loaded_image.read(
      0,
      0,
      /*force=*/true
      );

For some reason the image is not being loaded on demand for HEIF in particular 🤔

lgritz commented 1 year ago

Yes, in retrospect, this all makes sense! What's happening is that the IB is read lazily, and in this case it's backed underneath by an ImageCache. For such images, the pixeladdr() will be null, but of course, to_OpenCV needs the whole thing in the buffer. Duh. I will have a patch submitted shortly.

lgritz commented 1 year ago

I think this is the correct fix: https://github.com/AcademySoftwareFoundation/OpenImageIO/pull/4013

jorgemb commented 1 year ago

Awesome! Thank you @lgritz 🙌

lgritz commented 1 year ago

@jorgemb Are you able to test that patch on your end to verify that it fully solves it for you?

jorgemb commented 1 year ago

Hi! Yes, tested it locally and fully solved the issue for me.

lgritz commented 1 year ago

Thanks, @jorgemb. This fix will be in the next OIIO release.