[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