Revert "apngdec: use side data to pass extradata to the decoder"

This reverts commit e0c6b32046f4bab7d34be77dd2f03b2a80c86d39.

Said commit changed the behavior of the demuxer and decoder in a non
backwards compatible way.
Demuxers should make extradata available at init if possible, and send
new extradata as side data within a packet if needed.

A better fix for the remuxing crash will follow.

Signed-off-by: James Almer <jamrial@gmail.com>

libavcodec/pngdec.c

@@ -45,9 +45,6 @@ typedef struct PNGDecContext {
     ThreadFrame last_picture;
     ThreadFrame picture;
 
-    uint8_t* extra_data;
-    int extra_data_size;
-
     int state;
     int width, height;
     int cur_w, cur_h;
@@ -1364,28 +1361,14 @@ static int decode_frame_apng(AVCodecContext *avctx,
     p = s->picture.f;
 
     if (!(s->state & PNG_IHDR)) {
-        int side_data_size = 0;
-        uint8_t *side_data = NULL;
-        if (avpkt)
-            side_data = av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA, &side_data_size);
-
-        if (side_data_size) {
-            av_freep(&s->extra_data);
-            s->extra_data = av_mallocz(side_data_size + AV_INPUT_BUFFER_PADDING_SIZE);
-            if (!s->extra_data)
-                return AVERROR(ENOMEM);
-            s->extra_data_size = side_data_size;
-            memcpy(s->extra_data, side_data, s->extra_data_size);
-        }
-
-        if (!s->extra_data_size)
+        if (!avctx->extradata_size)
             return AVERROR_INVALIDDATA;
 
         /* only init fields, there is no zlib use in extradata */
         s->zstream.zalloc = ff_png_zalloc;
         s->zstream.zfree  = ff_png_zfree;
 
-        bytestream2_init(&s->gb, s->extra_data, s->extra_data_size);
+        bytestream2_init(&s->gb, avctx->extradata, avctx->extradata_size);
         if ((ret = decode_frame_common(avctx, s, p, avpkt)) < 0)
             goto end;
     }
@@ -1511,8 +1494,6 @@ static av_cold int png_dec_end(AVCodecContext *avctx)
     s->last_row_size = 0;
     av_freep(&s->tmp_row);
     s->tmp_row_size = 0;
-    av_freep(&s->extra_data);
-    s->extra_data_size = 0;
 
     return 0;
 }

libavformat/apngdec.c

@@ -49,10 +49,6 @@ typedef struct APNGDemuxContext {
 
     int is_key_frame;
 
-    uint8_t *extra_data;
-    int extra_data_size;
-    int extra_data_updated;
-
     /*
      * loop options
      */
@@ -126,9 +122,9 @@ end:
     return AVPROBE_SCORE_MAX;
 }
 
-static int append_extradata(APNGDemuxContext *ctx, AVIOContext *pb, int len)
+static int append_extradata(AVCodecParameters *par, AVIOContext *pb, int len)
 {
-    int previous_size = ctx->extra_data_size;
+    int previous_size = par->extradata_size;
     int new_size, ret;
     uint8_t *new_extradata;
 
@@ -136,30 +132,18 @@ static int append_extradata(APNGDemuxContext *ctx, AVIOContext *pb, int len)
         return AVERROR_INVALIDDATA;
 
     new_size = previous_size + len;
-    new_extradata = av_realloc(ctx->extra_data, new_size + AV_INPUT_BUFFER_PADDING_SIZE);
+    new_extradata = av_realloc(par->extradata, new_size + AV_INPUT_BUFFER_PADDING_SIZE);
     if (!new_extradata)
         return AVERROR(ENOMEM);
-    ctx->extra_data = new_extradata;
-    ctx->extra_data_size = new_size;
+    par->extradata = new_extradata;
+    par->extradata_size = new_size;
 
-    if ((ret = avio_read(pb, ctx->extra_data + previous_size, len)) < 0)
+    if ((ret = avio_read(pb, par->extradata + previous_size, len)) < 0)
         return ret;
 
     return previous_size;
 }
 
