'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/]
From a65825fa45a1c393899eeb1e4532236964ea3a2c Mon Sep 17 00:00:00 2001 From: Colin Guthrie <cguth...@mandriva.org> Date: Sat, 4 Sep 2010 11:58:05 +0100 Subject: [PATCH] alsa: Set the rewind safeguard proportionally to sample spec Currently when rewinding alsa, a fixed value of 256 bytes is used, which represents 1.33ms @ 48kHz (2ch, 16bit). This is typically fine and due to DMA constraints we would not want to rewind less than this. However with more demanding sample specs, (e.g. 8ch 192kHz 32bit) 256 bytes is likely not sufficient, so calculate what 1.33ms would be and use which ever value is bigger. Discussed with David Henningsson and Pierre-Louis Bossart here: http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/7286 --- src/modules/alsa/alsa-sink.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c index 9e200bd..4a5fe3f 100644 --- a/src/modules/alsa/alsa-sink.c +++ b/src/modules/alsa/alsa-sink.c @@ -83,7 +83,8 @@ #define VOLUME_ACCURACY (PA_VOLUME_NORM/100) /* don't require volume adjustments to be perfectly correct. don't necessarily extend granularity in software unless the differences get greater than this level */ -#define DEFAULT_REWIND_SAFEGUARD_BYTES (256) /* 1.33ms @48kHz, should work for most hardware */ +#define DEFAULT_REWIND_SAFEGUARD_BYTES (256U) /* 1.33ms @48kHz, we'll never rewind less than this */ +#define DEFAULT_REWIND_SAFEGUARD_USEC (1330) /* 1.33ms, depending on channels/rate/sample we may rewind more than 256 above */ struct userdata { pa_core *core; @@ -1733,7 +1734,7 @@ pa_sink *pa_alsa_sink_new(pa_module *m, pa_modargs *ma, const char*driver, pa_ca goto fail; } - rewind_safeguard = DEFAULT_REWIND_SAFEGUARD_BYTES; + rewind_safeguard = PA_MAX(DEFAULT_REWIND_SAFEGUARD_BYTES, pa_usec_to_bytes(DEFAULT_REWIND_SAFEGUARD_USEC, &ss)); if (pa_modargs_get_value_u32(ma, "rewind_safeguard", &rewind_safeguard) < 0) { pa_log("Failed to parse rewind_safeguard argument"); goto fail; -- 1.7.2.2
_______________________________________________ pulseaudio-discuss mailing list pulseaudio-discuss@mail.0pointer.de https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
