On 16.02.2017 12:59, Tanu Kaskinen wrote:
> On Fri, 2017-02-10 at 14:38 +0100, Georg Chini wrote:
>> Hi,
>>
>> during my work on module-loopback I came upon an issue regarding the
>> handling
>> of the port latency offsets.
>> The functions pa_{source,sink}_get_latency_within_thread() return the
>> latencies
>> including the offset. Those functions are used to determine the amount
>> that has
>> to be rewound during a sink move and also to calculate the time a volume
>> change
>> is delayed.
>>
>> I think it is wrong in those situations to include the offset,
>> especially if a user sets
>> very large or very negative offsets. If you have a large negative offset
>> for example,
>> the sink will never be rewound on a move because get_latency_in_thread()
>> always
>> returns 0. If the offset is large and positive, you might end up
>> rewinding too much,
>> although this is limited by max_rewind.
>> A similar thought applies to the delay of the volume changes, although
>> it is surely
>> less critical there.
>>
>> Am I right or do I miss something?
> You're right. The total latency is not the same thing as the rewindable
> amount. I think there's a FIXME comment about this somewhere in the
> stream moving code.
Could this not be fixed quite easily by subtracting the offset again
in these places? Should I send a patch?
>
>> Would it be a good idea to let the functions return a structure
>> containing the
>> actual latency and the offset in separate variables and let the caller
>> sort out
>> if it needs the offset or not?
> The term "actual latency" includes the offset, if the offset is
> correct. The important distinction is between "total latency" and
> "rewindable amount". And yes, it would be a good idea to report these
> separately.
>
I think it is not really necessary, because there is the port_latency_offset
variable, which can be directly accessed in all places where you might
need the offset. (And if there will be some sink_latency_offset in the
future, the same applies)
