woelper / oculante

A fast and simple image viewer / editor for many operating systems
https://github.com/woelper/oculante
MIT License
1.01k stars 45 forks source link

Investigate jxl-oxide for jpegxl decoding #181

Closed Quackdoc closed 1 year ago

Quackdoc commented 1 year ago

jxl-oxide is a pure rust jpeg-xl decoder. not only is the licence MIT (or BSD, it's dual licensed), better aligning with this repo's licence. but it also being in rust should help cross platform support.

https://github.com/tirr-c/jxl-oxide

currently it is not a feature complete decoder, however afaik it should decode most jxl images in the wild, I dont think it's ready for inclusion quite yet. but it's at the stage where it is worth keeping an eye on, it's also possible that someone may wish to add this to image-rs directly eventually.

woelper commented 1 year ago

Thank you so much. I actually had this repo starred, as the current options were a nightmare to set up - I grew a couple of grey hairs making the CI produce executables with everything statically linked on all platforms, and I would love to switch to some rust-native library sooner than later. I'll leave this in to track this issue.

Quackdoc commented 1 year ago

just as an update jxl-oxide has released 0.1.0 and has been published to crates, will wait for news to see if image-rs will integrate it first before further investigation

woelper commented 1 year ago

Cool, thank you for the update! If image won't integrate it (or I find some time) I will add it!

Right now decoding is based on the extension with fallback to image, so there would be no harm if I manually add jxl-oxide and image integrates it later.

woelper commented 1 year ago

I've switched to jxl-oxide now. I've tested a couple of images I could find, and it opened everything so far, including images with and without alpha channels. Do you have access to more test images? I am having trouble displaying them in my browser as support has been removed :(

Quackdoc commented 1 year ago

you can view some conformance test images here https://github.com/libjxl/conformance/tree/master/testcases

as well as a known trouble image here (this is the same uber large image as the black frame issue) https://files.catbox.moe/2kdb4f.jxl (see https://github.com/tirr-c/jxl-oxide/issues/22)

as for browser support, both firefox and chrome have jpeg-xl extensions, but they are a bit slow, thorium (chromium fork) and waterfox (firefox fork) have good jpeg-xl support if you need a test browser

Quackdoc commented 1 year ago

it's probably worth noting, in my entire library i've tested, this was the only problem image that straight up had issues.

woelper commented 1 year ago

Thanks, just what I need! I'll merge the branch if the test images work!

woelper commented 1 year ago

I am quite happy now - I found the pixel format is exposed as an enum, so now I just match against it.

Animations also "just work", which is nice. The only thing I don't understand is their timing. The animation header has a tps_denominator and a tps_numerator. Apparently tps stands for "ticks per second", and I don't understand what that means. If i look at newton's cradle, the numerator is 100, and the denominator is 1 - so 100/1 = 100? 100 ticks per second? Or 100 out of 1000 milliseconds? The original docs seem to he here, but I don't understand them either. In the newton's cradle json, frames have a duration of 0.02, and I assume that's seconds. This would make sense and produce a 50 fps animation.

It's all not too bad, I can hardcode animation delay and it's better than what I had before, which was no animation support :)

Quackdoc commented 1 year ago

the first is right 100/1 would be 100 ticks per second. like wise 100/2 would be 50 ticks per second. you can see 4.6 of this https://arxiv.org/ftp/arxiv/papers/1908/1908.03565.pdf (specifically (tps_numerator_minus_1 + 1) / (tps_denominator_minus_1 + 1) shall indicate the number of ticks per second.

you also need to see the duration which is the duration between frames in ticks

so say you have 100/1 for the animation and the frame's duration is 50, that means the next frame comes after 50 ticks. so half a second, this is important because if the next frame's duration is 100, that means it should last for 100 ticks, or one second later.

video formats (just a series of frames) often use fractional timecodes another common one you might see is 24000/1001 which roughly represents 24fps.

EDIT: btw gif96a and apng have similar data blocks

Quackdoc commented 1 year ago

btw here is a problematic JXL

Source: https://medium.afileditch.ch/m3/KoJMzgVvHZulSmfNEQol.gif JXL: https://files.catbox.moe/ttrgar.jxl

you can run jxl-info on it and it will show the per frame info but basically the output is more or less this, you can see that the first frame has 56 ticks, because of an initial delay, then it's 9 ticks for the rest of them,

JPEG XL image
  Image dimension: 2016x1064
  Bit depth: 8 bits
  Color encoding:
    Colorspace: RGB
    White point: D65
    Primaries: sRGB
    Transfer function: sRGB
  Animated (100/1 ticks per second)
Frame #0
  Modular (maybe lossless)
  2016x1064; (0, 0)
  Duration: 56 ticks
Frame #1
  Modular (maybe lossless)
  2016x1064; (0, 0)
  Duration: 9 ticks
  ...
woelper commented 1 year ago

Aaah, that was helpful! Thanks!

I was confused by this in the header:

animation: Some(
    AnimationHeader {
        tps_numerator: 100,
        tps_denominator: 1,
        num_loops: 0,
        have_timecodes: false,
    },
),

I thought this was all there is and have_timecodes: false meant that there were no individual durations. I had a hard time imagining how one would construct a valid duration from it. Now I see that the render result has a duration() function. I'll try that!

Quackdoc commented 1 year ago

timecodes can be ignored, they are SMPTE timecodes "HH:MM:SS:FF" and that flag merely exists to tell if they exist or not. it's optional metadata that probably has no use in occulante.

Quackdoc commented 1 year ago

this jxl animation causes a crash, maybe due to offset, windows build doesnt seem to dump any issue so Im not sure whats going on off the top of my head https://files.catbox.moe/3nkqjx.jxl

woelper commented 1 year ago

Interesting! It panics in the image library itself, on this unwrap:

// from f32:
// Note that in to-integer-conversion we are performing rounding but NumCast::from is implemented
// as truncate towards zero. We emulate rounding by adding a bias.

impl FromPrimitive<f32> for u8 {
    fn from_primitive(float: f32) -> Self {
        let inner = (float.clamp(0.0, 1.0) * u8::MAX as f32).round();
        NumCast::from(inner).unwrap()
    }
}

I might try to do float -> u8 conversion myself, but I guess this should not happen. This is nothing I can even catch, as the function is infallible.

woelper commented 1 year ago

This is now addressed by manually converting to u8. This could even be faster as it's multithreaded.

woelper commented 1 year ago

I am feeling like merging the PR now unless you find some flaws :)

Quackdoc commented 1 year ago

I have found another animated image it's crashing on, however tha'ts a jxl-oxide issue, so other then that, so it seems good, animations are playing at the proper rate, and decoding is sufficiently fast

woelper commented 1 year ago

Would you mind linking the problematic image? Have you raised a ticket with jxl-oxide? I am happy to do that as well.

Quackdoc commented 1 year ago

Thanks! the ticket is here https://github.com/tirr-c/jxl-oxide/issues/26, it already has a PR to fix it

tirr-c commented 1 year ago

The AVX2 version of color conversion had a bug, it's just merged. I'm going to fix some more bugs and release it as a patch version.

woelper commented 1 year ago

You are both amazing, thank you for that (and the great library)!