OpenEXR support desired?

[johannesvollmer]

Hi! Im the creator of the pure Rust implementation of the OpenEXR format. Would you be interested in having EXR support? I'd be happy to implement that and would create a PR if this is valuable :)

The EXR crate already supports most images. Only one compression method and a few special features are still on the roadmap. Simple RGBA images already have great support, and can be read and written.

OpenEXR is a popular image format, especially in VFX workflows, as it stores real floating-point pixels which can be compressed losslessly or lossy. Pixels can contain any type of channels, for example RGBA, LAB, CMYK, XYZ, or even depth and motion vectors. Arbitrary meta data can be added to any image. Most software already supports exr.

[fintelia]

I think OpenEXR would be a good addition. One of the hard parts would probably be mapping the complexity of the format down onto the simpler abstractions provided by this crate.

[johannesvollmer]

In the worst case, we can decide to support only a small subset of exr images. I'll have a look into the exact abstractions of the image crate to estimate the amount of work for this feature.

You will have to specify if the exr library needs some changes. I'm happy to add improvements to the exr library. For example, the exr crate always includes rayon, but it's a feature flag in the image crate. It could be a feature flag in the exr library too, if necessary.

[johannesvollmer]

First challenge: The current exr implementation needs Write to also be Seek. The meta data contains a lookup table, storing which pixel tile can be found at what byte in the file. After compressing and writing each tile, the writer seeks back to the beginning and then updates the byte locations of the compressed tiles.

Would it be possible to get a seekable writer in the image crate? This would probably not be a backwards-compatible change.

The only alternative I see, would be to write the exr image to an in-memory vector, which is Seek, and then copy that vector to the Write. It it is possible to open the same file twice, once for writing, and then for updating the byte locations, that could also maybe work.

[HeroicKatora]

The only alternative I see, would be to write the exr image to an in-memory vector, which is Seek, and then copy that vector to the Write. It it is possible to open the same file twice, once for writing, and then for updating the byte locations, that could also maybe work.

Incidentally, avif has similar concerns and uses an in-memory vector with a fallback strategy. This is definitely an interface problem that we should address for the next major version.

[johannesvollmer]

The reasons to make it Seek are: Most writers are seek (file is), and for all other writers, the user can just write to a temporary vector themselves.

The only disadvantage is losing backwards compatibility. I guess this would be fine, as most people only write to files anyways. But you'll have to decide that in the end.

[johannesvollmer]

I'm a little lost with the details of the implementation. I'm confused about the float pixel story. The RGB<T: Primitive> seems to allow float precision. But the ColorType of a decoder only has Rgb8 and Rgb16 and Decoder::read_image(bytes) expected i8/u16 values. So I assume Rgb16 only allows u16. On the other hand, the HDR decoder has a separate HDR adapter, which internally can output f32. But it seems like this is not really integrated into the core crate.

Is it possible to store f16 or even f32 data in an image buffer? Will the high-dynamic-range values have to be clamped to the 0-1 range? Could we add true high-dynamic-range float processing to the crate?

[fintelia]

What I would suggest would be first to add a ColorType::Rgba32F variant and/or other f32 variants. We try to be sparing with the number of variants of ColorType so half float and 32-bit integer variants are probably out of scope. The reasoning is that any downstream user kind of has to deal with any possible color type, so we prefer decoders to convert to one of the existing ones if possible.

With that you should be able to implement the ImageDecoder trait, and produce high-dynamic-range values. The output of read_image() is a &mut [u8], but that is expected to be reinterpreted as native endian samples of the type indicated by color_type().

I think ImageBuffer should already work with floating point samples, while DynamicImage can probably be dealt with in a follow up.

Does this all make sense?

[johannesvollmer]

Sounds good.

For image processing, f32 would be enough. We don't need f16 image processing, as f16 must always be converted to f32 anyways for calculations.

Nevertheless, it would be nice to have f16 storage internally, to save memory. On the other hand, if you are choosing high quality as a user, you will probably use 32 bit anyways, at least while processing. When writing the file, letting the exr encoder convert the f32 values to f16 might be a nice feature later on.

So my plan would be:

• enable storage method F32
• add F32 to the processing interface

Given that this would probably add about 4-8 new variants to the color type enum, I wondered if it might make sense to split up the color type into two fields: Channels (e.g. L, LA, RGB, RGBA) and SampleType (U8, U16, F16, F32, ..).

What do you think?

[fintelia]

