Re: [FFmpeg-devel] [PATCH] avformat/gifdec: cleanup

[Paul B Mahol <onemda@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 199 bytes --]

On 5/21/23, Paul B Mahol <onemda@gmail.com> wrote:
> Attached patches.
>
> This finally removes giant hacks in gif demuxer and allows using gif
> files via pipe reliably.
>

Hopefully final version.

[-- Attachment #2: 0001-avformat-demux-add-support-to-derive-timestamps-from.patch --]
[-- Type: text/x-patch, Size: 1081 bytes --]

From 4dee724ff112967b8aa2e8ca5a45ec2726a7f409 Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Sun, 21 May 2023 02:15:26 +0200
Subject: [PATCH 1/2] avformat/demux: add support to derive timestamps from
 packet durations for video

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavformat/demux.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libavformat/demux.c b/libavformat/demux.c
index dec02a1a6b..b3f563ccc7 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -1195,6 +1195,11 @@ static int parse_packet(AVFormatContext *s, AVPacket *pkt,
                                      st->time_base,
                                      AV_ROUND_DOWN);
             }
+        } else if ((s->iformat->flags & AVFMT_NOTIMESTAMPS) && st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
+            if (st->time_base.num > 0 && st->time_base.den > 0 &&
+                sti->parser->duration) {
+                out_pkt->duration = sti->parser->duration;
+            }
         }
 
         out_pkt->stream_index = st->index;
-- 
2.39.1


