On 17.04.2017 18:52, Tanu Kaskinen wrote:
> On Fri, 2017-04-14 at 17:02 +0200, Georg Chini wrote:
>> The reported latency of source or sink is based on measured initial conditions.
>> If the conditions contain an error, the estimated latency values may become negative.
>> This does not indicate that the latency is indeed negative but can be considered
>> merely an offset error. The current get_latency_in_thread() calls and the
>> implementations of the PA_{SINK,SOURCE}_MESSAGE_GET_LATENCY messages truncate negative
>> latencies because they do not make sense from a physical point of view. In fact,
>> the values are truncated twice, once in the message handler and a second time in
>> the pa_{source,sink}_get_latency_within_thread() call itself.
>> This leads to two problems for the latency controller:
> The commit references "the latency controller" without giving any clue
> about what that is, so please mention that you're talking about module-
> loopback.
>
> If I understood correctly, the two problems you mention here are not
> yet fixed, but will be fixed in a subsequent patch. That could be
> stated more clearly in the commit message.
>
>> - Truncating leads to discontinuities in the latency reports which then trigger
>> unwanted end to end latency corrections
>> - If a large negative port latency offsets is set, the reported latency is always 0,
>> making it impossible to control the end to end latency at all
>>
>> This patch adds a flag to pa_{sink,source}_get_latency_within_thread() to allow
>> negative return values. Truncating is also removed in all implementations of the
>> PA_{SINK,SOURCE}_MESSAGE_GET_LATENCY message handlers. The allow_negative flag
>> is set to false for all calls of pa_{sink,source}_get_latency_within_thread()
>> except when used within PA_{SINK,SOURCE}_MESSAGE_GET_LATENCY. This means that the
>> original behavior is not altered in most cases. Only if a positive latency offset
>> is set and the message returns a negative value, the reported latency is smaller
>> because the values are not truncated twice.
>>
>> Additionally let PA_SOURCE_MESSAGE_GET_LATENCY return -pa_sink_get_latency_within_thread()
>> for monitor sources because the source gets the data before it is played.
> Even though I complained about a couple of things, the commit message
> is pleasantly clear and detailed. Thank you!
>
> The code looks good to me.
>
Thanks! Modified the commit message and pushed both patches.
