[FFmpeg-devel] [PATCH] Animated GIF Support

[Vitaliy Sugrobov]

On 10/14/12, Clément Bœsch <ubitux at gmail.com> wrote:
> BTW, are you going to work on the encode? There is room for improvement
> there; I think it is intra-only at the moment (because the output gif are
> pretty huge). Also, you need some tricks to get a nice gif output (like
> doing things such as -vf ...,format=rgb8,format=rgb24 to get better
> colors).

Rather no than yes. Gif encoding is a story about dithering and optimal
color reduction strategies.. I'm not good in that.

> Anyway, following is a small review of your code, with a lot of style &
> nit stuff, sorry :)

Thank you for such detailed review:) I've fixed those little defects
regarding to style, comments, const pointers, vertical align and type
casts.

> > +/**
> > + * GIF format allows variable delay between frames without having
> > + * any notion of "frames per second". But since frames decoded with ffmpeg demuxer,
> > + * we have to adhere requirement of some fixed fps.
> > + */
> > +#define GIF_DEFAULT_RATE    25
> > +/**
> > + * Major web browsers display gifs at ~10-15fps when rate
> > + * is not explicitly set or have too low values. We assume default rate to be 10.
> > + * Default delay = 100hundredths of second / 10fps = 10hos per frame.
> > + */
> > +#define GIF_DEFAULT_DELAY   10
> 
> It really plays it faster in Firefox and Chromium for me: I'm trying with
> the following generated gif, and ffplay has a way slower playback:
> 
> ./ffmpeg -i ~/samples/big_buck_bunny_1080p_h264.mov -ss 45 -vf \
>         'scale=320:160,format=rgb8,format=rgb24' -r 20  -frames:v 50 -y bbb.gif

I tried this and got the same result. After some digging I fixed this.
There were two reasons for that behavior:
1) gif encoder produces wrong delay values in output: -r 20 results in
0.02 sec delay between frames which is 50 frames per second instead of
20! I found that libavformat/gif.c (gif muxer) uses some "jiffies"
instead of just converting timebase to delay values.
2) Firefox and Chromium slow down playback of fast gifs. It seems that
they set delay between frames to 10 hundredths of second (10fps) when
they found delay less than 2 (>50fps). I think this is a reasonable
approach: playing animation at the speed of 100 frames per second would
be resource consuming task. BTW: Internet Explorer has different delay
threshold, around 3 hundredths of second.

In previous version of this patch default threshold was 3, now I changed
it to 2. Of course, this value can be changed by -gif_min_delay option.

> > +/**
> > + * Converts gif time to pts where
> > + * t is integer time (in hundredths of second) and r is frame rate.
> > + * Note that result is rounded to the nearest integer.
> > + */
> > +#define GIF_TIME_TO_PTS(t,r) ((int)(((t) * (r) / 100.0f) + 0.5f))
> > +
> 
> Can't you just set the timebase correctly and let the FFmpeg internals
> handle the rescale?

You're right. Offloaded this task to ffmpeg internals.

> Overall comment: can't you avoid the image2 demuxer to be selected by
> default? IIRC a mjpeg stream is similar to this gif thing, and is playable
> directly;
> 
> try ./ffmpeg -f lavfi -i testsrc=d=5 out.mjpeg && ./ffplay out.mjpeg for
> instance.

Unfortunately I can't. ff_image2_demuxer has AVFMT_NOFILE flag and its
read_probe() called even before ffmpeg opened the file. It checks for
presence of ".gif" extension in the filename and returns some non-zero
score. FFmpeg satisfied with score greater than zero and do not bother
itself with reading file contents. Thus gif_read_probe() do not even
have a change to be invoked with input file named "<some_name>.gif".
However, gif file with no extension accepted well without -f gif option.

> Also, a bit unrelated, but I think the gif decoder should be moved to the
> bytestream2 API...

Now I think so too. However, I want to deal with current patch before
moving further.

On 10/13/12, Paul B Mahol <onemda at gmail.com> wrote:
>>> How will this work with seeking?
>>>
>>> This means that first frame is keyframe and any other frame with
>>> transparency is not.> I think that right place to handle this is not in
>>> decoder but from user.
>>>
>>> AFAIK mplayer should handle it and ffplay not.
>>
>> According to gif spec any
>> subsequent frame depends on all previous frames. And this is not only
>> because of transparency, the other reason is variable frame dimensions. In
>> more detail: gif header provides us initial image dimensions { width,
>> height }. Then, any subsequent frame specifies its { x, y, width, height }
>> characteristics within bounds of the canvas, which means than only portion
>> of the canvas is updated. Thus first frame might be considered as a
>> keyframe and any other frame with or without transparency is not a
>> keyframe. So there is no way to seek frame F without calculating preceding
>> frames A, B, C, D and E. 
>
>Then mark packets/frames as such in demuxer/decoder.

Done. Packets flagged with AV_PKT_FLAG_KEY in demuxer, frames marked
with AV_PICTURE_TYPE_(I/P) and .key_frame set to (1/0) in decoder.
Please correct me if I misunderstood.

Mindful of request to break the patch into two parts, I attached two
patch files to this single letter, so as not to break the discussion.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Animated-GIF-Support.patch
Type: text/x-patch
Size: 29481 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121018/e36da196/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-gif-decoder:-fixed-return-codes.patch
Type: text/x-patch
Size: 1480 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121018/e36da196/attachment-0001.bin>