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

Uploaded Image Filesize Optimization (Stripping Metadata) #2142

Closed
jumph4x opened this Issue Oct 24, 2012 · 5 comments

Comments

Projects
None yet
3 participants
@jumph4x
Member

jumph4x commented Oct 24, 2012

Not a correctness issue with the existing codebase

Rather, a performance improvement for the storefront frontend, as well as, cost savings for S3 users.

For store operators that use Photoshop (for instance) to convert images, a significant chunk of metadata is embedded into the images. Once Spree resizes the images to multiple size, each one of those will carry the said metadata, unless the user specifically stripped it (by using 'Save for Web', for instance).

What you end up with is a thumbnail that could be 2.5kB weighing 85kB with the addition of the metadata. Potential for dramatic improvements.

Blog post: http://jumph4x.net/post/11666938332/jpeg-compression-w-imagemagick-convert-thru

Tested in production on our store first with 0.60, then 0.70 and now 1-1-stable. The blog accurately shows this as year old.

ImageMagick option documentation: http://www.imagemagick.org/script/command-line-options.php#strip
Should you want to include this, it would look like this:

    has_attached_file :attachment,
                      :convert_options => { :mini => "-strip",
                                            :small => "-strip",
                                            :product => "-strip",
                                            :large => "-strip" }
@schof

This comment has been minimized.

Show comment
Hide comment
@schof

schof Oct 24, 2012

Member

Wow. That sounds like a really good enhancement. There's no loss in quality right? Just the metadata?

Member

schof commented Oct 24, 2012

Wow. That sounds like a really good enhancement. There's no loss in quality right? Just the metadata?

@jumph4x

This comment has been minimized.

Show comment
Hide comment
@jumph4x

jumph4x Oct 24, 2012

Member

Correct, this only touches the profiles and I imagine some of the EXIF.

For a lot of the folks, the metadata might not even be there, so the improvement is probably more of an edge case one. But this certainly does not hurt quality.

Member

jumph4x commented Oct 24, 2012

Correct, this only touches the profiles and I imagine some of the EXIF.

For a lot of the folks, the metadata might not even be there, so the improvement is probably more of an edge case one. But this certainly does not hurt quality.

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Oct 24, 2012

Member

The -strip option "strip[s] image of all profiles and comments". I think
this is a great idea.

On Thu, Oct 25, 2012 at 4:57 AM, Denis Ivanov notifications@github.comwrote:

Correct, this only touches the profiles and I imagine some of the EXIF.

For a lot of the folks, the metadata might not even be there, so the
improvement is probably more of an edge case one. But this certainly does
not hurt quality.


Reply to this email directly or view it on GitHubhttps://github.com/spree/spree/issues/2142#issuecomment-9749530.

Member

radar commented Oct 24, 2012

The -strip option "strip[s] image of all profiles and comments". I think
this is a great idea.

On Thu, Oct 25, 2012 at 4:57 AM, Denis Ivanov notifications@github.comwrote:

Correct, this only touches the profiles and I imagine some of the EXIF.

For a lot of the folks, the metadata might not even be there, so the
improvement is probably more of an edge case one. But this certainly does
not hurt quality.


Reply to this email directly or view it on GitHubhttps://github.com/spree/spree/issues/2142#issuecomment-9749530.

@jumph4x

This comment has been minimized.

Show comment
Hide comment
@jumph4x

jumph4x Oct 24, 2012

Member

I'll prepare a pull request.

Member

jumph4x commented Oct 24, 2012

I'll prepare a pull request.

@jumph4x

This comment has been minimized.

Show comment
Hide comment
@jumph4x

jumph4x Oct 24, 2012

Member

Wow, a mention right in the options key documentation: http://rdoc.info/gems/paperclip/2.3.8/Paperclip/ClassMethods

Member

jumph4x commented Oct 24, 2012

Wow, a mention right in the options key documentation: http://rdoc.info/gems/paperclip/2.3.8/Paperclip/ClassMethods

@radar radar closed this in f22355a Oct 26, 2012

radar added a commit that referenced this issue Oct 26, 2012

radar added a commit that referenced this issue Oct 26, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment