Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for transparency #11

Closed
wants to merge 4 commits into from
Closed

Add support for transparency #11

wants to merge 4 commits into from

Conversation

G4Vi
Copy link
Contributor

@G4Vi G4Vi commented May 17, 2021

Adds a transparent_index parameter to ge_new_gif that controls what color in the palette to make transparent if any. Pass -1 if no transparency is desired.

While transparency is implemented using the Graphics Control Extension block (already used for delay on every frame). For color consistency because there isn't a frame-local palette it was added to ge_new_gif instead of ge_add_frame.

@BuonOmo
Copy link

BuonOmo commented Aug 10, 2021

@G4Vi nice idea!

I've tested your branch and there is still an issue however (from my poor understanding of gif encoding).

The issue is that there is overlapping between images, as in this stackoverflow (python).

It looks like this can be solved with something that the pillow python library calls disposal. I've tried to look a bit deeper into that without success, I'm not fluent in C nor GIF unfortunately.

In the GIF spec there is a part about disposal! I think could be able to provide a patch to this PR, or another one.

      c. Syntax.

      7 6 5 4 3 2 1 0        Field Name                    Type
     +---------------+
  0  |               |       Extension Introducer          Byte
     +---------------+
  1  |               |       Graphic Control Label         Byte
     +---------------+

     +---------------+
  0  |               |       Block Size                    Byte
     +---------------+
  1  |     |     | | |       <Packed Fields>               See below
     +---------------+
  2  |               |       Delay Time                    Unsigned
     +-             -+
  3  |               |
     +---------------+
  4  |               |       Transparent Color Index       Byte
     +---------------+

     +---------------+
  0  |               |       Block Terminator              Byte
     +---------------+


      <Packed Fields>  =     Reserved                      3 Bits
                             Disposal Method               3 Bits
                             User Input Flag               1 Bit
                             Transparent Color Flag        1 Bit

...

            iv) Disposal Method - Indicates the way in which the graphic is to
            be treated after being displayed.

            Values :    0 -   No disposal specified. The decoder is
                              not required to take any action.
                        1 -   Do not dispose. The graphic is to be left
                              in place.
                        2 -   Restore to background color. The area used by the
                              graphic must be restored to the background color.
                        3 -   Restore to previous. The decoder is required to
                              restore the area overwritten by the graphic with
                              what was there prior to rendering the graphic.
                     4-7 -    To be defined.

EDIT: I have something ongoing, yet some pixels are flickering. The issue seems to be related to the disposal method itself.

To reproduce, just change the out last byte:

    uint8_t out[4] = {'!', 0xF9, 0x04, 0x08};

And here is an example of a flickering gif (before / after):

game_of_life

game_of_life

@G4Vi
Copy link
Contributor Author

G4Vi commented Aug 10, 2021

@BuonOmo

The encoding of the graphics control extension block is here:

gifenc/gifenc.c

Lines 279 to 285 in 001a81d

