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.

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

Expose mozjpeg quant_table flag #1285

Merged
merged 1 commit into from
Jul 10, 2018
Merged

Expose mozjpeg quant_table flag #1285

merged 1 commit into from
Jul 10, 2018

Conversation

rexxars
Copy link
Contributor

@rexxars rexxars commented Jul 3, 2018

This exposes the quant_table flag from mozjpeg, which previously had the libjpeg/mozjpeg default of 0. Will use 0 as the default, but now it's possible to choose a specific quantisation table when paired with the optimize_coding flag that was recently added.

This can be very useful in a certain cases where some tables provide a better quality/compression ratio for certain images, which opens up a few cool tricks.

I unfortunately could not figure out a way to run the tests in a reliable way since mozjpeg is usually not installed, but it seems other tests using mozjpeg features use a similar "same or less" approach - let me know if you have any pointers on how this can be improved. Perhaps if there was some way to check if mozjpeg is available, it could be exposed by sharp somehow and we could do a skip on the specific tests if not available?

@coveralls
Copy link

coveralls commented Jul 3, 2018

Coverage Status

Coverage increased (+0.007%) to 99.018% when pulling 66989b8 on rexxars:jpeg-specify-quant-table into 5cb3548 on lovell:master.

@lovell
Copy link
Owner

lovell commented Jul 4, 2018

Hei Espen, thanks for this PR, I should be able to take a closer look in a couple of days.

I think it might reveal a small bug in the way libvips currently only sets the table based on quant_table being non-zero. However mozjpeg uses table 3 by default, which means you can never "reset" mozjpeg to use table 0 via libvips. It also means that you might think you're using table 0 "by default" when you're actually getting table 3.

@rexxars
Copy link
Contributor Author

rexxars commented Jul 4, 2018

I've compiled against libvips 8.6.4 and mozjpeg 3.3.1, and I think you're onto something, but I'm not seeing quite the behavior you are suggesting. It seems to me that (with this PR, at least) I am able to freely change between the different quantization tables:

onst sharp = require('./')

async function main() {
  for (let i = 8; i >= 0; i--) {
    await sharp('img.jpg')
      .resize(1024, 1024)
      .max()
      .jpeg({
        optimizeCoding: false,
        quantisationTable: i
      })
      .toBuffer()
      .then(res => console.log('Table %d, %d bytes', i, res.length))
      .catch(err => console.error(err))
  }
}

main()

Yields the following output:

Table 8, 165658 bytes
Table 7, 186603 bytes
Table 6, 146376 bytes
Table 5, 117315 bytes
Table 4, 126824 bytes
Table 3, 115800 bytes
Table 2, 119055 bytes
Table 1, 202427 bytes
Table 0, 126018 bytes

As you pointed out however, libjpegs default quantization table is 0, while mozjpeg is 3, and this PR sets it to 0 by default. Not sure how you'd prefer it to be set by default?

@lovell
Copy link
Owner

lovell commented Jul 9, 2018

Thanks for checking. I'd forgotten about the libvips logic, which I added, to reset mozjpeg to use libjpeg defaults and therefore table 0 - see jcupitt/libvips@00e27de#diff-6e6f82dc292e7f7f8d223ff3dff4c7faR913

test/unit/io.js Outdated Show resolved Hide resolved
@lovell lovell merged commit 7bbc517 into lovell:master Jul 10, 2018
@lovell
Copy link
Owner

lovell commented Jul 10, 2018

Thank you, this will be in v0.20.6.

@lovell lovell added this to the v0.20.6 milestone Jul 10, 2018
lovell added a commit that referenced this pull request Jul 10, 2018
@rexxars
Copy link
Contributor Author

rexxars commented Jul 10, 2018

Thanks a bunch, Lovell! sharp is fantastic ❤️

@rexxars rexxars deleted the jpeg-specify-quant-table branch July 10, 2018 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants