image icon indicating copy to clipboard operation
image copied to clipboard

Allow using `zune-jpeg` for JPEG decoding

Open Shnatsel opened this issue 1 year ago • 11 comments

zune-jpeg is a pure-Rust JPEG decoder with performance on par with libjpeg-turbo, and far exceeding that of jpeg-decoder.

It now passes rigorous correctness tests on real-world images, and shows no issues when fuzzed. I believe it is mature enough to add to image as an optional backend for JPEG decoding, and you know I don't make claims like these lightly.

Shnatsel avatar Jan 21 '23 21:01 Shnatsel

Sounds good with regards to maturity. There's some API details for implementation:

  • multithreading, we need to not spawn threads on wasm targets for instance. Maybe switch to not-zune-jpeg on these targets?
  • the ImageDecoder interfaces in image assumes that the buffer information (size and color space) is available before the main decoding loop is entered. How should the API be used here?
  • we'll can replace (JpegDecoder::scale) with downsampling but it's odd
  • the image interface assumes a Reader, zune-jpeg takes byte slice fully in memory. The buffering shouldn't be too much of a problem itself, but we need to be able to effectively stop reading from the underlying reader at the end-of-stream marker. How best to implement this? Decoding the image to decide what data consistitutes the full encoded buffer is obv. non-sensical.

HeroicKatora avatar Feb 01 '23 17:02 HeroicKatora

Note that development has moved from https://github.com/etemesi254/zune-jpeg to https://github.com/etemesi254/zune-image repository, and the decoder is now always single-threaded. There have been API changes as well, so please take a look at the zune-image repository.

I'll defer the rest of the questions to the author, @etemesi254

Shnatsel avatar Feb 01 '23 18:02 Shnatsel

I assume some combination of downsampling and plain old resizing will do the trick. For example, downsample only to 2x of the requested size in all dimensions, and let the user then run the final resizing step. This will save a lot of memory on huge images without degrading the output quality much.

  • the image interface assumes a Reader, zune-jpeg takes byte slice fully in memory. The buffering shouldn't be too much of a problem itself, but we need to be able to effectively stop reading from the underlying reader at the end-of-stream marker. How best to implement this?

Unless image already provides an API for decoding multiple image from the same stream without knowing where one ends and the other starts, then simply ignoring the marker and reading till EOF doesn't break anything. It will use more memory when decoding rarJPEGs, but I don't see a way around this without a streaming parser.

Shnatsel avatar Feb 01 '23 18:02 Shnatsel

the ImageDecoder interfaces in image assumes that the buffer information (size and color space) is available before the main decoding loop is entered. How should the API be used here?

All decoders in zune provide a decode_header interface. These provide colorspace infromation, depth information, width height, etc before decoding, from jpeg to ppm, you can call decode_header to decode this information, and the other get_ methods to get the information.

the image interface assumes a Reader, zune-jpeg takes byte slice fully in memory. The buffering shouldn't be too much of a problem itself, but we need to be able to effectively stop reading from the underlying reader at the end-of-stream marker. How best to implement this? Decoding the image to decide what data consistitutes the full encoded buffer is obv. non-sensical.

This is a bit trickier. When we hit the EOF marker, there are instances where we may request extra bytes from the decoder (aggressive refill), but the underlying bytestream on having no bits usually returns 0, which the huffman decoder treats as a no-op(to match libjpeg-turbo),in case there are extra bytes, then those will be read. But as @Shnatsel pointed out, I haven't seen multiple images defined within a single reader

etemesi254 avatar Feb 01 '23 18:02 etemesi254

The concern isn't so much multiple images in a single reader, but that buffering all the available data beyond the image presents a change in semantics that has somewhat unknown consequences. Just not sure about it and if it's cheap I'd rather terminate the reading properly.

HeroicKatora avatar Feb 01 '23 19:02 HeroicKatora

All remaining panics on fuzzer inputs are fixed and the fuzzer has run for 250,000,000 inputs without any panics. It has also been tested for correctness on a whole imageboard, 300,000 images. It's time to wire it up.

Shnatsel avatar Feb 25 '23 01:02 Shnatsel

Simple plan for png: A reader that only checks the chunking until EOF to create an in-memory buffer of the whole file. Should be doable.

HeroicKatora avatar Feb 25 '23 21:02 HeroicKatora

Won't that be expensive?

etemesi254 avatar Feb 26 '23 09:02 etemesi254

I have a compiling but untested zune-jpeg adapter in #1876

Shnatsel avatar Mar 09 '23 21:03 Shnatsel

It looks like the latest MR for this is #1877.

anarcat avatar Nov 13 '23 16:11 anarcat

Yep. And the plan is to adopt it in the next major release

fintelia avatar Dec 17 '23 22:12 fintelia

Oh! I see 0.25 has shipped. Do you want any help with writing a release announcement?

Shnatsel avatar Mar 10 '24 19:03 Shnatsel

@Shnatsel that would be awesome. I wrote a changelog entry but haven't written up anything beyond that

fintelia avatar Mar 10 '24 20:03 fintelia