diff mbox series

[FFmpeg-devel,v4,09/24] avcodec: add FF_CODEC_CAP_INIT_CLEANUP for all codecs which use ff_mpv_common_init()

Message ID 1591111618-15778-9-git-send-email-lance.lmwang@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v4,01/24] avcodec/h264dec: cosmetics | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Lance Wang June 2, 2020, 3:26 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavcodec/h261dec.c       | 1 +
 libavcodec/h263dec.c       | 4 ++--
 libavcodec/mpeg12dec.c     | 9 ++++-----
 libavcodec/mpeg4videodec.c | 3 ++-
 libavcodec/mpegpicture.c   | 1 -
 libavcodec/mpegvideo.c     | 2 --
 libavcodec/msmpeg4dec.c    | 8 ++++----
 libavcodec/rv10.c          | 2 ++
 libavcodec/svq1enc.c       | 4 +---
 libavcodec/wmv2dec.c       | 1 +
 10 files changed, 17 insertions(+), 18 deletions(-)

Comments

Michael Niedermayer June 3, 2020, 10:43 a.m. UTC | #1
On Tue, Jun 02, 2020 at 11:26:43PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavcodec/h261dec.c       | 1 +
>  libavcodec/h263dec.c       | 4 ++--
>  libavcodec/mpeg12dec.c     | 9 ++++-----
>  libavcodec/mpeg4videodec.c | 3 ++-
>  libavcodec/mpegpicture.c   | 1 -
>  libavcodec/mpegvideo.c     | 2 --
>  libavcodec/msmpeg4dec.c    | 8 ++++----
>  libavcodec/rv10.c          | 2 ++
>  libavcodec/svq1enc.c       | 4 +---
>  libavcodec/wmv2dec.c       | 1 +
>  10 files changed, 17 insertions(+), 18 deletions(-)

this patch makes this
./ffmpeg -debug vis_qp  -i ~/videos/mm-short.mpg    -f framecrc -
segfault

[mpeg2video @ 0x2d2df4c0] [Eval @ 0x1ffeffe940] Undefined constant or missing '(' in 'vis_qp'
[mpeg2video @ 0x2d2df4c0] Unable to parse option value "vis_qp"
[mpeg2video @ 0x2d2df4c0] Error setting option debug to value vis_qp.
==28696== Invalid read of size 4
==28696==    at 0x909059: ff_mpeg_unref_picture (in ffmpeg_g)
==28696==    by 0x90FC85: ff_mpv_common_end (in ffmpeg_g)
==28696==    by 0x27868C: mpeg_decode_end (in ffmpeg_g)
==28696==    by 0xA760F5: avcodec_open2 (in ffmpeg_g)
==28696==    by 0x2F4DB3: transcode_init (in ffmpeg_g)
==28696==    by 0x2F55F8: transcode (in ffmpeg_g)
==28696==    by 0x2D043B: main (in ffmpeg_g)
==28696==  Address 0x18 is not stack'd, malloc'd or (recently) free'd

the video seems not to matter,
if you cannot reproduce say so and ill provide more detailed debug information

thx


[...]
Lance Wang June 3, 2020, 1:28 p.m. UTC | #2
On Wed, Jun 03, 2020 at 12:43:34PM +0200, Michael Niedermayer wrote:
> On Tue, Jun 02, 2020 at 11:26:43PM +0800, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavcodec/h261dec.c       | 1 +
> >  libavcodec/h263dec.c       | 4 ++--
> >  libavcodec/mpeg12dec.c     | 9 ++++-----
> >  libavcodec/mpeg4videodec.c | 3 ++-
> >  libavcodec/mpegpicture.c   | 1 -
> >  libavcodec/mpegvideo.c     | 2 --
> >  libavcodec/msmpeg4dec.c    | 8 ++++----
> >  libavcodec/rv10.c          | 2 ++
> >  libavcodec/svq1enc.c       | 4 +---
> >  libavcodec/wmv2dec.c       | 1 +
> >  10 files changed, 17 insertions(+), 18 deletions(-)
> 
> this patch makes this
> ./ffmpeg -debug vis_qp  -i ~/videos/mm-short.mpg    -f framecrc -
> segfault
> 
> [mpeg2video @ 0x2d2df4c0] [Eval @ 0x1ffeffe940] Undefined constant or missing '(' in 'vis_qp'
> [mpeg2video @ 0x2d2df4c0] Unable to parse option value "vis_qp"
> [mpeg2video @ 0x2d2df4c0] Error setting option debug to value vis_qp.
> ==28696== Invalid read of size 4
> ==28696==    at 0x909059: ff_mpeg_unref_picture (in ffmpeg_g)
> ==28696==    by 0x90FC85: ff_mpv_common_end (in ffmpeg_g)
> ==28696==    by 0x27868C: mpeg_decode_end (in ffmpeg_g)
> ==28696==    by 0xA760F5: avcodec_open2 (in ffmpeg_g)
> ==28696==    by 0x2F4DB3: transcode_init (in ffmpeg_g)
> ==28696==    by 0x2F55F8: transcode (in ffmpeg_g)
> ==28696==    by 0x2D043B: main (in ffmpeg_g)
> ==28696==  Address 0x18 is not stack'd, malloc'd or (recently) free'd
> 
> the video seems not to matter,
> if you cannot reproduce say so and ill provide more detailed debug information

