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

[Georg Chini]

On 16.08.2016 18:04, Tanu Kaskinen wrote:
> 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?

The smoother relies on some kind of start time. This is true for the
current smoother code and also for the new code I have been using.
An error in that start time is never corrected in both cases.

> Does the source keep on producing negative latency reports long after
> the initial startup?

Yes, this even happens after days of runtime.

>
> Can the initial error be avoided?

As said above, I don't think so. The error is nearly 0 for hda cards,
but for USB devices it is in the range of about 500 usec and
due to the alsa driver it is currently not possible to evaluate
the start time with an error better than 500 usec. Anyway, all
measurement have errors, it is just a matter of magnitude.

>
>>>> 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..."

I need it.

>
>>   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?
>
I don't know what you mean with perceivable here. When you mean
hearable I don't think it is, but the controller is disturbed and the goal
is to get sub-millisecond stability of the end-to-end latency. If that
wasn't the goal, I could leave out half of the code. With the final
loopback code and the new smoother code (which you probably will
not accept) I am reaching 50 usec stability. Take a look at the result
section of the document I sent with the code.
In the end it is definitely wrong to sometimes ignore an offset,
sometimes ignore part of it (if it is not fully negative) and sometimes
include it. It should be either included or excluded always, and due
to the fact that we don't know the exact amount, it can only be always
included. With the inclusion, the reported latency varies smoothly
without jumps.