ImageOptim / mozjpeg-rust

Safe Rust wrapper for the MozJPEG library
https://lib.rs/mozjpeg
Other
74 stars 18 forks source link

Add ability to enable full JPEG baseline compatibility #3

Open lautat opened 5 years ago

lautat commented 5 years ago

Compress::set_quality calls jpeg_set_quality with force_baseline parameter as FALSE, which is not an issue as long as a decoder capable of reading non-baseline JPEG, like the IJG decoder, is used. For example, optimised JPEG decoder on macOS and iOS can not decode 16-bit quantization table entries which could be generated when force_baseline parameter is FALSE. While iOS does use libjpeg as a fallback, it is slower and causes performance regressions. (This was an issue a year ago, it is possible that it has been fixed in latest versions of iOS and macOS).

Would it be possible to add the ability to set this parameter as TRUE? Simply adding another parameter to Compress::set_quality would be a breaking change, so adding another method like Compress::set_quality_and_force_baseline could be a viable solution.

Relevant section in libjpeg manual: https://github.com/mozilla/mozjpeg/blob/master/libjpeg.txt#L903

kornelski commented 5 years ago

I'm assuming this is only about baseline quantization tables, not disabling progressive rendering.

It would be good to generate 8-bit tables indeed.

If I recall correctly 16-bit tables are generated only when necessary. If values fit in 8 bit, libjpeg-turbo will output 8-bit even if baseline wasn't requested. And values exceed 255 only at qualities lower than 30 or so (depending on qtable), so I'm not sure if that's a widespread problem.

I see two solutions:

  1. From API perspective, a separate set_baseline() seems nicer. It would be trickier to change though, because the Rust side would have to "buffer" the settings in case quality is was first. But maybe that's a good thing anyway, given how fragile internal state of libjpeg is.

  2. Don't add any option. Automatically force baseline qtables for qualities > 30. Below 30 JPEG falls apart anyway, so nobody should be using these.

lautat commented 5 years ago

I'm assuming this is only about baseline quantization tables, not disabling progressive rendering.

Exactly, this is what I meant. Progressive rendering is not an issue.

If I recall correctly 16-bit tables are generated only when necessary. If values fit in 8 bit, libjpeg-turbo will output 8-bit even if baseline wasn't requested. And values exceed 255 only at qualities lower than 30 or so (depending on qtable), so I'm not sure if that's a widespread problem.

I'm not very familiar with libjpeg-turbo implementation, but I also recall that it uses 8-bit tables when all values are in range 1..255.

I see two solutions:

  1. From API perspective, a separate set_baseline() seems nicer. It would be trickier to change though, because the Rust side would have to "buffer" the settings in case quality is was first. But maybe that's a good thing anyway, given how fragile internal state of libjpeg is.

  2. Don't add any option. Automatically force baseline qtables for qualities > 30. Below 30 JPEG falls apart anyway, so nobody should be using these.

Latter is easy to implement, while I agree that the former may be a better solution.