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

[Dave Airlie]

On Thu, 22 Jun 2023 at 07:36, James Almer <jamrial at gmail.com> wrote:
>
> On 6/21/2023 6:15 PM, Dave Airlie wrote:
> > On Thu, 22 Jun 2023 at 02:36, James Almer <jamrial at gmail.com> wrote:
> >>
> >> On 6/20/2023 8:36 PM, 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,
> >>>                                         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;
> >>> +            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);
> >>
> >> Dimensions can change between frames without a seq header showing up to
> >> change the max_width/height values. get_pixel_format() would need to be
> >> called on frame headers instead.
> >
> > It can but my reading of the spec is that it is illegal to go beyond
> > the max in sequence header.
> >
> > 6.8 Frame header OBU semantics
> > 6.8.4 Frame size semantics
> >
> > "
> > It is a requirement of bitstream conformance that frame_width_minus_1
> > is less than or equal to max_frame_width_minus_1.
> > It is a requirement of bitstream conformance that frame_height_minus_1
> > is less than or equal to max_frame_height_minus_1.
> > "
> >
> > Dave.
>
> So Vulkan always allocates buffers for the max allowed dimensions and
> not for the current frame's dimensions?

It doesn't seem to be in vulkan specific code:

av1dec.c:set_context_with_sequence
    int width = seq->max_frame_width_minus_1 + 1;
    int height = seq->max_frame_height_minus_1 + 1;

is where it sets the values later used to allocate the frames.

Dave.