-static int send_extradata(APNGDemuxContext *ctx, AVPacket *pkt)
-{
-    if (!ctx->extra_data_updated) {
-        uint8_t *side_data = av_packet_new_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA, ctx->extra_data_size);
-        if (!side_data)
-            return AVERROR(ENOMEM);
-        memcpy(side_data, ctx->extra_data, ctx->extra_data_size);
-        ctx->extra_data_updated = 1;
-    }
-    return 0;
-}
-
 static int apng_read_header(AVFormatContext *s)
 {
     APNGDemuxContext *ctx = s->priv_data;
@@ -194,15 +178,15 @@ static int apng_read_header(AVFormatContext *s)
         return ret;
 
     /* extradata will contain every chunk up to the first fcTL (excluded) */
-    ctx->extra_data = av_malloc(len + 12 + AV_INPUT_BUFFER_PADDING_SIZE);
-    if (!ctx->extra_data)
+    st->codecpar->extradata = av_malloc(len + 12 + AV_INPUT_BUFFER_PADDING_SIZE);
+    if (!st->codecpar->extradata)
         return AVERROR(ENOMEM);
-    ctx->extra_data_size = len + 12;
-    AV_WB32(ctx->extra_data,    len);
-    AV_WL32(ctx->extra_data+4,  tag);
-    AV_WB32(ctx->extra_data+8,  st->codecpar->width);
-    AV_WB32(ctx->extra_data+12, st->codecpar->height);
-    if ((ret = avio_read(pb, ctx->extra_data+16, 9)) < 0)
+    st->codecpar->extradata_size = len + 12;
+    AV_WB32(st->codecpar->extradata,    len);
+    AV_WL32(st->codecpar->extradata+4,  tag);
+    AV_WB32(st->codecpar->extradata+8,  st->codecpar->width);
+    AV_WB32(st->codecpar->extradata+12, st->codecpar->height);
+    if ((ret = avio_read(pb, st->codecpar->extradata+16, 9)) < 0)
         goto fail;
 
     while (!avio_feof(pb)) {
@@ -234,11 +218,11 @@ static int apng_read_header(AVFormatContext *s)
         switch (tag) {
         case MKTAG('a', 'c', 'T', 'L'):
             if ((ret = avio_seek(pb, -8, SEEK_CUR)) < 0 ||
-                (ret = append_extradata(ctx, pb, len + 12)) < 0)
+                (ret = append_extradata(st->codecpar, pb, len + 12)) < 0)
                 goto fail;
             acTL_found = 1;
-            ctx->num_frames = AV_RB32(ctx->extra_data + ret + 8);
-            ctx->num_play   = AV_RB32(ctx->extra_data + ret + 12);
+            ctx->num_frames = AV_RB32(st->codecpar->extradata + ret + 8);
+            ctx->num_play   = AV_RB32(st->codecpar->extradata + ret + 12);
             av_log(s, AV_LOG_DEBUG, "num_frames: %"PRIu32", num_play: %"PRIu32"\n",
                                     ctx->num_frames, ctx->num_play);
             break;
@@ -252,15 +236,15 @@ static int apng_read_header(AVFormatContext *s)
             return 0;
         default:
             if ((ret = avio_seek(pb, -8, SEEK_CUR)) < 0 ||
-                (ret = append_extradata(ctx, pb, len + 12)) < 0)
+                (ret = append_extradata(st->codecpar, pb, len + 12)) < 0)
                 goto fail;
         }
     }
 
 fail:
-    if (ctx->extra_data_size) {
-        av_freep(&ctx->extra_data);
-        ctx->extra_data_size = 0;
+    if (st->codecpar->extradata_size) {
+        av_freep(&st->codecpar->extradata);
+        st->codecpar->extradata_size = 0;
     }
     return ret;
 }
@@ -409,16 +393,16 @@ static int apng_read_packet(AVFormatContext *s, AVPacket *pkt)
         pkt->pts = ctx->pkt_pts;
         pkt->duration = ctx->pkt_duration;
         ctx->pkt_pts += ctx->pkt_duration;
