[FFmpeg-devel] Fix ctts_index calculation

[Sasi Inguva]

The new patch looks good to me.

On Fri, Feb 9, 2018 at 1:34 PM, Xiaohan Wang (王消寒) <xhwang at chromium.org>
wrote:

> I uploaded a new patch. Is this what you meant?
>
> I am confused with the code though. In the case *ctts_index == ctts_count,
> this while loop will do nothing. Is this what we want? Also, the "wrong"
> |ctts_index| will be available outside of this function. How do we make
> sure it won't cause trouble later?
>
> On Fri, Feb 9, 2018 at 12:26 PM, Sasi Inguva <isasi-at-google.com at ffmpeg.
> org
> > wrote:
>
> > >
> > > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > > index 5adba52e08..f0bd3e3623 100644
> > > --- a/libavformat/mov.c
> > > +++ b/libavformat/mov.c
> > > @@ -3148,7 +3148,7 @@ static int find_prev_closest_index(AVStream *st,
> > >          *ctts_index = 0;
> > >          *ctts_sample = 0;
> > >          for (index_ctts_count = 0; index_ctts_count < *index;
> > > index_ctts_count++) {
> > > -            if (*ctts_index < ctts_count) {
> >
> > +            if (*ctts_index < ctts_count - 1) {
> > >
> > If you do this, then we are skipping, looking at ctts_samples in the last
> > ctts entry where (*ctts_index == ctts_count -1), which is not desired.
> >
> >                  (*ctts_sample)++;
> > >                  if (ctts_data[*ctts_index].count == *ctts_sample) {
> > >                      (*ctts_index)++;
> > >
> >
> > Here *ctts_index == ctts_count  only iff   the *ctts_index is the last
> CTTS
> > entry, and we still didn't reach   *index. That means that it's file with
> > CTTS truncated i.e. no corresponding CTTS entries for some STTS entries.
> > This issue can be fixed by checking for  *ctts_index <  ctts_count, in
> the
> > line after this loop.
> > while (*index
> > <https://cs.corp.google.com/piper///depot/google3/third_
> > party/ffmpeg/next/libavformat/mov.c?l=3123&ct=xref_jump_to_
> > def&gsn=index&rcl=184932376>
> > >= 0 && (*ctts_index
> > <https://cs.corp.google.com/piper///depot/google3/third_
> > party/ffmpeg/next/libavformat/mov.c?l=3124&ct=xref_jump_to_
> > def&gsn=ctts_index&rcl=184932376>)
> > >= 0) {
> > we already check for (*ctts_index) >=0 so we can check for the upper
> bound
> > too.
> >
> > On Sat, Feb 3, 2018 at 12:36 PM, Michael Niedermayer
> > <michael at niedermayer.cc
> > > wrote:
> >
> > > On Fri, Feb 02, 2018 at 05:55:38PM -0800, Xiaohan Wang (王消寒) wrote:
> > > >
> > >
> > > >  mov.c |    2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 87157b4053de0e044e78db72ef13e8058075c235
> > 0001-Fix-ctts_index-calculatio
> > > n.patch
> > > > From bb376fd2de5da5f9ecdef41621a579252b899d7d Mon Sep 17 00:00:00
> 2001
> > > > From: Xiaohan Wang <xhwang at chromium.org>
> > > > Date: Fri, 2 Feb 2018 17:33:56 -0800
> > > > Subject: [PATCH] Fix ctts_index calculation
> > > >
> > > > An index should never be equal to the count. Hence we must make sure
> > > > *ctts_index < ctts_count.
> > > > ---
> > > >  libavformat/mov.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > should be reviewed by sasi idealy, he wrote this
> > >
> > > thx
> > >
> > >
> > > [...]
> > >
> > > --
> > > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC7
> 87040B0FAB
> > >
> > > Democracy is the form of government in which you can choose your
> dictator
> > >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>