On Fri, Apr 19, 2019 at 08:47:34AM +0000, Andreas Håkon via ffmpeg-devel wrote: > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ > On Thursday, 18 de April de 2019 11:01, Andreas Håkon via ffmpeg-devel > <ffmpeg-devel@ffmpeg.org> wrote: > > > Hi, > > > > This patch resolves one very specific use case: > > > > - When you use the mpegts muxer; > > - And use the global parameter “-copyts”; > > - And use the parameter “-muxrate” for the mpegts muxer; > > - And use too the parameter “-mpegts_copyts”. > > > > The problem is created because the member “first_pcr” of the > > MpegTSWrite struct isn’t initialized with a correct timestamp (so when > > copying the timestamps the initial value is 0). And in this case an > > infinite loop is created because the code never writes PES packets when the > > “mux_rate” isn’t VBR (equals to 1). See the block that creates the loop > > here (note the "continue" command at end): > > > > https://github.com/FFmpeg/FFmpeg/blob/a0559fcd81f42f446c93357a943699f9d44eeb79/libavformat/mpegtsenc.c#L1211 > > > > So, this patch fixes the problem initializing the “first_pcr” with the > > first DTS value that comes when using the incoming timestamps. > > > > Regards. > > A.H. > > > > Hi, > > Here a new version of the patch. > > The "fist_pcr" value is now derived from DTS in a more consistent way. > > Regards, > A.H. > > > --- >
> From c59569ca9426fef455edabfa648cb2ff678c1640 Mon Sep 17 00:00:00 2001 > From: Andreas Hakon <andreas.ha...@protonmail.com> > Date: Fri, 19 Apr 2019 09:32:33 +0100 > Subject: [PATCH] libavformat: fix copyts and muxrate in mpegts muxer v2 > > When using "-copyts" and "-muxrate" with the mpegts muxer the muxing fails > because the member "first_pcr" of the MpegTSWrite struct isn't initialized > with a correct timestamp. > > The behaviour of the error is an infinite loop created in the function > mpegts_write_pes() because the code never writes PES packets. > > This patch resolves the problem initializing the "first_pcr" with a value > derived from the first DTS value seen. > > Signed-off-by: Andreas Hakon <andreas.ha...@protonmail.com> > --- > libavformat/mpegtsenc.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c > index fc0ea22..858b0d7 100644 > --- a/libavformat/mpegtsenc.c > +++ b/libavformat/mpegtsenc.c > @@ -971,6 +971,9 @@ static int mpegts_init(AVFormatContext *s) > > if (ts->copyts < 1) > ts->first_pcr = av_rescale(s->max_delay, PCR_TIME_BASE, > AV_TIME_BASE); > + else > + ts->first_pcr = AV_NOPTS_VALUE; > + > } else { > /* Arbitrary values, PAT/PMT will also be written on video key > frames */ > ts->sdt_packet_period = 200; > @@ -1186,12 +1189,16 @@ static void mpegts_write_pes(AVFormatContext *s, > AVStream *st, > int64_t pcr = -1; /* avoid warning */ > int64_t delay = av_rescale(s->max_delay, 90000, AV_TIME_BASE); > int force_pat = st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && key && > !ts_st->prev_payload_key; > + int last_payload_size = 0; > > av_assert0(ts_st->payload != buf || st->codecpar->codec_type != > AVMEDIA_TYPE_VIDEO); > if (ts->flags & MPEGTS_FLAG_PAT_PMT_AT_FRAMES && > st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) { > force_pat = 1; > } > > + if (ts->mux_rate > 1 && dts != AV_NOPTS_VALUE && ts->first_pcr == > AV_NOPTS_VALUE) > + ts->first_pcr = (dts * 300) - av_rescale(s->max_delay, > PCR_TIME_BASE, AV_TIME_BASE); > + if this doesnt execute first_pcr could remain AV_NOPTS_VALUE, this looks a bit fragile to me as there is code using first_pcr with implicit assumtation that it is not AV_NOPTS_VALUE in get_pcr(). is something preventing this combination ? > is_start = 1; > while (payload_size > 0) { > retransmit_si_info(s, force_pat, dts); > @@ -1209,12 +1216,13 @@ static void mpegts_write_pes(AVFormatContext *s, > AVStream *st, > } > > if (ts->mux_rate > 1 && dts != AV_NOPTS_VALUE && > - (dts - get_pcr(ts, s->pb) / 300) > delay) { > + last_payload_size != payload_size && (dts - get_pcr(ts, s->pb) / > 300) > delay) { > /* pcr insert gets priority over null packet insert */ > if (write_pcr) > mpegts_insert_pcr_only(s, st); > else > mpegts_insert_null_packet(s); > + last_payload_size = payload_size; why is the check on payload_size needed ? (it is not for the testcase) the commit message also seems not to explain this part thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In a rich man's house there is no place to spit but his face. -- Diogenes of Sinope
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".