cgifsave per frame cmap #2445
cgifsave per frame cmap #2445
Conversation
|
We could maybe set the first frame palette as the global colourmap, and only start using local cmaps if we find we need a new one. The most common case is probably all frames pretty similar, so we'd often get a single cmap per file, as before. |
|
Regarding local vs global cmaps, I see: So it's ~5% worse than master because of this cmap difference, and 8.11 is 1% worse again, perhaps because of LZW effort differences. |
Start using local cmaps only if we need to make a new palette
|
... I pushed something to use a global cmap for the start of processing, and only flip to local if we have to. The change detection isn't working very well, and it seems to leave transparent holes in some GIFs. I think this needs some work still :( This GIF ends up with holes:
|
|
Here's another test GIF that seems to produce incorrect output after processing: This is probably because the transparency index of the image is set incorrectly. |
|
Yes, cgif likes index 0 to be transparent, but I don't think libimagequant knows that. I'll have a look. |
and drop the change thresh to 1%
|
I fixed the issue with transparency and dropped the change threshold to 1%, it seems to work much better now. |
sorry, I keep changing this
|
I noticed a case where this PR could introduce noticeable artifacts. $ curl https://user-images.githubusercontent.com/12746591/134692488-e830184c-d801-4c29-9582-23786b1612f3.gif -o waves.gif
$ vips gaussblur waves.gif[n=-1] x.gif 10Details
Presumably this is due to the use of local palettes or the threshold of the change detector. |
|
Here's an example, the infamous 301 frame Before:
After:
(This page is now a good test for CPU fans.) |
|
I tried to improve the initialization of
/cc @dloebl (sorry for the ping) |
|
Wow, what a terrible GIF! I added a So it's still able to reuse the previous map 2/3rds of the time. Here's the new image:
|
Cool! I'll just run some benchmarks, then merge this.
Yes, I noticed that. I used the global config genflags field to hold frame gen options I didn't want to vary frame to frame.
Ah I hadn't seen that. I found the names a little confusing --- it's not clear from reading the header which define is part of which flag. Perhaps the genFlags should be called |
|
Benchmarks, after Kleis's fixes: So this branch is half the speed of master on this GIF, but uses less memory. As you suggested Lovell, perhaps we can use lower quality cmaps, since we are optimising per frame rather than for the whole file? The file size has really shot up, I'm not sure why. That 3x increase can't just be the extra cmaps. |
|
Another benchmark: This is a short video clip, so there's a new cmap per frame, but it's still quicker, I expect because the GIF write is done in the background. File size seems somewhat higher, as expected, |
|
I could fix the artifacts mentioned in comment #2445 (comment) with commit kleisauke@d10a879.
|
|
I wasn't sure why that fixed it, I didn't debug it further. It was found by accident, when I was looking at how gifski uses the libimagequant API. See for example commit ImageOptim/gifski@a3fb2ff. (It also uses the interesting |
Yes, Quick note on the size difference (local vs global color table): If local color tables are used, the width / height + transparency optimization are not performed for the affected frame(s) at the moment (Flags CGIF_FRAME_GEN_USE_TRANSPARENCY and CGIF_FRAME_GEN_USE_DIFF_WINDOW are ignored in this case). That might be the reason for the size difference. Adding support for the optimizations in the case of local color tables is already on my ToDo for cgif. However, it will increase the complexity of the code a bit. |
|
Ah yes, that sounds like it. I had a look through the generated GIFs, and with per-frame tables most frames (but not all, curiously) are: Which I suppose explains the size change and why experimenting with the transparency options did nothing. Also: thank you for this nice library, @dloebl. |
|
Are we thinking the change in this PR should be optional and, for now at least, not the default? |
|
Sorry, I posted this in gitter, but perhaps it's better here for reference. Here are a set of test GIFs plus a benchmark script and results for local-cmap, master and 8.11 (just for fun): http://www.rollthepotato.net/~john/test-gifs It's just 12 of the GIFs I had handy, but it's a mix of static images, video clips, and animated vector art with and without transparency. The test is making an animated thumbnail of each one. Memory behaviour:
master and local-cmaps are very similar, since we're downsizing to thumbnail size before doing the GIF save, so not keeping the whole GIF around makes little difference here. If the test was copying a large GIF, it'd look very different. Time behaviour:
local-cmaps is generally a bit quicker, though slightly slower in one case, perhaps 10% saving overall. Again, we're only writing a small GIF, so time savings in cgifsave don't have a huge effect. Perhaps copying a GIF should also be benchmarked. |
|
I added benchmarks for copy as well, but they are not very interesting.
|
And remove the redundant liq_image_add_fixed_color call, which should probably only be used in combination with liq_image_set_background.
Looking into this further (after reading the docs), it seems that the It looks like the issue with that test case is that the palette could be improved during remapping, so we must call Regarding |
|
Wow, I wouldn't have guessed that |
|
PR dloebl/cgif#23 would help here to reduce the file size, with that I see this (tested with the infamous $ /usr/bin/time -f %M:%e vips copy sample.gif[n=-1] x.per-frame-cmap.gif
107196:17.82
$ /usr/bin/time -f %M:%e vips copy sample.gif[n=-1] x.master.gif
1757196:11.42
$ ls -l sample.gif x.*
-rw-r--r--. 1 kleisauke kleisauke 3449741 Sep 26 11:00 sample.gif
-rw-r--r--. 1 kleisauke kleisauke 3355980 Sep 29 10:06 x.master.gif
-rw-r--r--. 1 kleisauke kleisauke 3401174 Sep 29 10:04 x.per-frame-cmap.gif |
|
Nice! Yes, that PR seems to mostly fix the size problem, and helps performance a little too. |
|
dloebl/cgif#23 has now been merged so I've updated the (temporary) cgif Ubuntu PPA and Homebrew tap to use the latest commit - see https://github.com/lovell/cgif-packaging |
|
Oh, that's great. I updated the benchmark above:
Summary: this branch keeps memory use low and constant, but save is up to 2x slower for some (not all) very large GIFs. |
|
I suppose my feeling is that the lower speed in some cases is unfortunate, but the much better memory behaviour is probably worth it. The lower speed is not that significant for thumbnailing since we only save small GIFs (the worst case is 2.36s vs 1.65s for Does anyone else have any thoughts? |
This patch makes the GIF a frame at a time, and only redoes the palette optimisation if the new frame is significantly different from the frame it last ran the optimisation on.
It detects differences by simply summing pixels and checking of the sum has changed by more than 20%. I've no idea if this is good enough, we'll need some testing.
Pros:
Cons:
Benchmark with master:
And with this branch:
@lovell any thoughts?
And the same test with 8.11, just to see how far we've come: