diff mbox series

[FFmpeg-devel,v3,5/6] ffmpeg: move field order decision making to encoder initialization

Message ID 20201016131649.4361-6-jeebjp@gmail.com
State Superseded
Headers show
Series ffmpeg: late A/V encoder init, AVFrame metadata usage | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

Jan Ekström Oct. 16, 2020, 1:16 p.m. UTC 
We now have the possibility of getting AVFrames here, and we should
not touch the muxer's codecpar after writing the header.
---
 fftools/ffmpeg.c                              | 27 ++++++++++---------
 .../fate/concat-demuxer-extended-lavf-mxf_d10 |  2 +-
 .../fate/concat-demuxer-simple1-lavf-mxf_d10  |  2 +-
 tests/ref/fate/rgb24-mkv                      |  4 +--
 tests/ref/lavf/mxf_d10                        |  2 +-
 5 files changed, 19 insertions(+), 18 deletions(-)

Comments

Anton Khirnov Oct. 27, 2020, 11:13 a.m. UTC | #1 
Quoting Jan Ekström (2020-10-16 15:16:48)
> We now have the possibility of getting AVFrames here, and we should
> not touch the muxer's codecpar after writing the header.
> ---
>  fftools/ffmpeg.c                              | 27 ++++++++++---------
>  .../fate/concat-demuxer-extended-lavf-mxf_d10 |  2 +-
>  .../fate/concat-demuxer-simple1-lavf-mxf_d10  |  2 +-
>  tests/ref/fate/rgb24-mkv                      |  4 +--
>  tests/ref/lavf/mxf_d10                        |  2 +-
>  5 files changed, 19 insertions(+), 18 deletions(-)

Why do the tests change?
Jan Ekström Oct. 27, 2020, 2:59 p.m. UTC | #2 
On Tue, Oct 27, 2020, 13:13 Anton Khirnov <anton@khirnov.net> wrote:
>
> Quoting Jan Ekström (2020-10-16 15:16:48)
> > We now have the possibility of getting AVFrames here, and we should
> > not touch the muxer's codecpar after writing the header.
> > ---
> >  fftools/ffmpeg.c                              | 27 ++++++++++---------
> >  .../fate/concat-demuxer-extended-lavf-mxf_d10 |  2 +-
> >  .../fate/concat-demuxer-simple1-lavf-mxf_d10  |  2 +-
> >  tests/ref/fate/rgb24-mkv                      |  4 +--
> >  tests/ref/lavf/mxf_d10                        |  2 +-
> >  5 files changed, 19 insertions(+), 18 deletions(-)
>
> Why do the tests change?

1. Matroska and mxf write interlaced/progressive field
2. Before the codecpar was prodded after init/write_header was
executed for muxer.
3. Now the codecpar is properly prodded before init/write_header is
executed for muxer and the encoder itself is initialized.

So basically I verified that all the changes seemed to make sense.
Will add a remark about it to the commit message This logic already
existed, and now it is just done at the proper spot.

Jan
diff mbox series

