plugins/gimp: Add support for encoding JPEG XL animations #1874
Conversation
|
Could you rebase so it's easier to see what's new? |
fb8948e to
143b8ab
Compare
|
I took the liberty to make a squashed rebase, so that only the 4 files that have been changed show up in the review here... This contains exactly the same data as in fb8948e |
|
Thanks, wasn't sure how |
commit fb8948e Merge: 3f6ec60 1018c31 Author: Fred Brennan <copypaste@kittens.ph> Date: Fri Nov 18 04:56:44 2022 -0500 Merge branch 'gimp_animation_encode' of ssh://github.com/ctrlcctrlv/libjxl into origin commit 1018c31 Merge: 63cb827 404374c Author: Fredrick Brennan <copypaste@kittens.ph> Date: Wed Nov 16 07:17:52 2022 -0500 Merge branch 'main' into gimp_animation_encode commit 63cb827 Author: Fred Brennan <copypaste@kittens.ph> Date: Mon Nov 7 00:34:32 2022 -0500 plugins/gimp: Add support for encoding JPEG XL animations Closes libjxl#1864. commit 7b63f07 Author: Fred Brennan <copypaste@kittens.ph> Date: Sat Nov 5 19:25:40 2022 -0400 plugins/gimp: If last frame was blank, never combine commit 2a160ca Author: Fred Brennan <copypaste@kittens.ph> Date: Sat Nov 5 19:16:16 2022 -0400 plugins/gimp: Start layer index from 1 to match WebP/gif plugins commit 5a454e5 Author: Fred Brennan <copypaste@kittens.ph> Date: Sat Nov 5 19:15:36 2022 -0400 plugins/gimp: Omit animation frames of 0 duration. commit 185e151 Merge: b6f4bd8 ddda9b2 Author: Fred Brennan <copypaste@kittens.ph> Date: Sat Nov 5 19:01:23 2022 -0400 Merge branch 'gimp_animation_decode' commit b6f4bd8 Author: Fred Brennan <copypaste@kittens.ph> Date: Sat Nov 5 15:50:12 2022 -0400 plugins/gimp: Fix progressive decoding (closes libjxl#1845) commit ddda9b2 Merge: b81ff91 997069d Author: Fred Brennan <copypaste@kittens.ph> Date: Sat Nov 5 15:15:28 2022 -0400 Merge branch 'main' of https://github.com/libjxl/libjxl into gimp_animation_decode commit b81ff91 Author: Fred Brennan <copypaste@kittens.ph> Date: Sat Nov 5 13:01:34 2022 -0400 plugins/gimp: Fix maths fail commit 97a1ca9 Author: Fred Brennan <copypaste@kittens.ph> Date: Sat Nov 5 02:23:13 2022 -0400 plugins/gimp: Handle named frames commit 21adf35 Author: Fred Brennan <copypaste@kittens.ph> Date: Sat Nov 5 01:59:05 2022 -0400 plugins/gimp: Handle the two blend modes GIMP understands. commit 9f69640 Author: Moritz Firsching <firsching@google.com> Date: Fri Nov 4 15:54:07 2022 +0100 fix authors check commit afe8f72 Author: Moritz Firsching <firsching@google.com> Date: Fri Nov 4 15:52:58 2022 +0100 lint commit f2bc3ad Author: Fred Brennan <copypaste@kittens.ph> Date: Fri Nov 4 03:56:43 2022 -0400 plugins/gimp: Support for decoding animations
143b8ab to
cbafe73
Compare
|
rebasing once more, because #1903 just landed and that modifies the same files (this one was trivial to rebase, though..) |
|
Not necessarily for this merge request, but I think this should be extended to also allow saving/loading a layered JPEG XL image. It's basically the same thing, except it's not an animation, i.e. all layers have duration zero. It should give a warning/error if other GIMP blend modes are used than Normal blending (since jxl only supports normal blending plus some other blend modes that do not correspond to GIMP blend modes), since then the jxl will not render in the same way. (one possible workaround in such cases would be to have a layer name convention to store the GIMP blend mode and layer opacity, and to optionally add a redundant layer at the very top that is the merged image with kReplace, which would be the equivalent of the "Maximize compatibility" option in Photoshop) |
| GimpColorProfile *profile_icc = nullptr; | ||
| GimpColorProfile *profile_int = nullptr; | ||
| bool is_linear = false; | ||
| bool last_frame_blank = false; |
There was a problem hiding this comment.
What is the purpose of this variable? It seems to be never-read.
| } else { | ||
| if (frame_duration == 0) { | ||
| last_frame_blank = true; | ||
| continue; |
There was a problem hiding this comment.
Simply ignoring/skipping zero-duration frames is not correct, since in principle they can contain information.
I think it is better to keep them in, with duration set to zero in Gimp too, and change the Gimp animation playback function so it blends all consecutive zero duration frames before showing anything (which is the way this case is supposed to be handled). If changing the Gimp animation playback code is not feasible, then I suppose for now you could avoid poor playback by ignoring frame_duration == 0 frames if info.have_animation == true and the frame is all-transparent (or figure out why the jxl apng or gif loader is adding these dummy frames in the first place and see if it can be avoided).
In the case of layered still images, you would have only zero-duration frames and you should not skip them but add them as layers (with normal blending if blend_mode == JXL_BLEND_BLEND, and if another blend mode is used in jxl then it cannot be represented as a Gimp blend mode so if this happens probably the best thing to do is to start over with coalescing enabled and just load the image as a single-layer merged image).
| gchar** endptr = &remainder2; | ||
| gint64 ms_len = g_ascii_strtoll(*remainder + 2, endptr, 10); | ||
| if (ms_len == 0) { | ||
| ms_len = 1; |
There was a problem hiding this comment.
Why force frames to have a duration of at least 1 ms? JXL does support zero-duration frames and specifies that all consecutive zero-duration frames should be blended before showing a frame. While GIF/WebP do not support this (they have a minimum frame duration for historical reasons, which is longer than 1ms btw), JXL does and it seems useful to allow using this feature.
|
@jonsneyers I think we should move forward on this and even accept a version that can later be improved. Better to have slighly buggy animation support than none... |
|
I plan to rebase and resovle the conflicts after #2190 is merged... |
plugins/gimp: Add support for encoding JPEG XL animations
Closes #1864.
Because I use the same semantics as the GIMP's existing GIF and WebP plugins, one can open a WebP and then immediately save a JPEG XL. E.g.:
Note: This assumes the acceptance of my other PR's for the GIMP plugin, #1863 and #1870. This should be merged only when those are merged. It became too difficult to try to maintain separate histories given how intertwined some of my (necessary) changes are.