| Message ID | 20180806233229.6454-1-isasi@google.com |
|---|---|
| State | Accepted |
| Commit | 12673bb25342c772e1bf5fd2adebd85f97dc616e |
| Headers | show |
Attaching the fate file. On Mon, Aug 6, 2018 at 4:33 PM <isasi@google.com> wrote: > From: Sasi Inguva <isasi@google.com> > > Fixes vorbis mp4 audio files, with edit list specified. Since > st->skip_samples is not set in case of vorbis , ffmpeg computes the > start_time as negative. > > Signed-off-by: Sasi Inguva <isasi@google.com> > --- > libavformat/mov.c | 4 ++-- > tests/fate/mov.mak | 5 +++++ > tests/ref/fate/mov-neg-firstpts-discard-vorbis | 3 +++ > 3 files changed, 10 insertions(+), 2 deletions(-) > create mode 100644 tests/ref/fate/mov-neg-firstpts-discard-vorbis > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 82cd410a72..c0f90edef7 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -3684,9 +3684,9 @@ static void mov_fix_index(MOVContext *mov, AVStream > *st) > st->index_entries[i].timestamp -= msc->min_corrected_pts; > } > } > - // Start time should be equal to zero or the duration of any > empty edits. > - st->start_time = empty_edits_sum_duration; > } > + // Start time should be equal to zero or the duration of any empty > edits. > + st->start_time = empty_edits_sum_duration; > > // Update av stream length, if it ends up shorter than the track's > media duration > st->duration = FFMIN(st->duration, edit_list_dts_entry_end - > start_dts); > diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak > index 6f0e28d21e..3d9e4280bb 100644 > --- a/tests/fate/mov.mak > +++ b/tests/fate/mov.mak > @@ -19,6 +19,7 @@ FATE_MOV = fate-mov-3elist \ > fate-mov-stream-shorter-than-movie \ > > FATE_MOV_FFPROBE = fate-mov-neg-firstpts-discard \ > + fate-mov-neg-firstpts-discard-vorbis \ > fate-mov-aac-2048-priming \ > fate-mov-zombie \ > fate-mov-init-nonkeyframe \ > @@ -89,6 +90,10 @@ fate-mov-bbi-elst-starts-b: CMD = framemd5 -flags > +bitexact -acodec aac_fixed -i > # Makes sure that the stream start_time is not negative when the first > packet is a DISCARD packet with negative timestamp. > fate-mov-neg-firstpts-discard: CMD = run ffprobe$(PROGSSUF)$(EXESUF) > -show_entries stream=start_time -bitexact > $(TARGET_SAMPLES)/mov/mov_neg_first_pts_discard.mov > > +# Makes sure that the VORBIS audio stream start_time is not negative when > the first few packets are DISCARD packets > +# with negative timestamps (skip_samples is not set for Vorbis, so ffmpeg > computes start_time as negative if not specified by demuxer). > +fate-mov-neg-firstpts-discard-vorbis: CMD = run > ffprobe$(PROGSSUF)$(EXESUF) -show_entries stream=start_time -bitexact > $(TARGET_SAMPLES)/mov/mov_neg_first_pts_discard_vorbis.mp4 > + > # Makes sure that expected frames are generated for > mov_neg_first_pts_discard.mov with -vsync 1 > fate-mov-neg-firstpts-discard-frames: CMD = framemd5 -flags +bitexact -i > $(TARGET_SAMPLES)/mov/mov_neg_first_pts_discard.mov -vsync 1 > > diff --git a/tests/ref/fate/mov-neg-firstpts-discard-vorbis > b/tests/ref/fate/mov-neg-firstpts-discard-vorbis > new file mode 100644 > index 0000000000..2e295e3b68 > --- /dev/null > +++ b/tests/ref/fate/mov-neg-firstpts-discard-vorbis > @@ -0,0 +1,3 @@ > +[STREAM] > +start_time=0.000000 > +[/STREAM] > -- > 2.18.0.597.ga71716f1ad-goog > >
On Mon, Aug 06, 2018 at 04:32:29PM -0700, isasi-at-google.com@ffmpeg.org wrote: > From: Sasi Inguva <isasi@google.com> > > Fixes vorbis mp4 audio files, with edit list specified. Since > st->skip_samples is not set in case of vorbis , ffmpeg computes the > start_time as negative. > > Signed-off-by: Sasi Inguva <isasi@google.com> > --- > libavformat/mov.c | 4 ++-- > tests/fate/mov.mak | 5 +++++ > tests/ref/fate/mov-neg-firstpts-discard-vorbis | 3 +++ > 3 files changed, 10 insertions(+), 2 deletions(-) > create mode 100644 tests/ref/fate/mov-neg-firstpts-discard-vorbis The commit message speaks of audio, this changes subtitles too is this intended ? thx [...]
i intended it only for audio, but i don't see any harm if it gets applied to subtitle tracks also . On Tue, Aug 7, 2018 at 3:23 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Mon, Aug 06, 2018 at 04:32:29PM -0700, isasi-at-google.com@ffmpeg.org > wrote: > > From: Sasi Inguva <isasi@google.com> > > > > Fixes vorbis mp4 audio files, with edit list specified. Since > > st->skip_samples is not set in case of vorbis , ffmpeg computes the > > start_time as negative. > > > > Signed-off-by: Sasi Inguva <isasi@google.com> > > --- > > libavformat/mov.c | 4 ++-- > > tests/fate/mov.mak | 5 +++++ > > tests/ref/fate/mov-neg-firstpts-discard-vorbis | 3 +++ > > 3 files changed, 10 insertions(+), 2 deletions(-) > > create mode 100644 tests/ref/fate/mov-neg-firstpts-discard-vorbis > > The commit message speaks of audio, this changes subtitles too > is this intended ? > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > I have often repented speaking, but never of holding my tongue. > -- Xenocrates > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Wed, Aug 08, 2018 at 12:20:34PM -0700, Sasi Inguva wrote: > i intended it only for audio, but i don't see any harm if it gets applied > to subtitle tracks also . if you want subs incldued then the commit message is wrong at least if you dont want subs incldued the change is wrong [...]
Ok. i'll correct the commit message. On Wed, Aug 8, 2018 at 4:25 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Aug 08, 2018 at 12:20:34PM -0700, Sasi Inguva wrote: > > i intended it only for audio, but i don't see any harm if it gets applied > > to subtitle tracks also . > > if you want subs incldued then the commit message is wrong at least > if you dont want subs incldued the change is wrong > > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > If you drop bombs on a foreign country and kill a hundred thousand > innocent people, expect your government to call the consequence > "unprovoked inhuman terrorist attacks" and use it to justify dropping > more bombs and killing more people. The technology changed, the idea is > old. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
diff --git a/libavformat/mov.c b/libavformat/mov.c index 82cd410a72..c0f90edef7 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -3684,9 +3684,9 @@ static void mov_fix_index(MOVContext *mov, AVStream *st) st->index_entries[i].timestamp -= msc->min_corrected_pts; } } - // Start time should be equal to zero or the duration of any empty edits. - st->start_time = empty_edits_sum_duration; } + // Start time should be equal to zero or the duration of any empty edits. + st->start_time = empty_edits_sum_duration; // Update av stream length, if it ends up shorter than the track's media duration st->duration = FFMIN(st->duration, edit_list_dts_entry_end - start_dts); diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak index 6f0e28d21e..3d9e4280bb 100644 --- a/tests/fate/mov.mak +++ b/tests/fate/mov.mak @@ -19,6 +19,7 @@ FATE_MOV = fate-mov-3elist \ fate-mov-stream-shorter-than-movie \ FATE_MOV_FFPROBE = fate-mov-neg-firstpts-discard \ + fate-mov-neg-firstpts-discard-vorbis \ fate-mov-aac-2048-priming \ fate-mov-zombie \ fate-mov-init-nonkeyframe \ @@ -89,6 +90,10 @@ fate-mov-bbi-elst-starts-b: CMD = framemd5 -flags +bitexact -acodec aac_fixed -i # Makes sure that the stream start_time is not negative when the first packet is a DISCARD packet with negative timestamp. fate-mov-neg-firstpts-discard: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_entries stream=start_time -bitexact $(TARGET_SAMPLES)/mov/mov_neg_first_pts_discard.mov +# Makes sure that the VORBIS audio stream start_time is not negative when the first few packets are DISCARD packets +# with negative timestamps (skip_samples is not set for Vorbis, so ffmpeg computes start_time as negative if not specified by demuxer). +fate-mov-neg-firstpts-discard-vorbis: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_entries stream=start_time -bitexact $(TARGET_SAMPLES)/mov/mov_neg_first_pts_discard_vorbis.mp4 + # Makes sure that expected frames are generated for mov_neg_first_pts_discard.mov with -vsync 1 fate-mov-neg-firstpts-discard-frames: CMD = framemd5 -flags +bitexact -i $(TARGET_SAMPLES)/mov/mov_neg_first_pts_discard.mov -vsync 1 diff --git a/tests/ref/fate/mov-neg-firstpts-discard-vorbis b/tests/ref/fate/mov-neg-firstpts-discard-vorbis new file mode 100644 index 0000000000..2e295e3b68 --- /dev/null +++ b/tests/ref/fate/mov-neg-firstpts-discard-vorbis @@ -0,0 +1,3 @@ +[STREAM] +start_time=0.000000 +[/STREAM]