Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: don't free AVOption settable fields in avcodec_close()

[James Almer <jamrial@gmail.com>]

On 3/20/2022 8:26 PM, Andreas Rheinhardt wrote:
> James Almer:
>> It can uninitialize fields that may still be used after the context was closed,
>> so do it instead in avcodec_free_context().
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/avcodec.c | 1 -
>>   libavcodec/options.c | 2 +-
>>   2 files changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
>> index 38bdaad4fa..122d09b63a 100644
>> --- a/libavcodec/avcodec.c
>> +++ b/libavcodec/avcodec.c
>> @@ -524,7 +524,6 @@ av_cold int avcodec_close(AVCodecContext *avctx)
>>   
>>       if (avctx->priv_data && avctx->codec && avctx->codec->priv_class)
>>           av_opt_free(avctx->priv_data);
>> -    av_opt_free(avctx);
>>       av_freep(&avctx->priv_data);
>>       if (av_codec_is_encoder(avctx->codec)) {
>>           av_freep(&avctx->extradata);
>> diff --git a/libavcodec/options.c b/libavcodec/options.c
>> index 33f11480a7..91335415c1 100644
>> --- a/libavcodec/options.c
>> +++ b/libavcodec/options.c
>> @@ -172,7 +172,7 @@ void avcodec_free_context(AVCodecContext **pavctx)
>>       av_freep(&avctx->intra_matrix);
>>       av_freep(&avctx->inter_matrix);
>>       av_freep(&avctx->rc_override);
>> -    av_channel_layout_uninit(&avctx->ch_layout);
>> +    av_opt_free(avctx);
>>   
>>       av_freep(pavctx);
>>   }
> 
> This will lead to memleaks for users that use avcodec_close(avctx) +
> av_free(avctx) to free an AVCodecContext (e.g. our frame-threaded
> encoders do this). Notice that avcodec_free_context() violates the
> documentation of AVCodecContext.extradata (documented to not be freed
> for decoders) and AVCodecContext.subtitle_header and
> AVCodecContext.rc_override (documented to not be freed by lavc for
> encoders), so there is a reason for using it instead of
> avcodec_free_context() (even when not reusing the context).

That's an absolute mess of a situation. av_free(avctx) should not be an 
allowed or supported scenario when avcodec_free_context() exists. And 
why is the latter violating its own documentation?

> 
> - Andreas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".