[FFmpeg-devel] [PATCH 2/2] ffmpeg: Made execution of reap_filters() more deterministic with respect to when headers are written

[Aaron Levinson]

Purpose: Made execution of reap_filters() more deterministic with
respect to when headers are written in relationship with the
initialization of output streams and the processing of input audio
and/or video frames.  This change fixes a bug in ffmpeg encountered
when decoding interlaced video.  In some cases, ffmpeg would treat it
as progressive.

Detailed description of problem: Run the following command: "ffmpeg -i
test8_1080i.h264 -c:v mpeg2video test8_1080i_mp2.ts".  In this case,
test8_1080i.h264 is an H.264-encoded file with 1080i59.94
(interlaced).  Prior to the patch, the following output is generated:

Input #0, h264, from 'test8_1080i.h264':
  Duration: N/A, bitrate: N/A
    Stream #0:0: Video: h264 (High), yuv420p(top first), 1920x1080 [SAR 1:1 DAR 16:9], 29.97 fps, 29.97 tbr, 1200k tbn, 59.94 tbc
Stream mapping:
  Stream #0:0 -> #0:0 (h264 (native) -> mpeg2video (native))
Press [q] to stop, [?] for help
Output #0, mpegts, to 'test8_1080i_mp2_2.ts':
  Metadata:
    encoder         : Lavf57.72.100
    Stream #0:0: Video: mpeg2video (Main), yuv420p, 1920x1080 [SAR 1:1 DAR 16:9], q=2-31, 200 kb/s, 29.97 fps, 90k tbn, 29.97 tbc
    Metadata:
      encoder         : Lavc57.92.100 mpeg2video

which demonstrates the bug.  The output stream should instead look like:

    Stream #0:0: Video: mpeg2video (Main), yuv420p(top coded first (swapped)), 1920x1080 [SAR 1:1 DAR 16:9], q=2-31, 200 kb/s, 29.97 fps, 90k tbn, 29.97 tbc

The bug is caused by the fact that reap_filters() calls
init_output_stream(), which is then immediately followed by a call to
check_init_output_file(), and this is all done prior to the first call
to do_video_out().  An initial call to do_video_out() is necessary to
populate the interlaced video information to the output stream's
codecpar (mux_par->field_order in do_video_out()).  However,
check_init_output_file() calls avformat_write_header() prior to the
initial call to do_video_out(), so field_order is populated too late,
and the header is written with the default field_order value,
progressive.

Signed-off-by: Aaron Levinson <alevinsn at aracnet.com>
---
 ffmpeg.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/ffmpeg.c b/ffmpeg.c
index 3cd45ba665..7b044b068c 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -1434,6 +1434,7 @@ static int reap_filters(int flush)
         AVFilterContext *filter;
         AVCodecContext *enc = ost->enc_ctx;
         int ret = 0;
+        int do_check_init_output_file = 0;
 
         if (!ost->filter || !ost->filter->graph->graph)
             continue;
@@ -1448,9 +1449,7 @@ static int reap_filters(int flush)
                 exit_program(1);
             }
 
-            ret = check_init_output_file(of, ost->file_index);
-            if (ret < 0)
-                exit_program(1);
+            do_check_init_output_file = 1;
         }
 
         if (!ost->filtered_frame && !(ost->filtered_frame = av_frame_alloc())) {
@@ -1526,6 +1525,44 @@ static int reap_filters(int flush)
             }
 
             av_frame_unref(filtered_frame);
+
+            /*
+             * It is important to call check_init_output_file() here, after
+             * do_video_out() was called, instead of in init_output_stream(),
+             * as was done previously.
+             * If called from init_output_stream(), it is possible that not
+             * everything will have been fully initialized by the time that
+             * check_init_output_file() is called, and if
+             * check_init_output_file() determines that all output streams
+             * have been initialized, it will write the header.  An example
+             * of initialization that depends on do_video_out() being called
+             * at least once is the specification of interlaced video, which
+             * happens in do_video_out().  This is particularly relevant when
+             * decoding--without processing a video frame, the interlaced
+             * video setting is not propagated before the header is written,
+             * and that causes problems.
+             * TODO:  should probably handle interlaced video differently
+             * and not depend on it being setup in do_video_out().  Another
+             * solution would be to set it up once by examining the input
+             * header.
+             */
+            if (do_check_init_output_file) {
+                ret = check_init_output_file(of, ost->file_index);
+                if (ret < 0)
+                    exit_program(1);
+                do_check_init_output_file = 0;
+            }
+        }
+
+        /*
+         * Can't wait too long to call check_init_output_file().
+         * Otherwise, bad things start to occur.
+         * If didn't do it earlier, do it by the time it gets here.
+         */
+        if (do_check_init_output_file) {
+            ret = check_init_output_file(of, ost->file_index);
+            if (ret < 0)
+                exit_program(1);
         }
     }
 
-- 
2.12.2.windows.2