[FFmpeg-devel] [PATCH] avformat/mpegts: unset DTS/PTS for subtitle PES packets if PCR not available

[Jan Ekström]

On Wed, Dec 19, 2018 at 8:56 PM Michael Niedermayer
<michael at niedermayer.cc> wrote:
>
> On Sat, Dec 15, 2018 at 08:50:41PM +0200, Jan Ekström wrote:
> > Fixes issues when a subtitle packet is received before PCR for the
> > program has been received, leading to wildly jumping timestamps
> > on the lavf client side as well as in the re-ordering logic.
> >
> > This usually happens in case of multiplexes where the PCR of a
> > program is not taken into account with subtitle tracks' DTS/PTS.
> > ---
> >  libavformat/mpegts.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
>
> For the record, so this is not lost on IRC and causes misunderstandings
> <michaelni> JEEB, if you prefer the simple solution iam fine with that too but i might (if i find time) rewrite it to do the buffering so the timestamps arent lost in the cases this affects
>

My opinion generally is that anything before PCR is received is
unsure/non-trustable. So if values are received before PCR that cause
issues, I would prefer not to pass on those values - which API clients
would then take at face value.

Additionally, if you currently attempt to transcode a sample - a la
the one I linked - with ffmpeg.c, the initial subtitle timestamp
received leads to zero video frames getting encoded (due to the vsync
logic thinking that there was an 8-hour jump in the timestamps or so).
That could be its own bug, but the timestamp jump is caused by us
returning the original timestamp before PCR is available - while all
following packets are adjusted to be within margin of the PCR.

In other words, this seems to improve handling of such samples, and
generally speaking in a spec-compliant MPEG-TS file you should receive
PCR quite quickly, and thus the possible negative effects of this
change should not be protracted. Thus I would like for this patch to
proceed unless someone has noticed issues with such handling. I
requested for someone to pass this patch through FATE and according to
a helpful soul it passes.

I do understand the wish to keep a timestamp that is received, of
course. Cases like this always remind me of how upipe has three
different timestamps in its packets. One for what was there originally
in the input, second that is adjusted for wrap-arounds and such to
always be rising - and third that contains the receival timestamp. A
lot of applications would only utilize the second one, but having the
first and the last one for specific use cases could be quite helpful.

Best regards,
Jan