[pulseaudio-discuss] [PATCH v6 17/25] loopback: Track and use average adjust time

[Tanu Kaskinen]

On Sat, 2016-08-20 at 14:55 +0200, Georg Chini wrote:
> On 20.08.2016 14:23, Tanu Kaskinen wrote:
> > 
> > On Sun, 2016-06-05 at 21:05 +0200, Georg Chini wrote:
> > > 
> > > > 
> > > > @@ -79,11 +79,18 @@ struct userdata {
> > >   
> > >       pa_time_event *time_event;
> > >   
> > > +    /* Variables used to calculate the average time between
> > > +     * subsequent calls of adjust_rates() */
> > > +    pa_usec_t time_stamp;
> > "time_stamp" is quite generic name. I'd prefer
> > "last_adjustment(_time_stamp)" or something like that.
> > 
> > > 
> > > +    pa_usec_t real_adjust_time;
> > > +    pa_usec_t real_adjust_time_sum;
> > > +
> > >       /* Various counters */
> > >       int64_t recv_counter;
> > >       int64_t send_counter;
> > >       uint32_t iteration_counter;
> > >       uint32_t underrun_counter;
> > > +    uint32_t adjust_counter;
> > iteration_counter and adjust_counter can be merged to just one
> > counter.
> > 
> > The counters have two differences. One difference is that
> > iteration_counter counts all adjust_rate() calls, while adjust_counter
> > skips the first call. The other difference is that iteration_counter is
> > reset when streams move or device latency changes, while adjust_counter
> > is not.
> > 
> > iteration_counter is used for calculating the time since the last reset
> > (stream move or device latency change), but that calculation ends up
> > being wrong, because the first adjust_rate() call is done much quicker
> > than the other calls. If iteration_counter would also skip the first
> > adjust_rate() call, the calculation would be more correct, although
> > still wrong, because the short first interval isn't included in the
> > calculation.
> 
> This is not really correct. When you take a look at the patch, you
> can see that iteration_counter is used before it is incremented, so
> the actual state is, that the first short interval is not included.
> I ignored that error, because the counter is only used to calculate
> the number of hours module-loopback is running under the same
> conditions and I am only using the difference of the hours, so an
> error of 333 ms for the first hour seems negligible.

Ah, indeed, so for this part iteration_counter and adjust_counter
behave identically.

> > The other difference between the counters is that iteration_counter is
> > reset when streams move or device latency changes, while adjust_counter
> > is not reset. However, I don't see any harm in resetting adjust_counter
> > too.
> 
> There is a reason why I am using two counters. If I would reset
> adjust_counter each time source or sink changed, I would also
> have to reset the adjust_time_sum. This however would mean
> starting to re-calculate the average adjust time each time
> source or sink changes. Since the adjust time cannot be modified
> during the runtime of module-loopback, it does not make sense
> to restart the calculation and would lower the precision of the
> average.

Well, changing the sink or source of a loopback is rare, I believe, and
I also believe that the harm from the loss of precision is negligible.
But if you think that the precision issue is grave enough to justify
another counter, then ok, let's leave that in.

-- 
Tanu