[alsa-devel] [RFC] ALSA: Reduce delay for a blocking pcm_drain.
This patch addresses two issues related to calling snd_pcm_drain.
If no_period_wakeup is set, then snd_pcm_drain would wait MAX_SCHEDULE_TIMEOUT jiffies, without a wakeup pending this will leave the calling task blocked indefinitely. Instead block for double the buffer size played back at a low sample rate (4k chosen arbitrarily).
Also if the stream is running with period wakeups but with a long period, the delay could be seconds. If only a small part of the buffer is being used, this is unnecessary. Instead wait for the remaining samples to play out, plus one millisecond. This allows systems that fill the buffer only part way to still do a blocking drain before closing to avoid losing samples.
Change-Id: I84ee1a52d9bdf80ea6065fb9533e565d3e6c8021 Signed-off-by: Dylan Reid dgreid@chromium.org ---
This can be worked around in user space as well by using a nonblocking drain and sleeping based on how many samples are queued. Would that be the right thing to do or is this (or something similar) worth fixing here.
To test this I opened a pcm and set no period wakeups, played a few samples and then called drain, which hung indefinitely. Using a USB headset with a max period size of 128k but only 15ms of the buffer filled, having the drain time shorter without losing samples makes a blocking call feasible.
--- sound/core/pcm_native.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index f4aaf5a..a5f0695 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1481,7 +1481,7 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream, }
for (;;) { - long tout; + long tout, tout_ms; struct snd_pcm_runtime *to_check; if (signal_pending(current)) { result = -ERESTARTSYS; @@ -1505,16 +1505,13 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream, snd_pcm_stream_unlock_irq(substream); up_read(&snd_pcm_link_rwsem); snd_power_unlock(card); - if (runtime->no_period_wakeup) - tout = MAX_SCHEDULE_TIMEOUT; - else { - tout = 10; - if (runtime->rate) { - long t = runtime->period_size * 2 / runtime->rate; - tout = max(t, tout); - } - tout = msecs_to_jiffies(tout * 1000); + tout_ms = runtime->buffer_size * 2 * 1000 / 4000; + if (runtime->rate) { + snd_pcm_sframes_t hw_avail; + hw_avail = snd_pcm_playback_hw_avail(runtime); + tout_ms = 1 + hw_avail * 1000 / runtime->rate; } + tout = msecs_to_jiffies(tout_ms); tout = schedule_timeout_interruptible(tout); snd_power_lock(card); down_read(&snd_pcm_link_rwsem);
At Fri, 27 Sep 2013 20:59:45 -0700, Dylan Reid wrote:
This patch addresses two issues related to calling snd_pcm_drain.
If no_period_wakeup is set, then snd_pcm_drain would wait MAX_SCHEDULE_TIMEOUT jiffies, without a wakeup pending this will leave the calling task blocked indefinitely. Instead block for double the buffer size played back at a low sample rate (4k chosen arbitrarily).
Also if the stream is running with period wakeups but with a long period, the delay could be seconds. If only a small part of the buffer is being used, this is unnecessary. Instead wait for the remaining samples to play out, plus one millisecond. This allows systems that fill the buffer only part way to still do a blocking drain before closing to avoid losing samples.
Change-Id: I84ee1a52d9bdf80ea6065fb9533e565d3e6c8021 Signed-off-by: Dylan Reid dgreid@chromium.org
This can be worked around in user space as well by using a nonblocking drain and sleeping based on how many samples are queued. Would that be the right thing to do or is this (or something similar) worth fixing here.
To test this I opened a pcm and set no period wakeups, played a few samples and then called drain, which hung indefinitely. Using a USB headset with a max period size of 128k but only 15ms of the buffer filled, having the drain time shorter without losing samples makes a blocking call feasible.
The no_period_wakeup option is a kind of "all your PCM timing control are belong to us" policy. So it's better to keep the kernel behavior as simple as possible in that case, IMO.
thanks,
Takashi
sound/core/pcm_native.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index f4aaf5a..a5f0695 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1481,7 +1481,7 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream, }
for (;;) {
long tout;
long tout, tout_ms;@@ -1505,16 +1505,13 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream, snd_pcm_stream_unlock_irq(substream); up_read(&snd_pcm_link_rwsem); snd_power_unlock(card);
if (runtime->no_period_wakeup)tout = MAX_SCHEDULE_TIMEOUT;else {tout = 10;if (runtime->rate) {long t = runtime->period_size * 2 / runtime->rate;tout = max(t, tout);}tout = msecs_to_jiffies(tout * 1000);
tout_ms = runtime->buffer_size * 2 * 1000 / 4000;if (runtime->rate) {snd_pcm_sframes_t hw_avail;hw_avail = snd_pcm_playback_hw_avail(runtime);tout_ms = 1 + hw_avail * 1000 / runtime->rate;tout = msecs_to_jiffies(tout_ms);-- 1.8.1.3.605.g02339dd
Dylan Reid wrote:
This patch addresses two issues related to calling snd_pcm_drain.
If no_period_wakeup is set, then snd_pcm_drain would wait MAX_SCHEDULE_TIMEOUT jiffies, without a wakeup pending this will leave the calling task blocked indefinitely.
no_period_wakeup is used by applications that do _not_ want to use any of ALSA's blocking functions (and thus want to avoid the interrupt overhead) and use their own timers instead. This also applies to snd_pcm_drain.
Is there any actual application that tries to use snd_pcm_drain together with no_period_wakeup?
Also if the stream is running with period wakeups but with a long period, the delay could be seconds. If only a small part of the buffer is being used, this is unnecessary.
Wakeup at period boundaries is part of the ALSA API. (This is all what periods are for.)
Instead wait for the remaining samples to play out, plus one millisecond.
The device's clock and the Linux system clock might have larger differences. The only reliable synchronization source is the period interrupt.
Regards, Clemens
On Mon, Sep 30, 2013 at 1:45 AM, Clemens Ladisch clemens@ladisch.de wrote:
Dylan Reid wrote:
This patch addresses two issues related to calling snd_pcm_drain.
If no_period_wakeup is set, then snd_pcm_drain would wait MAX_SCHEDULE_TIMEOUT jiffies, without a wakeup pending this will leave the calling task blocked indefinitely.
no_period_wakeup is used by applications that do _not_ want to use any of ALSA's blocking functions (and thus want to avoid the interrupt overhead) and use their own timers instead. This also applies to snd_pcm_drain.
Is there any actual application that tries to use snd_pcm_drain together with no_period_wakeup?
Not a real one. I am using no_period_wakeup and looking for an easy way to flush at the end. User space already knows the buffer level, so it can wait for the appropriate time and wait for an underrun just as easily. Thanks for the explanation all.
Dylan
participants (3)
-
Clemens Ladisch -
Dylan Reid -
Takashi Iwai