WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 6 weeks ago

#28474 assigned defect (bug)

WordPress destroys animation in animated GIF when it resizes

Reported by: archon810 Owned by: markoheijnen
Milestone: Future Release Priority: normal
Severity: normal Version: 3.9.1
Component: Media Keywords: has-patch needs-testing
Focuses: Cc:

Description (last modified by SergeyBiryukov)

When uploading animated GIFs to WordPress and then resizing them, the end result is the resized versions lose all frames and become static. This is a bug and should be fixed to allow animated GIFs to be resized properly.

Attachments (3)

gif-as-featured-image.mp4 (649.1 KB) - added by ericlewis 3 years ago.
28474.diff (5.9 KB) - added by mikeschroder 2 years ago.
Rough First Pass. Partially works. This does not check for all necessary functions existing, and doesn't seem to always thumbnail correctly.
28474.2.diff (7.0 KB) - added by markoheijnen 2 years ago.

Download all attachments as: .zip

Change History (40)

#1 @SergeyBiryukov
4 years ago

  • Description modified (diff)
  • Summary changed from Wordpress destroys animation in animated GIF when it resizes to WordPress destroys animation in animated GIF when it resizes

Related: #22543

#2 @markoheijnen
4 years ago

If we decide to fix this, then I rather only do this for Imagick and not for GD. For GD it takes a lot of work to make it work and I don't think it make sense to do so.

Imagick: http://www.php.net/manual/en/imagick.coalesceimages.php.

#3 @archon810
4 years ago

I think that's fair but I also think a warning that resized animated gifs would become static should also be implemented if possible, for those without ImageMagick. Maybe even with a suggestion or a link to the fix (I. E. Installing ImageMagick).

#4 @mikeschroder
4 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

I'd love to see this fixed.

If someone can come up with a sane way to do it in GD as well, I'd see no problem including it. However, as far as I'm aware, this is currently something that's difficult to do (especially with low memory usage) with GD.

In the meantime, I think it's fine to do it in Imagick, but well-document the differences. This certainly wouldn't be the only improvement that a user gets when using Imagick.

#5 @archon810
4 years ago

Anyone care to tackle this one?

#6 @markoheijnen
4 years ago

I will look into this to see how we wan't to do this but you can assume it will not be tackled for GD.

#7 @archon810
4 years ago

As I use imagick, I (and many others) will be happy with that.

#8 @ericlewis
3 years ago

Without support for resizing gifs, this sort of featured post thumbnail can't happen: attachment:gif-as-featured-image.mp4

#9 @gabrielstuff
3 years ago

Hey there, I made several test using imagemagick in command line and it seems to handle it pretty easily. Currently by passing this limitation by checking the extension and calling manually the

coalesceImages 

instance.

What is the right way to contribute / make things move for that. Looks like GD could not do it natively and need extensive extension such as : https://github.com/coldume/imagecraft

Thanks

#11 @wcvendors
3 years ago

Bump? :)

#12 @archon810
3 years ago

Bump bump.

#13 @mikeschroder
2 years ago

  • Milestone changed from Future Release to 4.5
  • Owner set to mikeschroder
  • Status changed from new to assigned

Let's look at how feasible this is in 4.5.

#14 @markoheijnen
2 years ago

https://github.com/markoheijnen/Improved-image-editor/commit/0937865453b6a0f46403d7c385386a506c469484 was the start of my attempt more then a year ago. It did saw the frames but resizing didn't worked out.

This ticket was mentioned in Slack in #feature-respimg by mike. View the logs.


2 years ago

This ticket was mentioned in Slack in #core by joemcgill. View the logs.


2 years ago

This ticket was mentioned in Slack in #feature-respimg by mike. View the logs.


2 years ago

@mikeschroder
2 years ago

Rough First Pass. Partially works. This does not check for all necessary functions existing, and doesn't seem to always thumbnail correctly.

This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.


2 years ago

#19 @azaozz
2 years ago

As far as I see pretty much all animated GIFs don't need hard resizing (make smaller image files). They are at the right size already, not like 4000px X 6000px photos from a digital camera. They can be soft-resized by CSS or the tag width/height attributes.

At the same time we can get hard resizing working only in Imagick leaving about 20-30% of the users out.

In these terms best would be to not try to hard resize animated GIFs at all. Just make a thumbnail, keeping the animation if possible (like 28474.diff). We should also exclude animated GIFs from the image editor. Even thinking we can start adding a flag to image meta to easily exclude them when adding srcset, etc.

If we can't create animated thumbnail the users will be able to see that and insert the full-size image, then soft resize it when needed. We can also add something in screen help about that (no matter how few users look there).

#20 follow-up: @wcvendors
2 years ago

I would disagree with @azaozz -- If the gif animation is a 5MB file, resizing in CSS still forces a 5MB file to load. The entire issue here is that a CSS resize would still require the full bandwidth, and the point of the resize is to retain the GIF animation *and* resize it to the correct, optimized file size at the same time. azaozz's argument would mean we should just CSS resize jpg/png's too because you can and that's easier, which defeats the purpose entirely.

If the site doesn't have Imagick and leaves out 20-30% of the sites that dont have it enabled/installed, I would suggest replacing the option to resize gif's as grey'd out with a helper text that requires Imagick and they don't have it installed.

