[FFmpeg-devel] Reducing time spent in av_find_stream_info()

[Diego Santa Cruz]

> > OK I have done both :)
> 
> [...]
> 

Thanks for the remarks, see below...

> > Second patch attached (ffmpeg-info-fps.patch) modifies the table of
> > standard framerates. The new set is much reduced (from 725 to 42). I
> > replaced all framerates 1 to 60 in 1/12 steps by all regular
framerates
> > derived from 18, 24, 30 and 60 which are not below 1 fps and which
are
> > also a multiple of 1/12 (as was the case before). Note that this
second
> > patch is independent of the one above, although it may not apply
cleanly
> > without it due to nearby changes. I also added 17.98 = 18*1000/1001,
> > since I guess this might appear in some cases...
> 
> i doubt these will be enough framerates also it looks a little more
> complex than what i would have hoped ...
> 

Do you have any hints on what else is common? Any ideas on how to
simplify it without having a number as large as before?

> 
> tabs are forbidden in svn

Sorry about that... I'll change the Emacs settings :)

> 
> 
> >                  for(i=1; i<MAX_STD_TIMEBASES; i++){
> >                      int framerate= get_std_framerate(i);
> > -                    int ticks= lrintf(dur*framerate/(1001*12));
> > -                    double error= dur -
ticks*1001*12/(double)framerate;
> > -                    duration_error[index][i] += error*error;
> > +		    int ticks_den = st->time_base.den * 1001 * 12;
> 
> this can overflow, also it can be moved out of this loop

I was wondering if there was a maximum value for st->time_base.den and
what it might be... Or is the full range of int used?

> 
> 
> > +		    int64_t ticks =
> > +			(2 * duration * st->time_base.num *
> > +			 framerate + ticks_den - 1) / (2*ticks_den);
> 
> 
> 
> > +		    /* 'error' is the difference between the duration
> > +		     * as integer multiple of the frame rate and the
> > +		     * actual duration in .16 fixed-point */
> > +		    int64_t error=
> > +			( (duration * st->time_base.num * framerate -
> 
> duration * st->time_base.num * framerate can be factorized out
> 
> 
> > +			   ticks * ticks_den) << 16) /
> > +			(st->time_base.den * framerate);
> > +                    duration_error[index][i] += (error*error);
> 
> this is not accurate also the denominator is not needed all
> values added have the same

Yep, there is a loss of precision due to the fixed point nature, but
that is the problem with fixed-point. But I assumed time_base.den can
change between different packets of the same stream, in which case I
think it cannot be factored out, or is it constant once the codec has
been opened? If constant it can be removed from the computation.

> 
> 
> >                  }
> >                  duration_count[index]++;
> >              }
> 
> > @@ -1939,11 +1950,11 @@
> >                 && (st->codec->time_base.num*101LL <=
st->codec->time_base.den || st->codec-
> >codec_id == CODEC_ID_MPEG2VIDEO) /*&&
> >                 //FIXME we should not special case mpeg2, but this
needs testing with non mpeg2 ...
> >
st->time_base.num*duration_sum[i]/duration_count[i]*101LL >
st->time_base.den*/){
> > -                double best_error= 2*av_q2d(st->time_base);
> > +                int64_t best_error=
(st->time_base.num<<17)/st->time_base.den;
> >                  best_error=
best_error*best_error*duration_count[i]*1000*12*30;
> >
> >                  for(j=1; j<MAX_STD_TIMEBASES; j++){
> > -                    double error= duration_error[i][j] *
get_std_framerate(j);
> > +                    int64_t error= duration_error[i][j] *
get_std_framerate(j);
> >  //                    if(st->codec->codec_type == CODEC_TYPE_VIDEO)
> >  //                        av_log(NULL, AV_LOG_ERROR, "%f %f\n",
get_std_framerate(j) / 12.0/1001,
> error);
> >                      if(error < best_error){
> 
> i dont think this matters speed wise, also both these changes can
cause
> overflows

You are right, this does not matter much speed-wise, but I did the
change mostly for consistency with the above changes, although I
overlooked the overflow...

BTW, why is the duration_error weighted by the framerate instead of just
using the lowest error without weighting? Weighting favors large frame
rates.

-- 
Diego