We used to break up ColorType into two parts, but the vast number of combinations meant that almost all code declared most of them unimplemented. I think it would be good to think through how few additional variants could be added. In my view, a key role of a decoder is to convert the pixel data from the source format into a much smaller list of common encodings even if that conversion is sightly lossy or uses more memory. Thus, F16 types should be unnecessary because the F32 versions should work well enough. I've even been debating whether all four of {R32F, Rg32F, Rgb32F, and Rgba32F} are required so I'd be curious your thoughts on condensing down to a subset of those.

We'll also need to clearly define what the floating point values mean. I think the usual meaning for high dynamic-range is that [0,1] is the usual image range and values greater than that are brightness values beyond what low dynamic range can represent.

The ExtendedColorType and associated ImageDecoder::original_color_type method are designed to communicate how the image data is represented in the file, and it should be fine to go ahead and add variants for all the pixel types you've been describing. I've been thinking there should be an additional method on decoders to signal "actually I'd like to read the image data in the format of original_color_type() instead of color_type()". The read_image() function already just works with byte buffers, so it wouldn't actually need to change.

[johannesvollmer]

key role of a decoder is to convert the pixel data from the source format into a much smaller list of common encodings, even if that conversion is sightly lossy or uses more memory. Thus, F16 types should be unnecessary because the F32 versions should work well enough

Agree, f32 should be fine.

Thinking this further... shouldn't Bgr be converted to Rgb when decoding? As a user of the image library, I personally don't really care about the order of the channels in the file. It adds an extra special case, probably without much of an advantage. In what case do other users need to distinguish between Bgr and Rgb? I would expect this to only be in the original_color_type and not in the normal color type.

We used to break up ColorType into two parts, but the vast number of combinations meant that almost all code declared most of them unimplemented

When only using L8, La8, Rgb8, Rgba8, L16, La16, Rgb16, Rgba16, which are all implemented currently, they are the perfect combination of (L|La|Rgb|Rgba) x (8|16). The only suspicious thing is Bgr, which did not add any value in my previous personal experience when I used this crate.

---

We'll also need to clearly define what the floating point values mean. I think the usual meaning for high dynamic-range is that [0,1] is the usual image range and values greater than that are brightness values beyond what low dynamic range can represent.

Yes, this will require some basic investigation. I think, most software handles in like this: 0 is black and 1 is white, meaning, 1 is the brightest color your non-hdr monitor could output. Defining (1,1,1) to be white could also work for the image crate.

By the way, does the image crate convert the file pixels to sRGB or to linear RGB?

---

I've even been debating whether all four of {R32F, Rg32F, Rgb32F, and Rgba32F} are required so I'd be curious your thoughts on condensing down to a subset of those.

Even though an OpenEXR could contain arbitrary channels, I estimate that 95% of the time they will simply contain linear RGBA. The users who work with the remaining 5% of weird images would probably not use a generic image processing crate any ways. I guess the image-rs crate is used in places where you just want to open the damn RGB/RGBA image file already, regardless of the file format, and have a reliably RGB color.

Furthermore, I have yet to see a luminance-only exr in real life, even though it may make sense to save memory. And I have never ever seen, or even used, RG images, so I would absolutely not support those with 2 channels right now haha. Therefore, I would suggest one of the following combinations (sorted by complexity, but the more the better I think):

1. only Rgba32, alpha being 1 if not specified
2. Rgb32, Rgba32
3. Rgb32, Rgba32, L32, La32 (most complete = favourite)

Does someone of you have the time to implement the float precision stuff? I have attempted it but I realized this will take quite some time for me, as I'm not sure in which places changes need to be made.

[fintelia]

Thinking this further... shouldn't Bgr be converted to Rgb when decoding? As a user of the image library, I personally don't really care about the order of the channels in the file. It adds an extra special case, probably without much of an advantage. In what case do other users need to distinguish between Bgr and Rgb?

I agree. Unfortunately removing it now is a rather large change because it is entangled with a lot of the rest of the library and would also have the potential to break a bunch of downstream users (though we don't actually have a great sense of how many). Hopefully, we'll be able to do it at some point.

This is part of the reason I'm hesitant to add more color types than necessary: it is very hard to remove them later...

By the way, does the image crate convert the file pixels to sRGB or to linear RGB?

Currently the crate does no conversion. The output color space in whatever the original file had.

Therefore, I would suggest one of the following combinations (sorted by complexity, but the more the better I think):

1. only Rgba32, alpha being 1 if not specified
2. Rgb32, Rgba32
3. Rgb32, Rgba32, L32, La32 (most complete = favourite)

Does someone of you have the time to implement the float precision stuff? I have attempted it but I realized this will take quite some time for me, as I'm not sure in which places changes need to be made.

I think either (1) or (2) would be best. I'm not quite sure what you mean by float precision stuff though. Are you talking about converting f16 -> f32?

[johannesvollmer]

Cool. With float precision I mean actually implementing the F32 ColorFormat, which seems rather involved to me.

I had the impression that adding F32 would be a lot easier if there was no Bgr variant and the color format was actually two separate enum ... at least that's what I thought on first glance, because then it would be possible to use generics much more abstractly. But I don't know for sure, I hope a contributor with more experience could estimate the workload of adding the F32 color type :)

