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 <jamr...@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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to