[FFmpeg-devel] avcodec/vaapi:free slice_buffers when decoding failed

Submitted by Linjie Fu on Sept. 14, 2018, 7:25 a.m.

Details

Message ID 20180914072534.9737-1-linjie.fu@intel.com
State New
Headers show

Commit Message

Linjie Fu Sept. 14, 2018, 7:25 a.m.
If vaEndPicture failed in ff_vaapi_decode_issue, free
the pic->slice_buffer.

Fix the memory leak issue in ticket #7385

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
 libavcodec/vaapi_decode.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Mark Thompson Sept. 18, 2018, 10:44 p.m.
On 14/09/18 08:25, Linjie Fu wrote:
> If vaEndPicture failed in ff_vaapi_decode_issue, free
> the pic->slice_buffer.
> 
> Fix the memory leak issue in ticket #7385
> 
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
>  libavcodec/vaapi_decode.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
> index d0a6b5817d..700cd5c681 100644
> --- a/libavcodec/vaapi_decode.c
> +++ b/libavcodec/vaapi_decode.c
> @@ -216,6 +216,11 @@ fail_with_picture:
>  fail:
>      ff_vaapi_decode_destroy_buffers(avctx, pic);
>  fail_at_end:
> +    pic->nb_param_buffers = 0;
> +    pic->nb_slices        = 0;
> +    pic->slices_allocated = 0;
> +    av_freep(&pic->slice_buffers);
> +
>      return err;
>  }

Makes sense, but would you mind uniting the two return paths rather than duplicating the code?

Thanks,

- Mark
Linjie Fu Sept. 19, 2018, 1:50 a.m.
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Mark Thompson

> Sent: Wednesday, September 19, 2018 06:45

> To: ffmpeg-devel@ffmpeg.org

> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/vaapi:free slice_buffers when

> decoding failed

> 

> On 14/09/18 08:25, Linjie Fu wrote:

> > If vaEndPicture failed in ff_vaapi_decode_issue, free

> > the pic->slice_buffer.

> >

> > Fix the memory leak issue in ticket #7385

> >

> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>

> > ---

> >  libavcodec/vaapi_decode.c | 5 +++++

> >  1 file changed, 5 insertions(+)

> >

> > diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c

> > index d0a6b5817d..700cd5c681 100644

> > --- a/libavcodec/vaapi_decode.c

> > +++ b/libavcodec/vaapi_decode.c

> > @@ -216,6 +216,11 @@ fail_with_picture:

> >  fail:

> >      ff_vaapi_decode_destroy_buffers(avctx, pic);

> >  fail_at_end:

> > +    pic->nb_param_buffers = 0;

> > +    pic->nb_slices        = 0;

> > +    pic->slices_allocated = 0;

> > +    av_freep(&pic->slice_buffers);

> > +

> >      return err;

> >  }

> 

> Makes sense, but would you mind uniting the two return paths rather than

> duplicating the code?

> 

> Thanks,

> 

> - Mark

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Thanks for your review. I'll commit [V2] to unit the two return paths under an "exit"
 tag at the end of the function.

Actually, I've thought about this problem, and I  think it's a trade-off between code
duplication and the code formatting consistency of  other function in vaapi_decode.c
(which have two return paths). 

Thanks,
Linjie Fu

Patch hide | download patch | download mbox

diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
index d0a6b5817d..700cd5c681 100644
--- a/libavcodec/vaapi_decode.c
+++ b/libavcodec/vaapi_decode.c
@@ -216,6 +216,11 @@  fail_with_picture:
 fail:
     ff_vaapi_decode_destroy_buffers(avctx, pic);
 fail_at_end:
+    pic->nb_param_buffers = 0;
+    pic->nb_slices        = 0;
+    pic->slices_allocated = 0;
+    av_freep(&pic->slice_buffers);
+
     return err;
 }