diff mbox

[FFmpeg-devel,4/5] avcodec/mediacodecdec: use ff_hevc_uninit_parameter_sets()

Message ID 20180120211253.9676-4-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer Jan. 20, 2018, 9:12 p.m. UTC
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/mediacodecdec.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

wm4 Jan. 20, 2018, 11 p.m. UTC | #1
On Sat, 20 Jan 2018 18:12:52 -0300
James Almer <jamrial@gmail.com> wrote:

> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/mediacodecdec.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
> index 6c5d3ddd79..b360e7a7f1 100644
> --- a/libavcodec/mediacodecdec.c
> +++ b/libavcodec/mediacodecdec.c
> @@ -258,6 +258,8 @@ static int hevc_set_extradata(AVCodecContext *avctx, FFAMediaFormat *format)
>      }
>  
>  done:
> +    ff_hevc_uninit_parameter_sets(&ps);
> +
>      av_freep(&vps_data);
>      av_freep(&sps_data);
>      av_freep(&pps_data);

Did this leak before? Or is it only needed to make it work with
patch 5/5?
James Almer Jan. 20, 2018, 11:02 p.m. UTC | #2
On 1/20/2018 8:00 PM, wm4 wrote:
> On Sat, 20 Jan 2018 18:12:52 -0300
> James Almer <jamrial@gmail.com> wrote:
> 
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libavcodec/mediacodecdec.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
>> index 6c5d3ddd79..b360e7a7f1 100644
>> --- a/libavcodec/mediacodecdec.c
>> +++ b/libavcodec/mediacodecdec.c
>> @@ -258,6 +258,8 @@ static int hevc_set_extradata(AVCodecContext *avctx, FFAMediaFormat *format)
>>      }
>>  
>>  done:
>> +    ff_hevc_uninit_parameter_sets(&ps);
>> +
>>      av_freep(&vps_data);
>>      av_freep(&sps_data);
>>      av_freep(&pps_data);
> 
> Did this leak before? Or is it only needed to make it work with
> patch 5/5?

It most likely leaked before. Notice how hevc parser and decoder both
free the buffers, and even mediacodecdec here does it with h264.

I can't test this module in any case, so someone else will have to
confirm this.
James Almer Jan. 30, 2018, 4:27 p.m. UTC | #3
On 1/20/2018 8:02 PM, James Almer wrote:
> On 1/20/2018 8:00 PM, wm4 wrote:
>> On Sat, 20 Jan 2018 18:12:52 -0300
>> James Almer <jamrial@gmail.com> wrote:
>>
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>>  libavcodec/mediacodecdec.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
>>> index 6c5d3ddd79..b360e7a7f1 100644
>>> --- a/libavcodec/mediacodecdec.c
>>> +++ b/libavcodec/mediacodecdec.c
>>> @@ -258,6 +258,8 @@ static int hevc_set_extradata(AVCodecContext *avctx, FFAMediaFormat *format)
>>>      }
>>>  
>>>  done:
>>> +    ff_hevc_uninit_parameter_sets(&ps);
>>> +
>>>      av_freep(&vps_data);
>>>      av_freep(&sps_data);
>>>      av_freep(&pps_data);
>>
>> Did this leak before? Or is it only needed to make it work with
>> patch 5/5?
> 
> It most likely leaked before. Notice how hevc parser and decoder both
> free the buffers, and even mediacodecdec here does it with h264.
> 
> I can't test this module in any case, so someone else will have to
> confirm this.

Pushed. It's pretty clear there are memleaks without it.
diff mbox

Patch

diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
index 6c5d3ddd79..b360e7a7f1 100644
--- a/libavcodec/mediacodecdec.c
+++ b/libavcodec/mediacodecdec.c
@@ -258,6 +258,8 @@  static int hevc_set_extradata(AVCodecContext *avctx, FFAMediaFormat *format)
     }
 
 done:
+    ff_hevc_uninit_parameter_sets(&ps);
+
     av_freep(&vps_data);
     av_freep(&sps_data);
     av_freep(&pps_data);