| Message ID | 20171118191217.15350-1-isasi@google.com |
|---|---|
| State | Superseded |
| Headers | show |
On Sat, Nov 18, 2017 at 11:12:17AM -0800, Sasi Inguva wrote: > Signed-off-by: Sasi Inguva <isasi@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 ? video_delay = 0 is also a valid value so it can be set to 0 already intentionally > + 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 > + 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 [...]
On Sun, Nov 19, 2017 at 1:17 AM, Michael Niedermayer <michael@niedermayer.cc > wrote: > On Sat, Nov 18, 2017 at 11:12:17AM -0800, Sasi Inguva wrote: > > Signed-off-by: Sasi Inguva <isasi@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 > >
On Mon, Nov 20, 2017 at 09:58:05PM +0530, Sasi Inguva wrote: > On Sun, Nov 19, 2017 at 1:17 AM, Michael Niedermayer <michael@niedermayer.cc > > wrote: > > > On Sat, Nov 18, 2017 at 11:12:17AM -0800, Sasi Inguva wrote: > > > Signed-off-by: Sasi Inguva <isasi@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. yes, please restrict this. I very much doubt that the mov/mp4 demuxer has more correct data about the codec than the codec itself when such data is needed to decode and cannot be ommited. I kind of find it a poor design choice that this can be ommited in h264 & hevc, but theres nothing we can do about this [...]
On Mon, Nov 20, 2017 at 11:06 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Mon, Nov 20, 2017 at 09:58:05PM +0530, Sasi Inguva wrote: >> On Sun, Nov 19, 2017 at 1:17 AM, Michael Niedermayer <michael@niedermayer.cc >> > wrote: >> >> > On Sat, Nov 18, 2017 at 11:12:17AM -0800, Sasi Inguva wrote: >> > > Signed-off-by: Sasi Inguva <isasi@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. > > yes, please restrict this. > I very much doubt that the mov/mp4 demuxer has more correct data > about the codec than the codec itself when such data is needed to > decode and cannot be ommited. > > I kind of find it a poor design choice that this can be ommited in > h264 & hevc, but theres nothing we can do about this > I don't belive hevc needs this value to be set, our decoder certainly doesn't. It seems to fill it straight from the SPS, without conditions (and the decoding logic never actually reads it). Our h264 decoder is the only one that actually needs this value to not drop frames, most other decoders just set it (potentially for the generic timestamp stuff?) - Hendrik
Ok. Just restricting this to H264 . On Tue, Nov 21, 2017 at 3:56 AM, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > On Mon, Nov 20, 2017 at 11:06 PM, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > On Mon, Nov 20, 2017 at 09:58:05PM +0530, Sasi Inguva wrote: > >> On Sun, Nov 19, 2017 at 1:17 AM, Michael Niedermayer > <michael@niedermayer.cc > >> > wrote: > >> > >> > On Sat, Nov 18, 2017 at 11:12:17AM -0800, Sasi Inguva wrote: > >> > > Signed-off-by: Sasi Inguva <isasi@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. > > > > yes, please restrict this. > > I very much doubt that the mov/mp4 demuxer has more correct data > > about the codec than the codec itself when such data is needed to > > decode and cannot be ommited. > > > > I kind of find it a poor design choice that this can be ommited in > > h264 & hevc, but theres nothing we can do about this > > > > I don't belive hevc needs this value to be set, our decoder certainly > doesn't. It seems to fill it straight from the SPS, without conditions > (and the decoding logic never actually reads it). > Our h264 decoder is the only one that actually needs this value to not > drop frames, most other decoders just set it (potentially for the > generic timestamp stuff?) > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Reattaching Fate sample. On Tue, Nov 21, 2017 at 9:57 AM, Sasi Inguva <isasi@google.com> wrote: > Signed-off-by: Sasi Inguva <isasi@google.com> > --- > libavformat/mov.c | 50 ++++++++++++++++++++++++++++++ > ++++++++++ > tests/fate/mov.mak | 7 ++++++ > tests/ref/fate/mov-guess-delay-1 | 3 +++ > tests/ref/fate/mov-guess-delay-2 | 3 +++ > tests/ref/fate/mov-guess-delay-3 | 3 +++ > 5 files changed, 66 insertions(+) > create mode 100644 tests/ref/fate/mov-guess-delay-1 > create mode 100644 tests/ref/fate/mov-guess-delay-2 > create mode 100644 tests/ref/fate/mov-guess-delay-3 > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index fd170baa57..afb0d4ca5c 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -3213,6 +3213,54 @@ 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 min_prev_pts = AV_NOPTS_VALUE; > + int64_t prev_max_pts = AV_NOPTS_VALUE; > + int num_steps = 0; > + > + if (st->codecpar->video_delay <= 0 && msc->ctts_data && > + st->codecpar->codec_id == AV_CODEC_ID_H264) { > + 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; > + > + // Everytime we encounter a new max_pts we reset num_steps > and compute again. > + if (curr_pts > prev_max_pts) { > + st->codecpar->video_delay = FFMIN(FFMAX(st->codecpar->video_delay, > num_steps), 16); > + num_steps = 0; > + prev_max_pts = curr_pts; > + min_prev_pts = curr_pts; > + } else { > + // Compute delay as the length of the path from max PTS > to min PTS. > + // Frames: I0 I1 B0 B1 B2 > + // PTS: 0 4 1 2 3 -> num_steps = delay = 1 (4->1) > + // > + // Frames: I0 I1 B1 B0 B2 > + // PTS: 0 4 2 1 3 -> num_steps = delay = 2 (4->2, > 2->1) > + if (min_prev_pts != AV_NOPTS_VALUE) { > + if (curr_pts < min_prev_pts) { > + ++num_steps; > + min_prev_pts = curr_pts; > + } > + } > + } > + > + ctts_sample++; > + if (ctts_sample == msc->ctts_data[ctts_ind].count) { > + ctts_ind++; > + ctts_sample = 0; > + } > + } > + av_log(c->fc, AV_LOG_DEBUG, "Setting codecpar->delay to %d for > stream st: %d\n", > + st->codecpar->video_delay, st->index); > + } > +} > + > static void mov_current_sample_inc(MOVStreamContext *sc) > { > sc->current_sample++; > @@ -3846,6 +3894,8 @@ static void mov_build_index(MOVContext *mov, > AVStream *st) > // Fix index according to edit lists. > mov_fix_index(mov, st); > } > + > + mov_guess_video_delay(mov, st); > } > > static int test_same_origin(const char *src, const char *ref) { > diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak > index 76f66ff498..6e79f0aba8 100644 > --- a/tests/fate/mov.mak > +++ b/tests/fate/mov.mak > @@ -11,6 +11,9 @@ FATE_MOV = fate-mov-3elist \ > fate-mov-440hz-10ms \ > fate-mov-ibi-elst-starts-b \ > fate-mov-elst-ends-betn-b-and-i \ > + fate-mov-guess-delay-1 \ > + fate-mov-guess-delay-2 \ > + fate-mov-guess-delay-3 \ > > FATE_MOV_FFPROBE = fate-mov-aac-2048-priming \ > fate-mov-zombie \ > @@ -72,3 +75,7 @@ fate-mov-spherical-mono: CMD = run > ffprobe$(PROGSSUF)$(EXESUF) -show_entries str > fate-mov-gpmf-remux: CMD = md5 -i $(TARGET_SAMPLES)/mov/fake-gp-media-with-real-gpmf.mp4 > -map 0 -c copy -fflags +bitexact -f mp4 > fate-mov-gpmf-remux: CMP = oneline > fate-mov-gpmf-remux: REF = 8f48e435ee1f6b7e173ea756141eabf3 > + > +fate-mov-guess-delay-1: CMD = run ffprobe$(PROGSSUF)$(EXESUF) > -show_entries stream=has_b_frames -select_streams v > $(TARGET_SAMPLES)/h264/h264_3bf_nopyramid_nobsrestriction.mp4 > +fate-mov-guess-delay-2: CMD = run ffprobe$(PROGSSUF)$(EXESUF) > -show_entries stream=has_b_frames -select_streams v > $(TARGET_SAMPLES)/h264/h264_3bf_pyramid_nobsrestriction.mp4 > +fate-mov-guess-delay-3: CMD = run ffprobe$(PROGSSUF)$(EXESUF) > -show_entries stream=has_b_frames -select_streams v > $(TARGET_SAMPLES)/h264/h264_4bf_pyramid_nobsrestriction.mp4 > \ No newline at end of file > diff --git a/tests/ref/fate/mov-guess-delay-1 b/tests/ref/fate/mov-guess- > delay-1 > new file mode 100644 > index 0000000000..96cb67be0c > --- /dev/null > +++ b/tests/ref/fate/mov-guess-delay-1 > @@ -0,0 +1,3 @@ > +[STREAM] > +has_b_frames=1 > +[/STREAM] > diff --git a/tests/ref/fate/mov-guess-delay-2 b/tests/ref/fate/mov-guess- > delay-2 > new file mode 100644 > index 0000000000..248de1c3ea > --- /dev/null > +++ b/tests/ref/fate/mov-guess-delay-2 > @@ -0,0 +1,3 @@ > +[STREAM] > +has_b_frames=2 > +[/STREAM] > diff --git a/tests/ref/fate/mov-guess-delay-3 b/tests/ref/fate/mov-guess- > delay-3 > new file mode 100644 > index 0000000000..248de1c3ea > --- /dev/null > +++ b/tests/ref/fate/mov-guess-delay-3 > @@ -0,0 +1,3 @@ > +[STREAM] > +has_b_frames=2 > +[/STREAM] > -- > 2.15.0.448.gf294e3d99a-goog > >
On Tue, Nov 21, 2017 at 09:59:08AM +0530, Sasi Inguva wrote:
> Reattaching Fate sample.
uploaded
[...]
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)) { + 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. + 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) + if (prev_pts != AV_NOPTS_VALUE) { + if (curr_pts < prev_pts) + ++num_swaps; + } + + prev_pts = curr_pts; + ctts_sample++; + if (ctts_sample == msc->ctts_data[ctts_ind].count) { + ctts_ind++; + ctts_sample = 0; + } + } + av_log(c->fc, AV_LOG_DEBUG, "Setting codecpar->delay to %d for stream st: %d\n", + st->codecpar->video_delay, st->index); + } +} + static void mov_current_sample_inc(MOVStreamContext *sc) { sc->current_sample++; @@ -3846,6 +3898,8 @@ static void mov_build_index(MOVContext *mov, AVStream *st) // Fix index according to edit lists. mov_fix_index(mov, st); } + + mov_guess_video_delay(mov, st); } static int test_same_origin(const char *src, const char *ref) { diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak index 76f66ff498..ef89e62096 100644 --- a/tests/fate/mov.mak +++ b/tests/fate/mov.mak @@ -11,6 +11,8 @@ FATE_MOV = fate-mov-3elist \ fate-mov-440hz-10ms \ fate-mov-ibi-elst-starts-b \ fate-mov-elst-ends-betn-b-and-i \ + fate-mov-guess-delay-1 \ + fate-mov-guess-delay-2 \ FATE_MOV_FFPROBE = fate-mov-aac-2048-priming \ fate-mov-zombie \ @@ -72,3 +74,6 @@ fate-mov-spherical-mono: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_entries str fate-mov-gpmf-remux: CMD = md5 -i $(TARGET_SAMPLES)/mov/fake-gp-media-with-real-gpmf.mp4 -map 0 -c copy -fflags +bitexact -f mp4 fate-mov-gpmf-remux: CMP = oneline fate-mov-gpmf-remux: REF = 8f48e435ee1f6b7e173ea756141eabf3 + +fate-mov-guess-delay-1: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_entries stream=has_b_frames -select_streams v $(TARGET_SAMPLES)/h264/h264_3bf_nopyramid_nobsrestriction.mp4 +fate-mov-guess-delay-2: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_entries stream=has_b_frames -select_streams v $(TARGET_SAMPLES)/h264/h264_3bf_pyramid_nobsrestriction.mp4 \ No newline at end of file diff --git a/tests/ref/fate/mov-guess-delay-1 b/tests/ref/fate/mov-guess-delay-1 new file mode 100644 index 0000000000..96cb67be0c --- /dev/null +++ b/tests/ref/fate/mov-guess-delay-1 @@ -0,0 +1,3 @@ +[STREAM] +has_b_frames=1 +[/STREAM] diff --git a/tests/ref/fate/mov-guess-delay-2 b/tests/ref/fate/mov-guess-delay-2 new file mode 100644 index 0000000000..248de1c3ea --- /dev/null +++ b/tests/ref/fate/mov-guess-delay-2 @@ -0,0 +1,3 @@ +[STREAM] +has_b_frames=2 +[/STREAM]
Signed-off-by: Sasi Inguva <isasi@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