Skip to content
This repository has been archived by the owner on Jan 16, 2023. It is now read-only.

Add support for animated WebP images #437

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Ytrewq13
Copy link

@Ytrewq13 Ytrewq13 commented Feb 2, 2021

sxiv doesn't support loading animated .webp images (since Imlib2 doesn't support it). I wrote a function img_load_webp() which mirrors the img_load_gif() function used to load animated gifs.

I had to also write a function to load the first frame of a webp file (I called it load_webp_firstframe_im, if you can think of a better name please rename it), because loading with Imlib2 fails and therefore the thumbnail was unable to load, deleting the image if thumbnail mode is entered. This caused a large amount of duplicated boilerplate for libWebP but I can't think of a better way to fix this issue.

I think I fixed all the memory leaks I had introduced with my changes. I removed all memory losses introduced in my test cases, but there is a chance that my tests were not extensive enough. There are still memory leaks but these existed before I changed anything, and they are small and don't seem to grow as the process remains active. This should probably be an issue, so I will create one if it isn't already.

Compiling the version with these changes requires all libraries that sxiv previously required and also libWebP (-lwebp and -lwebpdemux). I included #if HAVE_LIBWEBP ... #endif guards so sxiv can be compiled without webp support if desired (this can be toggled in the Makefile similar to HAVE_GIFLIB).

Created a function `img_load_webp()` which does the same thing as the
function `img_load_gif()`, but for webp images. Added a small amount of
code to make sxiv call this function when the file given is a webp image
file (using file extension to check at first, then doing proper tests in
the function).
@bakkeby
Copy link

bakkeby commented Apr 12, 2021

I initially thought this was about WebP support in general, but actually this is only specifically about animated WebP images.

If I test with still images from https://www.phoca.cz/phocacartdemo/webp-images-gallery then it looks like sxiv shows these just fine with Imlib2, but animated WebP images fail to load.

With this change animated WebP images show (and animate with the -a flag), but still images look to have problems with transparency. I tried to tinker and it sort of works if I add imlib_image_set_has_alpha(1); towards the end of the load_webp_firstframe_im function and this gives a checkered background at least. I may revisit later.

@GRFreire
Copy link

I know that muennich probably wont merge this since he is not particularly active, but I was thinking into merging in my own fork. But I found some problems and I'd like to share them here.

It is loading the animated webp images with no problem, but the thing is that when it tries to load some image with transparency, it just don't work properly.

On the left there is my fork of sxiv, which I think does not have changes related to webp when compared with this main fork. On the right there is your fork, which has the animated webp support.
before-after

@Ytrewq13
Copy link
Author

Thanks for bringing this to my attention. I'll admit that this pull request was mostly a learning exercise for WebP (the format) rather than for sxiv, so the implementation into sxiv was a little rushed.

I'll look into this as soon as I get some free time, but if you don't want to wait I would suspect that this issue relates to the TODO comment in image.c (line 390), so the first thing I would do is try changing things there and see if anything interesting happens.

@bakkeby
Copy link

bakkeby commented Aug 26, 2021

@Ytrewq13 I get this to display correctly with these changes.

diff --git a/image.c b/image.c
index 4e96d64..27e5583 100644
--- a/image.c
+++ b/image.c
@@ -372,7 +372,7 @@ bool img_load_webp(img_t* img, const fileinfo_t* file)
        flags = WebPDemuxGetI(demux, WEBP_FF_FORMAT_FLAGS);
        img->w = WebPDemuxGetI(demux, WEBP_FF_CANVAS_WIDTH);
        img->h = WebPDemuxGetI(demux, WEBP_FF_CANVAS_HEIGHT);
-       img->alpha = (flags & ALPHA_FLAG) > 0;
+       img->alpha = ALPHA_LAYER;
        img->multi.cap = info.frame_count;
        img->multi.sel = 0;
        img->multi.frames = emalloc(info.frame_count * sizeof(img_frame_t));
@@ -387,10 +387,7 @@ bool img_load_webp(img_t* img, const fileinfo_t* file)
                imlib_image_set_format("webp");
                // Get an iterator of this frame - used for frame info (duration, etc.)
                WebPDemuxGetFrame(demux, img->multi.cnt+1, &iter);
-               // TODO: iter refers to the sub-frame, not the total frame. im is the
-               // total frame, entirely reconstructed. How can we see if the total
-               // frame has transparency?
-               if (iter.has_alpha) imlib_image_set_has_alpha(1);
+               imlib_image_set_has_alpha((flags & ALPHA_FLAG) > 0);
                // Store info for this frame
                img->multi.frames[img->multi.cnt].im = im;
                delay = iter.duration > 0 ? iter.duration : DEF_WEBP_DELAY;
@@ -494,6 +491,7 @@ Imlib_Image load_webp_firstframe_im(const fileinfo_t *file)
                        info.canvas_width, info.canvas_height, (DATA32*)buf);
        imlib_context_set_image(im);
        imlib_image_set_format("webp");
+       imlib_image_set_has_alpha(1);

        WebPAnimDecoderDelete(dec);
        free((uint8_t*)data.bytes);

I don't really understand this all too well, but I think img->alpha in sxiv is just about whether to use a checkered background for the alpha layer or not, ref. config:

/* if true, use a checkerboard background for alpha layer,
 * toggled with 'A' key binding
 */
static const bool ALPHA_LAYER = false;

You can see this effect if you trigger the Shift+a keybinding.

As for the TODO, it is a plain guess on my part that perhaps we should set alpha based on the flag rather than the frame. I tried with a transparent animated webp image and it didn't seem to break anything at least.

Credit for this fix goes to @bakkeby on github.
@Ytrewq13
Copy link
Author

@bakkeby Your thinking on the TODO makes sense to me, I don't see any reason why different frames of an animated image should have different alpha statuses.

I'm going to apply these changes to my fork. Thanks for taking the time to fix this issue.

@kdkasad
Copy link

kdkasad commented Sep 7, 2021

You should add the new libwebp dependency to the README file

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants