[FFmpeg-devel] [PATCH] ALSA: fix timefilter divergence

[Michael Niedermayer]

On Thu, Feb 16, 2012 at 09:35:45PM +0100, Nicolas George wrote:
> L'octidi 28 pluviôse, an CCXX, Michael Niedermayer a écrit :
> > Iam quite interrested to see it failing, also heres some simple change
> > that vastly improves the timefilters performance in its selftest
> 
> I had some time to look at the code, but not tinker with it.
> 
> Could you explain in a few words the maths behind your suggested changes?

of course
what we have are noisy time values at arbitrary time intervals
we also know these time intervals to a pretty good approximation.
There is a systematic error in the time intervals due to clocks not
being exact while i assumed that this systematic error is sufficiently
close to constant over time. That is 44100 hz may actually be
44098 but wont drift slowly around between that and 44102. If this
assumtation is wrong the filter becomes less optimal but
should not catastrophically fail.

The way i implemented that is quite simply to take the first and
last timestamps and the "number of samples" to get the true sample
rate and use this instead of a exponentially decaying average.


> 
> > note, the variable names in the patch below are not very good, dont
> > be mislead by them
> 
> They were already awful anyway.
> 
> > @@ -41,9 +42,9 @@ TimeFilter *ff_timefilter_new(double clock_period,
> >                                double feedback3_factor)
> >  {
> >      TimeFilter *self       = av_mallocz(sizeof(TimeFilter));
> > -    self->clock_period     = clock_period;
> > +    self->feedback3_factor = clock_period/feedback3_factor;
> > +    self->clock_period_den = 1.0/feedback3_factor;
> >      self->feedback2_factor = feedback2_factor;
> > -    self->feedback3_factor = feedback3_factor;
> >      return self;
> >  }
> > 
> > @@ -62,16 +63,18 @@ double ff_timefilter_update(TimeFilter *self, double system_time, double period)
> >      self->count++;
> >      if (self->count == 1) {
> >          /// init loop
> > +        self->first_time =
> >          self->cycle_time = system_time;
> >      } else {
> >          double loop_error;
> > -        self->cycle_time += self->clock_period * period;
> > +        double num = FFMAX(self->cycle_time - self->first_time, 0) + self->feedback3_factor;
> > +        self->cycle_time +=  num * period / self->clock_period_den;
> >          /// calculate loop error
> >          loop_error = system_time - self->cycle_time;
> > 
> >          /// update loop
> >          self->cycle_time   += FFMAX(self->feedback2_factor, 1.0 / self->count) * loop_error;
> > -        self->clock_period += self->feedback3_factor * loop_error / period;
> > +        self->clock_period_den += period;
> >      }
> >      return self->cycle_time;
> >  }
> 
> If I read things correctly, the second-order feedback factor becomes now, if
> I eliminate all unnecessary multiplicative coefficients:
> 
> 	out <- out + (out + c) / (in + c)
> 
> Which means that as time passes, the filter becomes less reactive, I do not
> think this is a good property for such a filter.
> 
> For example, if the factor between input and output changes (practical case:
> ntpd just got the network back and starts correcting a drift), the time the
> filter will take to adjust to it will be roughly ten times the time it has
> already run. I do not think this is acceptable.

I dont think either of the filters behaves very well in this case
having the timestamps significantly wrong for a long time and then
correcting that suddenly likely leads to some issues.
It might be better to detect this case and force a (partial?) reset of
the internal state.

Also theres another issue, if theres a significnat correction from
NTPD that may mean the previous hours of recording used wrong
timestamps. And if there are 2 streams (audio & video) which use
seperate timefilter, they may with either implementation recover at
different rates makeing the whole issue even worse


> 
> I will need to run some tests to check if my hypothesis is valid, though.

Maybe we can extend the selftest code to cover a wider variety of
cases so that we can at first approximation compare different
implementations simply by running the tests instead of lengthly
analysis of their behavior

[...]

Thanks
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120216/50d1becd/attachment.asc>