[FFmpeg-devel] [PATCH 1/7] avcodec/avcodec: Actually honour the documentation of subtitle_header

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Thu Apr 22 21:50:39 EEST 2021


James Almer:
> On 4/22/2021 2:34 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 4/18/2021 11:00 PM, Andreas Rheinhardt wrote:
>>>> It is only supposed to be freed by libavcodec for decoders, yet
>>>> avcodec_open2() always frees it on failure.
>>>> Furthermore, avcodec_close() doesn't free it for decoders.
>>>> Both of this has been changed.
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>>>> ---
>>>> This might be squashed with the next patch.
>>>>
>>>>    libavcodec/avcodec.c | 7 +++++--
>>>>    1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
>>>> index 760a98d8ef..24f6922d4f 100644
>>>> --- a/libavcodec/avcodec.c
>>>> +++ b/libavcodec/avcodec.c
>>>> @@ -418,7 +418,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>>>          av_dict_free(&tmp);
>>>>        av_freep(&avctx->priv_data);
>>>> -    av_freep(&avctx->subtitle_header);
>>>> +    if (av_codec_is_decoder(avctx->codec))
>>>> +        av_freep(&avctx->subtitle_header);
>>>>      #if FF_API_OLD_ENCDEC
>>>>        av_frame_free(&avci->to_free);
>>>> @@ -589,7 +590,9 @@ FF_DISABLE_DEPRECATION_WARNINGS
>>>>            av_frame_free(&avctx->coded_frame);
>>>>    FF_ENABLE_DEPRECATION_WARNINGS
>>>>    #endif
>>>> -    }
>>>> +    } else if (av_codec_is_decoder(avctx->codec))
>>>> +        av_freep(&avctx->subtitle_header);
>>>> +
>>>>        avctx->codec = NULL;
>>>>        avctx->active_thread_type = 0;
>>>
>>> LGTM. Same should be done for extradata, it seems.
>>
>> You mean adding documentation that avcodec_free_context() frees it
>> despite the documentation saying that this doesn't happen for decoders?
> 
> No, i meant adapting the code to follow the doxy.
> 
Adapting avcodec_free_context() (avcodec_close() and avcodec_open2()
already honour the doxy) would probably break lots of code (i.e.
introduce leaks), so I'd rather opt for adapting the doxy.

- Andreas


More information about the ffmpeg-devel mailing list