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

Add support for jpeg-recompress from danielgtaylor/jpeg-archive #65

Merged
merged 2 commits into from
Nov 8, 2014

Conversation

wjordan
Copy link
Contributor

@wjordan wjordan commented Oct 5, 2014

Adds support for the jpeg-recompress utility from danielgtaylor/jpeg-archive:

jpeg-recompress

Compress JPEGs by re-encoding to the smallest JPEG quality while keeping perceived visual quality the same and by making sure huffman tables are optimized.

I've set the program to be disabled by default. It is enabled by using the --jpegoptim-max-quality N option to set a JPEG quality preset other than lossless.

Other configuration options are available aside from the quality preset, but I haven't added them to the image_optim options:

    -t, --target [arg]            Set target quality [0.9999]
    -n, --min [arg]               Minimum JPEG quality [40]
    -x, --max [arg]               Maximum JPEG quality [95]
    -l, --loops [arg]             Set the number of runs to attempt [6]
    -a, --accurate                Favor accuracy over speed
    -m, --method [arg]            Set comparison method to one of 'mpe', 'ssim', 'ms-ssim', 'smallfry' [ssim]
    -s, --strip                   Strip metadata
    -d, --defish [arg]            Set defish strength [0.0]
    -z, --zoom [arg]              Set defish zoom [1.0]

@toy
Copy link
Owner

toy commented Oct 6, 2014

Looks interesting, thanks for contribution.
I see that you are using %i which doesn't work in ruby < 2.0.
Also you didn't change comments for class and run_order method.
I'll check how it works.

def optimize(src, dst)
quality_str = QUALITY_OPTIONS[quality]
return false if quality_str == :lossless
src.copy(dst)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why copy instead of using src for input and dst for output in arguments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No intentional reason, I think I just used jpeg_optim as a starter template, I can probably remove this.

@toy
Copy link
Owner

toy commented Oct 17, 2014

Things I've notices:

  • You did not add changelog entry.
  • I've commented on line where you copy src to dst and then use dst for both input and output.
  • Worker should use --no-copy argument so no useless data transfer happen.

Not sure about other arguments, with --accurate worker was much slower and did not optimise as good, but there was no difference in quality loss measured by root mean squared error.

  • Also I am still thinking if it would be better to have global option like allow_lossy which will be passed to all workers and can be used in jpegoptim and pngquant.

toy added a commit that referenced this pull request Nov 8, 2014
toy added a commit that referenced this pull request Nov 8, 2014
toy added a commit that referenced this pull request Nov 8, 2014
toy added a commit that referenced this pull request Nov 8, 2014
@toy toy merged commit 93c3820 into toy:master Nov 8, 2014
@toy
Copy link
Owner

toy commented Nov 8, 2014

@wjordan Once more thanks for contribution, merged and released v0.19.0

@wjordan
Copy link
Contributor Author

wjordan commented Nov 17, 2014

Thanks for finishing up and integrating the contribution! Glad it was helpful. I think the allow_lossy option is a great addition to the program.

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.

2 participants