On 23.03.2016 11:50, Tanu Kaskinen wrote:
On Wed, 2016-03-23 at 11:18 +0100, Georg Chini wrote:
On 23.03.2016 10:55, Tanu Kaskinen wrote:
On Tue, 2016-03-22 at 12:12 +0100, Georg Chini wrote:
On 06.12.2015 22:38, Tanu Kaskinen wrote:
This seems buggy (also without your patch). This code is guarded by "if
(!in_pop)". The message queue is drained in the beginning of the pop
callback, and if the POST message that ends an underrun is processed in
the pop callback, then the rewind is not done, and the underrun won't
be counted in u->underruns.
Neither of the problems is particularly serious, so we could ignore
them, but the fix seems simple to me: it should be safe to just remove
the message queue draining from the pop callback. It would mean that if
the memblockq is empty, and a POST message is ready for processing when
the pop callback is called, the audio in the message won't be used, and
an unnecessary underrun will occur. I believe that's not a real
problem, however. The memblockq should be empty only when the sink and
source buffers are full, and if the sink buffer is full, the pop
callback won't be called.
Hi Tanu,
it looks like this _is_ relevant when running very short latencies.
Those "unnecessary underruns" occur for example when you
try to run HDA -> HDA at 5 ms latency. So I would suggest
to keep it mostly like it is.
Instead of masking the whole block, I would suggest to only mask
the rewind with "if (!in_pop)", so that at least the underruns are still
counted.
What are the configured sink and source latencies in that "5 ms
latency" setup, and is timer-based scheduling enabled or not? My
recollection of the past discussion is that you never agreed to
configure the target loopback latency high enough to cover the case
where both the sink and source buffers are full. If the target latency
is too low in relation to the sink and source latencies, that would
explain the underruns.
Sink & source latency are at 2.33 ms, which seems to be the lowest
latency that HDA devices support without producing underruns
or "memblock pool full" messages. Timer based scheduling is enabled.
Ok, 2 x 2.33 ms seems fine.
If it's impossible for me to convince you to avoid configuring
inherently unsafe target latencies,
5ms should not be inherently unsafe and it is working fine with
the current code. You can run it for hours without issues, tried
it often enough meanwhile. Just with your proposed change I am
hitting underruns nearly immediately and I think the code was put
there in the first place to avoid this situation. It was not me who
coded this part.
Ok, so it's a mystery why checking the message queue is necessary. This
mystery should at least be noted in a comment in the code.
Maybe the reason is that the measured latency contains the full latency
as reported by snd_pcm_delay, while the target latency configuration
only considers the buffer sizes? If the extra latency that is not
included in the buffer size is significant, that makes the latency
controller reduce the amount that is kept in the loopback memblockq.
What to do about this? I suggest just adding a comment. Feel free to
use the above speculation, if you don't have a better theory.
--
Thanks, I'll add some comment. Anyway this was a valuable hint.
I can stop to track the "no peek" events now, it looks like this was
only necessary because I missed the underuns that were masked
by the "if (!in_pop)" condition.
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
