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

[Anton Khirnov]

Quoting James Almer (2021-01-21 14:29:22)
> 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.

I'm not sure that won't have other side effects - the decoder might have
different ideas about dimensions than the demuxer, which might change
something in some obscure cases. I guess try and see if FATE passes?

-- 
Anton Khirnov