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

[James Almer]

On 9/29/2020 3:10 PM, Mark Thompson wrote:
> 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

Pushed, thanks.