OpenEXR basic support

[johannesvollmer]

Hi! Just experimented with adding OpenEXR support. Does not compile yet, work in progress.

Here's a list of things to do:

• Solve sendable Read/Write (exrs only supports sendable stuff due to multi threading for now, might be able to relax this in exrs)
• Reduce memcpy calls (requires exrs refactor)
• Try f32 support (using u16, clamping 0-1, for now)
• Use exr meta information more precisely (do not allocate alpha channel if none was in the file etc)
• Basic tests
• Try seekable writer if possible (kind of checked: this change must be done in the next branch)
• Not ignore data window position and display window (allocate transparent display window instead of data window, and load only display window blocks?). Reason: Auto cropped images should not be tiny.
• Investigate color conversion linearrgb/srgb

Current limitations:

• only pixel type Rgba32F and Rgba16F are supported
• only non-deep rgb/rgba files supported, no conversion from/to YCbCr or similar
• only the first non-deep rgb layer is used
• only the largest mip map level is used
• pixels outside display window are lost
• meta data is lost
• dwaa/dwab compressed images not supported yet by the exr library
• (chroma) subsampling not supported yet by the exr library

I license past and future contributions under the dual MIT/Apache-2.0 license,
allowing licensees to chose either at their option.

[johannesvollmer]

Ah wait I branched off master lol. Should I have used branch next instead`?

[fintelia]

Ah wait I branched off master lol. Should I have used branch next instead`?

They're mostly the same. ColorType is marked non-exhaustive so you might be able to avoid actually making any breaking changes, in which case the master branch is right. (We can also change it later if necessary)

[johannesvollmer]

@fintelia @HeroicKatora Are there any challenges with making the writer seekable? The reader already is. The user can always write to a byte vector first, if their writer is not seekable. As most writers are seekable anyways, this change would probably result in memory usage reduction. I can open a separate pull request for this change.

[fintelia]

@johannesvollmer please go ahead and make a PR against the next branch (we can't include it in the 0.23.x series because it would be a breaking change).

[johannesvollmer]

In the exr library, I tried relaxing the requirements of the Read, to remove the Send requirement. Frankly, after some time, I can't find a solution where the parallel decompression works without requiring the reader to be sendable. File is sendable, so that is not a problem. but an arbitrary reader might not be sendable. Do we want fast decompression or support non-sendable readers in the image library?

[fintelia]

The more bounds we add to the interface the more restrictive it is. Could you go into more detail on why the library requires the reader to beSend?

[johannesvollmer]

The exr library reads individual pixel tiles one after another from a byte source. Multiple tiles can be decompressed in parallel. This should happen on the fly, without allocating all tiles at once. As a consequence, multiple threads have to pull the next pixel tile from the file, which apparently requires the byte source to be sendable between the threads.

I have tried it again today and I think I got it working such that the reading from the file is only ever done in the main thread, and the blocks are sent to different threads for processing. Not sure yet whether it works correctly 100% of the time though, still doing some tests.

[johannesvollmer]

So, I have managed to not require sendable readers and writers. And I think it is possible to keep it this way in the future. I have released a new version of the exr library with the necessary changes. I'll try to get the seekable reader and writer PR done next.

[johannesvollmer]

I got it compiling, but there are no tests that could ever fail for the exr codec.

I'd like to ask for some suggestions concerning tests. My initial idea was to read and compare an HDR and EXR file of the same image, any objections? Are there any tests that should be done for every image type (and can also be done for high dynamic range f32 images)?

I've had a look at the test image folders, but I must admit that I couldn't understand the Readme in the references folder. Should the exr codec have such a reference too?

[johannesvollmer]

Also, is there no standardized way to obtain an image buffer from a decoder? I got stuck with the following code:

    let hdr: Vec<Rgb<f32>,> = image::codecs::hdr::HdrDecoder::new(
        io::BufReader::new(fs::File::open(&reference_path).unwrap())
    ).unwrap().read_image_hdr().unwrap();

    let exr_pixels: ImageBuffer<Rgba<f32>, Vec<f32>> = image::codecs::openexr::ExrDecoder::read(
        io::BufReader::new(fs::File::open(&exr_path).unwrap())
    ).unwrap().read_f32_image_buffer().unwrap();

    // TODO let hdr = ImageBuffer::from_raw(___)
    assert_eq!(exr_pixels.dimensions(), hdr.____);
    assert_eq!(exr_pixels.pixels(), hdr.____);

[johannesvollmer]

Also, I couldn't find whether the image crate stores images upside down or upside up. Is positive y coordinate considered top of the image, or positive y considered the bottom of the image?

The image buffer .as_raw() seems to store with pixels in y positive being the top of the image.

[johannesvollmer]

Got one tests working, but it feels like there should be an easier way of doing it

[HeroicKatora]

(0, 0) is considered top-left for the buffers in this library.

[johannesvollmer]

Using ImageBuffer, I added some tests:

• compare_exr_hdr
• roundtrip_rgba
• roundtrip_rgb
• compare_rgba_rgb

[johannesvollmer]

Hey! I've finally had the time to press the push commits button in my IDE! Sorry for the delay.

So, from my perspective, we could merge this soon, as a minimal support.

There's of course a lot of other things left to optimize, including the exr library itself. But for now, I estimate that 95% of uses cases are already covered, which is RGB images with F32 and F16 precision. I included an extensive list of limitations in the module documentation. Here's the contents:

• only color type Rgba32F and Rgba16F are supported
• only non-deep rgb/rgba files supported, no conversion from/to YCbCr or similar
• only the first non-deep rgb layer is used
• only the largest mip map level is used
• pixels outside display window are lost
• meta data is lost
• dwaa/dwab compressed images not supported yet by the exr library
• (chroma) subsampling not supported yet by the exr library

Some of these limitations are simplifications, which may possibly never change: Only the first layer and largest mip map level. However, conversion between color models, meta data, and subsampling might be something we could add later.

Note: The YCbCr conversion is specified in the OpenEXR standard, so it should be easy to implement, as it is fully defined.

[HeroicKatora]

Very nice iteration overall. LGTM.

Regarding linear RGB vs. sRGB I hope we can solve it with the meta data PR and f32 in the next major release.

[johannesvollmer]

That would be great :)

