| Message ID | 6ba95ea3-746d-aba8-0986-7fe79d3002ef@aracnet.com |
|---|---|
| State | Superseded |
| Headers | show |
Am 26.03.2017 um 11:50 schrieb Aaron Levinson: > 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. Which FFmpeg version did you test this with? There was a change related to this just short time ago. Does it happen with current git HEAD? Matthias
On 3/26/2017 4:41 AM, Matthias Hunstock wrote: > Am 26.03.2017 um 11:50 schrieb Aaron Levinson: >> 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. > > Which FFmpeg version did you test this with? > > There was a change related to this just short time ago. > > Does it happen with current git HEAD? > > Matthias This issue occurs with the current git HEAD. I'm aware of the Blackmagic improvement that was added in February to add support for interlaced video modes on output, and actually that's one of the reasons why I'm using the latest git sources, as opposed to, say, 3.2.4. This particular issue has nothing to do with Blackmagic, and I only used Blackmagic in the example that reproduces the bug because it is something that can be reproduced on both Windows and Linux (and presumably also on OS/X). The issue also occurs if I do something like -f rawvideo out.avi on Windows, and I'm sure that there are plenty of other examples. Aaron Levinson
On 3/26/2017 10:34 AM, Aaron Levinson wrote: > On 3/26/2017 4:41 AM, Matthias Hunstock wrote: >> Am 26.03.2017 um 11:50 schrieb Aaron Levinson: >>> 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. >> >> Which FFmpeg version did you test this with? >> >> There was a change related to this just short time ago. >> >> Does it happen with current git HEAD? >> >> Matthias > > This issue occurs with the current git HEAD. I'm aware of the > Blackmagic improvement that was added in February to add support for > interlaced video modes on output, and actually that's one of the reasons > why I'm using the latest git sources, as opposed to, say, 3.2.4. This > particular issue has nothing to do with Blackmagic, and I only used > Blackmagic in the example that reproduces the bug because it is > something that can be reproduced on both Windows and Linux (and > presumably also on OS/X). The issue also occurs if I do something like > -f rawvideo out.avi on Windows, and I'm sure that there are plenty of > other examples. > > Aaron Levinson Has anyone had a chance to review this patch yet, which I submitted on March 26th? To demonstrate the impact of this patch, here's some output of before and after for an .h264 file with interlaced 1080i59.94 video content: Command-line: ffmpeg -i test8_1080i.h264 -c:v mpeg2video test8_1080i_mp2.ts Before patch: -------------------------------------- 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 Side data: cpb: bitrate max/min/avg: 0/0/200000 buffer size: 0 vbv_delay: -1 -------------------------------------- After patch: -------------------------------------- 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(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 Metadata: encoder : Lavc57.92.100 mpeg2video Side data: cpb: bitrate max/min/avg: 0/0/200000 buffer size: 0 vbv_delay: -1 -------------------------------------- As can be seen, before the patch, after decoding the .h264 file and then re-encoding it as mpeg2video in an mpegts container, the interlaced aspect of the video has been lost in the output, and it is now effectively 1080p29.97, although the video hasn't actually been converted to progressive. ffmpeg simply thinks that the video is progressive when it is not. With the patch, the interlaced aspect is not lost and propagates to the output. So, this conclusively demonstrates that the issue has nothing to do with Blackmagic and is a more general issue with interlaced video and decoding. I can make the input file available if that would be helpful. Anyway, it would be great if this bug fix could make it into ffmpeg. Thanks, Aaron Levinson
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; }
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@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(-)