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

[James Almer]

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?