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

[Paul B Mahol]

On 12/17/18, 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(+)
>>
>> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
>> index edf6b5701d..8f6ee81cda 100644
>> --- a/libavformat/mpegts.c
>> +++ b/libavformat/mpegts.c
>> @@ -1219,6 +1219,7 @@ skip:
>>                          || pes->st->codecpar->codec_id ==
>> AV_CODEC_ID_DVB_SUBTITLE)
>>                      ) {
>>                      AVProgram *p = NULL;
>> +                    int pcr_found = 0;
>>                      while ((p = av_find_program_from_stream(pes->stream,
>> p, pes->st->index))) {
>>                          if (p->pcr_pid != -1 && p->discard !=
>> AVDISCARD_ALL) {
>>                              MpegTSFilter *f = pes->ts->pids[p->pcr_pid];
>> @@ -1242,6 +1243,7 @@ skip:
>>                                      // and the pcr error to this packet
>> should be no more than 100 ms.
>>                                      // TODO: we should interpolate the
>> PCR, not just use the last one
>>                                      int64_t pcr = f->last_pcr / 300;
>> +                                    pcr_found = 1;
>>                                      pes->st->pts_wrap_reference =
>> st->pts_wrap_reference;
>>                                      pes->st->pts_wrap_behavior =
>> st->pts_wrap_behavior;
>>                                      if (pes->dts == AV_NOPTS_VALUE ||
>> pes->dts < pcr) {
>> @@ -1258,6 +1260,15 @@ skip:
>>                              }
>>                          }
>>                      }
>> +
>> +                    if (!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
>> "
>> +                               "PCR hasn't been received yet.\n",
>> +                               pes->pid);
>> +                        pes->dts = pes->pts = AV_NOPTS_VALUE;
>> +                    }
>>                  }
>>              }
>>              break;
>
> Assuming i understand the intend correctly ...
> could the packet be put in a buffer and once a PCR has been encountered be
> returned based on that?
> This seems better as no timestamp would be lost
>
> thx

No, that is awful idea. You do not waste memory on that.