[FFmpeg-devel] lavf/mov.c: Set start_time for audio too (in case of edit lists).

Submitted by Sasi Inguva on Aug. 6, 2018, 11:32 p.m.

Details

Message ID 20180806233229.6454-1-isasi@google.com
State Accepted
Commit 12673bb25342c772e1bf5fd2adebd85f97dc616e
Headers show

Commit Message

Sasi Inguva Aug. 6, 2018, 11:32 p.m.
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

Comments

Sasi Inguva Aug. 6, 2018, 11:34 p.m.
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
>
>
Michael Niedermayer Aug. 7, 2018, 10:22 p.m.
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

[...]
Sasi Inguva Aug. 8, 2018, 7:20 p.m.
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
>
Michael Niedermayer Aug. 8, 2018, 11:25 p.m.
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


[...]
Sasi Inguva Aug. 9, 2018, 12:24 a.m.
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
>

Patch hide | download patch | download mbox

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]