On 1/23/2021 3:17 PM, Anton Khirnov wrote:
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 <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.

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?

If i apply the following, the output of three remuxing (codec copy) tests change

diff --git a/libavformat/utils.c b/libavformat/utils.c
index 8ac6bc04b8..0cdf3156a6 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -4110,13 +4110,12 @@ FF_ENABLE_DEPRECATION_WARNINGS
             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) {
+
+            // The decoder might change the video size.
+            if (orig_w && orig_h) {
                 st->codecpar->width = orig_w;
                 st->codecpar->height = orig_h;
             }
-#endif
         }

What i assume happens is that without this change st->codecpar is being populated with dimensions set by whatever decoder was used during probing, and then propagated to the muxer codecpar, whereas with this change the dimensions from the source container are kept unchanged.

Case in point

diff --git a/tests/ref/fate/cbs-vp9-vp90-2-05-resize 
b/tests/ref/fate/cbs-vp9-vp90-2-05-resize
index 8f036bba81..37a37ff1ea 100644
--- a/tests/ref/fate/cbs-vp9-vp90-2-05-resize
+++ b/tests/ref/fate/cbs-vp9-vp90-2-05-resize
@@ -1 +1 @@
-6838422ebb45df353a2bad62b9aff8e9
+1c39300b93fe110e1db30974e5d3479d
diff --git a/tests/ref/fate/redcode-demux b/tests/ref/fate/redcode-demux
index 45119ec71e..c6e0b6de5c 100644
--- a/tests/ref/fate/redcode-demux
+++ b/tests/ref/fate/redcode-demux
@@ -1,7 +1,7 @@
 #tb 0: 1/240000
 #media_type 0: video
 #codec_id 0: jpeg2000
-#dimensions 0: 2048x1152
+#dimensions 0: 4096x2304
 #sar 0: 0/1
 #tb 1: 1/240000
 #media_type 1: audio
diff --git a/tests/ref/fate/wtv-demux b/tests/ref/fate/wtv-demux
index abe85a4ab6..39d395699c 100644
--- a/tests/ref/fate/wtv-demux
+++ b/tests/ref/fate/wtv-demux
@@ -3,7 +3,7 @@
 #tb 0: 1/10000000
 #media_type 0: video
 #codec_id 0: mpeg2video
-#dimensions 0: 720x576
+#dimensions 0: 704x480
 #sar 0: 64/45
 #tb 1: 1/10000000
 #media_type 1: audio
--

I personally think that for a codec copy scenario (Where you could have no decoders at all), this behavior is more consistent. Some samples have different resolution per frame, like that vp9 one, and a decoder could return the one from the last frame probed.
_______________________________________________
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