[FFmpeg-devel] [PATCH 3/4] lavf/demux: stop calling avcodec_close()

[James Almer]

On 2/1/2024 5:29 AM, Anton Khirnov wrote:
> Replace it with recreating the codec context.
> 
> This is the last remaining blocker for deprecating avcodec_close().
> ---
>   libavformat/demux.c | 53 ++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/demux.c b/libavformat/demux.c
> index 6f640b92b1..c1640c459c 100644
> --- a/libavformat/demux.c
> +++ b/libavformat/demux.c
> @@ -1250,6 +1250,52 @@ static int64_t ts_to_samples(AVStream *st, int64_t ts)
>       return av_rescale(ts, st->time_base.num * st->codecpar->sample_rate, st->time_base.den);
>   }
>   
> +static int codec_close(FFStream *sti)

This returns an error code but you're not checking it.

> +{
> +    AVCodecContext *avctx_new = NULL;
> +    AVCodecParameters *par_tmp = NULL;
> +    int ret = 0;
> +
> +    avctx_new = avcodec_alloc_context3(sti->avctx->codec);
> +    if (!avctx_new) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }
> +
> +    par_tmp = avcodec_parameters_alloc();
> +    if (!par_tmp) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }
> +
> +    ret = avcodec_parameters_from_context(par_tmp, sti->avctx);
> +    if (ret < 0)
> +        goto fail;
> +
> +    ret = avcodec_parameters_to_context(avctx_new, par_tmp);
> +    if (ret < 0)
> +        goto fail;
> +
> +    avctx_new->pkt_timebase = sti->avctx->pkt_timebase;
> +
> +#if FF_API_TICKS_PER_FRAME
> +FF_DISABLE_DEPRECATION_WARNINGS
> +    avctx_new->ticks_per_frame = sti->avctx->ticks_per_frame;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> +
> +    avcodec_free_context(&sti->avctx);
> +    sti->avctx = avctx_new;
> +
> +    avctx_new = NULL;
> +
> +fail:
> +    avcodec_free_context(&avctx_new);
> +    avcodec_parameters_free(&par_tmp);
> +
> +    return ret;

For success scenarios, this will return whatever was last set on ret, 
which in this case is the return value of 
avcodec_parameters_to_context() that just happens to be 0. But something 
else could be added later that also sets ret after it, so the proper 
thing to do is to set ret to 0 immediately above the fail label.

> +}
> +
>   static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
>   {
>       FFFormatContext *const si = ffformatcontext(s);
> @@ -1286,7 +1332,7 @@ static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
>           if (sti->need_context_update) {
>               if (avcodec_is_open(sti->avctx)) {
>                   av_log(s, AV_LOG_DEBUG, "Demuxer context update while decoder is open, closing and trying to re-open\n");
> -                avcodec_close(sti->avctx);
> +                codec_close(sti);
>                   sti->info->found_decoder = 0;
>               }
>   
> @@ -3017,10 +3063,7 @@ find_stream_info_err:
>               av_freep(&sti->info->duration_error);
>               av_freep(&sti->info);
>           }
> -        avcodec_close(sti->avctx);
> -        // FIXME: avcodec_close() frees AVOption settable fields which includes ch_layout,
> -        //        so we need to restore it.
> -        av_channel_layout_copy(&sti->avctx->ch_layout, &st->codecpar->ch_layout);
> +        codec_close(sti);
>           av_bsf_free(&sti->extract_extradata.bsf);
>       }
>       if (ic->pb) {