-        return send_extradata(ctx, pkt);
+        return ret;
     case MKTAG('I', 'E', 'N', 'D'):
         ctx->cur_loop++;
         if (ctx->ignore_loop || ctx->num_play >= 1 && ctx->cur_loop == ctx->num_play) {
             avio_seek(pb, -8, SEEK_CUR);
             return AVERROR_EOF;
         }
-        if ((ret = avio_seek(pb, ctx->extra_data_size + 8, SEEK_SET)) < 0)
+        if ((ret = avio_seek(pb, s->streams[0]->codecpar->extradata_size + 8, SEEK_SET)) < 0)
             return ret;
-        return send_extradata(ctx, pkt);
+        return 0;
     default:
         {
         char tag_buf[32];
@@ -433,14 +417,6 @@ static int apng_read_packet(AVFormatContext *s, AVPacket *pkt)
     return AVERROR_PATCHWELCOME;
 }
 
-static int apng_read_close(AVFormatContext *s)
-{
-    APNGDemuxContext *ctx = s->priv_data;
-    av_freep(&ctx->extra_data);
-    ctx->extra_data_size = 0;
-    return 0;
-}
-
 static const AVOption options[] = {
     { "ignore_loop", "ignore loop setting"                         , offsetof(APNGDemuxContext, ignore_loop),
       AV_OPT_TYPE_BOOL, { .i64 = 1 }              , 0, 1      , AV_OPT_FLAG_DECODING_PARAM },
@@ -466,7 +442,6 @@ AVInputFormat ff_apng_demuxer = {
     .read_probe     = apng_probe,
     .read_header    = apng_read_header,
     .read_packet    = apng_read_packet,
-    .read_close     = apng_read_close,
     .flags          = AVFMT_GENERIC_INDEX,
     .priv_class     = &demuxer_class,
 };

tests/lavf-regression.sh

@@ -216,9 +216,9 @@ if [ -n "$do_apng" ] ; then
 file=${outfile}lavf.apng
 do_avconv $file $DEC_OPTS -f image2 -vcodec pgmyuv -i $raw_src $ENC_OPTS -t 1 -pix_fmt rgb24
 do_avconv_crc $file $DEC_OPTS -i $target_path/$file -pix_fmt rgb24
-file_copy=${outfile}lavf.copy.apng
-do_avconv $file_copy $DEC_OPTS -i $file $ENC_OPTS -c copy
-do_avconv_crc $file_copy $DEC_OPTS -i $target_path/$file_copy
+#file_copy=${outfile}lavf.copy.apng
+#do_avconv $file_copy $DEC_OPTS -i $file $ENC_OPTS -c copy
+#do_avconv_crc $file_copy $DEC_OPTS -i $target_path/$file_copy
 file=${outfile}lavf.png
 do_avconv $file $DEC_OPTS -f image2 -vcodec pgmyuv -i $raw_src $ENC_OPTS -pix_fmt rgb24 -frames:v 1 -f apng
 do_avconv_crc $file $DEC_OPTS -i $target_path/$file -pix_fmt rgb24

tests/ref/lavf/apng

@@ -1,9 +1,6 @@
 a4c46fad7716ad094eb3c78b74ca0244 *./tests/data/lavf/lavf.apng
 6209864 ./tests/data/lavf/lavf.apng
 ./tests/data/lavf/lavf.apng CRC=0x87b3c15f
-a4c46fad7716ad094eb3c78b74ca0244 *./tests/data/lavf/lavf.copy.apng
-6209864 ./tests/data/lavf/lavf.copy.apng
-./tests/data/lavf/lavf.copy.apng CRC=0x87b3c15f
 c5900fdd1b2fc30b985793f5226fd0c4 *./tests/data/lavf/lavf.png
 248854 ./tests/data/lavf/lavf.png
 ./tests/data/lavf/lavf.png CRC=0xd8c7b7a1