From 8fb33ddd4957d18e8a196aa9647b203b6d0685e5 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Fri, 2 Feb 2024 13:33:00 +0100 Subject: [PATCH] improve write() underrun handling, take 2 we *really* should not paper over underruns, as they require attention. however, the previous attempt (c2a6b6e) caused an exception to be thrown (see #130), which was a bit excessive, and was consequently reverted (438e52e). so instead we make the handling consistent with what we do in read(): return the verbatim -EPIPE in this case. this can be simply ignored, and the next write will resume the stream, so this is mostly backwards- compatible (the failing write will be discarded and would need repeating, but that will just cause a skip after the interruption, which does not seem particularly relevant). as a drive-by, again stop using snd_pcm_recover(), as it still just obfuscates the snd_pcm_prepare() call it does in the end. --- CHANGES.md | 3 ++- alsaaudio.c | 42 +++++++++++++++++++++++------------------- doc/libalsaaudio.rst | 19 ++++++++++++++----- 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 425ccf6..2f23f74 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,6 @@ # Version 0.10.1 -- restore previous xrun behaviour, #131 +- revert to not throwing an exception on playback buffer underrun; + instead, return -EPIPE like `PCM.read()` does on overrun; #131 - type hints # Version 0.10.0 diff --git a/alsaaudio.c b/alsaaudio.c index 780b1e3..6a2b98c 100644 --- a/alsaaudio.c +++ b/alsaaudio.c @@ -1473,37 +1473,41 @@ static PyObject *alsapcm_write(alsapcm_t *self, PyObject *args) } int res; + // After drop() and drain(), we need to prepare the stream again. + // Note that fresh streams are already prepared by snd_pcm_hw_params(). snd_pcm_state_t state = snd_pcm_state(self->handle); - - - if ((state != SND_PCM_STATE_XRUN && state != SND_PCM_STATE_SETUP) || - (res = snd_pcm_prepare(self->handle)) >= 0) { + if ((state != SND_PCM_STATE_SETUP) || + !(res = snd_pcm_prepare(self->handle))) { Py_BEGIN_ALLOW_THREADS res = snd_pcm_writei(self->handle, data, datalen/self->framesize); - if (res == -EPIPE) { - /* EPIPE means underrun */ - res = snd_pcm_recover(self->handle, res, 1); - if (res >= 0) { - res = snd_pcm_writei(self->handle, data, datalen/self->framesize); - } - } Py_END_ALLOW_THREADS + + if (res == -EPIPE) { + // This means buffer underrun, which we need to report. + // However, we recover the stream, so the next PCM.write() will work + // again. If recovery fails (very unlikely), report that instead. + if (!(res = snd_pcm_prepare(self->handle))) + res = -EPIPE; + } } - if (res == -EAGAIN) { - res = 0; - } - else if (res < 0) + if (res != -EPIPE) { - PyErr_Format(ALSAAudioError, "%s [%s]", snd_strerror(res), - self->cardname); + if (res == -EAGAIN) + { + res = 0; + } + else if (res < 0) { + PyErr_Format(ALSAAudioError, "%s [%s]", snd_strerror(res), + self->cardname); #if PY_MAJOR_VERSION >= 3 - PyBuffer_Release(&buf); + PyBuffer_Release(&buf); #endif - return NULL; + return NULL; + } } #if PY_MAJOR_VERSION >= 3 diff --git a/doc/libalsaaudio.rst b/doc/libalsaaudio.rst index 8bcbe51..a78e396 100644 --- a/doc/libalsaaudio.rst +++ b/doc/libalsaaudio.rst @@ -338,14 +338,23 @@ PCM objects have the following methods: period. If less than 'period size' frames are provided, the actual playout will not happen until more data is written. - If the device is not in :const:`PCM_NONBLOCK` mode, this call will block if - the kernel buffer is full, and until enough sound has been played - to allow the sound data to be buffered. The call always returns the - size of the data provided. + If the data was successfully written, the call returns the size of the + data *in frames*. + + If the device is not in :const:`PCM_NONBLOCK` mode, this call will block + if the kernel buffer is full, and until enough sound has been played + to allow the sound data to be buffered. In :const:`PCM_NONBLOCK` mode, the call will return immediately, with a return value of zero, if the buffer is full. In this case, the data - should be written at a later time. + should be written again at a later time. + + In case of a buffer underrun, this function will return the negative + size :const:`-EPIPE`, and no data is written. + At this point, the playback was already corrupted. If you want to play + the data nonetheless, call write again with the same data. + To avoid the problem in the future, try using a larger period size + and/or more periods, at the cost of higher latency. Note that this call completing means only that the samples were buffered in the kernel, and playout will continue afterwards. Make sure that the