Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: James Almer <jamrial@gmail.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 3/4] lavf/demux: stop calling avcodec_close()
Date: Sat, 3 Feb 2024 00:13:55 -0300
Message-ID: <6c436bf8-cbf3-4882-8ff4-f882ffc8bb46@gmail.com> (raw)
In-Reply-To: <20240201082938.16687-3-anton@khirnov.net>

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) {
_______________________________________________
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".

  parent reply	other threads:[~2024-02-03  3:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-01  8:29 [FFmpeg-devel] [PATCH 1/4] lavf/mpegts: drop a cargo-culted check Anton Khirnov
2024-02-01  8:29 ` [FFmpeg-devel] [PATCH 2/4] lavf/flacdec: stop accessing FFStream.avctx Anton Khirnov
2024-02-01  8:29 ` [FFmpeg-devel] [PATCH 3/4] lavf/demux: stop calling avcodec_close() Anton Khirnov
2024-02-03  2:35   ` Leo Izen
2024-02-05 16:59     ` Anton Khirnov
2024-02-03  3:13   ` James Almer [this message]
2024-02-05 17:03   ` [FFmpeg-devel] [PATCH v2 " Anton Khirnov
2024-02-01  8:29 ` [FFmpeg-devel] [PATCH 4/4] lavc: deprecate avcodec_close() Anton Khirnov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6c436bf8-cbf3-4882-8ff4-f882ffc8bb46@gmail.com \
    --to=jamrial@gmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git