Patch

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index b2e210c814..1c95890f08 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1117,7 +1117,6 @@  static void do_video_out(OutputFile *of,
     int ret, format_video_sync;
     AVPacket pkt;
     AVCodecContext *enc = ost->enc_ctx;
-    AVCodecParameters *mux_par = ost->st->codecpar;
     AVRational frame_rate;
     int nb_frames, nb0_frames, i;
     double delta, delta0;
@@ -1279,18 +1278,6 @@  static void do_video_out(OutputFile *of,
         if (!check_recording_time(ost))
             return;
 
-        if (enc->flags & (AV_CODEC_FLAG_INTERLACED_DCT | AV_CODEC_FLAG_INTERLACED_ME) &&
-            ost->top_field_first >= 0)
-            in_picture->top_field_first = !!ost->top_field_first;
-
-        if (in_picture->interlaced_frame) {
-            if (enc->codec->id == AV_CODEC_ID_MJPEG)
-                mux_par->field_order = in_picture->top_field_first ? AV_FIELD_TT:AV_FIELD_BB;
-            else
-                mux_par->field_order = in_picture->top_field_first ? AV_FIELD_TB:AV_FIELD_BT;
-        } else
-            mux_par->field_order = AV_FIELD_PROGRESSIVE;
-
         in_picture->quality = enc->global_quality;
         in_picture->pict_type = 0;
 
@@ -3435,6 +3422,20 @@  static int init_output_stream_encode(OutputStream *ost, AVFrame *frame)
             enc_ctx->field_order = AV_FIELD_TT;
         }
 
+        if (frame) {
+            if (enc_ctx->flags & (AV_CODEC_FLAG_INTERLACED_DCT | AV_CODEC_FLAG_INTERLACED_ME) &&
+                ost->top_field_first >= 0)
+                frame->top_field_first = !!ost->top_field_first;
+
+            if (frame->interlaced_frame) {
+                if (enc_ctx->codec->id == AV_CODEC_ID_MJPEG)
+                    enc_ctx->field_order = frame->top_field_first ? AV_FIELD_TT:AV_FIELD_BB;
+                else
+                    enc_ctx->field_order = frame->top_field_first ? AV_FIELD_TB:AV_FIELD_BT;
+            } else
+                enc_ctx->field_order = AV_FIELD_PROGRESSIVE;
+        }
+
         if (ost->forced_keyframes) {
             if (!strncmp(ost->forced_keyframes, "expr:", 5)) {
                 ret = av_expr_parse(&ost->forced_keyframes_pexpr, ost->forced_keyframes+5,
diff --git a/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 b/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
index e3e76f217a..f6efc00ca4 100644
--- a/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
+++ b/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
@@ -1 +1 @@ 
-d66177ea3922692bc91cd0f8aa907650 *tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe
+84496cfe2d668db395280ea67e5c6fbe *tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe
diff --git a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
index 79ce1e2306..8f3f2e5265 100644
--- a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
+++ b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
@@ -78,5 +78,5 @@  video|0|34|1.360000|34|1.360000|1|0.040000|N/A|N/A|150000|1924096|K_|1
 Strings Metadata
 audio|1|65280|1.360000|65280|1.360000|1920|0.040000|N/A|N/A|7680|2074624|K_|1
 Strings Metadata
-0|mpeg2video|0|video|1/25|[0][0][0][0]|0x0000|720|608|0|0|0|0|1:1|45:38|yuv422p|5|tv|unknown|unknown|unknown|topleft|tt|N/A|1|N/A|25/1|25/1|1/25|0|0.000000|N/A|N/A|30000000|N/A|N/A|N/A|N/A|35|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001300000000000000000000000000000000000001
+0|mpeg2video|0|video|1/25|[0][0][0][0]|0x0000|720|608|0|0|0|0|1:1|45:38|yuv422p|5|tv|unknown|unknown|unknown|topleft|tb|N/A|1|N/A|25/1|25/1|1/25|0|0.000000|N/A|N/A|30000000|N/A|N/A|N/A|N/A|35|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001300000000000000000000000000000000000001
 1|pcm_s16le|unknown|audio|1/48000|[0][0][0][0]|0x0000|s16|48000|2|unknown|16|N/A|0/0|0/0|1/48000|0|0.000000|N/A|N/A|1536000|N/A|N/A|N/A|N/A|35|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001300000000000000000000000000000000000001
diff --git a/tests/ref/fate/rgb24-mkv b/tests/ref/fate/rgb24-mkv
index 34d028cbfd..3b14cd0ef0 100644
--- a/tests/ref/fate/rgb24-mkv
+++ b/tests/ref/fate/rgb24-mkv
@@ -1,5 +1,5 @@ 
-fdc02d700dbe99315a9f0d928a9b935e *tests/data/fate/rgb24-mkv.matroska
-58213 tests/data/fate/rgb24-mkv.matroska
+fde8903c4df0ba8235dafcfd8a2f368c *tests/data/fate/rgb24-mkv.matroska
+58216 tests/data/fate/rgb24-mkv.matroska
 #tb 0: 1/10
 #media_type 0: video
 #codec_id 0: rawvideo
diff --git a/tests/ref/lavf/mxf_d10 b/tests/ref/lavf/mxf_d10
index 85e337d157..30701619e0 100644
--- a/tests/ref/lavf/mxf_d10
+++ b/tests/ref/lavf/mxf_d10
@@ -1,3 +1,3 @@ 
-36fc2a640368f6d33987d2b2d39df966 *tests/data/lavf/lavf.mxf_d10
+da0ebbebb50a530b14c0f06017f464b3 *tests/data/lavf/lavf.mxf_d10
 5332013 tests/data/lavf/lavf.mxf_d10
 tests/data/lavf/lavf.mxf_d10 CRC=0x6c74d488