[FFmpeg-devel] [PATCH] vp9: fix a race in exporting encoding parameters

[James Almer]

On 2/24/2021 6:33 AM, Anton Khirnov wrote:
> Modifying shared state after ff_thread_finish_setup() is not allowed, so
> set the encoding parameters directly on the output frame.

Does this also ensure the side data will be present in 
show_existing_frame frames? This means the "ret == 0" case when 
returning from decode_frame_header(), where it simply outputs an already 
decoded frame again directly from s->s.refs.

See vp90-2-10-show-existing-frame* in samples/vp9-test-vectors/

> 
> Found-by: James Almer <jamrial at gmail.com>

By Valgrind :p

> ---
>   libavcodec/vp9.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> index 4659f94ee8..46062f8a8f 100644
> --- a/libavcodec/vp9.c
> +++ b/libavcodec/vp9.c
> @@ -1499,7 +1499,8 @@ int loopfilter_proc(AVCodecContext *avctx)
>   }
>   #endif
>   
> -static int vp9_export_enc_params(VP9Context *s, VP9Frame *frame)
> +static int vp9_export_enc_params(VP9Context *s, AVFrame *frame_dst,
> +                                 const VP9Frame *frame)
>   {
>       AVVideoEncParams *par;
>       unsigned int tile, nb_blocks = 0;
> @@ -1509,7 +1510,7 @@ static int vp9_export_enc_params(VP9Context *s, VP9Frame *frame)
>               nb_blocks += s->td[tile].nb_block_structure;
>       }
>   
> -    par = av_video_enc_params_create_side_data(frame->tf.f,
> +    par = av_video_enc_params_create_side_data(frame_dst,
>           AV_VIDEO_ENC_PARAMS_VP9, nb_blocks);
>       if (!par)
>           return AVERROR(ENOMEM);
> @@ -1759,12 +1760,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
>           s->td->error_info = 0;
>           return AVERROR_INVALIDDATA;
>       }
> -    if (avctx->export_side_data & AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS) {
> -        ret = vp9_export_enc_params(s, &s->s.frames[CUR_FRAME]);
> -        if (ret < 0)
> -            return ret;
> -    }
> -
>   finish:
>       // ref frame setup
>       for (i = 0; i < 8; i++) {
> @@ -1778,6 +1773,13 @@ finish:
>       if (!s->s.h.invisible) {
>           if ((ret = av_frame_ref(frame, s->s.frames[CUR_FRAME].tf.f)) < 0)
>               return ret;
> +
> +        if (avctx->export_side_data & AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS) {
> +            ret = vp9_export_enc_params(s, frame, &s->s.frames[CUR_FRAME]);
> +            if (ret < 0)
> +                return ret;
> +        }
> +
>           *got_frame = 1;
>       }
>   
>