[FFmpeg-devel] [PATCH 5/8] avcodec/av1dec: parse dimensions from the sequence header in extradata

[Mark Thompson]

On 29/09/2020 17:23, James Almer wrote:
> On 9/29/2020 12:52 PM, Mark Thompson wrote:
>> On 25/09/2020 15:43, James Almer wrote:
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>>    libavcodec/av1dec.c | 10 ++++++++++
>>>    1 file changed, 10 insertions(+)
>>>
>>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>>> index 0bb04a3e44..f6b9fbbac3 100644
>>> --- a/libavcodec/av1dec.c
>>> +++ b/libavcodec/av1dec.c
>>> @@ -405,6 +405,9 @@ static av_cold int av1_decode_free(AVCodecContext
>>> *avctx)
>>>    static int set_context_with_sequence(AVCodecContext *avctx,
>>>                                         const AV1RawSequenceHeader *seq)
>>>    {
>>> +    int width = seq->max_frame_width_minus_1 + 1;
>>> +    int height = seq->max_frame_height_minus_1 + 1;
>>> +
>>>        avctx->profile = seq->seq_profile;
>>>        avctx->level = seq->seq_level_idx[0];
>>>    @@ -423,6 +426,13 @@ static int
>>> set_context_with_sequence(AVCodecContext *avctx,
>>>            break;
>>>        }
>>>    +    if (avctx->width != width || avctx->height != height) {
>>> +        int ret = ff_set_dimensions(avctx, width, height);
>>> +        if (ret < 0)
>>> +            return ret;
>>> +    }
>>> +    avctx->sample_aspect_ratio = (AVRational) { 1, 1 };
>>> +
>>>        if (seq->timing_info.num_units_in_display_tick &&
>>>            seq->timing_info.time_scale) {
>>>            av_reduce(&avctx->framerate.den, &avctx->framerate.num,
>>>
>>
>> Can you explain more about why this might be wanted?  There is no
>> requirement that the stream actually contains any frames with the size
>> stated in the sequence header.
> 
> It gives you an AVCodecContext with enough parameters for most non
> decoding scenarios immediately after avcodec_open2(). They will be
> updated with frame dimensions afterwards if any is parsed.
> Also, the AVCodecContext dimension fields are nonetheless documented to
> not necessarily match the latest returned frame's dimensions.
> 
> Applying this set without this patch, if this is the only decoder
> available and is therefore used for probing, some of the remuxing FATE
> tests will fail once a pix_fmt is not assigned and frames are then not
> parsed.

Ok, that seems fair enough.  Cases where this number is actually wrong are hopefully rare, anyway.

- Mark