[pulseaudio-discuss] [PATCH] Do not use tsched watermark if tsched is disabled

[Colin Guthrie]

'Twas brillig, and David Henningsson at 03/09/10 09:46 did gyre and gimble:
> 2010-09-02 16:06, pl bossart skrev:
>>> Agreed: You can pick those two patches, and then we add a third patch to
>>> both branches, which brings back the watermark for tsched devices and 20
>>> ms for non-tsched. Assuming my suspicion is not disproved, of course.
>>> What does Pierre think of that?
>>
>> I don't want the watermark to be used for rewinds. The watermark is
>> there for timer-based scheduling, so that you have enough time to
>> wake-up from sleep and still refill the buffer.
>> The rewinds happens when the processor is already awake, pulseaudio up
>> and running and only the remix part needs to happen. Plus the
>> watermark varies and the logic could really be improved.
>>
>> Also I think 20ms for rewinds is way too much. This will kill your
>> actual latency. Imagine you have a low-latency app that starts, the
>> first sample would be heard after at best 20ms. Not acceptable for
>> speech or interactive sounds.
>>
>> But I agree that 256-bytes isn't fool-proof for heavy duty use cases
>> such 8ch 192kHz 32-bit float.
>>
>> So how about we keep 256 bytes (1.33 ms for 48kHz) but add a 1.33 ms
>> threshold to make sure we never rewind below.
>>
>> rewind_safeguard = max(256, pa_usec_to_bytes(1330));
>>
>> This way you solve both the hardware issue (frequency independent) and
>> leave enough headroom for the system to avoid underflows.
> 
> Whether 1,33 ms or 20 ms is best - I assume your guess is as good as
> mine. Colin, feel free to go ahead with Pierre's suggestion - it's
> likely to be good enough.
> 
> As for the watermark usage, I admit to not knowing enough of CPU
> scheduling and wake-up times to either prove Pierre right or wrong.

OK, I've done this now.

The patch is attached. It's based on stable-queue with the two previous
patches cherry-picked first (and also Tanu's
0525807b63c11d3d71526cec553e8d80ad3f09cd which fixed a complier warning,
but shouldn't get in the way)

However, in testing this I had some problems. Likely this is due to me
testing hard/more thoroughly than before.

I found that using the attached patch fixed the chordtest.sh case for
tsched=0, however, when running pavucontrol at the same time, everything
started to go wrong pretty quickly (after two or three streams). When
things when wrong, they generally stayed wrong. i.e. ctrl+c on the
chordtest.sh kills all the streams, but if I rerun it, then the very
first stream is generally cocked up. Interestingly a paplay seemed to
work fine.

So I changed the 1330 usec to 20000 and tried again.

This had slightly better results, but still broke the chordtest.sh case
after three streams (fairly repeatable) when pavucontrol is running
(unsurprisingly it also worked fine when pavucontrol was not running).
The difference in this case however was the rerunning the test after an
initial failure worked fine. The sound was back to normal on the first
stream and only generally cocked up when it hit the third stream.

So what does this test mean? pavucontrol obviously affects the latency
of the sink due to it's VI meters. This obviously increases the
likelihood of a rewind being triggered. So, with this in mind what
values do you suggest we pick?


I'd be interested as to whether anyone else can repeat this experiment
and get similar results. Do you guys get a broken chordtest too (it's on
the RedHat bug I mentioned at the beginning of this thread)?

Col




-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mandriva Linux Contributor [http://www.mandriva.com/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-alsa-Set-the-rewind-safeguard-proportionally-to-samp.patch
URL: <http://lists.freedesktop.org/archives/pulseaudio-discuss/attachments/20100904/58d0aa2a/attachment.txt>