thanks, have reproduced it, will fix it.

> 
> thx
> 
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> While the State exists there can be no freedom; when there is freedom there
> will be no State. -- Vladimir Lenin



> _______________________________________________
> 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".
Lance Wang June 3, 2020, 2:17 p.m. UTC | #3
On Wed, Jun 03, 2020 at 12:43:34PM +0200, Michael Niedermayer wrote:
> On Tue, Jun 02, 2020 at 11:26:43PM +0800, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavcodec/h261dec.c       | 1 +
> >  libavcodec/h263dec.c       | 4 ++--
> >  libavcodec/mpeg12dec.c     | 9 ++++-----
> >  libavcodec/mpeg4videodec.c | 3 ++-
> >  libavcodec/mpegpicture.c   | 1 -
> >  libavcodec/mpegvideo.c     | 2 --
> >  libavcodec/msmpeg4dec.c    | 8 ++++----
> >  libavcodec/rv10.c          | 2 ++
> >  libavcodec/svq1enc.c       | 4 +---
> >  libavcodec/wmv2dec.c       | 1 +
> >  10 files changed, 17 insertions(+), 18 deletions(-)
> 
> this patch makes this
> ./ffmpeg -debug vis_qp  -i ~/videos/mm-short.mpg    -f framecrc -
> segfault
> 
> [mpeg2video @ 0x2d2df4c0] [Eval @ 0x1ffeffe940] Undefined constant or missing '(' in 'vis_qp'
> [mpeg2video @ 0x2d2df4c0] Unable to parse option value "vis_qp"
> [mpeg2video @ 0x2d2df4c0] Error setting option debug to value vis_qp.
> ==28696== Invalid read of size 4
> ==28696==    at 0x909059: ff_mpeg_unref_picture (in ffmpeg_g)
> ==28696==    by 0x90FC85: ff_mpv_common_end (in ffmpeg_g)
> ==28696==    by 0x27868C: mpeg_decode_end (in ffmpeg_g)
> ==28696==    by 0xA760F5: avcodec_open2 (in ffmpeg_g)
> ==28696==    by 0x2F4DB3: transcode_init (in ffmpeg_g)
> ==28696==    by 0x2F55F8: transcode (in ffmpeg_g)
> ==28696==    by 0x2D043B: main (in ffmpeg_g)
> ==28696==  Address 0x18 is not stack'd, malloc'd or (recently) free'd
> 
> the video seems not to matter,
> if you cannot reproduce say so and ill provide more detailed debug information

Have fixed the dump, for mpeg_decode_init may failed,so I remove the s->mpeg_enc_ctx_allocated
checking in the mpeg_decode_end() to clean memory before failed, but it'll cause s->avctx
is NULL and it hasn't been checked in ff_mpv_common_end().


> 
> thx
> 
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> While the State exists there can be no freedom; when there is freedom there
> will be no State. -- Vladimir Lenin



> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavcodec/h261dec.c b/libavcodec/h261dec.c
index 14a874c..8a49e7d 100644
--- a/libavcodec/h261dec.c
+++ b/libavcodec/h261dec.c
@@ -686,5 +686,6 @@  AVCodec ff_h261_decoder = {
     .close          = h261_decode_end,
     .decode         = h261_decode_frame,
     .capabilities   = AV_CODEC_CAP_DR1,
+    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
     .max_lowres     = 3,
 };
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index 31ac563..3b29a18 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -770,7 +770,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,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
     .flush          = ff_mpeg_flush,
     .max_lowres     = 3,
     .pix_fmts       = ff_h263_hwaccel_pixfmt_list_420,
@@ -788,7 +788,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,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
     .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 40d054d..4814e3b 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2876,8 +2876,7 @@  static av_cold int mpeg_decode_end(AVCodecContext *avctx)
 {
     Mpeg1Context *s = avctx->priv_data;
 
-    if (s->mpeg_enc_ctx_allocated)
-        ff_mpv_common_end(&s->mpeg_enc_ctx);
+    ff_mpv_common_end(&s->mpeg_enc_ctx);
     av_freep(&s->a53_caption);
     return 0;
 }
@@ -2894,7 +2893,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_SKIP_FRAME_FILL_PARAM,
+    .caps_internal         = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
     .flush                 = flush,
     .max_lowres            = 3,
     .update_thread_context = ONLY_IF_THREADS_ENABLED(mpeg_decode_update_thread_context),
@@ -2927,7 +2926,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_SKIP_FRAME_FILL_PARAM,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
     .flush          = flush,
     .max_lowres     = 3,
     .profiles       = NULL_IF_CONFIG_SMALL(ff_mpeg2_video_profiles),
@@ -2971,7 +2970,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_SKIP_FRAME_FILL_PARAM,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
     .flush          = flush,
     .max_lowres     = 3,
 };
diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
index 7e52bbe..4593477 100644
--- a/libavcodec/mpeg4videodec.c
+++ b/libavcodec/mpeg4videodec.c
@@ -3594,7 +3594,8 @@  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_ALLOCATE_PROGRESS |
+                             FF_CODEC_CAP_INIT_CLEANUP,
     .flush                 = ff_mpeg_flush,
     .max_lowres            = 3,
     .pix_fmts              = ff_h263_hwaccel_pixfmt_list_420,
diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
index 5fce25e..be90ece 100644
--- a/libavcodec/mpegpicture.c
+++ b/libavcodec/mpegpicture.c
@@ -90,7 +90,6 @@  int ff_mpeg_framesize_alloc(AVCodecContext *avctx, MotionEstContext *me,
 
     return 0;
 fail:
-    av_freep(&sc->edge_emu_buffer);
     return AVERROR(ENOMEM);
 }
 
diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
index 49fd1c9..34dfda5 100644
--- a/libavcodec/mpegvideo.c
+++ b/libavcodec/mpegvideo.c
@@ -991,7 +991,6 @@  av_cold int ff_mpv_common_init(MpegEncContext *s)
  fail_nomem:
     ret = AVERROR(ENOMEM);
  fail:
-    ff_mpv_common_end(s);
     return ret;
 }
 
@@ -1123,7 +1122,6 @@  int ff_mpv_common_frame_size_change(MpegEncContext *s)
 
     return 0;
  fail:
-    ff_mpv_common_end(s);
     return err;
 }
 
diff --git a/libavcodec/msmpeg4dec.c b/libavcodec/msmpeg4dec.c
index 16b6719..49df06a 100644
--- a/libavcodec/msmpeg4dec.c
+++ b/libavcodec/msmpeg4dec.c
@@ -888,7 +888,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,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
     .max_lowres     = 3,
     .pix_fmts       = (const enum AVPixelFormat[]) {
         AV_PIX_FMT_YUV420P,
@@ -906,7 +906,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,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
     .max_lowres     = 3,
     .pix_fmts       = (const enum AVPixelFormat[]) {
         AV_PIX_FMT_YUV420P,
@@ -924,7 +924,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,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
     .max_lowres     = 3,
     .pix_fmts       = (const enum AVPixelFormat[]) {
         AV_PIX_FMT_YUV420P,
@@ -942,7 +942,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,
+    .caps_internal  = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM | FF_CODEC_CAP_INIT_CLEANUP,
     .max_lowres     = 3,
     .pix_fmts       = (const enum AVPixelFormat[]) {
         AV_PIX_FMT_YUV420P,
diff --git a/libavcodec/rv10.c b/libavcodec/rv10.c
index 3b41d30..e594160 100644
--- a/libavcodec/rv10.c
+++ b/libavcodec/rv10.c
@@ -801,6 +801,7 @@  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,
@@ -818,6 +819,7 @@  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[]) {
diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
index 6510135..aa5a283 100644
--- a/libavcodec/svq1enc.c
+++ b/libavcodec/svq1enc.c
@@ -528,7 +528,6 @@  static av_cold int svq1_encode_init(AVCodecContext *avctx)
     s->current_picture = av_frame_alloc();
     s->last_picture    = av_frame_alloc();
     if (!s->current_picture || !s->last_picture) {
-        svq1_encode_end(avctx);
         return AVERROR(ENOMEM);
     }
 
@@ -545,7 +544,6 @@  static av_cold int svq1_encode_init(AVCodecContext *avctx)
     s->m.avctx             = avctx;
 
     if ((ret = ff_mpv_common_init(&s->m)) < 0) {
-        svq1_encode_end(avctx);
         return ret;
     }
 
@@ -563,7 +561,6 @@  static av_cold int svq1_encode_init(AVCodecContext *avctx)
 
     if (!s->m.me.temp || !s->m.me.scratchpad || !s->m.me.map ||
         !s->m.me.score_map || !s->mb_type || !s->dummy) {
-        svq1_encode_end(avctx);
         return AVERROR(ENOMEM);
     }
 
@@ -691,6 +688,7 @@  AVCodec ff_svq1_encoder = {
     .init           = svq1_encode_init,
     .encode2        = svq1_encode_frame,
     .close          = svq1_encode_end,
+    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
     .pix_fmts       = (const enum AVPixelFormat[]) { AV_PIX_FMT_YUV410P,
                                                      AV_PIX_FMT_NONE },
 };
diff --git a/libavcodec/wmv2dec.c b/libavcodec/wmv2dec.c
index afa6547..a16c446 100644
--- a/libavcodec/wmv2dec.c
+++ b/libavcodec/wmv2dec.c
@@ -537,6 +537,7 @@  AVCodec ff_wmv2_decoder = {
     .close          = wmv2_decode_end,
     .decode         = ff_h263_decode_frame,
     .capabilities   = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1,
+    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
     .pix_fmts       = (const enum AVPixelFormat[]) { AV_PIX_FMT_YUV420P,
                                                      AV_PIX_FMT_NONE },
 };