[FFmpeg-devel] [PATCH v2 1/2] avcodec/av1dec: Fix segfault upon allocation error

[James Almer]

On 9/16/2020 9:08 PM, Andreas Rheinhardt wrote:
> Up until now, the AV1 decoder always checks before calling its wrapper
> around ff_thread_release_buffer() whether the ThreadFrame was used at
> all, i.e. it checked whether the first data buffer of the AVFrame
> contained therein is NULL or not. Yet this presumes that the AVFrame has
> been successfully allocated, even though this can of course fail; and if
> it did, one would encounter a segfault.
> Fix this by removing the checks altogether: ff_thread_release_buffer()
> can handle both unallocated as well as empty frames (since commit
> f6774f905fb3cfdc319523ac640be30b14c1bc55).
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> Removing the checks is based upon a suggestion by James Almer. I have
> not removed the checks from the other callers of av1_frame_unref() as I
> don't know how probable it is for the frame to be empty at this point.
> I tested this as well as I can, but I have no hardware with AV1 hardware
> acceleration.

You can remove the check that aborts when no hwaccel is present, which
lets get_format() successfully return a software pixel format. This will
make the decoder "work", simply outputting zeroed buffers into frames,
which is enough to test behavior like the above.

> 
>  libavcodec/av1dec.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index bd8acdaafe..871db76b4d 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -388,12 +388,10 @@ static av_cold int av1_decode_free(AVCodecContext *avctx)
>      AV1DecContext *s = avctx->priv_data;
>  
>      for (int i = 0; i < FF_ARRAY_ELEMS(s->ref); i++) {
> -        if (s->ref[i].tf.f->buf[0])
> -            av1_frame_unref(avctx, &s->ref[i]);
> +        av1_frame_unref(avctx, &s->ref[i]);
>          av_frame_free(&s->ref[i].tf.f);
>      }
> -    if (s->cur_frame.tf.f->buf[0])
> -        av1_frame_unref(avctx, &s->cur_frame);
> +    av1_frame_unref(avctx, &s->cur_frame);
>      av_frame_free(&s->cur_frame.tf.f);
>  
>      av_buffer_unref(&s->seq_ref);
> 

LGTM.