[FFmpeg-devel] libavformat/utils: Interpolate missing timestamps in H264 and HEVC when no b-frames observed.

Submitted by Andriy Gelman on May 12, 2019, 3:20 a.m.

Details

Message ID 20190512032051.29529-1-andriy.gelman@gmail.com
State New
Headers show

Commit Message

Andriy Gelman May 12, 2019, 3:20 a.m.
From: Andriy Gelman <andriy.gelman@gmail.com>

Fixes Ticket #7895.

Currently, timestamp interpolation is disabled by default in H264 and
HEVC.  This creates playback issues when the demuxer does not output a
valid timestamp. This patch allows interpolation when no b-frames have
been observed during decoding, which fixes playback issues for some
missing timestamp cases.
---
 libavformat/utils.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Michael Niedermayer May 13, 2019, 10:04 a.m.
On Sat, May 11, 2019 at 11:20:51PM -0400, Andriy Gelman wrote:
> From: Andriy Gelman <andriy.gelman@gmail.com>
> 
> Fixes Ticket #7895.
> 
> Currently, timestamp interpolation is disabled by default in H264 and
> HEVC.  This creates playback issues when the demuxer does not output a
> valid timestamp. This patch allows interpolation when no b-frames have
> been observed during decoding, which fixes playback issues for some
> missing timestamp cases.
> ---
>  libavformat/utils.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index a63d71b0f4..0668ae3ad1 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -1233,7 +1233,9 @@ static void compute_pkt_fields(AVFormatContext *s, AVStream *st,
>      int64_t offset;
>      AVRational duration;
>      int onein_oneout = st->codecpar->codec_id != AV_CODEC_ID_H264 &&
> -                       st->codecpar->codec_id != AV_CODEC_ID_HEVC;
> +                       st->codecpar->codec_id != AV_CODEC_ID_HEVC ||
> +                       (!st->internal->avctx->max_b_frames &&

> +                        st->cur_dts != RELATIVE_TS_BASE);

This needs a comment explaining what it does. Assuming it is what i think it is
then its not completely obvious and could confuse someone working on the code
in the future.


>  
>      if (s->flags & AVFMT_FLAG_NOFILLIN)
>          return;
> @@ -1272,6 +1274,10 @@ static void compute_pkt_fields(AVFormatContext *s, AVStream *st,
>      delay = st->internal->avctx->has_b_frames;
>      presentation_delayed = 0;
>  
> +    /*update max_b_frames if delay is larger */
> +    if (delay > st->internal->avctx->max_b_frames)
> +      st->internal->avctx->max_b_frames = delay;

do we have a testcase for this ?

[...]
Andriy Gelman May 13, 2019, 7:37 p.m.
Hello, 

On Mon, 13. May 12:04, Michael Niedermayer wrote:
> On Sat, May 11, 2019 at 11:20:51PM -0400, Andriy Gelman wrote:
> > From: Andriy Gelman <andriy.gelman@gmail.com>
> > 
> > Fixes Ticket #7895.
> > 
> > Currently, timestamp interpolation is disabled by default in H264 and
> > HEVC.  This creates playback issues when the demuxer does not output a
> > valid timestamp. This patch allows interpolation when no b-frames have
> > been observed during decoding, which fixes playback issues for some
> > missing timestamp cases.
> > ---
> >  libavformat/utils.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index a63d71b0f4..0668ae3ad1 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -1233,7 +1233,9 @@ static void compute_pkt_fields(AVFormatContext *s, AVStream *st,
> >      int64_t offset;
> >      AVRational duration;
> >      int onein_oneout = st->codecpar->codec_id != AV_CODEC_ID_H264 &&
> > -                       st->codecpar->codec_id != AV_CODEC_ID_HEVC;
> > +                       st->codecpar->codec_id != AV_CODEC_ID_HEVC ||
> > +                       (!st->internal->avctx->max_b_frames &&
> 
> > +                        st->cur_dts != RELATIVE_TS_BASE);
> 
> This needs a comment explaining what it does. Assuming it is what i think it is
> then its not completely obvious and could confuse someone working on the code
> in the future.

I added this condition so that pts/dts interpolation is skipped when the demuxer
provides no timing information. I will add the comment in the updated patch.

> >  
> >      if (s->flags & AVFMT_FLAG_NOFILLIN)
> >          return;
> > @@ -1272,6 +1274,10 @@ static void compute_pkt_fields(AVFormatContext *s, AVStream *st,
> >      delay = st->internal->avctx->has_b_frames;
> >      presentation_delayed = 0;
> >  
> > +    /*update max_b_frames if delay is larger */
> > +    if (delay > st->internal->avctx->max_b_frames)
> > +      st->internal->avctx->max_b_frames = delay;
> 
> do we have a testcase for this ?
> 

I can add a testcase similar to the attachment in ticket #7895, where the pts/dts is interpolated 
on a HEVC stream encoded with the zerolatency option. Will this be ok? 

Thanks, 
Andriy
Michael Niedermayer May 14, 2019, 12:12 a.m.
On Mon, May 13, 2019 at 03:37:24PM -0400, Andriy Gelman wrote:
> Hello, 
> 
> On Mon, 13. May 12:04, Michael Niedermayer wrote:
> > On Sat, May 11, 2019 at 11:20:51PM -0400, Andriy Gelman wrote:
> > > From: Andriy Gelman <andriy.gelman@gmail.com>
> > > 
> > > Fixes Ticket #7895.
> > > 
> > > Currently, timestamp interpolation is disabled by default in H264 and
> > > HEVC.  This creates playback issues when the demuxer does not output a
> > > valid timestamp. This patch allows interpolation when no b-frames have
> > > been observed during decoding, which fixes playback issues for some
> > > missing timestamp cases.
> > > ---
> > >  libavformat/utils.c | 11 +++++++++--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > index a63d71b0f4..0668ae3ad1 100644
> > > --- a/libavformat/utils.c
> > > +++ b/libavformat/utils.c
> > > @@ -1233,7 +1233,9 @@ static void compute_pkt_fields(AVFormatContext *s, AVStream *st,
> > >      int64_t offset;
> > >      AVRational duration;
> > >      int onein_oneout = st->codecpar->codec_id != AV_CODEC_ID_H264 &&
> > > -                       st->codecpar->codec_id != AV_CODEC_ID_HEVC;
> > > +                       st->codecpar->codec_id != AV_CODEC_ID_HEVC ||
> > > +                       (!st->internal->avctx->max_b_frames &&
> > 
> > > +                        st->cur_dts != RELATIVE_TS_BASE);
> > 
> > This needs a comment explaining what it does. Assuming it is what i think it is
> > then its not completely obvious and could confuse someone working on the code
> > in the future.
> 
> I added this condition so that pts/dts interpolation is skipped when the demuxer
> provides no timing information. I will add the comment in the updated patch.
> 

> > >  
> > >      if (s->flags & AVFMT_FLAG_NOFILLIN)
> > >          return;
> > > @@ -1272,6 +1274,10 @@ static void compute_pkt_fields(AVFormatContext *s, AVStream *st,
> > >      delay = st->internal->avctx->has_b_frames;
> > >      presentation_delayed = 0;
> > >  
> > > +    /*update max_b_frames if delay is larger */
> > > +    if (delay > st->internal->avctx->max_b_frames)
> > > +      st->internal->avctx->max_b_frames = delay;
> > 
> > do we have a testcase for this ?
> > 
> 
> I can add a testcase similar to the attachment in ticket #7895, where the pts/dts is interpolated 
> on a HEVC stream encoded with the zerolatency option. Will this be ok? 

should be fine if it results in a reliable testcase

thx

[...]

Patch hide | download patch | download mbox

diff --git a/libavformat/utils.c b/libavformat/utils.c
index a63d71b0f4..0668ae3ad1 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -1233,7 +1233,9 @@  static void compute_pkt_fields(AVFormatContext *s, AVStream *st,
     int64_t offset;
     AVRational duration;
     int onein_oneout = st->codecpar->codec_id != AV_CODEC_ID_H264 &&
-                       st->codecpar->codec_id != AV_CODEC_ID_HEVC;
+                       st->codecpar->codec_id != AV_CODEC_ID_HEVC ||
+                       (!st->internal->avctx->max_b_frames &&
+                        st->cur_dts != RELATIVE_TS_BASE);
 
     if (s->flags & AVFMT_FLAG_NOFILLIN)
         return;
@@ -1272,6 +1274,10 @@  static void compute_pkt_fields(AVFormatContext *s, AVStream *st,
     delay = st->internal->avctx->has_b_frames;
     presentation_delayed = 0;
 
+    /*update max_b_frames if delay is larger */
+    if (delay > st->internal->avctx->max_b_frames)
+      st->internal->avctx->max_b_frames = delay;
+
     /* XXX: need has_b_frame, but cannot get it if the codec is
      *  not initialized */
     if (delay &&
@@ -1337,7 +1343,8 @@  static void compute_pkt_fields(AVFormatContext *s, AVStream *st,
             presentation_delayed, av_ts2str(pkt->pts), av_ts2str(pkt->dts), av_ts2str(st->cur_dts),
             pkt->stream_index, pc, pkt->duration, delay, onein_oneout);
 
-    /* Interpolate PTS and DTS if they are not present. We skip H264
+    /* Interpolate PTS and DTS if they are not present. H264/HEVC timestamps are 
+     * interpolated only if no b-frames have been observed. Otherwise, we skip H264/HEVC
      * currently because delay and has_b_frames are not reliably set. */
     if ((delay == 0 || (delay == 1 && pc)) &&
         onein_oneout) {