Removing the Bgr color type variant of course wouldn't mean removing the whole Bgr feature. It would probably be great to still have a method called to_bgr which simply returns the buffer with swapped the bytes, for when some UI expects Bgr pixels or something similar. Would be a breaking change, yes, but probably easy to fix for the users

[fintelia]

Just adding ColorType::Rgba32F and ExtendedColorType::Rgba32F is not very much work. Deeper integration would be more involved, but I'd suggest leaving that for a followup.

[HeroicKatora]

Currently the crate does no conversion. The output color space in whatever the original file had.

Well.. It depends. In jpeg for example we do CMYK -> sRGB but that is a bug. That issue also demonstrates why doing actual conversion is difficult: One has to parse and understand the exact colorimetric definitions in full, and be able to convert efficiently. Both should at least be optional since doing them in hardware is likely much more efficient.

Still, doing normalizations such as bgr(u16) -> rgba(u16) or rgba([u10; 3], u2) -> rgba(u16) very likely wouldn't hurt as long as the numerical value of color components stays unmodified. Note however that color range conversion, such as normalizing 0 to black and 1 to 'absolute white' is not immediately obvious and I think we should apply the same caution as with actual conversion.

I'm not sure if exr has this feature but in some cases a single unit of color values (texel) encodes more than one pixel. A common example is YUV420 where two chroma components are subsampled and apply to blocks of 2×2 pixels. It's slightly questionable if we should expand these because this increases the size, it's not obvious subsampling convention to apply (duplicate or interpolate values) and for read-only textures it is much faster if it's done in hardware later.

[johannesvollmer]

Solid points, for sure.

in some cases a single unit of color values (texel) encodes more than one pixel. A common example is YUV420

Yes this can happen when opening an exr file, however the rust implementation does not support that yet

ExtendedColorType::Rgba32F is not very much work.

I was able to do exactly these changes on my own, haha. I thought it will also be necessary to adjust the define_colors! macro to be able to actually be useful. It's it not important?

[fintelia]

I was able to do exactly these changes on my own, haha. I thought it will also be necessary to adjust the define_colors! macro to be able to actually be useful. It's it not important?

Yeah, you can already create an ImageBuffer<Rgba<f32>> right now. The only thing adjusting the define_colors! macro would do is make Rgba::<f32>::COLOR_TYPE have the correct value (instead of ColorType::Rgba16).

[johannesvollmer]

1 to 'absolute white'

Exr supports quite some color space meta data:
Chromaticities, White luminance and adopted neutral

Maybe these can be used to convert the RGB data to a unified color space that the image crate defines

[johannesvollmer]

make Rgba::::COLOR_TYPE have the correct value (instead of ColorType::Rgba16).

Interesting. What are currently the consequences of this mismatch? Will the image processing functions work as expected?

[johannesvollmer]

I'd argue we should add both Rgb32F and Rgba32F, as it can potentially save not only one fourth memory, but also processing time in certain algorithms: I assume the compiler can potentially optimize some functions if it knows the color is guaranteed to be fully opaque, right?

[johannesvollmer]

Here is some simplified calculation to convert the exr linear data to RGB.

https://github.com/AcademySoftwareFoundation/openexr/blob/e46ac0a70cad0d84f8e8be743a9b2d633056da5b/src/examples/previewImageExamples.cpp#L31

Do you think it would make sense to add at least an opt-in conversion for every image format to CIE XYZ? Otherwise, saving the image data of one format to another format will not produce the same image.

Also, if you do any kind of color interpolation, the color space should at least be linear right? Not sRGB

[HeroicKatora]

Do you think it would make sense to add at least an opt-in conversion for every image format to CIE XYZ?

Clamped XYZ as in your example is surely not a 'universal' color space, and even unclamped only makes a limited amount of sense for that purpose. As even that assumes that we only need to transform between color spaces for a particular human observer and using the same whitepoint, which is quite short sightned imho. Conversion between different whitepoints/viewing conditions can inherently not be completely lossless in a tri-stimulus model, you'll need a vastly more complex model to come even close. And it does not hold in general that any sequence of color space conversion is possible, or reversible.

