Revert "avcodec: add FF_CODEC_CAP_INIT_CLEANUP for all codecs which use ff_mpv_common...

This mostly reverts commit 4b2863ff01b1fe93d9a518523c9098d17a9d8c6f.
Said commit removed the freeing code from ff_mpv_common_init(),
ff_mpv_common_frame_size_change() and ff_mpeg_framesize_alloc() and
instead added the FF_CODEC_CAP_INIT_CLEANUP to several codecs that use
ff_mpv_common_init(). This introduced several bugs:

a) Several decoders using ff_mpv_common_init() in their init function were
forgotten: This affected FLV, Intel H.263, RealVideo 3.0 and V4.0 as well as
VC-1/WMV3.
b) ff_mpv_common_init() is not only called from the init function of
codecs, it is also called from AVCodec.decode functions. If an error
happens after an allocation has succeeded, it can lead to memleaks;
furthermore, it is now possible for the MpegEncContext to be marked as
initialized even when ff_mpv_common_init() returns an error and this can
lead to segfaults because decoders that call ff_mpv_common_init() when
decoding a frame can mistakenly think that the MpegEncContext has been
properly initialized. This can e.g. happen with H.261 or MPEG-4.
c) Removing code for freeing from ff_mpeg_framesize_alloc() (which can't
be called from any init function) can lead to segfaults because the
check for whether it needs to allocate consists of checking whether the
first of the buffers allocated there has been allocated. This part has
already been fixed in 76cea1d2ce3f23e8131c8664086a1daf873ed694.
d) ff_mpv_common_frame_size_change() can also not be reached from any
AVCodec.init function; yet the changes can e.g. lead to segfaults with
decoders using ff_h263_decode_frame() upon allocation failure, because
the MpegEncContext will upon return be flagged as both initialized and
not in need of reinitialization (granted, the fact that
ff_h263_decode_frame() clears context_reinit before the context has been
reinited is a bug in itself). With the earlier version, the context
would be cleaned upon failure and it would be attempted to initialize
the context again in the next call to ff_h263_decode_frame().

While a) could be fixed by adding the missing FF_CODEC_CAP_INIT_CLEANUP,
keeping the current approach would entail adding cleanup code to several
other places because of b). Therefore ff_mpv_common_init() is again made
to clean up after itself; the changes to the wmv2 decoder and the SVQ1
encoder have not been reverted: The former fixed a memleak, the latter
allowed to remove cleanup code.

Fixes: double free
Fixes: ff_free_picture_tables.mp4
Fixes: ff_mpeg_update_thread_context.mp4
Fixes: decode_colskip.mp4
Fixes: memset.mp4
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>

diff --git a/libavcodec/h261dec.c b/libavcodec/h261dec.c
index 5c25aa9cb39a59c1f6000b727da2cbbf64a2df8f..eb544e6046ae438c6bb93df34c947a5c65683513 100644 (file)
--- a/libavcodec/h261dec.c
+++ b/libavcodec/h261dec.c
@@ -678,6 +678,6 @@ AVCodec ff_h261_decoder = {
     .close          = h261_decode_end,
     .decode         = h261_decode_frame,
     .capabilities   = AV_CODEC_CAP_DR1,
-    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
     .max_lowres     = 3,
 };

diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index dafa54d8d4c34a06d573d03fabc0554467169e2a..e8b4d83e6ef4695f5aa1caa11b2ad4b9c6f3a7ae 100644 (file)
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -771,7 +771,7 @@ AVCodec ff_h263_decoder = {
     .decode         = ff_h263_decode_frame,
     .capabilities   = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1 |
                       AV_CODEC_CAP_TRUNCATED | AV_CODEC_CAP_DELAY,
-    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
     .flush          = ff_mpeg_flush,
     .max_lowres     = 3,
     .pix_fmts       = ff_h263_hwaccel_pixfmt_list_420,
@@ -789,7 +789,7 @@ AVCodec ff_h263p_decoder = {
     .decode         = ff_h263_decode_frame,
     .capabilities   = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1 |
                       AV_CODEC_CAP_TRUNCATED | AV_CODEC_CAP_DELAY,
-    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
     .flush          = ff_mpeg_flush,
     .max_lowres     = 3,
     .pix_fmts       = ff_h263_hwaccel_pixfmt_list_420,

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 92dd6a0b24bde831336e60a9867c55087689c23a..94221da2c158255e3b1db73299584f448ce634a5 100644 (file)
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2880,7 +2880,8 @@ static av_cold int mpeg_decode_end(AVCodecContext *avctx)
 {
     Mpeg1Context *s = avctx->priv_data;
 
-    ff_mpv_common_end(&s->mpeg_enc_ctx);
+    if (s->mpeg_enc_ctx_allocated)
+        ff_mpv_common_end(&s->mpeg_enc_ctx);
     av_buffer_unref(&s->a53_buf_ref);
     return 0;
 }
@@ -2897,7 +2898,7 @@ AVCodec ff_mpeg1video_decoder = {
     .capabilities          = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1 |
                              AV_CODEC_CAP_TRUNCATED | AV_CODEC_CAP_DELAY |
                              AV_CODEC_CAP_SLICE_THREADS,
-    .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP |
+    .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE |
                              FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
     .flush                 = flush,
     .max_lowres            = 3,
@@ -2931,7 +2932,7 @@ AVCodec ff_mpeg2video_decoder = {
     .capabilities   = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1 |
                       AV_CODEC_CAP_TRUNCATED | AV_CODEC_CAP_DELAY |
                       AV_CODEC_CAP_SLICE_THREADS,
-    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP |
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
                       FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
     .flush          = flush,
     .max_lowres     = 3,
@@ -2976,7 +2977,7 @@ AVCodec ff_mpegvideo_decoder = {
     .close          = mpeg_decode_end,
     .decode         = mpeg_decode_frame,
     .capabilities   = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1 | AV_CODEC_CAP_TRUNCATED | AV_CODEC_CAP_DELAY | AV_CODEC_CAP_SLICE_THREADS,
-    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP |
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
                       FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
     .flush          = flush,
     .max_lowres     = 3,

diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
index 2ca804fdc6eaf7d46dc00a4eb117be82418adab1..2c440a5026fcfca120321f777c7f46b62c1ab4be 100644 (file)
--- a/libavcodec/mpeg4videodec.c
+++ b/libavcodec/mpeg4videodec.c
@@ -3579,8 +3579,7 @@ AVCodec ff_mpeg4_decoder = {
                              AV_CODEC_CAP_TRUNCATED | AV_CODEC_CAP_DELAY |
                              AV_CODEC_CAP_FRAME_THREADS,
     .caps_internal         = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
-                             FF_CODEC_CAP_ALLOCATE_PROGRESS |
-                             FF_CODEC_CAP_INIT_CLEANUP,
+                             FF_CODEC_CAP_ALLOCATE_PROGRESS,
     .flush                 = ff_mpeg_flush,
     .max_lowres            = 3,
     .pix_fmts              = ff_h263_hwaccel_pixfmt_list_420,

diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
index 728e59195889ae6e833c8d28810898fc76b890ff..e03f174024136bc1fe264b2bf75ff00f389633bb 100644 (file)
--- a/libavcodec/mpegvideo.c
+++ b/libavcodec/mpegvideo.c
@@ -932,17 +932,17 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
     for (i = 0; i < MAX_PICTURE_COUNT; i++) {
         s->picture[i].f = av_frame_alloc();
         if (!s->picture[i].f)
-            return AVERROR(ENOMEM);
+            goto fail_nomem;
     }
 
     if (!(s->next_picture.f    = av_frame_alloc()) ||
         !(s->last_picture.f    = av_frame_alloc()) ||
         !(s->current_picture.f = av_frame_alloc()) ||
         !(s->new_picture.f     = av_frame_alloc()))
-        return AVERROR(ENOMEM);
+        goto fail_nomem;
 
     if ((ret = init_context_frame(s)))
-        return AVERROR(ENOMEM);
+        goto fail;
 
     s->parse_context.state = -1;
 
@@ -956,10 +956,10 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
             if (i) {
                 s->thread_context[i] = av_memdup(s, sizeof(MpegEncContext));
                 if (!s->thread_context[i])
-                    return AVERROR(ENOMEM);
+                    goto fail_nomem;
             }
             if ((ret = init_duplicate_context(s->thread_context[i])) < 0)
-                return ret;
+                goto fail;
             s->thread_context[i]->start_mb_y =
                 (s->mb_height * (i) + nb_slices / 2) / nb_slices;
             s->thread_context[i]->end_mb_y   =
@@ -967,7 +967,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
         }
     } else {
         if ((ret = init_duplicate_context(s)) < 0)
-            return ret;
+            goto fail;
         s->start_mb_y = 0;
         s->end_mb_y   = s->mb_height;
     }
@@ -975,6 +975,11 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
 //     }
 
     return 0;
+ fail_nomem:
+    ret = AVERROR(ENOMEM);
+ fail:
+    ff_mpv_common_end(s);
+    return ret;
 }
 
 /**
@@ -1067,17 +1072,17 @@ int ff_mpv_common_frame_size_change(MpegEncContext *s)
 
     if ((s->width || s->height) &&
         (err = av_image_check_size(s->width, s->height, 0, s->avctx)) < 0)
-        return err;
+        goto fail;
 
     /* set chroma shifts */
     err = av_pix_fmt_get_chroma_sub_sample(s->avctx->pix_fmt,
                                            &s->chroma_x_shift,
                                            &s->chroma_y_shift);
     if (err < 0)
-        return err;
+        goto fail;
 
     if ((err = init_context_frame(s)))
-        return err;
+        goto fail;
 
     memset(s->thread_context, 0, sizeof(s->thread_context));
     s->thread_context[0]   = s;
@@ -1089,11 +1094,12 @@ int ff_mpv_common_frame_size_change(MpegEncContext *s)
                 if (i) {
                     s->thread_context[i] = av_memdup(s, sizeof(MpegEncContext));
                     if (!s->thread_context[i]) {
-                        return AVERROR(ENOMEM);
+                        err = AVERROR(ENOMEM);
+                        goto fail;
                     }
                 }
                 if ((err = init_duplicate_context(s->thread_context[i])) < 0)
-                    return err;
+                    goto fail;
                 s->thread_context[i]->start_mb_y =
                     (s->mb_height * (i) + nb_slices / 2) / nb_slices;
                 s->thread_context[i]->end_mb_y   =
@@ -1102,7 +1108,7 @@ int ff_mpv_common_frame_size_change(MpegEncContext *s)
         } else {
             err = init_duplicate_context(s);
             if (err < 0)
-                return err;
+                goto fail;
             s->start_mb_y = 0;
             s->end_mb_y   = s->mb_height;
         }
@@ -1110,6 +1116,9 @@ int ff_mpv_common_frame_size_change(MpegEncContext *s)
     }
 
     return 0;
+ fail:
+    ff_mpv_common_end(s);
+    return err;
 }
 
 /* init common structure for both encoder and decoder */