static void
add_graphics_control_extension(ge_GIF *gif, uint16_t d)
{
uint8_t out[4] = {'!', 0xF9, 0x04, 0x04};
if(gif->transparent_index != -1)
out[3] |= 0x1;
write(gif->fd, out, sizeof(out));

Regardless of whether transparency is used the disposal method is set to no disposal. When a transparent color is used, the value of <Packed Fields> == 0b00000101 == 0x05.

I didn't change the disposal method in my PR, it's in set_delay in upstream. I believe gifenc attempts to just store a delta between frames with get_bbox to save space. Changing the disposal method would disable this optimization.

Could you share an example using gifenc with this issue by any chance? I'm not sure why it wouldn't detect the changed pixels or encode it properly?

@BuonOmo
Copy link

BuonOmo commented Aug 10, 2021

@G4Vi

believe gifenc attempts to just store a delta between frames with get_bbox to save space. Changing the disposal method would disable this optimization.

Ok I did not grasp that! I'll share my advancement below, however, it does certainly not take this bit into account.

Could you share an example using gifenc with this issue by any chance? I'm not sure why it wouldn't detect the changed pixels or encode it properly?

I've updated my answer, sadly, it was during github downtime 🙁 . Could you take a look above ?


Here's what I've added so far:

static void
add_graphics_control_extension(ge_GIF *gif, uint16_t delay_time)
{
    uint8_t packed_fields = 0;
    if (gif->transparent_index != -1) packed_fields |= 0b1;
    packed_fields |= (2 & 0b111) << 2; // TODO: gif->disposal rather than forcing 2

    write(gif->fd, (uint8_t[]){
        0x21, // Extension Introducer
        0xF9, // Graphic Control Label
        0x04, // Block Size
        packed_fields
    }, sizeof(uint8_t[4]));

    write_num(gif->fd, delay_time);
    write(gif->fd, (uint8_t[]){
        gif->transparent_index == -1 ? 0 : gif->transparent_index,
        0 // Block Terminator
    }, sizeof(uint8_t[2]));
}

@G4Vi
Copy link
Contributor Author

G4Vi commented Aug 10, 2021

@BuonOmo

Here's what I've added so far:

    (2 & 0b111)

I'm not sure why this is written instead of just 0x2 or 2. Your code does still set the disposal method to 2 (restore background color).

I'm not sure exactly what the before and after gif is showing. I might need to use actual gif software to look at them, but it appears before isn't transparent and after is. Converting to png before extracted to 96 frames and after extracted to 97 frames. The flicker in after is definitely there as the first frame of after extracted to

after_001

while it's second frame looks like

after_002

EDIT: Both those gifs appear to not be made by gifenc, before.gif doesn't even contain the graphics control extension. after.gif only has the delay set on the first frame. Github might be re-encoding them, maybe upload them in a zip file instead?

@BuonOmo
Copy link

BuonOmo commented Aug 10, 2021

@G4Vi to make sure we're talking about the same things here's a reproduction, with a way to set disposal (which is crucial for transparency imho):

#include <stdio.h>
#include "lib/gifenc.h"

int main(int argc, char const *argv[])
{
	if (argc != 3) {
		fprintf(stderr, "usage: %s <file.gif> <disposal>\n", argv[0]);
		return 1;
	}

	size_t i, j;
	size_t height = 100;
	size_t width = 100;
	ge_GIF* gif;
	gif = ge_new_gif(
		argv[1], width, height,
		(uint8_t []) { /* R, G, B */
			0xff, 0xff, 0xff,
			0xda, 0x09, 0xff
		},
		1,
		0, /* infinite loop */
		0 /* transparent background */
	);

	for (i = 0; i < 100; i++)
		for (j = 0; j < 100; j++)
			gif->frame[i*width + j] = i > 10 && i < 90 && j > 10 && j < 50;
	/*
	 0 or 1, the line does not shrink
	 2 or 3, absolute madness
	 */
	ge_set_disposal(gif, (uint8_t)(argv[2][0] - '0'));
	ge_add_frame(gif, 5);
	for (i = 50; i > 0; i--) {
		for (j = 60; j < 65; j++)
			gif->frame[i*width + j] = 1;
		ge_add_frame(gif, 5);
	}
	for (i = 0; i < 51; i++) {
		for (j = 60; j < 65; j++)
			gif->frame[i*width + j] = 0;
		ge_add_frame(gif, 5);
	}

	ge_close_gif(gif);
	return 0;
}

And here's a patch of the set_disposal way:

diff --git a/gifenc.c b/gifenc.c
index 795bbf4..e3c77e4 100644
--- a/gifenc.c
+++ b/gifenc.c
@@ -276,19 +276,29 @@ get_bbox(ge_GIF *gif, uint16_t *w, uint16_t *h, uint16_t *x, uint16_t *y)
     }
 }
 
+void ge_set_disposal(ge_GIF *gif, uint8_t disposal) {
+    gif->disposal = disposal & 0b111;
+}
+
 static void
-add_graphics_control_extension(ge_GIF *gif, uint16_t d)
+add_graphics_control_extension(ge_GIF *gif, uint16_t delay_time)
 {
-    uint8_t out[4] = {'!', 0xF9, 0x04, 0x04};
-    if(gif->transparent_index != -1)
-        out[3] |= 0x1;
-    write(gif->fd, out, sizeof(out));
-    write_num(gif->fd, d);
-    out[0] = 0;
-    out[1] = 0;
-    if(gif->transparent_index != -1)
-        out[0] = gif->transparent_index;
-    write(gif->fd, out, 2);
+    uint8_t packed_fields = 0;
+    if (gif->transparent_index != -1) packed_fields |= 0b1;
+    packed_fields |= (gif->disposal & 0b111) << 2;
+
+    write(gif->fd, (uint8_t[]){
+        0x21, // Extension Introducer
+        0xF9, // Graphic Control Label
+        0x04, // Block Size
+        packed_fields
+    }, sizeof(uint8_t[4]));
+
+    write_num(gif->fd, delay_time);
+    write(gif->fd, (uint8_t[]){
+        gif->transparent_index == -1 ? 0 : gif->transparent_index,
+        0 // Block Terminator
+    }, sizeof(uint8_t[2]));
 }
 
 void
