| Message ID | s5hr2ocp4ks.wl-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Takashi, but in this case each IO plugin has to have its own thread which checks for io->state == DRAINING to start draining. For example the pulse plugin do not have an own thread. Best regards Timo Wischer
On Thu, 22 Mar 2018 15:50:35 +0100, Wischer, Timo (ADITG/ESB) wrote: > > Hi Takashi, > > but in this case each IO plugin has to have its own thread which checks for io->state == DRAINING to start draining. > For example the pulse plugin do not have an own thread. I don't understand. The pulse plugin calls pa_stream_drain(), and pulse_wait_operation() to wait until the drain finishes. What does it have to do with threading? Takashi
> I don't understand. The pulse plugin calls pa_stream_drain(), and > pulse_wait_operation() to wait until the drain finishes. What does it > have to do with threading? As far as I understand you patch io->data->callback->drain() will not be called in nonblocking mode. + if (io->data->state == SND_PCM_STATE_DRAINING) { + if (io->data->nonblock) + return -EAGAIN; + + if (io->data->callback->drain) { + err = io->data->callback->drain(io->data); + if (err < 0) + return err; + } + } Therefore how should call pa_stream_drain() in case of nonblocking mode? Best regards Timo
On Thu, 22 Mar 2018 16:17:10 +0100, Wischer, Timo (ADITG/ESB) wrote: > > > I don't understand. The pulse plugin calls pa_stream_drain(), and > > pulse_wait_operation() to wait until the drain finishes. What does it > > have to do with threading? > > As far as I understand you patch io->data->callback->drain() will not be called in nonblocking mode. > + if (io->data->state == SND_PCM_STATE_DRAINING) { > + if (io->data->nonblock) > + return -EAGAIN; > + > + if (io->data->callback->drain) { > + err = io->data->callback->drain(io->data); > + if (err < 0) > + return err; > + } > + } > > Therefore how should call pa_stream_drain() in case of nonblocking mode? The application needs to sync manually via poll() instead. It's also the behavior of the kernel driver, which ioplug follows. Takashi
> The application needs to sync manually via poll() instead. You mean the user application which opens the ALSA virtual device (aka IO plugin), right? > It's also the behavior of the kernel driver, which ioplug follows. I know that your suggested solution is the behavior of the kernel. But in kernel space we have the DMA interrupt which checks the state for draining. To use the same mechanism in the IO plug each IO plug needs to have its own thread which checks state for draining. In case of pulse without this additional thread I have no idea how pulseaudio can be informed that draining starts because in nonblocking mode there is no function in the pulse IO plugin which is called to inform about the state change to draining. The only solution which I have in my mind is this additional thread state_check_thread() { while (true) { if (state == DRAINING) pa_stream_drain() ... } } Therefore with your proposed solution there is additional effort in each new IO plug required to support draining in nonblocking mode. (With my solution exactly the same mechanism (drain callback and poll) is used in blocking and nonblocking mode. Therefore new IO plugins will support both modes without additional efforts in the IO plugin implementation) I hope my concern is now more clear. Best regards Timo
On Fri, 23 Mar 2018 08:23:56 +0100, Wischer, Timo (ADITG/ESB) wrote: > > > The application needs to sync manually via poll() instead. > > You mean the user application which opens the ALSA virtual device (aka IO plugin), right? No, no matter which device is used. > > It's also the behavior of the kernel driver, which ioplug follows. > > I know that your suggested solution is the behavior of the kernel. > But in kernel space we have the DMA interrupt which checks the state for draining. > > To use the same mechanism in the IO plug each IO plug needs to have its own thread which checks state for draining. > In case of pulse without this additional thread I have no idea how pulseaudio can be informed that draining starts > because in nonblocking mode there is no function in the pulse IO plugin which is called to inform about the state change to draining. In non-blocking mode, drain is never called. > The only solution which I have in my mind is this additional thread > state_check_thread() > { > while (true) { > if (state == DRAINING) > pa_stream_drain() > ... > } > } > > Therefore with your proposed solution there is additional effort in each new IO plug required to support draining in nonblocking mode. (With my solution exactly the same mechanism (drain callback and poll) is used in blocking and nonblocking mode. Therefore new IO plugins will support both modes without additional efforts in the IO plugin implementation) No, again, in non-blocking mode, the drain callback will get never called. It's the responsibility of application to sync with poll() instead. Takashi
> No, again, in non-blocking mode, the drain callback will get never > called. It's the responsibility of application to sync with poll() > instead. Sorry but I do not get it anyway. The user application which is playing some audio has to do the following to drain in nonblocking mode, right? snd_pcm_poll_descriptors(pfds) while (snd_pcm_drain() == -EAGAIN) { poll(pfds) } But in nonblocking mode the drain callback of the IO plugin will never be called with your solution. Therefore in case of the pulse IO plugin which function should call pa_stream_drain()? The user application will not do it directly and poll can also not call it. Thanks for your time. Best regards Timo
On Fri, 23 Mar 2018 08:43:10 +0100, Wischer, Timo (ADITG/ESB) wrote: > > > No, again, in non-blocking mode, the drain callback will get never > > called. It's the responsibility of application to sync with poll() > > instead. > > Sorry but I do not get it anyway. > > The user application which is playing some audio has to do the following to drain in nonblocking mode, right? > snd_pcm_poll_descriptors(pfds) > while (snd_pcm_drain() == -EAGAIN) { > poll(pfds) > } > > > But in nonblocking mode the drain callback of the IO plugin will never be called with your solution. > Therefore in case of the pulse IO plugin which function should call pa_stream_drain()? > The user application will not do it directly and poll can also not call it. OK, now I understand your concern. Yes it's another missing piece, snd_pcm_ioplug_hw_ptr_update() needs to check the current state, and it drops to SETUP state instead of XRUN when it was DRAINING. Then application can simply do poll() and status update until it goes out of DRAINING state. But still it's outside the plugin, drain callback isn't called there. Takashi
On Fri, 23 Mar 2018 09:01:35 +0100, Takashi Iwai wrote: > > On Fri, 23 Mar 2018 08:43:10 +0100, > Wischer, Timo (ADITG/ESB) wrote: > > > > > No, again, in non-blocking mode, the drain callback will get never > > > called. It's the responsibility of application to sync with poll() > > > instead. > > > > Sorry but I do not get it anyway. > > > > The user application which is playing some audio has to do the following to drain in nonblocking mode, right? > > snd_pcm_poll_descriptors(pfds) > > while (snd_pcm_drain() == -EAGAIN) { > > poll(pfds) > > } > > > > > > But in nonblocking mode the drain callback of the IO plugin will never be called with your solution. > > Therefore in case of the pulse IO plugin which function should call pa_stream_drain()? > > The user application will not do it directly and poll can also not call it. > > OK, now I understand your concern. Yes it's another missing piece, > snd_pcm_ioplug_hw_ptr_update() needs to check the current state, and > it drops to SETUP state instead of XRUN when it was DRAINING. > Then application can simply do poll() and status update until it goes > out of DRAINING state. > > But still it's outside the plugin, drain callback isn't called there. .... and now thinking of this again, the whole story can be folded back: - The standard drain behavior can be implemented without plugin's own code; it's just a poll and status check. - For any special case (or better implementation than poll()), we may leave the whole draining callback action to each plugin; that's the case of PA. Takashi
> OK, now I understand your concern. Yes it's another missing piece, > snd_pcm_ioplug_hw_ptr_update() needs to check the current state, and > it drops to SETUP state instead of XRUN when it was DRAINING. > Then application can simply do poll() and status update until it goes > out of DRAINING state. Okay. Therefore the pulse plugin has to call pa_stream_drain() from its IO plug pointer callback in case of state DRAINING, right? > But still it's outside the plugin, drain callback isn't called there. I think this is not the best design decision because there is an IO plug callback for each state transitions. Only for transitions to draining in nonblocking mode there will be no IO plug callback. I think this could be confusing for some IO plugin developers. What do you think? Timo
diff --git a/src/pcm/pcm_ioplug.c b/src/pcm/pcm_ioplug.c index 8c0ed4836365..33f7c5c27b6f 100644 --- a/src/pcm/pcm_ioplug.c +++ b/src/pcm/pcm_ioplug.c @@ -494,12 +494,37 @@ static int snd_pcm_ioplug_drain(snd_pcm_t *pcm) ioplug_priv_t *io = pcm->private_data; int err; - if (io->data->state == SND_PCM_STATE_OPEN) + switch (io->data->state) { + case SND_PCM_STATE_OPEN: + case SND_PCM_STATE_DISCONNECTED: + case SND_PCM_STATE_SUSPENDED: return -EBADFD; + case SND_PCM_STATE_PREPARED: + if (pcm->stream == SND_PCM_STREAM_PLAYBACK) { + snd_pcm_lock(pcm); + err = snd_pcm_ioplug_start(pcm); + snd_pcm_unlock(pcm); + if (err < 0) + return err; + io->data->state = SND_PCM_STATE_DRAINING; + } + break; + case SND_PCM_STATE_RUNNING: + io->data->state = SND_PCM_STATE_DRAINING; + break; + } + + if (io->data->state == SND_PCM_STATE_DRAINING) { + if (io->data->nonblock) + return -EAGAIN; + + if (io->data->callback->drain) { + err = io->data->callback->drain(io->data); + if (err < 0) + return err; + } + } - io->data->state = SND_PCM_STATE_DRAINING; - if (io->data->callback->drain) - io->data->callback->drain(io->data); snd_pcm_lock(pcm); err = snd_pcm_ioplug_drop(pcm); snd_pcm_unlock(pcm);