diff --git a/libavcodec/msmpeg4dec.c b/libavcodec/msmpeg4dec.c
index 405fda4b83a6aa332a157ae8266b874d9750aef8..63f9faa1bb3d26b1b1a3cd691e56e88c0b6d8dd5 100644 (file)
--- a/libavcodec/msmpeg4dec.c
+++ b/libavcodec/msmpeg4dec.c
@@ -870,7 +870,7 @@ AVCodec ff_msmpeg4v1_decoder = {
     .close          = ff_h263_decode_end,
     .decode         = ff_h263_decode_frame,
     .capabilities   = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1,
-    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
     .max_lowres     = 3,
     .pix_fmts       = (const enum AVPixelFormat[]) {
         AV_PIX_FMT_YUV420P,
@@ -888,7 +888,7 @@ AVCodec ff_msmpeg4v2_decoder = {
     .close          = ff_h263_decode_end,
     .decode         = ff_h263_decode_frame,
     .capabilities   = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1,
-    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
     .max_lowres     = 3,
     .pix_fmts       = (const enum AVPixelFormat[]) {
         AV_PIX_FMT_YUV420P,
@@ -906,7 +906,7 @@ AVCodec ff_msmpeg4v3_decoder = {
     .close          = ff_h263_decode_end,
     .decode         = ff_h263_decode_frame,
     .capabilities   = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1,
-    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
     .max_lowres     = 3,
     .pix_fmts       = (const enum AVPixelFormat[]) {
         AV_PIX_FMT_YUV420P,
@@ -924,7 +924,7 @@ AVCodec ff_wmv1_decoder = {
     .close          = ff_h263_decode_end,
     .decode         = ff_h263_decode_frame,
     .capabilities   = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1,
-    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
     .max_lowres     = 3,
     .pix_fmts       = (const enum AVPixelFormat[]) {
         AV_PIX_FMT_YUV420P,

diff --git a/libavcodec/rv10.c b/libavcodec/rv10.c
index d5c7480611e0eed98d176802e09ef7f47f937c22..89b838ad1559661f6d944611a05fffc75fd0fa00 100644 (file)
--- a/libavcodec/rv10.c
+++ b/libavcodec/rv10.c
@@ -687,7 +687,6 @@ AVCodec ff_rv10_decoder = {
     .close          = rv10_decode_end,
     .decode         = rv10_decode_frame,
     .capabilities   = AV_CODEC_CAP_DR1,
-    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
     .max_lowres     = 3,
     .pix_fmts       = (const enum AVPixelFormat[]) {
         AV_PIX_FMT_YUV420P,
@@ -705,7 +704,6 @@ AVCodec ff_rv20_decoder = {
     .close          = rv10_decode_end,
     .decode         = rv10_decode_frame,
     .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
-    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
     .flush          = ff_mpeg_flush,
     .max_lowres     = 3,
     .pix_fmts       = (const enum AVPixelFormat[]) {