| Message ID | 20190512032051.29529-1-andriy.gelman@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
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 ? [...]
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
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 [...]
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) {
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(-)