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

[Colin Guthrie]

'Twas brillig, and David Henningsson at 13/09/10 11:14 did gyre and gimble:
> On 2010-09-04 14:10, Colin Guthrie wrote:
>> '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)?
> 
> I have now tried to repeat the experiment. The chordtest.sh seems to be
> buggy in itself (the cleanup does not remove the gst-launch, which in
> turn had to be renamed to gst-launch-0.10 here).

Yeah I have gst-launch-0.10 here too... not quite sure why, I'd have
thought we could ditch the old 0.8 support by now but hey ho. (I don't
follow gst dev super closely)

I thought that the script trapped ctrl+c and killed any processes
started. It seems to be clean for me.

Perhaps the problem is that /bin/sh is not actually bash on your system?
Perhaps just changing the first line to:

#!/bin/bash would cause it to tidy things up properly?


> Anyway, the results
> were not encouraging - with tsched=0, pavucontrol, and -vvvv to syslog
> on, three tones were heard, then things went quiet - however, pulseaudio
> started to eat more and more memory. Quickly my machine started swapping
> and became unresponsive, so I killed PA.
> Besides that, when I looked at pavucontrol, only the meters of the first
> three were moving, the other ones were silent. My log got filled up with
> "memblockq: pool full" as well. I'm getting the feeling that this
> problem is something different, unrelated to DMA controller hardware.

Interesting, can't say I noticed this, but I probably wasn't looking
closely enough.

> My suggestion is that you should commit your proposed patch as it
> improves the situation compared to the current situation. If there are
> additional problems, let's nail them down separately.

OK, sounds reasonable. Do you think the patch I posted is OK with the
1330 time?

I guess it's not super important as if it solves your original problem
that kicked off this whole thread, then that's the main thing!!

Cheers

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/]