[johannesvollmer]

No wait.... the decoder should panic instead? Whoopsie :D Changed it to a panic.

Would it make sense to unify this behaviour to be the same across decoding and encoding?

[johannesvollmer]

I'd say I'm ready to merge, but there seems to be a problem with the CI runner:

info: syncing channel updates for 'beta-x86_64-unknown-linux-gnu'
error: could not download file from 'https://static.rust-lang.org/dist/channel-rust-beta.toml.sha256' to '/home/runner/.rustup/tmp/vnafuiemrcd4vsc4_file': failed to make network request: error sending request for url (https://static.rust-lang.org/dist/channel-rust-beta.toml.sha256): error trying to connect: Connection reset by peer (os error 104): error trying to connect: Connection reset by peer (os error 104): Connection reset by peer (os error 104): Connection reset by peer (os error 104)

[johannesvollmer]

re-triggering the CI worked, now I'm ready to go :)

[johannesvollmer]

Thanks for your support <3

[jzarzeckis]

Hey. as a newbie I may get it all wrong, but I'm trying this out with the image in the tests folder, and I called:

let im = open("assets/cropping - uncropped original.exr").unwrap();

And got the following error:

Unsupported(UnsupportedError { format: Unknown, kind: Color(Rgba32F) })

I wonder why the format is Unknown. Did it fail to parse the extension? Just tested with a PNG, and it did work fine.

FYI - this is what I have in my cargo.toml:

image = { git = "https://github.com/image-rs/image", rev="3197e3d964f5d214cbdec98206fbc895723e5c72" }

[johannesvollmer]

Hi! Thanks for taking the time to post this. Next time, don't be afraid to open a full-blown issue and tag me in there :)

Summary: While the exr file format can be decoded and encoded, the rest of the image library needs more time to properly implement f32 support everywhere.

In detail: EXR files currently cannot be loaded as a DynamicImage. This is because the dynamic image does not support F32 data yet - regardless of the file format. As a workaround, you can still load an exr image to an f32 buffer by using the OpenEXRDecoder directly. We hope that in the next major release, this won't be necessary.

[jzarzeckis]

Thanks for the clarification. Yeah - for now I'm playing around with the decoder directly.

[johannesvollmer]

You're welcome. If you notice anything else, let me know :)