Media: Add an opt-in filter to generate animated image sub-sizes - #80385
Media: Add an opt-in filter to generate animated image sub-sizes#80385adamsilverstein wants to merge 14 commits into
Conversation
Match WordPress core's server-side behavior, where both GD and Imagick flatten animated images when resizing and wp_calculate_image_srcset() keeps flattened sub-sizes and the animated full-size image from mixing. Loading all frames ([n=-1]) re-encoded a full animated GIF per uncropped sub-size, which took 16-47 seconds per size for a 769-frame GIF and produced sub-sizes larger than the original file (5.5MB medium from a 2.2MB source). Cropped sizes already flattened to the first frame, so behavior was inconsistent, and Media Library uploads taking the server path already produced static sub-sizes. See #80266.
… output mediabunny's default 2-second key frame cadence roughly doubles the output size for long GIF conversions (2.2MB vs 1.14MB for a 769-frame GIF) with no encode-time benefit. These looping, autoplaying GIF replacements don't need fine seek granularity. See #80266.
…size-performance # Conflicts: # packages/vips/CHANGELOG.md # packages/vips/src/index.ts # test/e2e/specs/editor/various/gif-to-video.spec.js
Add the wp_generate_animated_image_subsizes filter (boolean, default false). When a site opts in, uncropped sub-sizes of animated GIFs keep their animation instead of flattening to the first frame, resolving the long-standing request in https://core.trac.wordpress.org/ticket/28474 without depending on server-side Imagick availability. The flag travels the same path as image_strip_meta: REST API root index field -> block editor setting -> upload-media store -> vips worker, where it restores the pre-#80268 [n=-1] load path for uncropped resizes. When writing an animated GIF, gifsave is tuned (effort 2, interframe_maxerror 8, interpalette_maxerror 16), measured 4-8x faster than the defaults and avoiding sub-sizes larger than the original. See #80383.
|
Size Change: +264 B (0%) Total Size: 7.73 MB 📦 View Changed
|
|
Core backport of the server-side changes: WordPress/wordpress-develop#12572 (draft; Trac ticket to follow). |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Understood. If so, I would be happy if we could merge this at an early stage. Will this PR and the core backport PR make it in time for Beta2? Also, it would be good to add an explanation of the new filters introduced in this PR to the dev note for client-side media. |
|
Nit: can we rename the flag?
Maybe prefix with "generate" or so. |
If approved, I can definitely land this and the backport before Beta 2. |
…ubsizes-optin # Conflicts: # packages/upload-media/CHANGELOG.md # packages/video-conversion/CHANGELOG.md # packages/vips/CHANGELOG.md # packages/vips/README.md # packages/vips/src/index.ts # packages/vips/src/test/resize-image.ts # test/e2e/specs/editor/various/gif-to-video.spec.js
Prefix the REST index field and editor setting with "generate" so the boolean nature of the flag is obvious, matching the wp_generate_animated_image_subsizes filter it carries. Suggested in PR review.
Add the new opt-in filter to the client-side media how-to guide and architecture reference, and correct the stale claim that sub-sizes of animated images preserve all frames (first-frame-only is the default since #80268).
|
Renamed in f976f20: the REST index field is now Also merged trunk to resolve the conflicts, and documented the new filter in the client-side media docs (10a1b7c). |
The PR touches lib/compat/wordpress-7.1, so CI requires a backport changelog entry pointing at the wordpress-develop backport PR.
Honestly 10-20 seconds doesn't seem like that long, but still is a significant change in behavior (storage also increases) so probably not what we want to make the default? Curious what others think the default behavior should be? Regardless, making it filterable seems like a good idea! cc: @swissspidy & @andrewserong |
I added it to docs in this PR in 10a1b7c and will add to my make core post (as yet unpublished) if merged. |
@swissspidy always appreciate the naming support! Renamed in f976f20 |
For me, these sorts of questions mean that I think we should park this feature for 7.1 and give it more time to explore for 7.2+. Speaking for myself, I'm a little spread thin across 7.1 features that I'm trying to support, and I'd be keen for us to carefully contain scope here if we can. It also seems that in order for GIF sub-size processing to feel stable, we'll need more UI state in order to balance out the longer processing time (#80329). Taken together, this feels like a good well-scoped feature for 7.2 ("WP now preserves animated GIFs at all image sub-sizes"), whereas in the context of client-side media processing, this seems more the nice-to-have territory than must have. What do you all think? I'll just ping @annezazu for visibility on this one, too, as I want to make sure I'm helping out with the high priority features in the release. (If this is high priority, happy to help review of course). |
swissspidy
left a comment
There was a problem hiding this comment.
I'll defer the final call to someone else, but code-wise this LGTM. It's a low-risk opt-in filter for devs who really would like to have animated thumbnails and are aware of the trade-offs.
| * take the server-side path (e.g. some Media Library uploads) also still | ||
| * produce static sub-sizes, as core has no animated resize support. | ||
| * | ||
| * @since 23.7.0 |
There was a problem hiding this comment.
| * @since 23.7.0 | |
| * @since 23.6.0 |
Nit: I intend to backport this PR not only to 7.1 but also to Gutenberg 23.6.
|
Hey folks! Thanks for tagging me in this. I read through the issue and, zooming out across the release, I'd like this to be punted to 7.2. This is a conservative decision and it comes from not looking solely at just this isolated change but looking at the weight of all of the features across the release when combined against the collective capacity of our current active contributors. Put another way, we already have a lot of features that we will need to be ready to do bug fixes for during beta and I'm hesitant to continue adding to it. Looking at this in isolation, I can see how it's lower risk since it's a dev focused change but it's not zero risk and everything we add takes up review time.
Relatedly, client side media wasn't noted as an area to be "blessed" and I want to honor that original commitment when it was discussed previously. Otherwise, it becomes easy to start "moving the goal posts" late in the game and I don't believe in doing that, unless project leadership overrides something. |
That makes sense, thanks for deciding! We will wait until 7.2 to land this one. |
What?
Adds a developer opt-in filter,
wp_generate_animated_image_subsizes, that re-enables animated (multi-frame) sub-size generation for animated GIFs in the client-side media processing pipeline:Fixes #80383.
Also fixes the 12-year-old core request Trac #28474 - "WordPress destroys animation in animated GIF when it resizes", as proposed in comment:58.
Core backport: Trac #65656 / WordPress/wordpress-develop#12572.
Note
Stacked on #80268, which makes static first-frame sub-sizes the default. This PR adds the opt-in path back on top of it. Only the last commit is new; review the diff from
fix/animated-gif-subsize-performance.Why?
#80268 switched sub-sizes of animated GIFs to static first-frame images, matching what WordPress core has always done server-side, because full animated re-encodes were extremely expensive (~88 s combined for a 769-frame GIF, with sub-sizes larger than the original - see #80266).
But there is long-standing, sustained demand for resized GIFs that keep their animation: Trac #28474 has been open since 2014 and stalled server-side because GD cannot do it and Imagick is only available on a subset of hosts. Client-side processing sidesteps both constraints - the cost is paid once in the uploading user's browser, and wasm-vips is available regardless of host configuration. That makes animated sub-sizes reasonable as an explicit developer opt-in while keeping the fast, core-consistent static behavior as the default.
How?
The flag follows the exact same path as
image_strip_meta/image_max_bit_depth(#80218):lib/media/load.php):wp_generate_animated_image_subsizes(boolean, defaultfalse) is applied ingutenberg_media_processing_filter_rest_index()and exposed asanimated_image_subsizeson the REST API root index. The field is also added to the preload/entities field lists (lib/compat/wordpress-7.1/preload.php,packages/core-data/src/entities.js), which must match exactly.generateAnimatedImageSubsizessetting inuse-block-editor-settings.jsand forwarded byuse-media-upload-settings.js.resizeCropItemreads the setting from the@wordpress/upload-mediastore and passes it to the vips worker as a newpreserveAnimationoption onresizeImage()(options object from Client Side Media: Consolidate optional positional params into options objects in vips / upload-media #80328).preserveAnimationis set and the resize is uncropped,resizeImage()restores the pre-Client-side media: generate animated image sub-sizes from the first frame only, matching core #80268[n=-1]load path so all frames are decoded and re-encoded. Cropped sizes (e.g.thumbnail) always flatten to the first frame, matching the pre-existing behavior - per-frame smart-cropping is out of scope.Tuned
gifsavesettingsThe profiling in #80266 showed ~85% of the animated resize cost is GIF re-encoding (per-frame palette quantization), and that default
gifsavesettings cause the output-larger-than-input bloat. When writing an animated GIF, this PR applies the settings benchmarked there:effort: 2,interframe_maxerror: 8,interpalette_maxerror: 16- measured 4-8x faster and eliminates the size bloat.That turns the opt-in cost from ~88 s into roughly 10-20 s for a very large GIF - still too slow to be the default, but a reasonable trade for a site that has explicitly chosen animated sub-sizes.
Notes
wp_calculate_image_srcset()still never mixes the full-size GIF and its sub-sizes in onesrcset. With animated sub-sizes that guard becomes overly conservative but harmless; relaxing it is a server-side follow-up.Testing Instructions
add_filter( 'wp_generate_animated_image_subsizes', '__return_true' );medium/largesub-sizes (e.g. via/wp-json/wp/v2/media/<id>): they should be animated GIFs (multiple frames), whilethumbnail(cropped) remains a static first frame.Documentation
The client-side media docs (#75895) are updated alongside: the how-to guide gains an "Animated image sub-sizes" section for the new filter, and the architecture reference adds it to the filter table and REST index field list (plus corrects the now-stale note that sub-sizes preserve all frames).
Automated tests
packages/vips/src/test/resize-image.ts: newpreserveAnimationsuite -[n=-1]+ tuned gifsave for uncropped animated resizes, first-frame flattening for crops, no effect on still formats.phpunit/media/media-processing-test.php: REST index exposesgenerate_animated_image_subsizes(defaultfalse, honors the filter, hidden withoutupload_files).test/e2e/specs/editor/various/gif-to-video.spec.js: new test uploads an animated GIF with the filter enabled (via a new e2e test plugin) and asserts themediumsub-size keeps all frames.