On Sun, Nov 19, 2017 at 1:17 AM, Michael Niedermayer <mich...@niedermayer.cc
> wrote:
> On Sat, Nov 18, 2017 at 11:12:17AM -0800, Sasi Inguva wrote:
> > Signed-off-by: Sasi Inguva <is...@google.com>
> > ---
> > libavformat/mov.c | 54 ++++++++++++++++++++++++++++++
> ++++++++++
> > tests/fate/mov.mak | 5 ++++
> > tests/ref/fate/mov-guess-delay-1 | 3 +++
> > tests/ref/fate/mov-guess-delay-2 | 3 +++
> > 4 files changed, 65 insertions(+)
> > create mode 100644 tests/ref/fate/mov-guess-delay-1
> > create mode 100644 tests/ref/fate/mov-guess-delay-2
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index fd170baa57..7354652c6e 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -3213,6 +3213,58 @@ static int64_t add_ctts_entry(MOVStts**
> ctts_data, unsigned int* ctts_count, uns
> > return *ctts_count;
> > }
> >
> > +static void mov_guess_video_delay(MOVContext *c, AVStream* st) {
> > + MOVStreamContext *msc = st->priv_data;
> > + int ind;
> > + int ctts_ind = 0;
> > + int ctts_sample = 0;
> > + int64_t curr_pts = AV_NOPTS_VALUE;
> > + int64_t prev_pts = AV_NOPTS_VALUE;
> > + int64_t prev_max_pts = AV_NOPTS_VALUE;
> > + int num_swaps = 0;
> > +
> > + if (st->codecpar->video_delay <= 0 && msc->ctts_data &&
> > + (st->codecpar->codec_id == AV_CODEC_ID_MPEG2VIDEO ||
> > + st->codecpar->codec_id == AV_CODEC_ID_MPEG4 ||
> > + st->codecpar->codec_id == AV_CODEC_ID_VC1 ||
> > + st->codecpar->codec_id == AV_CODEC_ID_H263 ||
> > + st->codecpar->codec_id == AV_CODEC_ID_H264 ||
> > + st->codecpar->codec_id == AV_CODEC_ID_HEVC)) {
>
> Do all these cases really need this ?
>
> I just wanted to list all codecs with B-frames. H264, HEVC need it in
case, the bitstream_restriction_flag is not set in the bitstream. Other
codecs might need it only if the bitstream has been written incorrectly
i.e. writing 0 for the delay bit when there are B-frames etc. I can
restrict to H264 , HEVC if needed.
video_delay = 0 is also a valid value so it can be set to 0 already
intentionally
>
> I just want to avoid the case where the code incorrectly estimates a
video_delay that is lesser than what is written in the bitstream, because
that would be a fatal error resulting in incorrect timestamps. The other
case where the code estimates non-zero video_delay when it is zero in the
bitstream is benign.
>
> > + st->codecpar->video_delay = 0;
> > + for(ind = 0; ind < st->nb_index_entries && ctts_ind <
> msc->ctts_count; ++ind) {
> > + curr_pts = st->index_entries[ind].timestamp +
> msc->ctts_data[ctts_ind].duration;
> > +
>
> > + // This is used as an indication that the previous GOP has
> ended and a
> > + // new GOP has started.
>
> this is not neccesary a GOP uless i misread the code
>
> Corrected the comment.
>
> > + if (curr_pts > prev_max_pts) {
> > + st->codecpar->video_delay =
> > FFMIN(FFMAX(st->codecpar->video_delay,
> num_swaps), 16);
> > + num_swaps = 0;
> > + prev_max_pts = curr_pts;
> > + }
> > +
> > + // Compute delay as the no. of "drop"s in PTS inside a GOP.
> > + // Frames: I0 I1 B0 B1 B2
> > + // PTS: 0 4 1 2 3 -> num_swaps = delay = 1 (4->1)
> > + //
> > + // Frames: I0 I1 B1 B0 B2
> > + // PTS: 0 4 2 1 3 -> num_swaps = delay = 2 (4->2,
> 2->1)
>
> This may be incorret
>
> consider
> PTS 0 5 2 1 4 3
> BUFFER 0 05 25 25 45 45 5
> OUT 0 1 2 3 4 5
>
> So this is a delay = 2 but your code would set 3 i think
>
> True. Thanks for catching it. The issue will only get worse, with more no.
of B-frames in-between ( 0 7 2 1 4 3 6 4 -> would have delay 4 ) . I have
changed the code to estimate the delay as the length of the patch from max.
PTS to min. PTS . (So only 5->2->1 will be counted). Sending the new
patch. PTAL. Thanks.
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Those who are best at talking, realize last or never when they are wrong.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
