[go: nahoru, domu]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ability to enable full JPEG baseline compatibility #3

Open
lautat opened this issue Jan 16, 2019 · 2 comments
Open

Add ability to enable full JPEG baseline compatibility #3

lautat opened this issue Jan 16, 2019 · 2 comments

Comments

@lautat
Copy link
lautat commented Jan 16, 2019

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
Copy link
Member

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
Copy link
Author
lautat commented Jan 16, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kornelski @lautat