From patchwork Sun Mar 26 09:50:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aaron Levinson X-Patchwork-Id: 3103 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.50.79 with SMTP id y76csp779244vsy; Sun, 26 Mar 2017 02:51:13 -0700 (PDT) X-Received: by 10.28.49.7 with SMTP id x7mr5203944wmx.3.1490521873745; Sun, 26 Mar 2017 02:51:13 -0700 (PDT) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id 60si10415954wrp.148.2017.03.26.02.51.12; Sun, 26 Mar 2017 02:51:13 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 65227688283; Sun, 26 Mar 2017 12:50:48 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from white.spiritone.com (white.spiritone.com [216.99.193.38]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 659756809A1 for ; Sun, 26 Mar 2017 12:50:41 +0300 (EEST) Received: from [192.168.3.101] (184-100-223-229.ptld.qwest.net [184.100.223.229]) by white.spiritone.com (Postfix) with ESMTPSA id 4070073403F4 for ; Sun, 26 Mar 2017 02:51:01 -0700 (PDT) To: ffmpeg-devel@ffmpeg.org From: Aaron Levinson Message-ID: <6ba95ea3-746d-aba8-0986-7fe79d3002ef@aracnet.com> Date: Sun, 26 Mar 2017 02:50:59 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] Fixed bug encountered when decoding interlaced video X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 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 -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; }