diff --git a/gifenc.h b/gifenc.h
index e1e6a11..a7c6805 100644
--- a/gifenc.h
+++ b/gifenc.h
@@ -14,6 +14,7 @@ typedef struct ge_GIF {
     int fd;
     int offset;
     int nframes;
+    uint8_t disposal;
     uint8_t *frame, *back;
     uint32_t partial;
     uint8_t buffer[0xFF];
@@ -26,6 +27,24 @@ ge_GIF *ge_new_gif(
 void ge_add_frame(ge_GIF *gif, uint16_t delay);
 void ge_close_gif(ge_GIF* gif);
 
+/*
+ * Indicates the way in which the graphic is to
+ * be treated after being displayed.
+ *
+ * Values:
+ *
+ *     0 -   No disposal specified. The decoder is
+ *           not required to take any action.
+ *     1 -   Do not dispose. The graphic is to be left
+ *           in place.
+ *     2 -   Restore to background color. The area used by the
+ *           graphic must be restored to the background color.
+ *     3 -   Restore to previous. The decoder is required to
+ *           restore the area overwritten by the graphic with
+ *           what was there prior to rendering the graphic.
+ *   4-7 -   To be defined. */
+void ge_set_disposal(ge_GIF *gif, uint8_t disposal);
+
 #ifdef __cplusplus
 }
 #endif

And the results (using gcc gifbug.c lib/gifenc.c -o g;for i in {0..3}; ./g file$i.gif $i):

0 1 2 3
file0 file1 file2 file3

I'm sorry if my english hinted in the wrong direction, the first frame flickering is solely due to my usage of gifenc, not a bug within the lib. This example should be clearer though!

I'm not sure why this is written instead of just 0x2 or 2. Your code does still set the disposal method to 2 (restore background color).

Disposal is encoded on three bits and 0b111 forces that while being quite clear IMHO!

Fix attempt

EDIT: with your hints of get_bbox usage, I think that I found a fix. Of course this would slow down gif generation, however: it works for my first case! Not entirely for the reproduction though..

static int
get_bbox(ge_GIF *gif, uint16_t *w, uint16_t *h, uint16_t *x, uint16_t *y)
{
    int i, j, k;
    int left, right, top, bottom;
    left = gif->w; right = 0;
    top = gif->h; bottom = 0;
    k = 0;
    if (gif->disposal > 1) { // <= the fix: if disposal care about the whole frame, render it all
        *w = gif->w;
        *h = gif->h;
        *x = 0;
        *y = 0;
        return 1;
    }
    for (i = 0; i < gif->h; i++) {
        for (j = 0; j < gif->w; j++, k++) {
            if (gif->frame[k] != gif->back[k]) {
                if (j < left)   left    = j;
                if (j > right)  right   = j;
                if (i < top)    top     = i;
                if (i > bottom) bottom  = i;
            }
        }
    }
    if (left != gif->w && top != gif->h) {
        *x = left; *y = top;
        *w = right - left + 1;
        *h = bottom - top + 1;
        return 1;
    } else {
        return 0;
    }
}

Another reproduction script, which rewrites frame every time

#include <stdio.h>
#include "lib/gifenc.h"

#define MIN(a,b) (((a)<(b))?(a):(b))

int main(int argc, char const *argv[])
{
	if (argc != 3) {
		fprintf(stderr, "usage: %s <file.gif> <disposal>\n", argv[0]);
		return 1;
	}

	size_t i, j;
	ge_GIF* gif;
	gif = ge_new_gif(
		argv[1], 100, 100,
		(uint8_t []) { /* R, G, B */
			0xff, 0xff, 0xff,
			0xda, 0x09, 0xff
		},
		1,
		0, /* infinite loop */
		0 /* transparent background */
	);

	/*
	 0 or 1, the line does not change
	 2 or 3, absolute madness
	 */
	ge_set_disposal(gif, (uint8_t)(argv[2][0] - '0'));
	for (size_t t = 0; t < 100; t++)
	{
		for (i = 0; i < 100; i++)
			for (j = 0; j < 100; j++)
				gif->frame[i*100 + j] = i > 10 && i < 90 && j > 10 && j < 50;
		for (i = 50; i > 0; i--)
			for (j = 60; j < 65; j++)
				gif->frame[i*100 + j] = i > MIN(t, 100 - t);
		ge_add_frame(gif, 5);
	}

	ge_close_gif(gif);
	return 0;
}
0 1 2 3
file0 file1 file2 file3

@G4Vi
Copy link
Contributor Author

G4Vi commented Aug 11, 2021

@BuonOmo Thanks for sharing some examples.

Disposal is encoded on three bits and 0b111 forces that while being quite clear IMHO!

That makes sense, if I was trying to clamp the values for disposal I'd probably use the equivalent of 0b11 as the only valid values right now is from 0-3.

I tested your first example with logging the frames to a log file.

static int frameindex = 0;
void add_frame(ge_GIF* gif, uint16_t delay)
{
    int height = 100;
	int width  = 100;

	printf("frame %03d: \n", frameindex++);
	for(int i =0; i < height; i++)
	{
		for(int j =0; j < width; j++)
		{
			printf("%c", gif->frame[i*width + j] ? 'Z' : ' ');
		}
		printf("\n");
	}

	printf("\n");
	ge_add_frame(gif, delay);
}

I tested with this branch of gifenc and with disabling the bbox optimization in ge_add_frame like this (still with no disposal (1), but makes it encode the entire frame every-time (should be the same thing as your modification in get_bbox):

//} else if (!get_bbox(gif, &w, &h, &x, &y)) {
//    /* image's not changed; save one pixel just to add delay */
//    w = h = 1;
//    x = y = 0;
//}
} else {
w = gif->w;
h = gif->h;
x = y = 0;
}

log: https://gist.github.com/G4Vi/7a364bd79960e278b11a16ef00bc6531

I noticed some of your input frames appear not be complete frames, alternating between (only containing the big rectangle or only containing the varying size right bar), that definitely doesn't seem correct for no disposal, with no disposal you are supposed to be able to pass only the whole frames to gifenc and it optimizes it now.

I don't think setting disposal method is crucial for transparency (unless there's a bug in this implementation), but it would be a nice feature to add. I'd like to figure out how to encode your animation with the existing gifenc before jumping to adding a new feature.

