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

[Jan Ekström]

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.

Jan