From patchwork Mon Jul 29 11:11:40 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: 14121 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 62922449F5C for ; Mon, 29 Jul 2019 14:11:55 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3CD00680AE5; Mon, 29 Jul 2019 14:11:55 +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 312396800FF for ; Mon, 29 Jul 2019 14:11:48 +0300 (EEST) Date: Mon, 29 Jul 2019 11:11:40 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=default; t=1564398706; bh=WMUfuuyEdXFWR7TumT8CycacOiAEbd9MulRpZGD5JnE=; h=Date:To:From:Reply-To:Subject:Feedback-ID:From; b=ik8si3+RDojJC2iLLbOT8UAQSqVYIFz0QdCaznSxrbKUcmultS8su+3sM6PRt2V5m f+tZ2nSlEW7mu9RUI2Ry+9jv/qrQ3WGzWfLy191ItJpE0ptH62lSbo3/Uz4NeQBUcG xkqjp8Sa7WTlpGPFSQH9W0jFV2LGpQC80FxFuUPA= 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 [v4] 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, An updated version that fixes all previous bugs: https://patchwork.ffmpeg.org/patch/14109/ https://patchwork.ffmpeg.org/patch/14099/ https://patchwork.ffmpeg.org/patch/14036/ It passes the tests pointed by Michael Niedermayer (Sample_cut.ts) and Andriy Gelman (day_flight.mpg). I hope this time the patch will be accepted. Regards. A.H. --- From 8381febd0e881cfcd53583b0ccdd7eb2c580e422 Mon Sep 17 00:00:00 2001 From: Andreas Hakon Date: Mon, 29 Jul 2019 12:02:06 +0100 Subject: [PATCH] libavformat/mpegtsenc: fix incorrect PCR with multiple programs [v4] The MPEG-TS muxer has a serious bug related to the use of multiple programs: in that case, the PCR pid selection is incomplete for all services except one. This patch solves this problem and select correct streams for each program. Furthermore, it passes all the checks and doesn't generate any regression. Also fully compatible with shared pids over services. You can check it with this example 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,2 -program st=1,2 -program st=2 -program st=0 -f mpegts out.ts And you will see something like: [mpegts @ 0x55f8452797c0] service 1 using PCR in pid=256 [mpegts @ 0x55f8452797c0] service 2 using PCR in pid=257 [mpegts @ 0x55f8452797c0] service 3 using PCR in pid=258 [mpegts @ 0x55f8452797c0] service 4 using PCR in pid=256 Signed-off-by: Andreas Hakon --- libavformat/mpegtsenc.c | 167 +++++++++++++++++++++++++++++------------------ 1 file changed, 105 insertions(+), 62 deletions(-) diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c index fc0ea22..6a0dd55 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. */ @@ -872,17 +878,6 @@ static int mpegts_init(AVFormatContext *s) goto fail; } - program = av_find_program_from_stream(s, NULL, i); - if (program) { - for (j = 0; j < ts->nb_services; j++) { - if (ts->services[j]->program == program) { - service = ts->services[j]; - break; - } - } - } - - ts_st->service = service; /* MPEG pid values < 16 are reserved. Applications which set st->id in * this range are assigned a calculated pid. */ if (st->id < 16) { @@ -895,11 +890,6 @@ static int mpegts_init(AVFormatContext *s) ret = AVERROR(EINVAL); goto fail; } - if (ts_st->pid == service->pmt.pid) { - av_log(s, AV_LOG_ERROR, "Duplicate stream id %d\n", ts_st->pid); - ret = AVERROR(EINVAL); - goto fail; - } for (j = 0; j < i; j++) { if (pids[j] == ts_st->pid) { av_log(s, AV_LOG_ERROR, "Duplicate stream id %d\n", ts_st->pid); @@ -913,12 +903,7 @@ static int mpegts_init(AVFormatContext *s) ts_st->first_pts_check = 1; ts_st->cc = 15; ts_st->discontinuity = ts->flags & MPEGTS_FLAG_DISCONT; - /* update PCR pid by using the first video stream */ - if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && - service->pcr_pid == 0x1fff) { - service->pcr_pid = ts_st->pid; - pcr_st = st; - } + if (st->codecpar->codec_id == AV_CODEC_ID_AAC && st->codecpar->extradata_size > 0) { AVStream *ast; @@ -949,50 +934,109 @@ static int mpegts_init(AVFormatContext *s) if (st->codecpar->codec_id == AV_CODEC_ID_OPUS) { ts_st->opus_pending_trim_start = st->codecpar->initial_padding * 48000 / st->codecpar->sample_rate; } + + /* program service checks */ + program = av_find_program_from_stream(s, NULL, i); + do { + for (j = 0; j < ts->nb_services; j++) { + if (program) { + /* search for the services corresponding to this program */ + if (ts->services[j]->program == program) + service = ts->services[j]; + else + continue; + } + + ts_st->service = service; + + /* check for PMT pid conflicts */ + if (ts_st->pid == service->pmt.pid) { + av_log(s, AV_LOG_ERROR, "Duplicate stream id %d\n", ts_st->pid); + ret = AVERROR(EINVAL); + goto fail; + } + + /* update PCR pid by using the first video stream */ + if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && + service->pcr_pid == 0x1fff) { + service->pcr_pid = ts_st->pid; + 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; + } + + /* exit when just one single service */ + if (!program) + break; + } + program = program ? av_find_program_from_stream(s, program, i) : NULL; + } while (program); } 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 (!service->pcr_st) { + av_log(s, AV_LOG_ERROR, "no PCR selected for the service %i\n", service->sid); + ret = AVERROR(EINVAL); + goto fail_at_end; + } + /* get the PCR stream (real or candidate) */ + ts_st = service->pcr_st->priv_data; + + /* if no video stream then 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); + } + + if (!ts_st->service) { + av_log(s, AV_LOG_ERROR, "empty services!\n"); + ret = AVERROR(EINVAL); + goto fail_at_end; } ts->last_pat_ts = AV_NOPTS_VALUE; @@ -1005,8 +1049,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; @@ -1016,7 +1058,7 @@ static int mpegts_init(AVFormatContext *s) av_log(s, AV_LOG_VERBOSE, "muxrate %d, ", ts->mux_rate); av_log(s, AV_LOG_VERBOSE, "pcr every %d pkts, sdt every %d, pat/pmt every %d pkts\n", - service->pcr_packet_period, + ts_st->service->pcr_packet_period, ts->sdt_packet_period, ts->pat_packet_period); if (ts->m2ts_mode == -1) { @@ -1031,6 +1073,7 @@ static int mpegts_init(AVFormatContext *s) fail: av_freep(&pids); +fail_at_end: return ret; }