search

inflation / jpegxl-rs

GNU General Public License v3.0
54 stars 8 forks source link

Using native pixel format of the image #6

Closed kornelski closed 3 years ago

kornelski commented 3 years ago

It seems that the decoder interface requires deciding on pixel format and number of channels ahead of time, when calling decode. But I don't see any way to discover what is the correct format for the image.

I'd like to decode image without unnecessary conversion, e.g. if it's opaque, then decode to RGB, if it's transparent, then decode to RGBA. If it's sRGB, then decode to 8-bit, if it's wide gamut, then decode to 16-bit pixels, and so on.

Currently I'd have to ask for 16-bit RGBA for every image in order to avoid data loss, and that's a waste when dealing with 8-bit opaque images.

inflation commented 3 years ago

This is the design of the underlying decoder library. It will do the internal conversion to any supported pixel format. So the idea is to specify the result format for either conversion or display, which usually is a fixed format.

Let me think of a better way to handle that.

inflation commented 3 years ago

Some attempt:

I can read basic_info.bits_per_sample to determine which pixel type the original image uses. However, I can't think of a way to tell that to Rust's type system.

For the C library, it's just bytes to write to, but in Rust's perspective, you can't transform a Vec<u8> to a Vec<u16> trivially without copying or UB (since the layout of allocation changed). The best I came up with is using a Vec<u32> to store the data with stricter alignment and have &[u8] and &[u16] slicing into it.

I'll try union next.

kornelski commented 3 years ago

In such cases I return an enum Image { RGB8(Vec<RGB8>), RGB16(Vec<RGB16>) }

Returning user-supplied pixel type is fine too, as long as you expose a way for the caller to chose the right type.

kornelski commented 3 years ago

BTW, PixelType is not exported from the crate, and doesn't show up in the docs:

https://docs.rs/jpegxl-rs/0.3.7/jpegxl_rs/decode/struct.JxlDecoder.html

inflation commented 3 years ago

PixelType does not mean to be a public trait. It's an abstraction over the pixel types internal decoder and encoder supported. Currently, it's u8, u16, and f32(without alpha channel). Also u32 is in the headers but not implemented.

There are some comments in the source code, I guess I should document this.