[FFmpeg-devel] [PATCH 4/7] lavc: move decoder bsf init into decoder-specific code

[James Almer]

On 3/10/2021 9:03 AM, Anton Khirnov wrote:
> ---
>   libavcodec/decode.c | 8 +++++++-
>   libavcodec/decode.h | 6 ------
>   libavcodec/utils.c  | 6 ------
>   3 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index e5a301ec58..d25b15e95a 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -184,7 +184,7 @@ static int extract_packet_props(AVCodecInternal *avci, const AVPacket *pkt)
>       return 0;
>   }
>   
> -int ff_decode_bsfs_init(AVCodecContext *avctx)
> +static int decode_bsfs_init(AVCodecContext *avctx)
>   {
>       AVCodecInternal *avci = avctx->internal;
>       int ret;
> @@ -2010,6 +2010,8 @@ int ff_reget_buffer(AVCodecContext *avctx, AVFrame *frame, int flags)
>   
>   int ff_decode_preinit(AVCodecContext *avctx)
>   {
> +    int ret = 0;
> +
>       /* if the decoder init function was already called previously,
>        * free the already allocated subtitle_header before overwriting it */
>       av_freep(&avctx->subtitle_header);
> @@ -2046,5 +2048,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>           avctx->export_side_data |= AV_CODEC_EXPORT_DATA_MVS;
>       }
>   
> +    ret = decode_bsfs_init(avctx);

Did you try to run fate with THREADS set to something other than 1? This 
moves the bsfs initialization after ff_thread_init(), so all the copies 
of avctx in frame threading scenarios will be missing the bsfs requested 
by the decoder.

fate-vp9 should be enough to confirm this, since afaik it's the only 
frame threaded decoder that uses a bsf.

> +    if (ret < 0)
> +        return ret;
> +
>       return 0;
>   }
> diff --git a/libavcodec/decode.h b/libavcodec/decode.h
> index a865fe954f..1467b1eb33 100644
> --- a/libavcodec/decode.h
> +++ b/libavcodec/decode.h
> @@ -69,12 +69,6 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt);
>    */
>   int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame);
>   
> -/**
> - * Called during avcodec_open2() to initialize avctx->internal->bsf.
> - * The bsf should be freed with av_bsf_free().
> - */
> -int ff_decode_bsfs_init(AVCodecContext *avctx);
> -
>   /**
>    * Make sure avctx.hw_frames_ctx is set. If it's not set, the function will
>    * try to allocate it from hw_device_ctx. If that is not possible, an error
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 6d5aeb4eaf..918cb1b4d1 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -730,12 +730,6 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
>               goto free_and_end;
>       }
>   
> -    if (av_codec_is_decoder(avctx->codec)) {
> -        ret = ff_decode_bsfs_init(avctx);
> -        if (ret < 0)
> -            goto free_and_end;
> -    }
> -
>       if (HAVE_THREADS
>           && !(avci->frame_thread_encoder && (avctx->active_thread_type&FF_THREAD_FRAME))) {
>           ret = ff_thread_init(avctx);
>