[FFmpeg-devel] [PATCH] av1dec: handle dimension changes via get_format

[Leo Izen]

On 6/20/23 19:36, airlied at gmail.com wrote:
> From: Dave Airlie <airlied at redhat.com>
> 
> av1-1-b8-03-sizeup.ivf on vulkan causes gpu hangs as none of the
> images get resized when dimensions change, this detects the dim
> change and calls the get_format to reinit the context.
> ---
>   libavcodec/av1dec.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index e7f98a6c81..1cec328563 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -721,6 +721,7 @@ static av_cold int av1_decode_free(AVCodecContext *avctx)
>   }
>   
>   static int set_context_with_sequence(AVCodecContext *avctx,
> +                                     int *dim_change,

I would add a comment above this function documenting that dim_change 
may be NULL, in which case it will be ignored. This is not a functional 
issue, it's just comments = readable = good.

Also, nitpick, but this fits on the previous line (77 wide, less than 
80). Ignore this if you think a new line is more readable, just keep in 
mind.

>                                        const AV1RawSequenceHeader *seq)
>   {
>       int width = seq->max_frame_width_minus_1 + 1;
> @@ -753,6 +754,8 @@ static int set_context_with_sequence(AVCodecContext *avctx,
>           int ret = ff_set_dimensions(avctx, width, height);
>           if (ret < 0)
>               return ret;
> +        if (dim_change)
> +            *dim_change = 1;
>       }
>       avctx->sample_aspect_ratio = (AVRational) { 1, 1 };
>   
> @@ -859,7 +862,7 @@ static av_cold int av1_decode_init(AVCodecContext *avctx)
>               goto end;
>           }
>   
> -        ret = set_context_with_sequence(avctx, seq);
> +        ret = set_context_with_sequence(avctx, NULL, seq);
>           if (ret < 0) {
>               av_log(avctx, AV_LOG_WARNING, "Failed to set decoder context.\n");
>               goto end;
> @@ -1202,7 +1205,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>           CodedBitstreamUnit *unit = &s->current_obu.units[i];
>           AV1RawOBU *obu = unit->content;
>           const AV1RawOBUHeader *header;
> -
> +        int dim_change = 0;
>           if (!obu)
>               continue;
>   
> @@ -1220,7 +1223,8 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>   
>               s->raw_seq = &obu->obu.sequence_header;
>   
> -            ret = set_context_with_sequence(avctx, s->raw_seq);
> +            dim_change = 0;

You don't need to initialize this variable to zero here, because you 
initialized it to zero when you declared it. If you prefer to leave this 
here, then remove the `= 0;` upon declaration.

> +            ret = set_context_with_sequence(avctx, &dim_change, s->raw_seq);
>               if (ret < 0) {
>                   av_log(avctx, AV_LOG_ERROR, "Failed to set context.\n");
>                   s->raw_seq = NULL;
> @@ -1229,7 +1233,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>   
>               s->operating_point_idc = s->raw_seq->operating_point_idc[s->operating_point];
>   
> -            if (s->pix_fmt == AV_PIX_FMT_NONE) {
> +            if (s->pix_fmt == AV_PIX_FMT_NONE || dim_change) {
>                   ret = get_pixel_format(avctx);
>                   if (ret < 0) {
>                       av_log(avctx, AV_LOG_ERROR,

- Leo Izen (Traneptora)