I'm about to check out your second example, but could you please share exactly what your test animation is supposed to look like? Thanks again making a more minimal images to help debug this.

@BuonOmo
Copy link

BuonOmo commented Aug 11, 2021

I'm sorry I'm just getting into IRL issues hence I will take more time to answer clearly (at least a week I guess...). In the meantime:

I'm about to check out your second example, but could you please share exactly what your test animation is supposed to look like? Thanks again making a more minimal images to help debug this.

Thank you! The square on the left is supposed to not be moving, the line on the right is suppose to shrink and grow back. Here is a representation without transparency, which makes it clearer (i just change transparency to -1 in the last example I shared):

file0


I'll take time to add a more detailed answer later, sorry for the inconvenience

@G4Vi
Copy link
Contributor Author

G4Vi commented Aug 11, 2021

@BuonOmo

I was able to replicate the issue with the second example.

I finally understand why a different disposal method is needed, when you layering on a transparent pixel is not the same as replacing it with nothing. I'm not sure how this wasn't caught in my previous testing.

It appears restore to background dispose is needed when making a attempting to make a non-transparent pixel transparent.
https://engineering.giphy.com/modifying-ffmpeg-to-support-transparent-gifs/

@G4Vi
Copy link
Contributor Author

G4Vi commented Aug 11, 2021

@BuonOmo

No rush on this, but can you try the version linked below? The example is in example_transparency.c The example and gifenc changes are very similar to what you had.

I'm not sure what you meant by "Not entirely for the reproduction though.", this is inefficient (encoding the whole frame for every frame), but I had no issues getting the test animation working.

example_transparency

G4Vi@758955a

@G4Vi
Copy link
Contributor Author

G4Vi commented Aug 12, 2021

Also please try G4Vi@e0f4c56

This version uses both DND (Do not dispose) and RTB (Restore to Background) to produce optimized transparent animations.

etb

31678 -> 11865 bytes

https://movableink.github.io/gif-inspector/ is a really good tool for inspecting individual frames of a gif.

@BuonOmo
Copy link

BuonOmo commented Aug 16, 2021

@G4Vi

Hi! I've tried your last branch with my first gif, it works like a charm!
game_of_life

IMHO, this is enough for having simple transparency support. And we don't need for a user to set the DM himself, keeping a simple interface.

I'd say LGTM, yet @lecram is the maintainer 🙂

On optimisation, on my computer this one is 540k, and the image before that PR, Without transparency is 580k, hence it is a win!

Moreover, if i post it to any gif compressor tool, I do not get anything better than what is achieved there.

Thank you for digging in this issue. I will be able to finish my article on game of life soon now 🙂

@G4Vi
Copy link
Contributor Author

G4Vi commented Aug 16, 2021

@BuonOmo

Glad it worked for you! I'll watch for that article 🙂 assuming it's on your blog.

I added the commits to the PR branch and fixed the style to match lecram's style.

