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;
struct snd_pcm_runtime *to_check; if (signal_pending(current)) { result = -ERESTARTSYS;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 = schedule_timeout_interruptible(tout); snd_power_lock(card); down_read(&snd_pcm_link_rwsem);tout = msecs_to_jiffies(tout_ms);
-- 1.8.1.3.605.g02339dd