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

[Michael Niedermayer]

Hi

On Fri, Jul 14, 2017 at 02:52:23PM -0700, Dale Curtis wrote:
> 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.

I dont think CTTS is the only affected atom.

what you describe sounds like an option. but i might be missing something,
possibly even something significant. I dont feel that confident with
this code after quickly looking at it.


> 
> - 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.
> >>
> >>
> >
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170715/c2f70b11/attachment.sig>