[-- Attachment #3: 0002-avformat-gifdec-switch-to-using-gif-parser.patch --]
[-- Type: text/x-patch, Size: 12822 bytes --]

From 899afe00bf969b8c47c6fb5afbde3a8f9be1536d Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Sat, 20 May 2023 14:13:27 +0200
Subject: [PATCH 2/2] avformat/gifdec: switch to using gif parser

Update fate test, more correct as last packet is not truncated.

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavcodec/gifdec.c      |  11 +-
 libavformat/gifdec.c     | 244 ++++++++++-----------------------------
 tests/ref/fate/gif-demux |   2 +-
 3 files changed, 70 insertions(+), 187 deletions(-)

diff --git a/libavcodec/gifdec.c b/libavcodec/gifdec.c
index 0835c5bdd0..f2ab783ef0 100644
--- a/libavcodec/gifdec.c
+++ b/libavcodec/gifdec.c
@@ -472,10 +472,6 @@ static int gif_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
 
     bytestream2_init(&s->gb, avpkt->data, avpkt->size);
 
-    s->frame->pts     = avpkt->pts;
-    s->frame->pkt_dts = avpkt->dts;
-    s->frame->duration = avpkt->duration;
-
     if (avpkt->size >= 6) {
         s->keyframe = memcmp(avpkt->data, gif87a_sig, 6) == 0 ||
                       memcmp(avpkt->data, gif89a_sig, 6) == 0;
@@ -522,6 +518,13 @@ static int gif_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
 
     if ((ret = av_frame_ref(rframe, s->frame)) < 0)
         return ret;
+    if (s->keyframe) {
+        rframe->pict_type = AV_PICTURE_TYPE_I;
+        rframe->flags |= AV_FRAME_FLAG_KEY;
+    } else {
+        rframe->pict_type = AV_PICTURE_TYPE_P;
+        rframe->flags &= ~AV_FRAME_FLAG_KEY;
+    }
     *got_frame = 1;
 
     return bytestream2_tell(&s->gb);
diff --git a/libavformat/gifdec.c b/libavformat/gifdec.c
index 1977f46e3a..11fcde36b7 100644
--- a/libavformat/gifdec.c
+++ b/libavformat/gifdec.c
@@ -28,9 +28,12 @@
 #include "libavutil/bprint.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/opt.h"
+#include "avio_internal.h"
 #include "internal.h"
 #include "libavcodec/gif.h"
 
+#define GIF_PACKET_SIZE 1024
+
 typedef struct GIFDemuxContext {
     const AVClass *class;
     /**
@@ -53,9 +56,6 @@ typedef struct GIFDemuxContext {
     int total_iter;
     int iter_count;
     int ignore_loop;
-
-    int nb_frames;
-    int last_duration;
 } GIFDemuxContext;
 
 /**
@@ -84,8 +84,8 @@ static int gif_probe(const AVProbeData *p)
 
 static int resync(AVIOContext *pb)
 {
-    int i;
-    for (i = 0; i < 6; i++) {
+    ffio_ensure_seekback(pb, 13);
+    for (int i = 0; i < 6; i++) {
         int b = avio_r8(pb);
         if (b != gif87a_sig[i] && b != gif89a_sig[i])
             i = -(b != 'G');
@@ -132,6 +132,9 @@ static int gif_read_header(AVFormatContext *s)
     if (!st)
         return AVERROR(ENOMEM);
 
+    if (!(pb->seekable & AVIO_SEEKABLE_NORMAL))
+        goto skip;
+
     if (flags & 0x80)
         avio_skip(pb, 3 * (1 << ((flags & 0x07) + 1)));
 
@@ -158,15 +161,37 @@ static int gif_read_header(AVFormatContext *s)
 
                     avio_skip(pb, 1);
                     delay = avio_rl16(pb);
-                    if (delay < gdc->min_delay)
-                        delay = gdc->default_delay;
-                    delay = FFMIN(delay, gdc->max_delay);
+                    delay = delay ? delay : gdc->default_delay;
                     duration += delay;
                     avio_skip(pb, 1);
                 } else {
                     avio_skip(pb, block_size);
                 }
                 gif_skip_subblocks(pb);
+            } else if (subtype == GIF_APP_EXT_LABEL) {
+                uint8_t data[256];
+                int sb_size;
+
+                sb_size = avio_r8(pb);
+                ret = avio_read(pb, data, sb_size);
+                if (ret < 0 || !sb_size)
+                    break;
+
+                if (sb_size == strlen(NETSCAPE_EXT_STR)) {
+                    sb_size = avio_r8(pb);
+                    ret = avio_read(pb, data, sb_size);
+                    if (ret < 0 || !sb_size)
+                        break;
+
+                    if (sb_size == 3 && data[0] == 1) {
+                        gdc->total_iter = AV_RL16(data+1);
+                        av_log(s, AV_LOG_DEBUG, "Loop count is %d\n", gdc->total_iter);
+
+                        if (gdc->total_iter == 0)
+                            gdc->total_iter = -1;
+                    }
+                }
+                gif_skip_subblocks(pb);
             } else {
                 gif_skip_subblocks(pb);
             }
@@ -183,203 +208,57 @@ static int gif_read_header(AVFormatContext *s)
         }
     }
 
+skip:
+    /* jump to start because gif decoder needs header data too */
+    if (avio_seek(pb, 0, SEEK_SET) != 0)
+        return AVERROR(EIO);
+
     /* GIF format operates with time in "hundredths of second",
      * therefore timebase is 1/100 */
     avpriv_set_pts_info(st, 64, 1, 100);
+    ffstream(st)->need_parsing = AVSTREAM_PARSE_FULL_RAW;
     st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
     st->codecpar->codec_id   = AV_CODEC_ID_GIF;
     st->codecpar->width      = width;
     st->codecpar->height     = height;
+    if (nb_frames > 1) {
+        av_reduce(&st->avg_frame_rate.num, &st->avg_frame_rate.den,
+                  100, duration / nb_frames, INT_MAX);
+    } else if (duration) {
+        st->avg_frame_rate   = (AVRational) { 100, duration };
+    }
     st->start_time           = 0;
     st->duration             = duration;
     st->nb_frames            = nb_frames;
-    if (n) {
-        st->codecpar->sample_aspect_ratio.num = n + 15;
-        st->codecpar->sample_aspect_ratio.den = 64;
-    }
-
-    /* jump to start because gif decoder needs header data too */
-    if (avio_seek(pb, 0, SEEK_SET) != 0)
-        return AVERROR(EIO);
+    if (n)
+        st->codecpar->sample_aspect_ratio = av_make_q(n + 15, 64);
 
     return 0;
 }
 
-static int gif_read_ext(AVFormatContext *s)
+static int gif_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
     GIFDemuxContext *gdc = s->priv_data;
     AVIOContext *pb = s->pb;
-    int sb_size, ext_label = avio_r8(pb);
     int ret;
 
-    if (ext_label == GIF_GCE_EXT_LABEL) {
-        if ((sb_size = avio_r8(pb)) < 4) {
-            av_log(s, AV_LOG_FATAL, "Graphic Control Extension block's size less than 4.\n");
-            return AVERROR_INVALIDDATA;
-        }
-
-        /* skip packed fields */
-        if ((ret = avio_skip(pb, 1)) < 0)
-            return ret;
-
-        gdc->delay = avio_rl16(pb);
-
-        if (gdc->delay < gdc->min_delay)
-            gdc->delay = gdc->default_delay;
-        gdc->delay = FFMIN(gdc->delay, gdc->max_delay);
-
-        /* skip the rest of the Graphic Control Extension block */
-        if ((ret = avio_skip(pb, sb_size - 3)) < 0 )
-            return ret;
-    } else if (ext_label == GIF_APP_EXT_LABEL) {
-        uint8_t data[256];
-
-        sb_size = avio_r8(pb);
-        ret = avio_read(pb, data, sb_size);
-        if (ret < 0 || !sb_size)
-            return ret;
-
-        if (sb_size == strlen(NETSCAPE_EXT_STR)) {
-            sb_size = avio_r8(pb);
-            ret = avio_read(pb, data, sb_size);
-            if (ret < 0 || !sb_size)
-                return ret;
-
-            if (sb_size == 3 && data[0] == 1) {
-                gdc->total_iter = AV_RL16(data+1);
-                av_log(s, AV_LOG_DEBUG, "Loop count is %d\n", gdc->total_iter);
-
-                if (gdc->total_iter == 0)
-                    gdc->total_iter = -1;
-            }
-        }
+    if ((pb->seekable & AVIO_SEEKABLE_NORMAL) &&
+        !gdc->ignore_loop && avio_feof(pb) &&
+        (gdc->total_iter < 0 || (++gdc->iter_count < gdc->total_iter))) {
+        avio_seek(pb, 0, SEEK_SET);
     }
-
-    if ((ret = gif_skip_subblocks(pb)) < 0)
+    if ((ret = av_new_packet(pkt, GIF_PACKET_SIZE)) < 0)
         return ret;
 
-    return 0;
-}
-
-static int gif_read_packet(AVFormatContext *s, AVPacket *pkt)
-{
-    GIFDemuxContext *gdc = s->priv_data;
-    AVIOContext *pb = s->pb;
-    int packed_fields, block_label, ct_size,
-        keyframe, frame_parsed = 0, ret;
-    int64_t frame_start = avio_tell(pb), frame_end;
-    unsigned char buf[6];
-
-    if ((ret = avio_read(pb, buf, 6)) == 6) {
-        keyframe = memcmp(buf, gif87a_sig, 6) == 0 ||
-                   memcmp(buf, gif89a_sig, 6) == 0;
-    } else if (ret < 0) {
+    pkt->pos = avio_tell(pb);
+    pkt->stream_index = 0;
+    ret = avio_read_partial(pb, pkt->data, GIF_PACKET_SIZE);
+    if (ret < 0) {
+        av_packet_unref(pkt);
         return ret;
-    } else {
-        keyframe = 0;
-    }
-
-    if (keyframe) {
-parse_keyframe:
-        /* skip 2 bytes of width and 2 of height */
-        if ((ret = avio_skip(pb, 4)) < 0)
-            return ret;
-
-        packed_fields = avio_r8(pb);
-
-        /* skip 1 byte of Background Color Index and 1 byte of Pixel Aspect Ratio */
-        if ((ret = avio_skip(pb, 2)) < 0)
-            return ret;
-
-        /* global color table presence */
-        if (packed_fields & 0x80) {
-            ct_size = 3 * (1 << ((packed_fields & 0x07) + 1));
-
-            if ((ret = avio_skip(pb, ct_size)) < 0)
-                return ret;
-        }
-    } else {
-        avio_seek(pb, -ret, SEEK_CUR);
-        ret = AVERROR_EOF;
     }
-
-    while (GIF_TRAILER != (block_label = avio_r8(pb)) && !avio_feof(pb)) {
-        if (block_label == GIF_EXTENSION_INTRODUCER) {
-            if ((ret = gif_read_ext (s)) < 0 )
-                goto resync;
-        } else if (block_label == GIF_IMAGE_SEPARATOR) {
-            /* skip to last byte of Image Descriptor header */
-            if ((ret = avio_skip(pb, 8)) < 0)
-                return ret;
-
-            packed_fields = avio_r8(pb);
-
-            /* local color table presence */
-            if (packed_fields & 0x80) {
-                ct_size = 3 * (1 << ((packed_fields & 0x07) + 1));
-
-                if ((ret = avio_skip(pb, ct_size)) < 0)
-                    return ret;
-            }
-
-            /* read LZW Minimum Code Size */
-            if (avio_r8(pb) < 1) {
-                av_log(s, AV_LOG_ERROR, "lzw minimum code size must be >= 1\n");
-                goto resync;
-            }
-
-            if ((ret = gif_skip_subblocks(pb)) < 0)
-                goto resync;
-
-            frame_end = avio_tell(pb);
-
-            if (avio_seek(pb, frame_start, SEEK_SET) != frame_start)
-                return AVERROR(EIO);
-
-            ret = av_get_packet(pb, pkt, frame_end - frame_start);
-            if (ret < 0)
-                return ret;
-
-            if (keyframe)
-                pkt->flags |= AV_PKT_FLAG_KEY;
-
-            pkt->stream_index = 0;
-            pkt->duration = gdc->delay;
-
-            gdc->nb_frames ++;
-            gdc->last_duration = pkt->duration;
-
-            /* Graphic Control Extension's scope is single frame.
-             * Remove its influence. */
-            gdc->delay = gdc->default_delay;
-            frame_parsed = 1;
-
-            break;
-        } else {
-            av_log(s, AV_LOG_ERROR, "invalid block label\n");
-resync:
-            if (!keyframe)
-                avio_seek(pb, frame_start, SEEK_SET);
-            if ((ret = resync(pb)) < 0)
-                return ret;
-            frame_start = avio_tell(pb) - 6;
-            keyframe = 1;
-            goto parse_keyframe;
-        }
-    }
-
-    if ((ret >= 0 && !frame_parsed) || ret == AVERROR_EOF) {
-        if (gdc->nb_frames == 1) {
-            s->streams[0]->r_frame_rate = (AVRational) {100, gdc->last_duration};
-        }
-        /* This might happen when there is no image block
-         * between extension blocks and GIF_TRAILER or EOF */
-        if (!gdc->ignore_loop && (block_label == GIF_TRAILER || avio_feof(pb))
-            && (gdc->total_iter < 0 || ++gdc->iter_count < gdc->total_iter))
-            return avio_seek(pb, 0, SEEK_SET);
-        return AVERROR_EOF;
-    } else
-        return ret;
+    av_shrink_packet(pkt, ret);
+    return ret;
 }
 
 static const AVOption options[] = {
@@ -405,6 +284,7 @@ const AVInputFormat ff_gif_demuxer = {
     .read_probe     = gif_probe,
     .read_header    = gif_read_header,
     .read_packet    = gif_read_packet,
-    .flags          = AVFMT_GENERIC_INDEX,
+    .flags          = AVFMT_GENERIC_INDEX | AVFMT_NOTIMESTAMPS,
+    .extensions     = "gif",
     .priv_class     = &demuxer_class,
 };
diff --git a/tests/ref/fate/gif-demux b/tests/ref/fate/gif-demux
index 3d46441d8e..a3267a0d2a 100644
--- a/tests/ref/fate/gif-demux
+++ b/tests/ref/fate/gif-demux
@@ -38,4 +38,4 @@
 0,         74,         74,        2,     4633, 0x8f64fda7, F=0x0
 0,         76,         76,        2,     4700, 0x45f40805, F=0x0
 0,         78,         78,        2,     5117, 0x4eb4c5fb, F=0x0
-0,         80,         80,        2,     5370, 0xb10c6910, F=0x0
+0,         80,         80,        2,     5371, 0x1a66694b, F=0x0
-- 
2.39.1


[-- Attachment #4: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".