Gif transparency fix #3
Conversation
Support for transparencies in animated gifs requires modifying both libavcodec/gif.c and libavformat/gif.c because both the graphics control extension (handled by libavformat/gif.c) and the raw frame data (handled by libavcodec/gif.c) must be changed. This is because transparencies in GIF can be used both to create a transparent image, and to provide optimization. How transparencies are interpreted in a given frame is controlled by the “disposal method”, which must be set appropriately in the graphics control extension. The “in place” disposal method is used when transparency indicates optimization, and the “background” disposal method is used when transparency is intended to be preserved. In order to support both disposal methods, libavcodec/gif.c must signal to libavformat/gif.c which disposal method is required for every frame. This is done with a new side data type: AV_PKT_DATA_GIF_FRAME_DISPOSAL. This requires a change to avcodec.h Unfortunately, the addition of a new side data type causes some of the FATE tests to fail, so the fate tests are updated here as well.
CR-1266: Fix infinite loop bug on transparent optimization
Upstream merge 03 14 19 v2
and add some debugging around gif writing
| @@ -136,7 +136,7 @@ static void gif_crop_translucent(AVCodecContext *avctx, | |||
| while (*y_start < y_end) { | |||
| int is_trans = 1; | |||
| for (int i = 0; i < w; i++) { | |||
| if (buf[w * *y_start + i] != trans) { | |||
| if (buf[linesize * *y_start + i] != trans) { | |||
There was a problem hiding this comment.
(ignoring this file as it's already mostly been accepted in mainline)
| @@ -1291,6 +1290,8 @@ static void do_video_out(OutputFile *of, | |||
|
|
|||
| while (1) { | |||
| ret = avcodec_receive_packet(enc, &pkt); | |||
| if (pkt.duration == AV_NOPTS_VALUE || pkt.duration <= 0) | |||
| pkt.duration = av_rescale_q(in_picture->pkt_duration, ost->mux_timebase, enc->time_base); | |||
There was a problem hiding this comment.
[from IRL discussion] jagraff explained to me that this is probably not the 'right' way to copy the duration between frames (since it's probably better to figure out why the duration isn't copied over and fix that). So this is unlikely to be merged into mainline - but I think it's fine to use this for GIPHY's purposes.
| @@ -88,6 +88,8 @@ static int gif_get_delay(GIFContext *gif, AVPacket *prev, AVPacket *new) | |||
| gif->duration = av_clip_uint16(new->pts - prev->pts); | |||
| else if (!new && gif->last_delay >= 0) | |||
| gif->duration = gif->last_delay; | |||
| else | |||
| gif->duration = prev->duration; | |||
There was a problem hiding this comment.
I'm hesitant to use this for all of our transcoding. I think we should submit a patch to the maintainers and get their feedback to see if this messes up something we don't realize. @bejayoharen do you want to take charge of that since you're known to them?
|
Have we talked to the maintained about this? Is this on me :P? |
|
Did these ever get fixed in upstream? |
|
@MarsVard the cropping fixes in |
No description provided.