[PATCH] fix snd_pcm_drain() excluding SETUP state from valid states
once draining is done, the pcm enters the SETUP state, which ought to be valid for snd_pcm_drain()
signed-off-by: Sylvain BERTRAND sylvain.bertrand@legeek.net ---
I missed this one in my previous patch because exiting with or without an error once draining is done was producing the same result.
--- a/src/pcm/pcm.c +++ b/src/pcm/pcm.c @@ -1329,7 +1329,7 @@ int snd_pcm_drain(snd_pcm_t *pcm) SNDMSG("PCM not set up"); return -EIO; } - err = bad_pcm_state(pcm, P_STATE_RUNNABLE); + err = bad_pcm_state(pcm, P_STATE_RUNNABLE | P_STATE(SETUP)); if (err < 0) return err; /* lock handled in the callback */
Dne 02. 05. 20 v 21:33 sylvain.bertrand@gmail.com napsal(a):
once draining is done, the pcm enters the SETUP state, which ought to be valid for snd_pcm_drain()
signed-off-by: Sylvain BERTRAND sylvain.bertrand@legeek.net
I missed this one in my previous patch because exiting with or without an error once draining is done was producing the same result.
NAK: You should not call drain when the PCM handle is in the SETUP field. It's an obvious caller problem. The streaming should be active somehow.
Jaroslav
--- a/src/pcm/pcm.c +++ b/src/pcm/pcm.c @@ -1329,7 +1329,7 @@ int snd_pcm_drain(snd_pcm_t *pcm) SNDMSG("PCM not set up"); return -EIO; }
- err = bad_pcm_state(pcm, P_STATE_RUNNABLE);
- err = bad_pcm_state(pcm, P_STATE_RUNNABLE | P_STATE(SETUP)); if (err < 0) return err; /* lock handled in the callback */
On Thu, May 14, 2020 at 03:52:25PM +0200, Jaroslav Kysela wrote:
NAK: You should not call drain when the PCM handle is in the SETUP field. It's an obvious caller problem. The streaming should be active somehow.
The pb here is the non-blocking calls of the drain function: in my test case, the first call to the drain function switches the pcm in draining state, but the pcm will be switched to the setup state somewhen in between 2 drain function calls! Naively, I was calling the drain function on a regular time basis to see if the draining was finished, namely expecting 0 to be returned.
Then if I understood you well, the right way(tm) to use the drain function in non-block mode, is to call only once the drain function, then inspect the state of the pcm till it not anymore in the draining state.
Am I right? Or did I miss something again?
regards,
Dne 14. 05. 20 v 23:06 sylvain.bertrand@gmail.com napsal(a):
On Thu, May 14, 2020 at 03:52:25PM +0200, Jaroslav Kysela wrote:
NAK: You should not call drain when the PCM handle is in the SETUP field. It's an obvious caller problem. The streaming should be active somehow.
The pb here is the non-blocking calls of the drain function: in my test case, the first call to the drain function switches the pcm in draining state, but the pcm will be switched to the setup state somewhen in between 2 drain function calls! Naively, I was calling the drain function on a regular time basis to see if the draining was finished, namely expecting 0 to be returned.
Then if I understood you well, the right way(tm) to use the drain function in non-block mode, is to call only once the drain function, then inspect the state of the pcm till it not anymore in the draining state.
Am I right? Or did I miss something again?
I looked to this problem again and the original patch seems more appropriate. The snd_pcm_drain() should return zero, if the state is SETUP, because there is no further work.
I applied your patch:
https://github.com/alsa-project/alsa-lib/commit/1b9104b5ff10be7f60441f622436...
with the (sanity) optimization in:
https://github.com/alsa-project/alsa-lib/commit/0b7f1441bb82903d45a29bf83c84...
It basically doesn't allow to call the plugin callback (otherwise we need to review all plugin drain callbacks, if the SETUP state is handled properly).
Thank you for your suggestion, Jaroslav
regards,
augment the documentation regarding the use of the snd_pcm_drain function in non-blocking mode.
signed-off-by: Sylvain BERTRAND sylvain.bertrand@legeek.net --- src/pcm/pcm.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) --- diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c index 1064044c..0d4b2930 100644 --- a/src/pcm/pcm.c +++ b/src/pcm/pcm.c @@ -1311,8 +1311,14 @@ int snd_pcm_drop(snd_pcm_t *pcm) * \return 0 on success otherwise a negative error code * \retval -ESTRPIPE a suspend event occurred * - * For playback wait for all pending frames to be played and then stop - * the PCM. + * For playback, in blocking mode, wait for all pending frames to be played + * and then stop the PCM. + * For playback, in non-blocking mode, will return -EAGAIN if the pcm is still + * being drained at the time of the call. A note of caution: the pcm can finish + * draining asynchronously from a snd_pcm_draw call. The pcm will be then in + * SND_PCM_STATE_SETUP state which means any subsequent calls to snd_pcm_drain + * will fail since you cannot switch the pcm to SND_PCM_STATE_DRAINING state + * from SND_PCM_STATE_SETUP state. * For capture stop PCM permitting to retrieve residual frames. * * For stopping the PCM stream immediately, use \link ::snd_pcm_drop() \endlink
ping?
regards,
participants (2)
-
Jaroslav Kysela -
sylvain.bertrand＠gmail.com