@lecram
Copy link
Owner

lecram commented Aug 17, 2021

Hi @G4Vi and @BuonOmo,

Thanks for working on this issue!

The PR seems to work well, but in the interest of keeping the code small I decided to push a simpler solution.
With this change (0044b0e), GIFs with transparent background simply use RTB exclusively. The drawback is that RTB prevents proper size optimization. Some slight optimization is still done by encoding only the rectangular region containing all opaque pixels. As for GIFs without transparent background, everything works as before, including proper size optimization.

Note that the interface chosen is a bit different from the PR: the transparent_index parameter is inserted before loop, to keep color-related parameters together, and it's called bgindex because the GIF spec refers to this "color" as Background Color.

Let me know if that works for you.

@lecram
Copy link
Owner

lecram commented Aug 17, 2021

Another thing worth mentioning about the pushed change:

If bgindex >= 0, then gifenc uses a single frame buffer (as opposed to the double buffer used for non-transparent GIFs). This means two things:

  1. less memory usage;
  2. it's not necessary to redraw or copy the entire canvas between calls to ge_add_frame().

Some people might prefer this behavior and it can even be used for non-transparent GIFs too if desired (although this disables proper size optimization, as discussed above): just use a non-negative bgindex that is outside the color range, e.g. 256.

@G4Vi
Copy link
Contributor Author

G4Vi commented Aug 18, 2021

@lecram

Thanks for looking into adding support!

It mostly works for me, however I discovered it doesn't encode frames with transparency correctly when delay is set to 0. I added a PR to fix this: #12

Only encoding the opaque bounding box is a great simple optimization with the common case of transparency surrounding the rest of the image.

My only other comment is while you can set bgindex in the gif header, here in ge_new_gif

gifenc/gifenc.c

Line 123 in 0044b0e

write(gif->fd, (uint8_t []) {0xF0 | (depth-1), (uint8_t) bgindex, 0x00}, 3);

It's a little misleading as it's not used for anything, RTB always clears pixels instead of restoring to the background color on all modern implementations [ even when this index is different from the index in the graphics control extension, RTB still clears pixels instead of restoring to the gif's "background color"].

@lecram
Copy link
Owner

lecram commented Aug 18, 2021

it's a little misleading as it's not used for anything, RTB always clears pixels instead of restoring to the background color on all modern implementations [ even when this index is different from the index in the graphics control extension, RTB still clears pixels instead of restoring to the gif's "background color"].

According to the GIF89 specification, the GCE's Transparency Index is not meant for "transparency" in the modern sense, it's just a way to leave a pixel unchanged from the last frame. That's why we can't use DND for actual transparency. Transparency is not really supported by the spec. What is supported is the ability to Restore to Background Color (RTB), which AFAICT refers to LSD's Background Color Index. Modern decoders have to break the spec one way or another in order to support actual transparency. They do seem to use Transparency Index for it, but I decided to set Background Color Index as well, because 1) that's what the spec says AFAICT, 2) apparently it's not hurting anything and 3) someone's decoder might need it.

GIF89 spec on transparency:

23 (GCE) [this is per frame]
viii) Transparency Index - The Transparency Index is such that when encountered, the corresponding pixel of the display device is not modified and processing goes on to the next pixel.

GIF89 spec on background color:

18 (LSD) [this is file-wide]
vii) Background Color Index - Index into the Global Color Table for the Background Color. The Background Color is the color used for those pixels on the screen that are not covered by an image.

GIF89 spec on disposal method:

23 (GCE)
iv) Disposal Method [...]
Values : [...] 2 - Restore to background color. The area used by the graphic must be restored to the background color.

@G4Vi
Copy link
Contributor Author

G4Vi commented Aug 19, 2021

Thanks for the detailed explanation. That does seem like decoders should restore to the LSD background color index and it should set to the pixel to be clear if GCE's transparency index matches it on RTB. It's a mystery why the current decoders don't do that, but there's no reason they couldn't. A new spec based on how gif has been implemented would be cool, ImageMagick had some decent docs on that part of it.

Closing this PR as transparency has been added.

@G4Vi G4Vi closed this Aug 19, 2021
@BuonOmo
Copy link

BuonOmo commented Sep 26, 2021

@lecram @G4Vi

Sorry for the delay, I just wanted to thank you both for the work on transparency, avoiding having to work with image magick or some other huge library.

I've tested the new implementation, and it works perfectly, the image is a bit bigger though (+6%).

I've mentionned you in my article leading to discuss this PR :) (https://ulysse.md)

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

Successfully merging this pull request may close these issues.

None yet

3 participants