[pulseaudio-discuss] [PATCH v6 14/25] source.c, sink.c: Implement pa_{source, sink}_get_raw_latency_within_thread() calls

[Tanu Kaskinen]

On Tue, 2016-08-16 at 08:07 +0200, Georg Chini wrote:
> On 15.08.2016 23:40, Tanu Kaskinen wrote:
> > 
> > On Sun, 2016-06-05 at 21:05 +0200, Georg Chini wrote:
> > > 
> > > In certain situations, the reported sink or source latency may
> > > become
> > > negative.
> > What are those certain situations?
> 
> I'm not really sure, I believe the reports get negative when the
> initial
> start time of the calculation is wrong. So the reported source or
> sink
> latencies have a permanent offset which can not be avoided because
> all reported times/latencies have errors.

Permanent offset? Surely the smoother smooths out any initial errors?
Does the source keep on producing negative latency reports long after
the initial startup?

Can the initial error be avoided?

> > > This does not indicate that the latency is indeed negative but
> > > can be considered
> > > merely an offset error. The current get_latency() calls truncate
> > > negative latencies
> > > because they do not make sense from a physical point of view.
> > > In the context of module loopback, negative source or sink
> > > latencies are acceptable
> > > because the overall latency never becomes negative. Truncating
> > > negative values leads
> > > to discontinuities in the latency reports, therefore this patch
> > > implements functions
> > > in source.c and sink.c which can return the raw value without
> > > truncation.
> > What's the practical problem with the discontinuities? A negative
> > latency report is always incorrect, so changing it to 0 can only
> > make
> > the latency report closer to truth. The explanation isn't
> > sufficient
> > for me to understand why that is such a big problem that it
> > warrants
> > adding this stuff to the core.
> 
> Actually you recommended to add it to the core.

This sounded familiar, but I didn't find the right thread yesterday.
Now I found it, and what I said was:

"If you really need access to "unfiltered" values, maybe
pa_sink_get_latency() could have an "allow_negative" parameter or
something similar."

So the recommendation was conditional, "if you really need..."

>  From a practical perspective your argument is not important. You only 
> consider
> an isolated source or sink latency, The end-to-end latency never gets 
> negative
> and should be continuous.
> 
> In effect, the calculated end-to-end latency might really be slightly 
> too small,
> because of an incorrect offset, but this offset applies to ALL reported 
> latencies,
> thereby lowering the reported end-to-end latency not only when one of the
> components gets negative.
> Truncating the latency when it gets negative would mean sometimes 
> ignoring the
> offset and sometimes not which not does not look like a good idea.
> 
> Let me give you an example of what I am seeing:
> 
>        source      sink    buffer   total
> 1.        0.5         10      50.5      61
> 2.       -0.4         12      49.4      61
> 3.        0.2           9      51.8      61
> 
> If you truncated the negative latency in the second measurement, you would
> end up with 61.4 ms, which may be nearer to the "real" end-to-end 
> latency, but
> will trigger an unwanted and unneeded correction due to an isolated jump 
> in the
> time series.

We're talking about sub-millisecond errors here. Are there any
perceivable problems due to the unneeded corrections?

-- 
Tanu