Cheers

#21 in reply to: ↑ 20 @azaozz
2 years ago

Replying to wcvendors:

If the gif animation is a 5MB file...

Yes, any animated GIF can be 5, 50 or even 100MB, it's a really bad format :) What I mean is that all animated GIFs are already at the right size for displaying on a web page, like 600px x 400px, etc. Not like photos from digital cameras or phones that are thousands of pixels. If you really want to resize 600x400px down, you would do a much better job resizing it before uploading.

I'm also a bit skeptical about a core feature that leaves out so many users. It just brings more confusion at the end of the day.

This ticket was mentioned in Slack in #core by joemcgill. View the logs.


2 years ago

@markoheijnen
2 years ago

#23 @markoheijnen
2 years ago

  • Keywords has-patch added; needs-patch removed

Added a new patch that should fix some if not all issues. I also added a method to check if it's an animation which I also added to GD for reasons azaozz mentioned found at http://stackoverflow.com/questions/280658/can-i-detect-animated-gifs-using-php-and-gd.

#24 @markoheijnen
2 years ago

Forgot to mention that I had to switch getImageIterations() to coalesceImages() to make it work. For the image I used it was returning 0 but coalesceImages() was returning 15

This ticket was mentioned in Slack in #core by joemcgill. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-images by markoheijnen. View the logs.


2 years ago

#27 @mikeschroder
2 years ago

  • Milestone changed from 4.5 to Future Release
  • Owner mikeschroder deleted

Per discussion in Slack, since implementation, or even whether this is the correct route forward, doesn't have consensus at this point, punting to future release.

If anyone is interested in tackling this, please feel free to dig in.

#28 @mikeschroder
2 years ago

  • Owner set to markoheijnen

#29 @gabrielstuff
18 months ago

As stated here (https://core.trac.wordpress.org/ticket/28474#comment:9) 22 months ago, I would be happy to help. I disagree with the idea of letting GIF in there own space. I really think Wordpress should be easy for everybody, even for people who upload 800x600 px GIF instead of 400x300 and who end up with frozen frame. It feels buggy. Every young people around upload gif and I always tend to explain to them to resize before upload but they won't. Even when a simple tool like giphy exists.

I was wondering why not using a library that abstract the use of GD and image magick and that would allow for more advanced image processing.

As for audio, Wordpress uses https://github.com/JamesHeinrich/getID3, why not using an abstraction librery such as https://github.com/kosinix/grafika/ or an other, to replace https://github.com/WordPress/WordPress/blob/0a349c964178343f7668abf065bb86fc114b8332/wp-includes/class-wp-image-editor-imagick.php and https://github.com/WordPress/WordPress/blob/dd6da701b286579819cd6aa518aa2d7018efd759/wp-includes/class-wp-image-editor-gd.php

Also if not possible why not just use the same method that @markoheijnen found and we confirmed 22 month ago ? (http://php.net/manual/fr/imagick.coalesceimages.php) It won't break anything to GD and will bring resized GIF for imagemagick compatible server.

Thanks !

#30 @archon810
11 months ago

Still no takers for this important ticket? :-/

#31 @gabrielstuff
11 months ago

I would still be happy to take it :) as mentioned earlier this year.

#32 @SergeyBiryukov
10 months ago

#41777 was marked as a duplicate.

#33 @archon810
6 months ago

@markoheijnen Will you be able to find some time to assist @gabrielstuff in fixing this?

#35 @mikeschroder
7 weeks ago

  • Keywords needs-testing added

@archon810 @gabrielstuff Thanks for the comments and feedback! Apologies for the latency in a reply.

First, as a quick note -- You definitely don't need to wait on a ticket owner if you're interested in helping out. Tickets being assigned to someone does not mean that a ticket isn't open for testing, feedback or patches.

I agree, the ticket is pretty stagnant, so it'd be great to have testing of the existing patch and/or to have alternate patches uploaded with updated or different approaches to the problem. It's not that it's not desired to fix the bug, but that the ticket doesn't have a complete and well-tested solution yet.

I see that you've provided a suggestion that we use an alternate library. Could you please walk through how that would be a better solution for this particular issue?

A solution that doesn't require swapping out/refactoring the image manipulation internals entirely is likely to be able to be merged more quickly, but I'm excited to see any approaches for fixing this bug.

#36 follow-up: @gabrielstuff
6 weeks ago

Hello @mikeschroder, I'm really happy to read you. What is the preferred way to propose a patch?

  • Should we first discuss about implementation and then put it in place ?
  • Should we develop a plugin, show it to you and then see how it lands in the core ?

Thanks for taking interest into this particular issue.

G

#37 in reply to: ↑ 36 @mikeschroder
6 weeks ago

Replying to gabrielstuff:

Hello @mikeschroder, I'm really happy to read you. What is the preferred way to propose a patch?

<snip>

I'd suggest proposing patches by submitting them to this ticket (there's an attach button at the top of the page). Then, we can discuss the approach(es), and iterate together.

If it's close to what you're thinking, it'd also be great to have testing (and refresh if necessary) of the existing patch, since it's been a while since it was initially added, to know how how far that is from a solution.

Note: See TracTickets for help on using tickets.