[FFmpeg-devel] [PATCH] Fixed bug encountered when decoding interlaced video

Aaron Levinson alevinsn at aracnet.com
Sun Mar 26 12:50:59 EEST 2017


Hopefully I went through the patch process correctly, as this is the first time that I've done this.  I didn't submit a ticket regarding this issue, but a detailed description of the problem can be found below.  It is possible that there is an already existing ticket that corresponds to this issue, but I didn't check.  I appreciate any comments.

Thanks,
Aaron Levinson
----------------------- next part ------------------------------------
>From 66796ca78e9b307ba0a8a7c33bd89d006c44ddf8 Mon Sep 17 00:00:00 2001
From: Aaron Levinson <alevinsn at aracnet.com>
Date: Sun, 26 Mar 2017 01:57:19 -0700
Subject: [PATCH] Fixed bug encountered when decoding interlaced video.

Purpose: Fixed bug encountered when decoding interlaced video.  In
 some cases, ffmpeg was treating it as progressive.  As a result of
 the change, the issues with interlaced video are fixed.  It is also
 possible that this change fixes other issues besides interlaced
 video.

Detailed description of problem: I generated two mpegts files, each
 with H.264 encoded 1080i59.94 video.  One of the mpegts files had an
 AAC audio stream, while the other mpegts file had an Opus audio
 stream.  When using the following command to play back either file:
 ffmpeg -i <mpegts file> -f decklink -pix_fmt uyvy422 "DeckLink SDI
 4K", I noticed that with the mpegts file with the AAC audio stream,
 it would correctly select an interlaced video mode for the video
 output stream, but with the mpegts file with the Opus audio stream,
 it would use a progressive video mode (1080p29.97) for the video
 output stream.  The reason why it works for AAC (usually, not always)
 and doesn't work for Opus has to do with the order in which
 init_output_stream() is called for each output stream.  When AAC is
 used, it usually calls init_output_stream() for video before it calls
 it for audio.  When Opus is used, it usually calls
 init_output_stream() for audio before it calls it for video.  When
 init_output_stream() is called for audio before it is called for
 video, this results in check_init_output_file() being called
 successfully (as in, all output streams are initialized, and it
 writes the header) before it creates the filtered frame at the end of
 reap_filters().  The process of creating and processing the filtered
 frame is what sets up interlaced video properly, but in this
 particular case, that happens after writing the header in
 check_init_output_file(), and by that point, it is too late.

Testing notes: Ran regression tests, all of which passed

Comments:
ffmpeg.c:
a) Enhanced init_output_stream() to take an additional input indicating
 whether or not check_init_output_file() should be called.  There are
 other places that call init_output_stream(), and it was important to
 make sure that these continued to work properly.
b) Adjusted reap_filters() such that, in the case that
 init_output_stream() is called, it indicates that it should not call
 check_init_output_file() in init_output_stream().  Instead, it makes
 the call to check_init_output_file() directly, but after it does the
 filtered frame setup and processing.

---
 ffmpeg.c | 42 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/ffmpeg.c b/ffmpeg.c
index 62e7d82..a5ac5e0 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -1395,7 +1395,7 @@ static void do_video_stats(OutputStream *ost, int frame_size)
     }
 }
 
-static int init_output_stream(OutputStream *ost, char *error, int error_len);
+static int init_output_stream(OutputStream *ost, char *error, int error_len, int do_check_output_file);
 
 static void finish_output_stream(OutputStream *ost)
 {
@@ -1410,6 +1410,8 @@ static void finish_output_stream(OutputStream *ost)
     }
 }
 
+static int check_init_output_file(OutputFile *of, int file_index);
+
 /**
  * Get and encode new output from any of the filtergraphs, without causing
  * activity.
@@ -1428,6 +1430,7 @@ static int reap_filters(int flush)
         AVFilterContext *filter;
         AVCodecContext *enc = ost->enc_ctx;
         int ret = 0;
+        int did_init_output_stream = 0;
 
         if (!ost->filter || !ost->filter->graph->graph)
             continue;
@@ -1435,12 +1438,14 @@ static int reap_filters(int flush)
 
         if (!ost->initialized) {
             char error[1024] = "";
-            ret = init_output_stream(ost, error, sizeof(error));
+            ret = init_output_stream(ost, error, sizeof(error), 0);
             if (ret < 0) {
                 av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- %s\n",
                        ost->file_index, ost->index, error);
                 exit_program(1);
             }
+
+            did_init_output_stream = 1;
         }
 
         if (!ost->filtered_frame && !(ost->filtered_frame = av_frame_alloc())) {
@@ -1517,6 +1522,25 @@ 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() may have been 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.
+        if (did_init_output_stream) {
+            ret = check_init_output_file(of, ost->file_index);
+            if (ret < 0)
+                return ret;
+        }
     }
 
     return 0;
@@ -1883,7 +1907,7 @@ static void flush_encoders(void)
                 finish_output_stream(ost);
             }
 
-            ret = init_output_stream(ost, error, sizeof(error));
+            ret = init_output_stream(ost, error, sizeof(error), 1);
             if (ret < 0) {
                 av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- %s\n",
                        ost->file_index, ost->index, error);
@@ -3380,7 +3404,7 @@ static int init_output_stream_encode(OutputStream *ost)
     return 0;
 }
 
-static int init_output_stream(OutputStream *ost, char *error, int error_len)
+static int init_output_stream(OutputStream *ost, char *error, int error_len, int do_check_output_file)
 {
     int ret = 0;
 
@@ -3534,9 +3558,11 @@ static int init_output_stream(OutputStream *ost, char *error, int error_len)
 
     ost->initialized = 1;
 
-    ret = check_init_output_file(output_files[ost->file_index], ost->file_index);
-    if (ret < 0)
-        return ret;
+    if (do_check_output_file) {
+        ret = check_init_output_file(output_files[ost->file_index], ost->file_index);
+        if (ret < 0)
+            return ret;
+    }
 
     return ret;
 }
@@ -3603,7 +3629,7 @@ static int transcode_init(void)
         if (output_streams[i]->filter)
             continue;
 
-        ret = init_output_stream(output_streams[i], error, sizeof(error));
+        ret = init_output_stream(output_streams[i], error, sizeof(error), 1);
         if (ret < 0)
             goto dump_format;
     }
-- 
2.7.4



More information about the ffmpeg-devel mailing list