elninotech / uppload

šŸ“ JavaScript image uploader and editor, no backend required
https://uppload.js.org
MIT License
1.84k stars 128 forks source link

Giphy images output as a jpg not gif #80

Open nicholasgriffinVF opened 4 years ago

nicholasgriffinVF commented 4 years ago

On upload v2, the giphy plugin seems to send the gif blob back as a jpg not a gif file so it doesn't come to the server as an animated file.

To Reproduce

Expected behavior The outputted type for giphy should be .gif.

AnandChowdhary commented 4 years ago

Thanks for opening this issue, @nicholasgriffinVF! I kind-of made a compromise which I think I should rethink now.

Essentially what's happening here is that the proxy we're using to download images from URLs (bypassing CORS) returns static images, not animated GIFs. Furthermore, our effects (like cropping, brightness, etc.) use the HTML5 Canvas API and also only support static output. This means that even if the GIFs were animated, after cropping or editing them, they'd stay on the first frame.

Perhaps we can:

  1. Remove GIPHY support altogether
  2. Make it clear in the GIPHY service page that only static images are uploaded
  3. Figure out a way to support animations in the proxy (weserv/images) or remove it altogether, and figure out how to export animated canvas blobs
nicholasgriffinVF commented 4 years ago

No problem, I'd be happy to take a look myself and see if I can help find a solution.

We did notice the weserv call yesterday and figured that had something to do with it.

I think if you're using one of the cropping tools then it's alright for them not to be GIFs as that's for sure not going to work, would be cool to be able to do uncropped animated GIFs though, definitely a feature I'm looking for so I'll definitely help take a look.

AnandChowdhary commented 4 years ago

As it turns out, version 5 of weserv/images does support animated WebP and GIF images. All we need to do it edit the canvas logic a bit and we should be able to upload animated GIFs with no trouble, albeit without editing.

More details: https://images.weserv.nl/news/2019/09/01/introducing-api-5/#support-for-animated-images

nicholasgriffintn commented 4 years ago

Awesome (commenting from my main account btw), got a fork going on now that I've got working with the new API query params, haven't got to the GIF output yet though unfortunately.

Also, not sure if the params.uppload.file.type is a thing, came out in console.log but doesn't seem to come through to the if, forcing a GIF for the Giphy service.

https://github.com/nicholasgriffintn/uppload/commit/fb69ef1386e5b08238c8c216e9720d24b4a0090f

Looks like the last part may be the compressImage function in elements.ts in terms of the type param.

I reckon joining the two together will also sort out the transparent PNG issue.

AnandChowdhary commented 4 years ago

Also, not sure if the params.uppload.file.type is a thing

Iā€™m not looking at the code, but if I remember correctly, itā€™s not a thing. file is where we keep the Blob containing the file to upload, so if instead of the blob itā€™s a native File (which might be case when using the Local file selector?) it might log properties like name and type that arenā€™t there on a regular Blob.

Looks like the last part may be the compressImage function in elements.ts in terms of the type param.

Ooh, and thereā€™s another problem ā€” compression only outputs JPG or WebP, I think. This is a limitation of the Canvas API with the quality option. Iā€™ll have a second look on whether weā€™ll need to disable compression for GIFs or is there another way.

Thanks for all your hard work!