[FFmpeg-devel] [PATCH] avformat/mpegts: only reset timestamps to NOPTS for DVB teletext

[Jan Ekström]

On Thu, Aug 13, 2020 at 12:52 PM Jan Ekström <jeebjp at gmail.com> wrote:
>
> On Thu, Aug 13, 2020 at 10:34 AM Marton Balint <cus at passwd.hu> wrote:
> >
> >
> >
> > On Thu, 13 Aug 2020, Jan Ekström wrote:
> >
> > > While having the possibility of non-NOPTS values that can suddenly
> > > jump in time due to adjustments to match PCR is not nice for DVB
> > > subtitles, apparently the parser for this format bases its behavior on
> > > whether the packets' timestamps are NOPTS or not.
> >
> > Actually what matters is that the parser separates packets which have
> > different PTS values. Having a DVB subtitle packet with no PTS is not even
> > valid based on what I read from the specs:
> >
> > "Each PES header shall carry a PTS, associated with all the subtitle data
> > contained within that PES packet. "
> >
> > So I guess current code assumes that if a packet has no PTS then it is
> > also part of the previous packet, but this also seems like a workaround
> > for bad streams...
> >
>
> Yup, it's layers upon layers of fun :) Just like a box of chocolates
> or an onion.
>
> I posted this patch to get a discussion rolling, as I was highlighted
> on the parser being broken with NOPTS.
>
> > > Thus while we can adjust timestamps, we should exclude DVB subtitles
> > > from the timestamp unsetting logic.
> >
> > Ok, but to be frank the timestamp setting logic (even when it is
> > used with PCR) also breaks the parser, because it might assign different
> > PTS to PES packets which have the same PTS...
> >
>
> Yup. Cascades of things and all that jazz are fun. I just addressed
> the most obvious bit which I caused by my unsetting of timestamps
> (which in turn was to make sure that the time line doesn't suddenly
> jump X hours back or so).
>
> On trac reverting
> http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=42aa02418e43802b4ebcca373d2413ab63a0307e
> was mentioned, but that seems to have been a case fixing
> https://trac.ffmpeg.org/ticket/4200 . The ticket lacks exact details,
> but I expect there to be a bad mux with the subtitle timestamps not
> matching the general time line according to PCR.
>
> > >
> > > Fixes #8844
> > > ---
> > > libavformat/mpegts.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > > index f71f18a57d..50d4d5e9bc 100644
> > > --- a/libavformat/mpegts.c
> > > +++ b/libavformat/mpegts.c
> > > @@ -1343,7 +1343,8 @@ skip:
> > >                         }
> > >                     }
> > >
> > > -                    if (!pcr_found) {
> > > +                    if (pes->st->codecpar->codec_id == AV_CODEC_ID_DVB_TELETEXT &&
> > > +                        !pcr_found) {
> > >                         av_log(pes->stream, AV_LOG_VERBOSE,
> > >                                "Forcing DTS/PTS to be unset for a "
> > >                                "non-trustworthy PES packet for PID %d as "
> >
> > Patch LGTM, I am just saying that the problems lies more deep...
> >
>
> Thanks. Unless people have a better idea of how to go forward with
> this, I'll apply this patch.
>

As no better ideas were brought up, applied this as
c820c2d4bfb7e2573f7bf8e2a3544a98e5a11343 .

Jan