[FFmpeg-devel] [PATCH] ffmpeg_opts: remove lowres check

[James Almer]

On 1/21/2021 9:59 AM, Anton Khirnov wrote:
> Quoting James Almer (2021-01-09 18:47:17)
>> The st->codec values are updated based on the lowres factor by
>> avformat_find_stream_info() when it runs an instance of the decoder internally,
>> and the same thing happens in ffmpeg.c when we open ist->dec_ctx with
>> avcodec_open2(), so these assignments are redundant.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> This chunk here is not properly wrapped with the relevant pre-processor check
>> for AVStream->codec, and seeing it's ultimately redundant, i figured we might
>> as well delete it now.
>>
>> For that matter, the deprecation of lowres in avcodec.h is in a very strange
>> state (the field is not removed, its offset is changed instead). Once the value
>> of FF_API_LOWRES is flipped, neither the field, the AVOption, or the usage
>> within decoders will be removed, but some code in libavformat/utils.c will be
>> disabled, and that may result in unexpected behavior.
> 
> IMO it should just be made a codec-private option. And lavf has no
> business treating it specially.

Technically speaking, there's no reason for lavf to check for lowres to 
do what it's currently doing.

The code

>             int orig_w = st->codecpar->width;
>             int orig_h = st->codecpar->height;
>             ret = avcodec_parameters_from_context(st->codecpar, st->internal->avctx);
>             if (ret < 0)
>                 goto find_stream_info_err;
>             ret = add_coded_side_data(st, st->internal->avctx);
>             if (ret < 0)
>                 goto find_stream_info_err;
> #if FF_API_LOWRES
>             // The decoder might reduce the video size by the lowres factor.
>             if (st->internal->avctx->lowres && orig_w) {
>                 st->codecpar->width = orig_w;
>                 st->codecpar->height = orig_h;
>             }
> #endif

Could be simplified to unconditionally set st->codecpar dimensions to 
the backed up ones. If you agree, i can send a patch.

So based on the above, turning it into a codec specific option sounds 
good to me. We could repeat the FF_API_PRIVATE_OPT deprecation mechanism 
for it, but the global option/field needs to stay for another two years 
since neither were truly deprecated (same for AVCodec->max_lowres), just 
wrapped in a pre-processor check.

> 
> Patch fine with me.
> 

Pushed, thanks.