Replies: 24 comments
-
It's not there by default, but you can pass options to the underlying processor: ImageProcessing::Vips
.source(image)
.resize_to_limit(400, 400)
.saver(interlace: true) # passed to #jpegsave and #pngsave
.call ImageProcessing::MiniMagick
.source(image)
.resize_to_limit(400, 400)
.saver(interlace: "...") # adds -interlace (I'm not sure whether it should be "Line", "Plane", "JPEG", "PNG", "GIF" or something else)
.call
Yeah, sounds like for generating JPEG thumbnails interlacing should be the default (ImageOptim thinks so too). For PNGs we'd probably want to leave it off by default, because if I understood https://github.com/jcupitt/libvips/issues/140#issuecomment-47515390 correctly, generating progressive PNGs might negatively impact performance.
Yes, that was something that I had in mind. For ImageMagick stripping EXIF and other metadata is as simple as using Both ImageMagick and libvips have a "strip" option, but for ImageMagick that also strips color profiles, for libvips probably as well. At the moment I'm reluctant to add it as default behaviour, as some people might not be using it only for generating thumbnails, e.g. they might resize the original image and not keep the original, so they might be surprised to find out that EXIF and other data is stripped.
I'm not sure how to do that yet, but if that's a reasonable default we might add it to ImageProcessing. For now we can just highlight how to do that in the README. Maybe we could add a |
Beta Was this translation helpful? Give feedback.
-
It's not a default setting as such things are best left up to the dev's using the library. The ImageProcessing API is a common generic wrapper to provide a uniform interface to underlying image processors like ImageMagick and Libvps. Different apps have different needs (e.g. image archiving, photographers, etc where preservation of quality and metadata is key). I think linking such recommendations is a good idea. We would welcome a PR for consideration to such links that you feel will benefit dev's. |
Beta Was this translation helpful? Give feedback.
-
Right, whether default or not, I think a common generic API to exersize these options (whether in IM or vips), without having to provide back-end-specific directives, might be a good idea? Is my suggestion. |
Beta Was this translation helpful? Give feedback.
-
(PS: I actually work in digital preservation/archiving, and while we don't want to touch the originals, there is no reason I know of not to make the derivative/version thumbs optimized for the web). |
Beta Was this translation helpful? Give feedback.
-
It does not matter for JPEG files according to this answer: https://stackoverflow.com/a/36436945/1377358 |
Beta Was this translation helpful? Give feedback.
-
There is from the vips command-line, which makes it seem like there might be from libvips? See This kind of having to figure stuff out, though, is part of why I suggest that stripping metadata and progressive JPG should be part of the ImageProcessing API, figure it out once and put it in ImageProcessing, and then every consumer doens't have to figure it out -- and can have it keep working if they switch from IM to vips backends. |
Beta Was this translation helpful? Give feedback.
-
@janko-m if you decide to add strip as default, please consider the additional time cost if any and provide option to turn it off. When I implemented this a year ago using ImageProcessing with MiniMagick, I discovered stripping added another 200-300 ms to process time. My optimum recipe for version generation was:
|
Beta Was this translation helpful? Give feedback.
-
For stripping metadata both processors coincidentally already have the same API: ImageProcessing::MiniMagick
.saver(strip: true) # appends the `-strip` option
# ... ImageProcessing::Vips
.saver(strip: true) # passes the `:strip` saver option
# ... However, as discussed in https://github.com/janko-m/image_processing/issues/28, it might not be desirable to strip by default, as people might be relying on EXIF tags existing, as was the case with earlier versions of ImageProcessing. Stripping also removes color profiles, which if I understood correctly might make the image display with different colors in the browser? From the post that @jrochkind linked to it seems that it should be a good as long as images are converted to the sRGB colorspace (if they aren't already). Progressive JPEGs/PNGs are also easy to do, as mentioned previously. I agree with @jrochkind that we should at least document these common tips & tricks. I'm thinking we should add it as a wiki page, then link to it from the README.
@hmistry Wow, that's good to know. I'm not getting any performance difference locally, but it may be visible with images that have more data.
@renchap That's good to know, thanks! |
Beta Was this translation helpful? Give feedback.
-
If anyone would like to take a stab at documenting these tips, or think that some should be applied by default, please feel free to edit the wiki and/or send a PR. |
Beta Was this translation helpful? Give feedback.
-
So I believe Yes, this stuff is confusing. That it is confusing to figure out is one reason I still suggest image_processing should provide operations that do it, not just docs on how minimagick/image_magick/vips do it. I'm not sure why the discussion has become about what should be default. The issue as I opened it does not suggest anything about defaults. Nowhere I have suggested any of this stuff should be default. I have no real opinion on what should be default. It may indeed be reasonable for none of it to be default. What I am still suggesting is that these operations should be part of the image_processing api, that image_processing should provide these operations. Because they are often best practices, recommended by many sources. Of course you can pass the specific back-end-specific options to the specific back-end. But if we were just going to do that, why would we need image_processing at all? We could just use MiniMagick or ruby-vips or whatever. The reason this gem is valuable is because it encapsulates common operations so we don't have to go figure out how to do them. (And, as of recently, in a backend-independent way, which is awesome). The same reason we want a Of course, this is just my opinion, you can disagree, but please let's not talk about what should be default anymore. And I hope you at least understand my suggestion now, I'm sorry I've been having trouble making it clear. |
Beta Was this translation helpful? Give feedback.
-
Yeah, sorry, that was something that I unnecessarily added to the discussion. Since all of us are ok with this being opt-in, let's talk about the API. Ok, these are some of my comments on what we've mentioned so far:
We could add |
Beta Was this translation helpful? Give feedback.
-
You gotta supply your own sRGB.icc color profile in vips for whatever reason. There are several open source ones under licenses that would allow you to package them with whatever you want, not sure why libvips chooses not to package one, maybe it doesn't want to decide for you which one to use. (I think under normal/default/non-expert use, any of them are just fine). https://github.com/jcupitt/libvips/issues/776 This stuff is totally crazy confusing to figure out! Which is why figuring it out in image_processing for everyone else would be so useful heh. |
Beta Was this translation helpful? Give feedback.
-
I think this is a good idea to provide built-in "profiles" (the first one being web thumbnail) in this gem, as image processing (the field, not the gem 🙂 ) is complex and not easy to grasp. Something to keep in mind is that the options will depend on the output image format. You dont want a progressive PNG for perf reasons, quality does not have the same meaning, … |
Beta Was this translation helpful? Give feedback.
-
It seems that we should probably use this profile for sRGB conversion.
You can say that again. I've never even heard of "image profiles" until I started reading other libvips wrappers.
Interesting! Yeah, we should definitely try to do the right thing. |
Beta Was this translation helpful? Give feedback.
-
yep, that's the profile I ended up using myself too, since it it is after all direct from the International Color Consortium, and is licensed for unrestricted use and redistribution. It's not the one jcupitt used/recommended for whatever reasons. I think really just about any will do (if someone is enough of a digital image wizard to know enough to have a preference, they can do it manually :) ). |
Beta Was this translation helpful? Give feedback.
-
I'm thinking about this again -- in your original directions for ["It's not there by default, but you can pass options to the underlying processor"] (https://github.com/janko-m/image_processing/issues/34#issuecomment-380170221) -- would there be any way to use those commands with ActiveStorage? If so, an example would be awesome. If not, that seems a lot of motivation for figuring out a way to bake it in as explicit built-in API to image_processing, so it can be used with ActiveStorage. (If there's no way to use ActiveStorage to get standard web best practices like interlaced JPG, dropped profiles, standardized color profiles... that would seem to be a rather bad mark against ActiveStorage, which in a sense is ActiveStorage's problem not yours.... but AS is likely to kick it back as "sure, you can do that, as soon as image_processing supports it" heh). |
Beta Was this translation helpful? Give feedback.
-
To my knowledge, the current ImageProcessing API should already support specifying options necessary for generating images according to web best practices. ImageProcessing can do anything that MiniMagick/ruby-vips can do, and if there is something they can't do, that should be fixed in those gems. ActiveStorage photo.image.variant(saver: { interlace: true }, ...) I don't think there is anything that needs to be done on the ImageProcessing side, in other words: "innocent until proven guilty" 😉 I also don't have the bandwidth and the knowledge to determine what are the important best practices for web images, and then how to exactly translate them into MiniMagick/ruby-vips settings. So I would be grateful to anyone who figures all of that out 😃 |
Beta Was this translation helpful? Give feedback.
-
Hello, I just stumbled across this interesting issue. That libvips blogpost about thumbnailing has been updated and worked up into a chapter in the main docs: https://libvips.github.io/libvips/API/current/Using-vipsthumbnail.md.html tldr: it recommends:
Or in Ruby: thumb = Vips::Image.thumbnail "filename-of-image", 128, eprofile: "filename-of-srgb-profile", :rotate true
thumb.jpegsave "filename-of-thumbnail", :optimize_coding: true, strip: true You can use any sRGB profile, that's just an example. They do vary more than you'd expect, so test the one you pick. libvips 8.8 (due RSN) will bundle this one (free, high-quality): https://github.com/libvips/libvips/blob/master/libvips/colour/profiles/sRGB.icm To use the bundled profiles, just set the filename to the profile name, so: thumb = Vips::Image.thumbnail "filename-of-image", 128, eprofile: "srgb", :rotate true You could add Please let me know if there's anything I can do to help! |
Beta Was this translation helpful? Give feedback.
-
Hello again, a quick update:. First (trivially), Secondly, P3 colourspace is becoming very widely used. This is significantly bigger than sRGB, and P3 images converted to sRGB will either be drastically clipped or desaturated. Either way, your users are likely to complain, especially the photographers. Rather than forcing everything to srgb, I think best practice now is to strip all metadata except the icc profile. There's a new image = image.mutate do |mutable_image|
mutable_image.get_fields.each { |field| mutable_image.remove! field unless field == "icc-profile-data"}
end To remove all metadata except the icc profile. A few users like to keep EXIF on thumbnails, so metadata stripping should probably be optional. |
Beta Was this translation helpful? Give feedback.
-
@jcupitt so useful to have your advice! Earlier you linked to https://jcupitt.github.io/libvips/API/current/Using-vipsthumbnail.md.html, which it looks like is no longer there. Is there an updated docs page with your new advice about (not) sRGB? |
Beta Was this translation helpful? Give feedback.
-
Hi @jrochkind, it's here now: https://www.libvips.org/API/current/Using-vipsthumbnail.md.html Though I've not got around to updating the colourspace advice, unfortunately. |
Beta Was this translation helpful? Give feedback.
-
OK thanks @jcupitt . Not relevant to this PR, but I'm trying to figure out if there's a way to "strip all metadata except the icc profile" using vips CLI. I think there may not be? If this is the current recommended best practice, perhaps there should be a way to do it with CLI too? But I'll stop talking about it here, not relevant to image_processing which does not use CLI! |
Beta Was this translation helpful? Give feedback.
-
Unclear if I should use
|
Beta Was this translation helpful? Give feedback.
-
I'm going to convert this into a discussion. I'd welcome any guides for progressive JPEGs, the wiki is publicly editable 😉 |
Beta Was this translation helpful? Give feedback.
-
Is making sure the JPG output is a progressive JPG part of the ImageProcessing API?
It makes so much sense to make sure your thumbnails are progressive JPGs. They are often (for mathematical reasons I don't understand) smaller in file size, but more importantly they load in a way better for UX under a slow connection.
Another, although less important, thing that makes a lot of sense for web thumb JPGs, is stripping EXIF metadata, and making sure they are translated to color-profile sRGB.
These are things that all the guides for creating JPG thumbs on the web recommend (including Google's), but I find often left out or not highlighted in documentation in ruby easy-thumbnail-gen libraries.
Beta Was this translation helpful? Give feedback.
All reactions