Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Sony ARW medium and small compression 15 bit per sample files are unsupported #4361

Open
lingyukongt opened this issue Jul 29, 2024 · 17 comments
Assignees

Comments

@lingyukongt
Copy link

Describe the bug

Importing a Sony ARW Medium or Small compression file with 15 bits per sample is unsupported in OIIO.

Other Sony ARW Medium or Small compression files which are not 15 bits per sample open correctly and are supported.

OpenImageIO version and dependencies

oiiotool --buildinfo
OIIO 2.5.12.0 | MacOS/ARM
    Build compiler: Apple clang 15.0 | C++14/201402
    HW features enabled at build: neon
Dependencies: Boost 1.85.0, BZip2 1.0.8, FFmpeg 6.0, fmt 10.2.1, Freetype
    2.13.2, GIF 5.2.2, Libheif 1.17.6, libjpeg-turbo 3.0.3, LibRaw 0.21.2,
    OpenColorIO 2.3.2, OpenEXR 3.2.4, OpenVDB NONE, PNG 1.6.43, pugixml 1.14,
    pybind11 2.12.0, Python 3.12.3, Robinmap, TBB 2021.12.0, TIFF 4.6.0, WebP
    1.4.0, ZLIB 1.2.12

To Reproduce

Steps to reproduce the behavior:

  1. Download the ARW file
  2. Try to convert to a different format with OIIO
  3. This error appears.
  4. I expected it to output.
oiiotool /Users/topazlabs/Downloads/A1A04973.ARW -o ~/Downloads/arw.png
oiiotool ERROR: read : Could not open file "/Users/topazlabs/Downloads/A1A04973.ARW", Unsupported file format or not RAW file
Full command line was:
> oiiotool /Users/topazlabs/Downloads/A1A04973.ARW -o /Users/topazlabs/Downloads/arw.png

Evidence

https://www.dropbox.com/scl/fi/y19jqq4q3rh1wv7810048/A1A04973.ARW?rlkey=pnrdm0oo0hlpbkep1goczk1da&st=ij4br4bn&dl=0

@lgritz
Copy link
Collaborator

lgritz commented Aug 10, 2024

We rely on libraw to decode that file type. I think that maybe this means that libraw itself doesn't handle that bit depth for that file?

I don't think there's a lot we can directly do until libraw supports it. Maybe you should file an issue against that project?

@LibRaw
Copy link

LibRaw commented Aug 12, 2024

What version of LibRaw do you use?
Sony Small/Medium Pseudo-RAW format is supported since 202403 snapshot published in our github repo on Mar 29, 2024.
Please upgrade if you're using an older version.

Here is sample above processed with dcraw_emu -T -w: https://www.dropbox.com/scl/fi/wm2ef2atvy8korg1nn32z/A1A04973.ARW.tiff?rlkey=4yt4so6osoevy74m129ka9p1d&dl=0

@lgritz
Copy link
Collaborator

lgritz commented Aug 12, 2024

Hi, @LibRaw , thanks so much for chiming in.

According to the log posted above, the OP was using LibRaw 0.21.2. I'm not sure exactly how that corresponds to the snapshot you refer to.

@LibRaw
Copy link

LibRaw commented Aug 12, 2024

LibRaw 0.21.0 was released in 2022. At the time of this release, this format did not yet exist in Sony cameras.

0.21.xx is bugfixes only, no new cameras/formats to preserve API/ABI.

Our release policy is described in details in project description on Github and/or on project homepage.

@lingyukongt
Copy link
Author

lingyukongt commented Aug 12, 2024

Initial testing for reproduction was done with homebrew oiiotool which is version 2.5.12.0 with libraw 0.21.2 as you said, but our application is using 2.4.11.0 internally with Libraw 20240710. Initial post was using the latest OIIO from homebrew since it was also reproducing the not being able to save issue, and our build system does not build oiiotool.

I'm not sure if a crash related to this was fixed between OIIO versions, but here is part of the crash report for reference.

Thread 17 Crashed:: QThread
0   libsystem_platform.dylib      	       0x18714e230 _platform_memmove + 144
1   libOpenImageIO.2.4.11.dylib   	       0x10d69c378 OpenImageIO_v2_4::convert_pixel_values(OpenImageIO_v2_4::TypeDesc, void const*, OpenImageIO_v2_4::TypeDesc, void*, int) + 240
2   libOpenImageIO.2.4.11.dylib   	       0x10d7b50f8 OpenImageIO_v2_4::RawInput::read_native_scanline(int, int, int, int, void*) + 1220
3   libOpenImageIO.2.4.11.dylib   	       0x10d693290 OpenImageIO_v2_4::ImageInput::read_native_scanlines(int, int, int, int, int, void*) + 180
4   libOpenImageIO.2.4.11.dylib   	       0x10d6933ac OpenImageIO_v2_4::ImageInput::read_native_scanlines(int, int, int, int, int, int, int, void*) + 180
5   libOpenImageIO.2.4.11.dylib   	       0x10d692ba4 OpenImageIO_v2_4::ImageInput::read_scanlines(int, int, int, int, int, int, int, OpenImageIO_v2_4::TypeDesc, void*, long long, long long) + 1328
6   libOpenImageIO.2.4.11.dylib   	       0x10d695da0 OpenImageIO_v2_4::ImageInput::read_image(int, int, int, int, OpenImageIO_v2_4::TypeDesc, void*, long long, long long, long long, bool (*)(void*, float), void*) + 1684

If a crash related to this was fixed we can try to update to OIIO 2.5 internally, but this will take us some time.

@LibRaw
Copy link

LibRaw commented Aug 12, 2024

@lingyukongt
Please make sure you use actual LibRaw public header files (libraw.h) when building the wrapper you use (OpenImageIO):

LibRaw snapshots are NOT binary (ABI) compatible unless such compatibility is declared (e.g. within single major version: 0.21.0, 0.21.1, 0.21.x). Internal layout may change without notice, while our C++ api do not isolate this.

Alternatively, use LibRaw C API, that provides such isolation.

@LibRaw
Copy link

LibRaw commented Aug 12, 2024

Just looked into raw.imageio/rawinput.cpp, read_native_scanline (HEAD/master fetched from github)

The issue is solved:
the read_native_scanline reads the imgdata.rawdata.raw_image pointer (without checking it for NULL value before).

But raw_image is actual for Bayer images only, while Sony YCC/Pseudo-RAW is 3-color image, so color3_image or color4_image will be used: https://www.libraw.org/docs/API-datastruct-eng.html#libraw_rawdata_t

So, please update OpenImageIO to support 3/4 color images.
Also, check imgdata.rawdata.raw_image against NULL is strongly suggested.

@lgritz
Copy link
Collaborator

lgritz commented Aug 12, 2024

We're closing in on releasing 3.0 (which is currently still called "2.6" in the master branch), and 2.5 is the supported release family. So 2.4 is 2 years old and we're no longer investigating problems in it. We haven't issued any patches for 2.4 for quite some time now, and since that predated any 2024 era libraw releases or snapshots, I would consider that untested and not a combination I would recommend.

@LibRaw:

I see now, 0.21.2 dates from January, so that is not the snapshot and thus doesn't contain support for the format in question. The new snapshot would be labelled 0.22.x, right? We do CI test against libraw master nightly, in addition to tagged releases, so I am confident that we build and run against your latest snapshots.

@lgritz
Copy link
Collaborator

lgritz commented Aug 12, 2024

@antond-weta Calling your attention to @LibRaw's comments above about Bayer images, since you seem to be currently working in our libraw code.

@lingyukongt
Copy link
Author

Thanks for the investigation guys

@LibRaw
Copy link

LibRaw commented Aug 12, 2024

@lgritz
Please take look on our update policy: https://www.libraw.org/#updatepolicy (same on GitHub page, but I do not know how to point to specific sections).

Our snapshots are carefully tested against normal user case (no specifically crafted files e.g. from fuzzers, but normal files from user cameras). These snapshots are published as master branch and not tagged (it does not make sense, see below).
After such publishing, only very minor fixes are added (including ones inspired by OSS-fuzz and similar).

So, LibRaw/master is generally OK for end-user cases: no unknown data sources (e.g. fuzzed/specially crafted files) just files from user's camera. It is not recommended to use in services opened to everyone (e.g. processing anonymous users uploads).

@LibRaw
Copy link

LibRaw commented Aug 12, 2024

BTW, null pointer access is a good reason for an immediate fix (and CVE)

@lgritz
Copy link
Collaborator

lgritz commented Aug 12, 2024

We CI test against a range of libraw releases, as well as whatever the current master is.

But we don't supply pre-built binaries, nor strictly dictate what version of libraw that users/builders compile against (other than enforcing a minimum version, which is currently 0.18 in OIIO 2.5 but in our master was recently raised to 0.20). The judgment about which version of LibRaw to build against, or any feature vs reliability tradeoffs in that decision, is up to the downstream builder of OIIO. I think most of them just use whatever is the latest tagged release provided by their OS or packaging system of choice. So in general, I would only expect the latest snapshots to be used by people who make a proactive decision to build them from source and understand the tradeoffs.

@lgritz
Copy link
Collaborator

lgritz commented Aug 12, 2024

@LibRaw I really appreciate your input here, thanks.

@LibRaw
Copy link

LibRaw commented Aug 12, 2024

This null-pointer-access problem is not Sony/YCC specific.
There are multiple non-bayer/full-color raw formats, e.g: Linear DNG, Kodak YCC, Kodak RGB, fast-load DNG, Sony ARQ, Sony YCC, Canon sRAW, Nikon Small RAW.

Such files will be processed OK in LibRaw part and will fail on null-pointer access in OIIO layer.

@lgritz
Copy link
Collaborator

lgritz commented Aug 12, 2024

Understood, thanks.
I'm hoping this is something that @antond-weta will roll into his next set of changes.

@antond-weta
Copy link
Contributor

I'll take a look, thanks!

@antond-weta antond-weta self-assigned this Sep 25, 2024
lgritz pushed a commit that referenced this issue Sep 27, 2024
This adds a null pointer check in the raw input plugin when the user
requested raw bayer data for a file format which doesn't have it. With
this fix OIIO shows an error instead of crashing in such case.

This fixes the issue mentioned in the comments of #4361. The title issue
of #4361 has been resolved in LibRaw 0.22.0, so we can close that issue,
I guess.

This also fixes the crash on 5 out of 6 examples in #3115. The last one
is unrelated, and I don't have a good fix for that at this time (crash
due to the raw buffer having half resolution, with the pixel aspect
ratio of 0.5). Perhaps we could create a separate issue and close #3115.

I have not added any tests for this change specifically, as it is fairly
trivial.
I have started working on a more comprehensive test suite for the raw
plugin / rawtoaces, but it's not ready yet, and I'm not sure where it
should live at this stage.

Signed-off-by: Anton Dukhovnikov <[email protected]>
lgritz pushed a commit to lgritz/OpenImageIO that referenced this issue Sep 29, 2024
…tion#4448)

This adds a null pointer check in the raw input plugin when the user
requested raw bayer data for a file format which doesn't have it. With
this fix OIIO shows an error instead of crashing in such case.

This fixes the issue mentioned in the comments of AcademySoftwareFoundation#4361. The title issue
of AcademySoftwareFoundation#4361 has been resolved in LibRaw 0.22.0, so we can close that issue,
I guess.

This also fixes the crash on 5 out of 6 examples in AcademySoftwareFoundation#3115. The last one
is unrelated, and I don't have a good fix for that at this time (crash
due to the raw buffer having half resolution, with the pixel aspect
ratio of 0.5). Perhaps we could create a separate issue and close AcademySoftwareFoundation#3115.

I have not added any tests for this change specifically, as it is fairly
trivial.
I have started working on a more comprehensive test suite for the raw
plugin / rawtoaces, but it's not ready yet, and I'm not sure where it
should live at this stage.

Signed-off-by: Anton Dukhovnikov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants