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