In total, the most future proof interface is one that does not assume this from the start. I think the best we can do is to clearly ensure there is an method that returns an optional conversion interface to CIE XYZ or any particular output color space. Clearly, that implies that we may want to avoid doing that transformation implicitly as this raises some amount of awareness to the necessary complexity and differentiates different failure modes (failure to decode vs. failure to bring to a unified color space).

[johannesvollmer]

Well yeah right. There are existing color conversion crates so that's not something the image crate must include. However as a user of the image library I have probably a lot less knowledge about the particular color space of a specific image format, or color theory in general, than you seem to have. If the point of this library is to provide a uniform interface to the pixels of images, regardless of format, I would expect it to unify the color handling as well. Especially since the RGB colors are all exposed through the same struct RGB which implies the RGB colors of one image are somehow compatible with the RGB colors of a different image.

This does not mean that all values are converted to a particular color space. Maybe being very explicit about the color space would be enough, maybe having JpegRGB and such, or Rgb<Colorspace, Whitepoint, ...>

[johannesvollmer]

Also, does the image processing part of the library simply produce incorrect results if the user does not convert his images to an appropriate color model on their own (depending on the format and meta data in the image)?

[HeroicKatora]

I want to add that ImageBuffer and Pixel might follow a different philosophy. Indeed, we already somewhat take the stance that all the buffers types represent/contain CIE XYZ compatible color because the trait contains conversion methods and because DynamicImage::to_rgba is functionality we definitely do not want to lose. That is, DynamicImage::from_decoder could implicltly converts to sRGB—without requiring the ImageDecoder itself to do so. That adds one additional error condition, where we don't have an internal conversion method from the image's color space, but at least it leaves downstream users that really care about it to implement their own buffer that do support the color types. And this way an inexperienced user is less likely to be confused about the non-(s)RGB color details.

[johannesvollmer]

Sounds nice

[johannesvollmer]

you'll need a vastly more complex model to come even close (link)

Very fascinating read. I'm still in the middle, of it, so please forgive me if I get something wrong :D

Completely different topic:

https://docs.rs/cint/0.2.1/cint/

This is a crate that aims to provide a unified interface to colors. No conversion methods, only traits. That way, we could communicate the exact color type of an image without forcing the user to use a specific crate for their color calculations, it's very lightweight. Maybe this would be worth a discussion?

[fintelia]

I'm not sure the interface in that crate is quite right for us. It seems to assume that the color space is known at the type level (or tracked individually for each pixel perhaps?). We could just implement cint::ColorInterop<CintTy = EncodedSrgb<T>> for image::Rgb<T> to indicate that we assume everything is sRGB, but I don't think that would do much.

If I understand correctly, the more useful thing for us would be an enum of different possible colorspaces so we could store / communicate that once per image.

[johannesvollmer]

Yeah seems like they don't have dynamic color spaces (yet?) (except a custom color space enum, but that misses the point of standardized interface).

[johannesvollmer]

On the other hand .... Nothing would stop us from defining a precisely crafted enum, for example

enum ExrPixels {
    LinearRgb(Vec<cint::LinearRGB<f32>>),
    YCbCr(Vec<cint::YCbCr<f32>>),
}

type JpegPixels = Vec<cint::sRgb<u8>>;
...

Which would be much more concise than a generic single dynamic pixel type for all images types.

[experiment9123]

I came here to make this exact suggestion.
Add openEXR , and I wanted to check if there are any floating point formats in the DynamicImage enum9

[johannesvollmer]

Cool. Frankly, the exr library has to get at least one larger update so we can give it non-sendable read and write objects. Therefore it will be a few days or weeks until we can support this, even on the most basic level.

[johannesvollmer]

In order to not stray too far, I propose we continue parts of the discussion in separate Colorspace and Float Support issues.

@fintelia will the master branch receive the float support implementation you linked earlier, or was it only meant to be an example?

[fintelia]

That commit I linked was only meant as an example; I wasn't planning on merging it anywhere.

[johannesvollmer]

Ok, gotcha. I'll probably do a separate PR for float support. Which branch should I chose for this change?

[fintelia]

If you don't need to make any breaking changes, just target master

[johannesvollmer]

I have browsed the issues regarding color spaces and color models and color profiles. It seems to me that there are currently great efforts being made to work out a solid color concept for the whole crate. Is there a single document or issue where the color concept is outlined or discussed?

I'm asking because as far as I know, we still have not settled on the meaning of the float colors, right? For adding exr, I'd like to have a clear vision for float colors.

[johannesvollmer]

Initial OpenEXR support was just merged 🎉 It has some limitations. Basically, the format supports only RGB(A)32F. Have a look at the module documentation to find out more about those.