From patchwork Mon Jul 22 12:22:34 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Andreas_H=C3=A5kon?= X-Patchwork-Id: 14036 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 9C6F2449EFE for ; Mon, 22 Jul 2019 15:22:47 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7C5EA68A16A; Mon, 22 Jul 2019 15:22:47 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-40130.protonmail.ch (mail-40130.protonmail.ch [185.70.40.130]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 334A8689909 for ; Mon, 22 Jul 2019 15:22:41 +0300 (EEST) Date: Mon, 22 Jul 2019 12:22:34 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=default; t=1563798159; bh=Peiody3yaVLZrXm2etJOGsrZXFwVaDL/LUq9iIXbqTU=; h=Date:To:From:Reply-To:Subject:Feedback-ID:From; b=XFkyNHxUwG66mvZ3VDKY6eebBP+SGcoeAedrcRhmAFq0dRv5VrDrv9EFBAAUXgE2p jMp5uJPAffQvBzRGhvSSZwR+KnUUasJStyHRRWxbm/l7j4hNHAGMGttXSgsuPXMySD ezRu/R1U974QTuHOFTy6+iOpWtLAg+JppoaxHvJU= To: FFmpeg development discussions and patches From: =?UTF-8?Q?Andreas_H=C3=A5kon?= Message-ID: Feedback-ID: Mx8CaiV20jk_fqXDN0fFpg3vRaGkb9VCTrYRnZNHwEija3aOdqvFspzl6ODkmHrlSKJSx29p-LzkuvS_96L02A==:Ext:ProtonMail MIME-Version: 1.0 X-Spam-Status: No, score=-1.2 required=7.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,HTML_MESSAGE autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on mail.protonmail.ch X-Content-Filtered-By: Mailman/MimeDel 2.1.20 Subject: [FFmpeg-devel] [PATCH] libavformat/mpegtsenc: fix incorrect PCR with multiple programs 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" Hi, Based on the discussion of my previous patch https://patchwork.ffmpeg.org/patch/13487/ Here I publish a first part of the patch that only addresses the PCR problem. This supersedes too: https://patchwork.ffmpeg.org/patch/13745/ Regards. A.H. --- From a9d51e71c546986c8ea3bd111d16fc81354ffa3d Mon Sep 17 00:00:00 2001 From: Andreas Hakon Date: Mon, 22 Jul 2019 13:10:20 +0100 Subject: [PATCH] libavformat/mpegtsenc: fix incorrect PCR with multiple programs The MPEG-TS muxer has a serious bug related to the PCR pid selection. This bug appears when more than one program is used. The root cause is because the current code targets only one program when selecting the stream for the PCR. This patch solves this problem. And you can check it with these commands: - First, make sure you have "ffmpeg-old" (a binary without the patch), "ffmpeg-new" (a binary with the patch), and the test file "Day_Flight.mpg" from https://samples.ffmpeg.org/MPEG2/mpegts-klv/Day%20Flight.mpg - Execute these commands: $ ffmpeg-old -loglevel verbose -y -f mpegts -i Day_Flight.mpg \ -map i:0x1e1 -c:v:0 copy -map i:0x1e1 -c:v:1 copy \ -program st=0 -program st=1 -f mpegts out-error.ts $ ffmpeg-new -loglevel verbose -y -f mpegts -i Day_Flight.mpg \ -map i:0x1e1 -c:v:0 copy -map i:0x1e1 -c:v:1 copy \ -program st=0 -program st=1 -f mpegts out-ok.ts - As a result, you have two files "out-error.ts" and "out-ok.ts". Both files with the same content (but not identical): two services, and each one with one video stream. However, the first file has errors in the PCR timestamps. - If you analyze the files, you can see that the file "out-error.ts" has PCR marks in every packet of the pid 256 (a total of 561862 packets with PCR marks). However, the pid 257 has 783 packets only with PCR marks. On the other hand, the file "out-ok.ts" has correct PCR marks, and both pids (256 & 257) only have 783 packets with PCR timestamps. - You can do additional checks with the tool "tsreport" from the tstools package. Try this to see the difference: $ tsreport -b -v out-ok.ts | more $ tsreport -b -v out-error.ts | more Signed-off-by: Andreas Hakon --- libavformat/mpegtsenc.c | 82 +++++++++++++++++++++++++---------------------- 1 file changed, 44 insertions(+), 38 deletions(-) diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c index fc0ea22..4a978ed 100644 --- a/libavformat/mpegtsenc.c +++ b/libavformat/mpegtsenc.c @@ -953,46 +953,54 @@ static int mpegts_init(AVFormatContext *s) av_freep(&pids); - /* if no video stream, use the first stream as PCR */ - if (service->pcr_pid == 0x1fff && s->nb_streams > 0) { - pcr_st = s->streams[0]; - ts_st = pcr_st->priv_data; - service->pcr_pid = ts_st->pid; - } else - ts_st = pcr_st->priv_data; - - if (ts->mux_rate > 1) { - service->pcr_packet_period = (int64_t)ts->mux_rate * ts->pcr_period / - (TS_PACKET_SIZE * 8 * 1000); - ts->sdt_packet_period = (int64_t)ts->mux_rate * SDT_RETRANS_TIME / - (TS_PACKET_SIZE * 8 * 1000); - ts->pat_packet_period = (int64_t)ts->mux_rate * PAT_RETRANS_TIME / - (TS_PACKET_SIZE * 8 * 1000); - - if (ts->copyts < 1) - ts->first_pcr = av_rescale(s->max_delay, PCR_TIME_BASE, AV_TIME_BASE); - } else { - /* Arbitrary values, PAT/PMT will also be written on video key frames */ - ts->sdt_packet_period = 200; - ts->pat_packet_period = 40; - if (pcr_st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { - int frame_size = av_get_audio_frame_duration2(pcr_st->codecpar, 0); - if (!frame_size) { - av_log(s, AV_LOG_WARNING, "frame size not set\n"); - service->pcr_packet_period = - pcr_st->codecpar->sample_rate / (10 * 512); + for (i = 0; i < ts->nb_services; i++) { + service = ts->services[i]; + + /* if no video stream, use the first stream as PCR */ + if (service->pcr_pid == 0x1fff && s->nb_streams > 0) { + pcr_st = s->streams[0]; + ts_st = pcr_st->priv_data; + service->pcr_pid = ts_st->pid; + } else + ts_st = pcr_st->priv_data; + + if (ts->mux_rate > 1) { + service->pcr_packet_period = (int64_t)ts->mux_rate * ts->pcr_period / + (TS_PACKET_SIZE * 8 * 1000); + ts->sdt_packet_period = (int64_t)ts->mux_rate * SDT_RETRANS_TIME / + (TS_PACKET_SIZE * 8 * 1000); + ts->pat_packet_period = (int64_t)ts->mux_rate * PAT_RETRANS_TIME / + (TS_PACKET_SIZE * 8 * 1000); + if (ts->copyts < 1) + ts->first_pcr = av_rescale(s->max_delay, PCR_TIME_BASE, AV_TIME_BASE); + } else { + /* Arbitrary values, PAT/PMT will also be written on video key frames */ + ts->sdt_packet_period = 200; + ts->pat_packet_period = 40; + if (pcr_st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { + int frame_size = av_get_audio_frame_duration2(pcr_st->codecpar, 0); + if (!frame_size) { + av_log(s, AV_LOG_WARNING, "frame size not set\n"); + service->pcr_packet_period = + pcr_st->codecpar->sample_rate / (10 * 512); + } else { + service->pcr_packet_period = + pcr_st->codecpar->sample_rate / (10 * frame_size); + } } else { + // max delta PCR 0.1s + // TODO: should be avg_frame_rate service->pcr_packet_period = - pcr_st->codecpar->sample_rate / (10 * frame_size); + ts_st->user_tb.den / (10 * ts_st->user_tb.num); } - } else { - // max delta PCR 0.1s - // TODO: should be avg_frame_rate - service->pcr_packet_period = - ts_st->user_tb.den / (10 * ts_st->user_tb.num); + if (!service->pcr_packet_period) + service->pcr_packet_period = 1; } - if (!service->pcr_packet_period) - service->pcr_packet_period = 1; + + // output a PCR as soon as possible + service->pcr_packet_count = service->pcr_packet_period; + + av_log(s, AV_LOG_VERBOSE, "service %i using PCR in pid=%i\n", service->sid, service->pcr_pid); } ts->last_pat_ts = AV_NOPTS_VALUE; @@ -1005,8 +1013,6 @@ static int mpegts_init(AVFormatContext *s) ts->sdt_packet_period = INT_MAX; } - // output a PCR as soon as possible - service->pcr_packet_count = service->pcr_packet_period; ts->pat_packet_count = ts->pat_packet_period - 1; ts->sdt_packet_count = ts->sdt_packet_period - 1;