JXL: initial support #1945
JXL: initial support #1945
Conversation
|
I license past and future contributions to image-rs under the dual MIT/Apache-2.0 license, |
src/codecs/jxl/decoder.rs
Outdated
| } | ||
|
|
||
| fn icc_profile(&mut self) -> Option<Vec<u8>> { | ||
| self.image.embedded_icc().map(Vec::from) |
There was a problem hiding this comment.
Decoded images are in rendered_icc() profile, but it seems like JxlImage lacks the method. I'll fix that in the next release of jxl-oxide.
There was a problem hiding this comment.
Thanks for opening this PR, it is off to a good start!
I left some minor comments, but the one big thing I'd really like to see before we enable jxl by default is to implement ImageDecoder::set_limits. Supporting resource limits for decoding can be rather tricky to implement if you don't have it right from the start (as we're unfortunately finding with some of our other decoders) but we can't really do fuzzing without it.
src/lib.rs
Outdated
| /// | PNM | PBM, PGM, PPM, standard PAM | Yes | | ||
| /// | DDS | DXT1, DXT3, DXT5 | No | | ||
| /// | TGA | Yes | Rgb8, Rgba8, Bgr8, Bgra8, Gray8, GrayA8 | | ||
| /// | OpenEXR | Rgb32F, Rgba32F (no dwa compression) | Rgb32F, Rgba32F (no dwa compression) | | ||
| /// | farbfeld | Yes | Yes | | ||
| /// | JPEG XL | Yes | No | |
There was a problem hiding this comment.
Uh, aren't JXL and JPEG XL the same thing?
src/codecs/jxl/decoder.rs
Outdated
| let pixfmt = renderer.pixel_format(); | ||
| let metadata = &image.image_header().metadata; | ||
| let bits_per_sample = metadata.bit_depth.bits_per_sample(); | ||
| let bitdepth = BitDepth::new(bits_per_sample); | ||
|
|
||
|
|
||
| let colortype = match (pixfmt, bitdepth) { | ||
| (PixelFormat::Gray, BitDepth::Eight) => ColorType::L8, | ||
| (PixelFormat::Gray, BitDepth::Sixteen) => ColorType::L16, | ||
| // | ||
| (PixelFormat::Graya, BitDepth::Eight) => ColorType::La8, | ||
| (PixelFormat::Graya, BitDepth::Sixteen) => ColorType::La16, | ||
| // | ||
| (PixelFormat::Rgb, BitDepth::Eight) => ColorType::Rgb8, | ||
| (PixelFormat::Rgb, BitDepth::Sixteen) => ColorType::Rgb16, | ||
| (PixelFormat::Rgb, BitDepth::ThirtyTwo) => ColorType::Rgb32F, | ||
| // | ||
| (PixelFormat::Rgba, BitDepth::Eight) => ColorType::Rgba8, | ||
| (PixelFormat::Rgba, BitDepth::Sixteen) => ColorType::Rgba16, | ||
| (PixelFormat::Rgba, BitDepth::ThirtyTwo) => ColorType::Rgba32F, | ||
| // | ||
| _ => { | ||
| return Err(unsupported_color(ExtendedColorType::Unknown( | ||
| bits_per_sample as u8, | ||
| ))) | ||
| } | ||
| }; |
There was a problem hiding this comment.
Doesn't this also need to check whether samples are integer or float?
| 17.. => Self::ThirtyTwo, | ||
| 9.. => Self::Sixteen, | ||
| _ => Self::Eight, |
There was a problem hiding this comment.
This makes me nervous that it will silently fail if passed images with other numbers of bits per sample
| Self::with_limits(r, Limits::default()) | ||
| } | ||
| /// Creates a new decoder that decodes from the stream ```r``` | ||
| pub fn with_limits(r: R, limits: Limits) -> ImageResult<JxlDecoder<R>> { |
There was a problem hiding this comment.
If possible, implementing set_limits should be preferred over with_limits
|
sorry haven't had a lot of time to work on this, ill try to make an update soon if I get the time |
The work done here was entirely @EugeneVert, I just cleaned it up a bit and intend to implement missing features at some point, this is a draft because it needs cleaned up and is missing a few features
jxl-oxide also has some issues decoding large image's often running into OOM issues if the image is large enough.
currently it's usable enough for a good chunk of images out there. decided to post it both to get comments on the current state as well since I'm using it in it's current state, it may as well be in the open
I license past and future contributions under the dual MIT/Apache-2.0 license,
allowing licensees to choose either at their option.