[FFmpeg-devel] [PATCH] lavf/mov: add support for sidx fragment indexes

Dale Curtis dalecurtis at chromium.org
Sat Jul 15 00:52:23 EEST 2017


After looking at this some more. I don't think the way ctts entries are
stored is correct unless you assume all ctts entries are known prior to any
seeks taking place. Either through reading all trun boxes ahead of time or
content having a ctts atom.

The current code is broken in the presence of seeks since seeks can not
know the proper ctts index unless all trun boxes have been read. Instead it
performs the seek+root switch and then ends up writing ctts entries into
the wrong slots of the global ctts index as trun boxes are encountered
after the seek.

To fix this I think the code needs to only use the global ctts index if the
ctts atom is present. If the ctts data is just being pulled from the trun
boxes, then it should probably be stored in along with each
MOVFragmentIndex.

Is anyone else familiar enough with this code to have opinions on direction
here? Rodger seems MIA.

- dale

On Fri, Jun 30, 2017 at 2:56 PM, Dale Curtis <dalecurtis at chromium.org>
wrote:

> Hmm, finally got around to looking into this again and this still isn't
> fixed. Just seeking a few times in ffplay can trigger this issue with the
> clip linked in my original message:
>
> http://storage.googleapis.com/dalecurtis-shared/buck480p30_na.mp4
>
> ./ffplay -v debug -drp 1 ~/Downloads/buck480p30_na.mp4
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fbce00008c0] invalid dts/pts combination
> 14583000
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fbce00008c0] invalid dts/pts combination
> 14586000
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fbce00008c0] found fragment index for track 1
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fbce00008c0] found fragment index entry for
> track 1 and moof_offset 16686198
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fbce00008c0] found frag time 14589000, using
> it for dts
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fbce00008c0] invalid dts/pts combination
> 14607000
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fbce00008c0] invalid dts/pts combination
> 14610000
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fbce00008c0] invalid dts/pts combination
> 14622000
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fbce00008c0] invalid dts/pts combination
> 14631000
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fbce00008c0] invalid dts/pts combination
> 14634000
> [mov,mp4,m4a,3gp,3g2,mj2 @ 0x7fbce00008c0] invalid dts/pts combination
> 14643000
>
> Disabled sidx processing resolves this issue:
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 63f84be782..919475f12f 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -5497,7 +5497,7 @@ static const MOVParseTableEntry
> mov_default_parse_table[] = {
>  { MKTAG('a','l','a','c'), mov_read_alac }, /* alac specific atom */
>  { MKTAG('a','v','c','C'), mov_read_glbl },
>  { MKTAG('p','a','s','p'), mov_read_pasp },
> -{ MKTAG('s','i','d','x'), mov_read_sidx },
> +// { MKTAG('s','i','d','x'), mov_read_sidx },
>
> Rodger, are you able to still look into this?
>
> - dale
>
> On Fri, Feb 12, 2016 at 2:21 AM, Rodger Combs <rodger.combs at gmail.com>
> wrote:
>
>> This issue is fixed by this patch, but I'm unsure of possible
>> implications on other files. It passes FATE, at least.
>>
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 149e3b4..c5e0a1e 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -3609,7 +3609,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext
>> *pb, MOVAtom atom)
>>                  }
>>                  av_log(c->fc, AV_LOG_DEBUG, "calculated into dts
>> %"PRId64"\n", dts);
>>              } else {
>> -                dts = frag->time;
>> +                dts = frag->time - sc->time_offset;
>>                  av_log(c->fc, AV_LOG_DEBUG, "found frag time %"PRId64
>>                          ", using it for dts\n", dts);
>>              }
>>
>>
>> > On Jan 15, 2016, at 16:57, Michael Niedermayer <michael at niedermayer.cc>
>> wrote:
>> >
>> > On Fri, Jan 15, 2016 at 10:24:43PM +0000, Dan Sanders wrote:
>> >> Michael, I wanted to check if you have you looked into this playback
>> issue,
>> >> or were planning to?
>> >
>> > i didnt look into it, i had thought rodger would look into it as it
>> > was his patch ...
>> >
>> > rodger, did you look into this ?
>> >
>> > [...]
>> > --
>> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>> >
>> > Rewriting code that is poorly written but fully understood is good.
>> > Rewriting code that one doesnt understand is a sign that one is less
>> smart
>> > then the original author, trying to rewrite it will not make it better.
>>
>>
>


More information about the ffmpeg-devel mailing list