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

Switch resizing algorithm to CV_INTER_AREA #143

Closed
GoogleCodeExporter opened this Issue Apr 6, 2015 · 14 comments

Comments

Projects
None yet
1 participant
@GoogleCodeExporter

GoogleCodeExporter commented Apr 6, 2015

Unfortunately, by default mod_pagespeed uses cvResize() with default 
interpolation method, which leads to unacceptable quality of resized jpg 
images. 

Ideally, the algorithm used shall be configurable. For now i've patched the 
code to use CV_INTER_AREA mode, which gives much better results. Patch attached.

Original issue reported on code.google.com by codepain...@gmail.com on 9 Dec 2010 at 1:06

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Thanks for the patch!  For my own curiosity, would you mind attaching 3 images: 
the original, the one generated by our system today, and the one generated by 
our system with this patch?

Original comment by jmara...@google.com on 9 Dec 2010 at 1:29

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Find the images attached - original one (640x480) and images resized to 
160x120, one of them with my patch applied. 

The picture is not perfect, but the difference is visible at leaf's edges.

Original comment by codepain...@gmail.com on 10 Dec 2010 at 5:06

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Hmm, it looks like the images were re-compressed after the upload, and the 
difference is no longer visible. Let me attach zipped original files then...

Original comment by codepain...@gmail.com on 10 Dec 2010 at 5:08

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

I think it makes sense to add a configuration setting for spending CPU cycles 
to improve quality.  In the future if we get our compressed-image caching 
working really well then it might make sense to turn it on by default.

Original comment by jmara...@google.com on 14 Jan 2011 at 6:22

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by sligocki@google.com on 23 Feb 2011 at 11:10

  • Added labels: Type-Enhancement
  • Removed labels: Type-Defect
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by sligocki@google.com on 24 Feb 2011 at 10:34

  • Changed title: Make image re-compression algorithm configurable
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by jmara...@google.com on 6 May 2011 at 5:22

  • Changed state: Fixed
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Reopened.  While recompression and resize are now separately enabled, we still 
do not permit the choice of algorithm for resizing.  I've retitled the report 
to reflect my understanding.

Original comment by jmaes...@google.com on 6 May 2011 at 7:07

  • Changed title: Make image resizing algorithm configurable
  • Changed state: Accepted
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

I think I'm having the same issue with resizing of a JPG (and the rewritten 
version looking very pixelated & not acceptable from an image quality 
perspective).

I've attached the original & rewritten version (excuse the image contents, for 
a swimwear & intimate apparel website :P).

I hope the quality shows, if not, then I'll delete my comment.

Original comment by mtwhe...@gmail.com on 22 Jun 2011 at 7:02

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Good, looks like the image quality differences is very clear.

Original comment by mtwhe...@gmail.com on 22 Jun 2011 at 7:04

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

The issue itself looks pretty simple, is there any estimate when can we expect 
the problem solved?

Original comment by codepain...@gmail.com on 22 Jun 2011 at 7:24

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Note that in the interim you can specifically disable image resizing in the 
most recent release of mod_pagespeed using:
   ModPagespeedDisableFilters resize_images
That said, I don't think it's the solution any of us wants.  I'll take a look 
at the plumbing here; the main knob  we have at our disposal is the 
filtering/interpolation method used during resize, and it looks like the 
default (bilinear interpolation) works badly for the hair and leaf edges in 
particular.

Looking at the OpenCV wiki, theirs a suggestion that we ought to change the 
default to area remapping (CV_INTER_AREA) as this may be better for image 
shrinking.  We could then offer bilinear and bicubic interpolation as options.  
I'll make the former change immediately; the latter change requires a bit of 
plumbing; I'll prioritize that if the sample images still cause trouble with 
the new setting.

Original comment by jmaes...@google.com on 23 Jun 2011 at 12:43

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Well, changing the default method to area remapping (CV_INTER_AREA) is exactly 
what my patch does - attached to this ticket when opening it. I'm not sure if 
it will apply cleanly now, as it was a few months ago, though. 

Original comment by codepain...@gmail.com on 23 Jun 2011 at 1:35

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

I've made the CV_INTER_AREA change in r803 (the image handling code has changed 
quite a bit in the intervening time!).  I'm closing this bug, and opening a 
fresh one for configurability.  If you think you'll need configurability beyond 
CV_INTER_AREA, please weigh in over there with sample images.

Original comment by jmaes...@google.com on 24 Jun 2011 at 3:45

  • Changed title: Switch resizing algorithm to CV_INTER_AREA
  • Changed state: Fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment