From patchwork Sun Jul 28 19:07:44 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: 14109 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 ED5D74469C4 for ; Sun, 28 Jul 2019 22:07:59 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id CE8806802F5; Sun, 28 Jul 2019 22:07:59 +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 B51AC680172 for ; Sun, 28 Jul 2019 22:07:52 +0300 (EEST) Date: Sun, 28 Jul 2019 19:07:44 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=default; t=1564340870; bh=JLPtZJb/0Qa9IZIPVBZTFrlqt/eR7CBuwfnGv2h1izY=; h=Date:To:From:Reply-To:Subject:Feedback-ID:From; b=pVeSapoeZ7A8bYDaxcVzt9IHoWv/IIE3gFur9zcqLjonE6VrRQqLMeO9RqkY0ldP4 VXLWY76jlfR/OzyGg4kLKjo95rrc7FacMSl0eaY/zNIaOCIZl5loIuOdcjSOrRri94 JkJuqtwEnLNEtUgrbzl9hfrVq1GHcZQZ10GTEsFQ= 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] libavformat/mpegtsenc: fix incorrect PCR with multiple programs [v3] 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, This last version fixes the small bug discovered by Michael Niedermayer: https://patchwork.ffmpeg.org/patch/14099/ This version is finally clean. Regards. A.H. --- From 08565b81aa2b6d75043e5e984da143480891c3b0 Mon Sep 17 00:00:00 2001 From: Andreas Hakon Date: Sun, 28 Jul 2019 19:59:51 +0100 Subject: [PATCH] libavformat/mpegtsenc: fix incorrect PCR with multiple programs [v3] 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 select correct streams for each program. Furthermore, it passes all the checks and doesn't generate any regression. You can check it with this command: $ ./ffmpeg -loglevel verbose -y -f data -i /dev/zero \ -filter_complex "nullsrc=s=60x60,split=2[v0][v1],anullsrc[a]" \ -map [v0] -c:v:0 rawvideo \ -map [v1] -c:v:1 rawvideo \ -map [a] -c:a:0 pcm_s8 \ -program st=0 -program st=1 -program st=2 -f mpegts out.ts And you will see something like: [mpegts @ 0x562f388cd800] service 1 using PCR in pid=256 [mpegts @ 0x562f388cd800] service 2 using PCR in pid=257 [mpegts @ 0x562f388cd800] service 3 using PCR in pid=258 Signed-off-by: Andreas Hakon --- libavformat/mpegtsenc.c | 103 ++++++++++++++++++++++++++++------------------- 1 file changed, 62 insertions(+), 41 deletions(-) diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c index fc0ea22..fc340cd 100644 --- a/libavformat/mpegtsenc.c +++ b/libavformat/mpegtsenc.c @@ -60,6 +60,7 @@ typedef struct MpegTSService { int pcr_packet_count; int pcr_packet_period; AVProgram *program; + AVStream *pcr_st; } MpegTSService; // service_type values as defined in ETSI 300 468 @@ -774,7 +775,7 @@ static int mpegts_init(AVFormatContext *s) MpegTSWrite *ts = s->priv_data; MpegTSWriteStream *ts_st; MpegTSService *service; - AVStream *st, *pcr_st = NULL; + AVStream *st; AVDictionaryEntry *title, *provider; int i, j; const char *service_name; @@ -831,6 +832,11 @@ static int mpegts_init(AVFormatContext *s) } } + for (i = 0; i < ts->nb_services; i++) { + service = ts->services[i]; + service->pcr_st = NULL; + } + ts->pat.pid = PAT_PID; /* Initialize at 15 so that it wraps and is equal to 0 for the * first packet we write. */ @@ -917,7 +923,12 @@ static int mpegts_init(AVFormatContext *s) if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && service->pcr_pid == 0x1fff) { service->pcr_pid = ts_st->pid; - pcr_st = st; + service->pcr_st = st; + } + /* store this stream as a candidate PCR if the service doesn't have any */ + if (service->pcr_pid == 0x1fff && + !service->pcr_st) { + service->pcr_st = st; } if (st->codecpar->codec_id == AV_CODEC_ID_AAC && st->codecpar->extradata_size > 0) { @@ -951,50 +962,62 @@ static int mpegts_init(AVFormatContext *s) } } - av_freep(&pids); + 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); + /* get the PCR stream (real or candidate) */ + if (!service->pcr_st) { + av_log(s, AV_LOG_ERROR, "no PCR selected for the service %i", service->sid); + ret = AVERROR(EINVAL); + goto fail; + } + ts_st = service->pcr_st->priv_data; + + /* if no video stream, use the pid of the candidate PCR */ + if (service->pcr_pid == 0x1fff) + service->pcr_pid = ts_st->pid; + + 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 (service->pcr_st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { + int frame_size = av_get_audio_frame_duration2(service->pcr_st->codecpar, 0); + if (!frame_size) { + av_log(s, AV_LOG_WARNING, "frame size not set\n"); + service->pcr_packet_period = + service->pcr_st->codecpar->sample_rate / (10 * 512); + } else { + service->pcr_packet_period = + service->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); } + av_freep(&pids); + ts->last_pat_ts = AV_NOPTS_VALUE; ts->last_sdt_ts = AV_NOPTS_VALUE; // The user specified a period, use only it @@ -1005,8 +1028,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;