strapi / rfcs

RFCs for Strapi future changes
68 stars 33 forks source link

Add webp and avif conversion to strapi-plugin-upload #47

Open Antoine-lb opened 1 year ago

Antoine-lb commented 1 year ago

Link to markdown: https://github.com/Antoine-lb/rfcs/blob/main/rfcs/xxxx-template.md

strapi-cla commented 1 year ago

CLA assistant check
All committers have signed the CLA.

Antoine-lb commented 1 year ago

Do you have some resource one which conversions are supported to webp?

From the documentation (https://sharp.pixelplumbing.com/#formats):

How does sharp handle non compatible formats ?

From my small non exhaustive testing:

Important note: Converting a large .jpeg to a .png caused the image to go from 2.7MB to 32.2MB ⚠️ (whereas when converted to .webp it went to 1.4MB). The test was done in just 1 picture. I will do further testing on the side effects that converting can have.

if so we would need to have a mapping to maintain?

You are dealing with a noob here 😅, I didn’t get what you mean

Can it be done dynamically

I guess, technically nothing is stopping it. I would double check security-wise if it is safe (I don’t know much about security, but it looks like vulnerabilities could get through)

Let's say you want to only convert png to webp and not jpg for some reason

At a first look, this could also be done.


Antoine-lb commented 1 year ago

Personal opinion and motivations I’m building a product with Strapi where multiple users can upload images. Users and I don’t care about the image format as long as it looks good and that it loads properly. If Google tells me that I should used WebP, I have no good argument for not doing it. So:

I see this feature for people that don't want to think about images instead of people who want more granular control.

A more "batteries included" approach. Very opinionated, but is most likely the best option for you.

Question:

knulpi commented 1 year ago

I'm wondering if we should allow some more configurations for this. Let's say you want to only convert png to webp and not jpg for some reason 🤔

This would be valid then also for other image formats, right? So e.g. transforming an uploaded PNG into an JPG and WebP, or transforming an uploaded WebP into an AVIV (just a made up example). While I do not really see a use case...what do you think about a configurable array here? I would suggest something like:

conversions: [
  {
    from: ["PNG", "JPG"]
    to: ["WebP", "TIFF", "AVIV"]
    keepSourceFile: true // will keep (and resize (if applicable)) originally uploaded file
  },
  {
    from: ["*"],
    to: ["AVIV"],
    keepSourceFile: false // will delete and not resize (if applicable) the original file 
  }
]

Something that I see might be a request would be to make this dynamic so you can decide at runtime which conversions to run. Would be nice to think about that in the rfc

Looking at the current configuration mechanism of the upload plugin (being statically configured in the plugins.js) I do not really see why it should be dynamic here. I agree though, that it might be a nice feature for the future. What about taking the static configuration for a first shot and thinking about an admin-controlled configuration mechanism as outlook? (admin-controlled, since I share some of the concerns @Antoine-lb brought up...)

Antoine-lb commented 1 year ago

I didn't have time to work on this, but the main blocker right now is whether it is a good idea to change the format altogether.

What I mean by that is that these algorithms are clearly good when you go from a raw or uncompressed image to a compressed image. But when you are dealing with already compressed images, things can go wrong.

I did some testing with sharp (the lib used by Strapi) and when going from a reasonably sized jpeg  to webp, the image actually got bigger, whereas just optimizing the jpeg got me great results.

And when converting jpegs to pngs they almost always got bigger, which makes sense for jpeg and webp.

So basically just transforming all images to webp may not be the best idea in all cases.

I did very little test and research, and I know very little about image compression, so take this very lightly.

Current hypothesis:

If I'm totally honest, we lost interest in building this because now we are converting all images to webp manually ourselves. But I still believe this is a feature Strapi should consider, specially for the fact that sharp already allows it, so it does not require much work.

Pin me if you still want me to work on it.

strapi-bot commented 7 months ago

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/image-file-format-webm-not-allowed-in-media-library-upload/34346/2