[FFmpeg-devel] [PATCH 2/2] avcodec/options: do a more thorough clean up in avcodec_copy_context()

[James Almer]

On 4/30/2017 11:10 PM, Michael Niedermayer wrote:
> On Sun, Apr 30, 2017 at 10:07:17AM -0700, Aaron Levinson wrote:
>> On 4/26/2017 12:47 PM, James Almer wrote:
>>> On 4/26/2017 2:46 AM, Aaron Levinson wrote:
>>>> On 4/24/2017 3:47 PM, James Almer wrote:
>>>>> Free coded_frame, coded_side_data and unref hw_device_ctx to prevent
>>>>> potential leaks.
>>>>>
>>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>>> ---
>>>>> libavcodec/options.c | 15 +++++++++++++++
>>>>> 1 file changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/libavcodec/options.c b/libavcodec/options.c
>>>>> index b98da9378a..82e12179a6 100644
>>>>> --- a/libavcodec/options.c
>>>>> +++ b/libavcodec/options.c
>>>>> @@ -190,14 +190,26 @@ void avcodec_free_context(AVCodecContext **pavctx)
>>>>> #if FF_API_COPY_CONTEXT
>>>>> static void copy_context_reset(AVCodecContext *avctx)
>>>>> {
>>>>> +    int i;
>>>>> +
>>>>>     av_opt_free(avctx);
>>>>> +#if FF_API_CODED_FRAME
>>>>> +FF_DISABLE_DEPRECATION_WARNINGS
>>>>> +    av_frame_free(&avctx->coded_frame);
>>>>> +FF_ENABLE_DEPRECATION_WARNINGS
>>>>> +#endif
>>>>>     av_freep(&avctx->rc_override);
>>>>>     av_freep(&avctx->intra_matrix);
>>>>>     av_freep(&avctx->inter_matrix);
>>>>>     av_freep(&avctx->extradata);
>>>>>     av_freep(&avctx->subtitle_header);
>>>>>     av_buffer_unref(&avctx->hw_frames_ctx);
>>>>> +    av_buffer_unref(&avctx->hw_device_ctx);
>>>>> +    for (i = 0; i < avctx->nb_coded_side_data; i++)
>>>>> +        av_freep(&avctx->coded_side_data[i].data);
>>>>> +    av_freep(&avctx->coded_side_data);
>>>>>     avctx->subtitle_header_size = 0;
>>>>> +    avctx->nb_coded_side_data = 0;
>>>>>     avctx->extradata_size = 0;
>>>>> }
>>>>>
>>>>> @@ -238,11 +250,14 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>>>>
>>>>>     /* reallocate values that should be allocated separately */
>>>>>     dest->extradata       = NULL;
>>>>> +    dest->coded_side_data = NULL;
>>>>>     dest->intra_matrix    = NULL;
>>>>>     dest->inter_matrix    = NULL;
>>>>>     dest->rc_override     = NULL;
>>>>>     dest->subtitle_header = NULL;
>>>>>     dest->hw_frames_ctx   = NULL;
>>>>> +    dest->hw_device_ctx   = NULL;
>>>>> +    dest->nb_coded_side_data = 0;
>>>>>
>>>>> #define alloc_and_copy_or_fail(obj, size, pad) \
>>>>>     if (src->obj && size > 0) { \
>>>>>
>>>>
>>>> I'm not sure if this patch is intended to be a replacement for your last
>>>> "2/2" patch, but this version is missing the the coded_side_data
>>>> population code that was in the first version of your "2/2" patch.
>>>>
>>>> Aaron Levinson
>>>
>>> It is. I'm keeping the functionality of the function as is, and only
>>> making sure cleanup is complete to prevent leaks.
>>> This function is deprecated and shouldn't be used, so i'm not going to
>>> make it copy even more stuff.
>>
>> I've reviewed both patches 1 and 2, and both are good to commit.
> 
> and i couldnt find a case the patches break
> 
> [...]
> 
> thx

Pushed, thanks.