diff mbox series

[FFmpeg-devel,1/4] avcodec/vp3: Fix memleak upon init failure

Message ID 20201020075356.185676-1-andreas.rheinhardt@gmail.com
State Accepted
Commit a01ca21bbbd41ad86ca58f2c7575c92a36a4b722
Headers show
Series [FFmpeg-devel,1/4] avcodec/vp3: Fix memleak upon init failure
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Oct. 20, 2020, 7:53 a.m. UTC
Up until now, there was no cleanup in case initializing the Theora VLC
tables failed, leading to memleaks. This commit gets rid of them by
setting the FF_CODEC_CAP_INIT_CLEANUP flag for all decoders in vp3.c;
this also allows to remove some (now redundant) cleanup code.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/vp3.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

Comments

Peter Ross Oct. 20, 2020, 9:26 p.m. UTC | #1
On Tue, Oct 20, 2020 at 09:53:52AM +0200, Andreas Rheinhardt wrote:
> Up until now, there was no cleanup in case initializing the Theora VLC
> tables failed, leading to memleaks. This commit gets rid of them by
> setting the FF_CODEC_CAP_INIT_CLEANUP flag for all decoders in vp3.c;
> this also allows to remove some (now redundant) cleanup code.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/vp3.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c
> index 6fe1ca46a3..0d8d1ff80f 100644
> --- a/libavcodec/vp3.c
> +++ b/libavcodec/vp3.c
> @@ -2287,7 +2287,6 @@ static av_cold int allocate_tables(AVCodecContext *avctx)
>          !s->superblock_fragments || !s->macroblock_coding      ||
>          !s->dc_pred_row ||
>          !s->motion_val[0]        || !s->motion_val[1]) {
> -        vp3_decode_end(avctx);
>          return -1;
>      }
>  
> @@ -2302,12 +2301,8 @@ static av_cold int init_frames(Vp3DecodeContext *s)
>      s->last_frame.f    = av_frame_alloc();
>      s->golden_frame.f  = av_frame_alloc();
>  
> -    if (!s->current_frame.f || !s->last_frame.f || !s->golden_frame.f) {
> -        av_frame_free(&s->current_frame.f);
> -        av_frame_free(&s->last_frame.f);
> -        av_frame_free(&s->golden_frame.f);
> +    if (!s->current_frame.f || !s->last_frame.f || !s->golden_frame.f)
>          return AVERROR(ENOMEM);
> -    }
>  
>      return 0;
>  }
> @@ -3221,7 +3216,8 @@ AVCodec ff_theora_decoder = {
>                               AV_CODEC_CAP_FRAME_THREADS,
>      .flush                 = vp3_decode_flush,
>      .update_thread_context = ONLY_IF_THREADS_ENABLED(vp3_update_thread_context),
> -    .caps_internal         = FF_CODEC_CAP_EXPORTS_CROPPING | FF_CODEC_CAP_ALLOCATE_PROGRESS,
> +    .caps_internal         = FF_CODEC_CAP_EXPORTS_CROPPING | FF_CODEC_CAP_ALLOCATE_PROGRESS |
> +                             FF_CODEC_CAP_INIT_CLEANUP,
>  };
>  #endif
>  
> @@ -3238,7 +3234,7 @@ AVCodec ff_vp3_decoder = {
>                               AV_CODEC_CAP_FRAME_THREADS,
>      .flush                 = vp3_decode_flush,
>      .update_thread_context = ONLY_IF_THREADS_ENABLED(vp3_update_thread_context),
> -    .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS,
> +    .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP,
>  };
>  
>  #if CONFIG_VP4_DECODER
> @@ -3255,6 +3251,6 @@ AVCodec ff_vp4_decoder = {
>                               AV_CODEC_CAP_FRAME_THREADS,
>      .flush                 = vp3_decode_flush,
>      .update_thread_context = ONLY_IF_THREADS_ENABLED(vp3_update_thread_context),
> -    .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS,
> +    .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP,
>  };
>  #endif
> -- 
> 2.25.1

looks good

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
diff mbox series

Patch

diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c
index 6fe1ca46a3..0d8d1ff80f 100644
--- a/libavcodec/vp3.c
+++ b/libavcodec/vp3.c
@@ -2287,7 +2287,6 @@  static av_cold int allocate_tables(AVCodecContext *avctx)
         !s->superblock_fragments || !s->macroblock_coding      ||
         !s->dc_pred_row ||
         !s->motion_val[0]        || !s->motion_val[1]) {
-        vp3_decode_end(avctx);
         return -1;
     }
 
@@ -2302,12 +2301,8 @@  static av_cold int init_frames(Vp3DecodeContext *s)
     s->last_frame.f    = av_frame_alloc();
     s->golden_frame.f  = av_frame_alloc();
 
-    if (!s->current_frame.f || !s->last_frame.f || !s->golden_frame.f) {
-        av_frame_free(&s->current_frame.f);
-        av_frame_free(&s->last_frame.f);
-        av_frame_free(&s->golden_frame.f);
+    if (!s->current_frame.f || !s->last_frame.f || !s->golden_frame.f)
         return AVERROR(ENOMEM);
-    }
 
     return 0;
 }
@@ -3221,7 +3216,8 @@  AVCodec ff_theora_decoder = {
                              AV_CODEC_CAP_FRAME_THREADS,
     .flush                 = vp3_decode_flush,
     .update_thread_context = ONLY_IF_THREADS_ENABLED(vp3_update_thread_context),
-    .caps_internal         = FF_CODEC_CAP_EXPORTS_CROPPING | FF_CODEC_CAP_ALLOCATE_PROGRESS,
+    .caps_internal         = FF_CODEC_CAP_EXPORTS_CROPPING | FF_CODEC_CAP_ALLOCATE_PROGRESS |
+                             FF_CODEC_CAP_INIT_CLEANUP,
 };
 #endif
 
@@ -3238,7 +3234,7 @@  AVCodec ff_vp3_decoder = {
                              AV_CODEC_CAP_FRAME_THREADS,
     .flush                 = vp3_decode_flush,
     .update_thread_context = ONLY_IF_THREADS_ENABLED(vp3_update_thread_context),
-    .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS,
+    .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP,
 };
 
 #if CONFIG_VP4_DECODER
@@ -3255,6 +3251,6 @@  AVCodec ff_vp4_decoder = {
                              AV_CODEC_CAP_FRAME_THREADS,
     .flush                 = vp3_decode_flush,
     .update_thread_context = ONLY_IF_THREADS_ENABLED(vp3_update_thread_context),
-    .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS,
+    .caps_internal         = FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP,
 };
 #endif