From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id CC56844D52 for ; Mon, 22 May 2023 16:03:51 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5CB4168BF7D; Mon, 22 May 2023 19:03:49 +0300 (EEST) Received: from mail-oi1-f169.google.com (mail-oi1-f169.google.com [209.85.167.169]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D695868BE2F for ; Mon, 22 May 2023 19:03:42 +0300 (EEST) Received: by mail-oi1-f169.google.com with SMTP id 5614622812f47-397f13944f2so1358826b6e.0 for ; Mon, 22 May 2023 09:03:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684771421; x=1687363421; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=C3I/aLLWzHLD/8ZlsoT4JvnM64HjctIXpP0YLn907wo=; b=cMU95MJ8Oyu4dIhQGb5tzD1qUHh8ALKOh1+hPC3UcXICMDDY2TNL+BCLYme8gCFXBt d3+pDDa9ZnQWPtBcg99uNC5ZwIwvoFfbmW62UZ6nspNyY0oS8CN99TxneEEGs/e5r+Oo C+do6M7TFu/C/QyJxgt16qSTC1U22V2Of2sCeqltEbYs/l8+M4JBvRzQTlSvgRTaa0zn RJHA2kY1x4fq6FDhojPDdZt/mhLgSVsAzCpBwIhaVno0TOLXAlBrlsHlRcJMUXpXTBNd 7AkUv9N5+XUC8Erq1zZ9IGdd5AK4hlg+dmbg0Nuwp97VoLzBy2Hk7uSK3XlTduj2U7CF Q6BQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684771421; x=1687363421; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=C3I/aLLWzHLD/8ZlsoT4JvnM64HjctIXpP0YLn907wo=; b=CjAJN0yodB/pAHrkdtvKP7UtdevJRzqmsGZ7sIJLo2bUWP4aPtWNnXM0dH3r7VtTSC qrOKCZRqWKZT9qAtxFP3FUP8GIH3pcARA32+tt7uO4bU2Q6WwWb2Qt2kwmEvsT9W4KcL 3aUxoIiDVuyc5Z0FlIsE5TYEP7Q+8qiq3GJ7PtNMqVDJdK/6DHFFr5v6ag3JybIpNDFo bmH/LXcYmxskZ1ZuadDXeMkhOm66b6O51aG0tL0Hk5IJ3B1jAisAGtCfo0tpZmL6eTVr UZwXF49vI4KkcL761PGX3O5xA7wdXrmZVl3imSrdiRJnNJTIGxbFID1lTakzlmQxEsVf f1Zg== X-Gm-Message-State: AC+VfDw/4xrnJM6GkHAsNwPp7QJv/dsuALJYm7q98wu431Z5M3aT3W3j OMSEBJmw70SU33sPHVQ6tPQjtu4917w= X-Google-Smtp-Source: ACHHUZ4BeqyTB0u3G4NPXFphgj/hJjflW3uFgkR9VfGbt4sPG15q0g0gMINoyxeGzcAmzTwRizGXXw== X-Received: by 2002:a54:4695:0:b0:398:581:aedd with SMTP id k21-20020a544695000000b003980581aeddmr2143873oic.37.1684771420482; Mon, 22 May 2023 09:03:40 -0700 (PDT) Received: from [192.168.0.12] (host197.190-225-105.telecom.net.ar. [190.225.105.197]) by smtp.gmail.com with ESMTPSA id v124-20020aca6182000000b003942a9bfdefsm2887276oib.33.2023.05.22.09.03.39 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 22 May 2023 09:03:39 -0700 (PDT) Message-ID: <4757bce8-1185-714a-88a3-2135bc51390b@gmail.com> Date: Mon, 22 May 2023 13:04:00 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: From: James Almer In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH] avformat/gifdec: cleanup X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 5/22/2023 12:49 PM, Paul B Mahol wrote: > On 5/21/23, Paul B Mahol wrote: >> Attached patches. >> >> This finally removes giant hacks in gif demuxer and allows using gif >> files via pipe reliably. >> > > Now with smaller diff. [...] > From 208d1e83ae9aef8e9b37007df16569cdd4cf25d2 Mon Sep 17 00:00:00 2001 > From: Paul B Mahol > Date: Sat, 20 May 2023 14:13:27 +0200 > Subject: [PATCH 2/2] avformat/gifdec: switch to using gif parser > > Update fate tests, more correct as timebase is no more reduced. This is not the case anymore. > > Signed-off-by: Paul B Mahol > --- > libavcodec/gifdec.c | 11 +- > libavformat/gifdec.c | 244 +++++++++++-------------------------------- > tests/ref/lavf/gif | 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/lavf/gif b/tests/ref/lavf/gif > index fc94b9df3d..30eb85325a 100644 > --- a/tests/ref/lavf/gif > +++ b/tests/ref/lavf/gif > @@ -1,3 +1,3 @@ > e35f5ea283bbcb249818e0078ec72664 *tests/data/lavf/lavf.gif > 2011766 tests/data/lavf/lavf.gif > -tests/data/lavf/lavf.gif CRC=0x2429faff > +tests/data/lavf/lavf.gif CRC=0x563cec26 > -- > 2.39.1 TEST gif-demux --- /home/Ethaniel/FFmpeg/tests/ref/fate/gif-demux 2023-05-21 22:13:04.358066200 -0300 +++ tests/data/fate/gif-demux 2023-05-22 13:00:37.259301700 -0300 @@ -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 Test gif-demux failed. Look at tests/data/fate/gif-demux.err for details. And TEST lavf-gif --- /home/Ethaniel/FFmpeg/tests/ref/lavf/gif 2023-05-22 13:00:14.259590500 -0300 +++ tests/data/fate/lavf-gif 2023-05-22 13:01:19.316371100 -0300 @@ -1,3 +1,3 @@ e35f5ea283bbcb249818e0078ec72664 *tests/data/lavf/lavf.gif 2011766 tests/data/lavf/lavf.gif -tests/data/lavf/lavf.gif CRC=0x563cec26 +tests/data/lavf/lavf.gif CRC=0x2429faff Test lavf-gif failed. Look at tests/data/fate/lavf-gif.err for details. This one giving the same result as pre-patch. Are you sure you ran fate on a clean tree and build folder? _______________________________________________ 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".