On Thu, May 28, 2015 at 06:49:02PM +0200, Andreas Cadhalpun wrote: > On 28.05.2015 04:29, Michael Niedermayer wrote: > > On Thu, May 28, 2015 at 12:11:00AM +0200, Andreas Cadhalpun wrote: > >> A negative sample duration is invalid according to the spec, but there > >> are samples that use it for the DTS calculation, e.g.: > >> http://files.1f0.de/samples/mp4-negative-stts-problem.mp4 > >> > >> These currently get out of A/V sync. > >> > >> Also change the logging type to AV_LOG_WARNING, because decoding the > >> sample can continue. > >> > >> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com> > >> --- > >> libavformat/mov.c | 20 ++++++++++++++------ > >> 1 file changed, 14 insertions(+), 6 deletions(-) > >> > >> diff --git a/libavformat/mov.c b/libavformat/mov.c > >> index 51cdd21..730f097 100644 > >> --- a/libavformat/mov.c > >> +++ b/libavformat/mov.c > >> @@ -2234,12 +2234,6 @@ static int mov_read_stts(MOVContext *c, AVIOContext > >> *pb, MOVAtom atom) > >> sample_count=avio_rb32(pb); > >> sample_duration = avio_rb32(pb); > >> > >> - /* sample_duration < 0 is invalid based on the spec */ > >> - if (sample_duration < 0) { > >> - av_log(c->fc, AV_LOG_ERROR, "Invalid SampleDelta %d in STTS, > >> at %d st:%d\n", > >> - sample_duration, i, c->fc->nb_streams-1); > >> - sample_duration = 1; > >> - } > >> if (sample_count < 0) { > >> av_log(c->fc, AV_LOG_ERROR, "Invalid sample_count=%d\n", > >> sample_count); > >> return AVERROR_INVALIDDATA; > >> @@ -2439,6 +2433,7 @@ static void mov_build_index(MOVContext *mov, > >> AVStream *st) > >> unsigned int distance = 0; > >> unsigned int rap_group_index = 0; > >> unsigned int rap_group_sample = 0; > >> + int64_t dts_correction = 0; > >> int rap_group_present = sc->rap_group_count && sc->rap_group; > >> int key_off = (sc->keyframe_count && sc->keyframes[0] > 0) || > >> (sc->stps_count && sc->stps_data[0] > 0); > >> > >> @@ -2522,6 +2517,19 @@ static void mov_build_index(MOVContext *mov, > >> AVStream *st) > >> > >> current_offset += sample_size; > >> stream_size += sample_size; > >> + current_dts += dts_correction; > > > > i think this could depending on the next stts value end up making > > DTS non monotone which seems wrong/unintended > > Indeed. Fixed patch attached. > > Best regards, > Andreas >
> mov.c | 29 +++++++++++++++++++++++------ > 1 file changed, 23 insertions(+), 6 deletions(-) > 24c730d9588822043175600b1bcde8f0fe0719c5 > 0001-mov-fix-DTS-calculation-for-samples-with-negative-st.patch > From 1ecae2a3b588c878f91b1c5c1cf93c2d9bc81812 Mon Sep 17 00:00:00 2001 > From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com> > Date: Wed, 27 May 2015 23:57:50 +0200 > Subject: [PATCH] mov: fix DTS calculation for samples with negative stts > duration > > A negative sample duration is invalid according to the spec, but there > are samples that use it for the DTS calculation, e.g.: > http://files.1f0.de/samples/mp4-negative-stts-problem.mp4 > > These currently get out of A/V sync. > > Also change the logging type to AV_LOG_WARNING, because decoding the > sample can continue. > > Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com> > --- > libavformat/mov.c | 29 +++++++++++++++++++++++------ > 1 file changed, 23 insertions(+), 6 deletions(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 51cdd21..7ce5a86 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -2234,12 +2234,6 @@ static int mov_read_stts(MOVContext *c, AVIOContext > *pb, MOVAtom atom) > sample_count=avio_rb32(pb); > sample_duration = avio_rb32(pb); > > - /* sample_duration < 0 is invalid based on the spec */ > - if (sample_duration < 0) { > - av_log(c->fc, AV_LOG_ERROR, "Invalid SampleDelta %d in STTS, at > %d st:%d\n", > - sample_duration, i, c->fc->nb_streams-1); > - sample_duration = 1; > - } > if (sample_count < 0) { > av_log(c->fc, AV_LOG_ERROR, "Invalid sample_count=%d\n", > sample_count); > return AVERROR_INVALIDDATA; > @@ -2439,10 +2433,13 @@ static void mov_build_index(MOVContext *mov, AVStream > *st) > unsigned int distance = 0; > unsigned int rap_group_index = 0; > unsigned int rap_group_sample = 0; > + int64_t last_dts = 0; > + int64_t dts_correction = 0; > int rap_group_present = sc->rap_group_count && sc->rap_group; > int key_off = (sc->keyframe_count && sc->keyframes[0] > 0) || > (sc->stps_count && sc->stps_data[0] > 0); > > current_dts -= sc->dts_shift; > + last_dts = current_dts; > > if (!sc->sample_count || st->nb_index_entries) > return; > @@ -2522,7 +2519,27 @@ static void mov_build_index(MOVContext *mov, AVStream > *st) > > current_offset += sample_size; > stream_size += sample_size; > + > + /* A negative sample duration is invalid based on the spec, > + * but some samples need it to correct the DTS. */ > + if (sc->stts_data[stts_index].duration < 0) { > + av_log(mov->fc, AV_LOG_WARNING, > + "Invalid SampleDelta %d in STTS, at %d st:%d\n", > + sc->stts_data[stts_index].duration, stts_index, > + st->index); > + dts_correction += sc->stts_data[stts_index].duration - 1; > + sc->stts_data[stts_index].duration = 1; > + } > current_dts += sc->stts_data[stts_index].duration; > + if (current_dts + dts_correction > last_dts) { > + current_dts += dts_correction; > + dts_correction = 0; > + } else { > + /* Avoid creating non-monotonous DTS */ > + dts_correction += current_dts - last_dts - 1; > + current_dts = last_dts + 1; this would enfore strict monotonicity not just monotoicity dts[i-1] == dts[i] was previously left as is is there a reason to change this ? Ive not checked what the spec says about it but i suspect there are files which have such dts [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Into a blind darkness they enter who follow after the Ignorance, they as if into a greater darkness enter who devote themselves to the Knowledge alone. -- Isha Upanishad
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
