[alsa-devel] [PATCH - JACK IO plug 0/3] snd_pcm_drain support
The following patches are required to support draining with the JACK IO plugin. Without these patches $ speaker-test -D jack -c2 will freeze.
It is required to support the draining feature implemented in commit ce2095c41f2 (pcm: ioplug: Implement proper drain behavior)
From: Timo Wischer twischer@de.adit-jv.com
This function can be called without calling snd_pcm_avail_update().
The call to snd_pcm_avail_update() can take some time. Therefore some developers would not like to call it from a real-time context (e.g. from JACK client context).
Signed-off-by: Timo Wischer twischer@de.adit-jv.com
diff --git a/include/pcm_ioplug.h b/include/pcm_ioplug.h index c1310e3..b16fc8b 100644 --- a/include/pcm_ioplug.h +++ b/include/pcm_ioplug.h @@ -235,6 +235,9 @@ int snd_pcm_ioplug_set_param_list(snd_pcm_ioplug_t *io, int type, unsigned int n int snd_pcm_ioplug_set_state(snd_pcm_ioplug_t *ioplug, snd_pcm_state_t state);
/* calucalte the available frames */ +snd_pcm_uframes_t snd_pcm_ioplug_avail(const snd_pcm_ioplug_t * const ioplug, + const snd_pcm_uframes_t hw_ptr, + const snd_pcm_uframes_t appl_ptr); snd_pcm_uframes_t snd_pcm_ioplug_hw_avail(const snd_pcm_ioplug_t * const ioplug, const snd_pcm_uframes_t hw_ptr, const snd_pcm_uframes_t appl_ptr); diff --git a/src/pcm/pcm_ioplug.c b/src/pcm/pcm_ioplug.c index 4d44ae2..6d52c27 100644 --- a/src/pcm/pcm_ioplug.c +++ b/src/pcm/pcm_ioplug.c @@ -1221,6 +1221,21 @@ int snd_pcm_ioplug_set_state(snd_pcm_ioplug_t *ioplug, snd_pcm_state_t state) * \param ioplug the ioplug handle * \param hw_ptr hardware pointer in frames * \param appl_ptr application pointer in frames + * \return available frames for the application + */ +snd_pcm_uframes_t snd_pcm_ioplug_avail(const snd_pcm_ioplug_t * const ioplug, + const snd_pcm_uframes_t hw_ptr, + const snd_pcm_uframes_t appl_ptr) +{ + return __snd_pcm_avail(ioplug->pcm, hw_ptr, appl_ptr); +} + +/** + * \brief Get the available frames. This function can be used to calculate the + * the available frames before calling #snd_pcm_avail_update() + * \param ioplug the ioplug handle + * \param hw_ptr hardware pointer in frames + * \param appl_ptr application pointer in frames * \return available frames for the hardware */ snd_pcm_uframes_t snd_pcm_ioplug_hw_avail(const snd_pcm_ioplug_t * const ioplug, @@ -1230,8 +1245,9 @@ snd_pcm_uframes_t snd_pcm_ioplug_hw_avail(const snd_pcm_ioplug_t * const ioplug, /* available data/space which can be transferred by the user * application */ - const snd_pcm_uframes_t user_avail = __snd_pcm_avail(ioplug->pcm, - hw_ptr, appl_ptr); + const snd_pcm_uframes_t user_avail = snd_pcm_ioplug_avail(ioplug, + hw_ptr, + appl_ptr);
if (user_avail > ioplug->pcm->buffer_size) { /* there was an Xrun */
From: Timo Wischer twischer@de.adit-jv.com
snd_pcm_avail_update() can call snd_pcm_jack_stop() but snd_pcm_jack_stop() should not be called by the JACK thread. It should only be called by the thread how has called snd_pcm_jack_start().
In addition the execution of snd_pcm_avail_update() can take a while. Therefore it should not be called by the JACK thread to not block this thread.
Signed-off-by: Timo Wischer twischer@de.adit-jv.com
diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c index e3df4d2..b643d93 100644 --- a/jack/pcm_jack.c +++ b/jack/pcm_jack.c @@ -67,7 +67,7 @@ static int pcm_poll_block_check(snd_pcm_ioplug_t *io)
if (io->state == SND_PCM_STATE_RUNNING || (io->state == SND_PCM_STATE_PREPARED && io->stream == SND_PCM_STREAM_CAPTURE)) { - avail = snd_pcm_avail_update(io->pcm); + avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr); if (avail >= 0 && avail < jack->min_avail) { while (read(io->poll_fd, &buf, sizeof(buf)) == sizeof(buf)) ; @@ -84,7 +84,7 @@ static int pcm_poll_unblock_check(snd_pcm_ioplug_t *io) snd_pcm_sframes_t avail; snd_pcm_jack_t *jack = io->private_data;
- avail = snd_pcm_avail_update(io->pcm); + avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr); if (avail < 0 || avail >= jack->min_avail) { write(jack->fd, &buf, 1); return 1;
On Tue, 03 Jul 2018 13:58:06 +0200, twischer@de.adit-jv.com wrote:
From: Timo Wischer twischer@de.adit-jv.com
snd_pcm_avail_update() can call snd_pcm_jack_stop() but snd_pcm_jack_stop() should not be called by the JACK thread. It should only be called by the thread how has called snd_pcm_jack_start().
In addition the execution of snd_pcm_avail_update() can take a while. Therefore it should not be called by the JACK thread to not block this thread.
Signed-off-by: Timo Wischer twischer@de.adit-jv.com
With that change, avail cannot be a negative, as snd_pcm_ioplug_avail() returns snd_pcm_uframes_t. A couple of checks are redundant and confuse readers as if it's still a signed value.
Takashi
diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c index e3df4d2..b643d93 100644 --- a/jack/pcm_jack.c +++ b/jack/pcm_jack.c @@ -67,7 +67,7 @@ static int pcm_poll_block_check(snd_pcm_ioplug_t *io)
if (io->state == SND_PCM_STATE_RUNNING || (io->state == SND_PCM_STATE_PREPARED && io->stream == SND_PCM_STREAM_CAPTURE)) {
avail = snd_pcm_avail_update(io->pcm);
avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr);@@ -84,7 +84,7 @@ static int pcm_poll_unblock_check(snd_pcm_ioplug_t *io) snd_pcm_sframes_t avail; snd_pcm_jack_t *jack = io->private_data;
- avail = snd_pcm_avail_update(io->pcm);
- avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr); if (avail < 0 || avail >= jack->min_avail) { write(jack->fd, &buf, 1); return 1;
-- 2.7.4
From: Timo Wischer twischer@de.adit-jv.com
snd_pcm_avail_update() can call snd_pcm_jack_stop() but snd_pcm_jack_stop() should not be called by the JACK thread. It should only be called by the thread how has called snd_pcm_jack_start().
In addition the execution of snd_pcm_avail_update() can take a while. Therefore it should not be called by the JACK thread to not block this thread.
Signed-off-by: Timo Wischer twischer@de.adit-jv.com
diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c index e3df4d2..5a57e13 100644 --- a/jack/pcm_jack.c +++ b/jack/pcm_jack.c @@ -62,13 +62,13 @@ static int snd_pcm_jack_stop(snd_pcm_ioplug_t *io); static int pcm_poll_block_check(snd_pcm_ioplug_t *io) { static char buf[32]; - snd_pcm_sframes_t avail; + snd_pcm_uframes_t avail; snd_pcm_jack_t *jack = io->private_data;
if (io->state == SND_PCM_STATE_RUNNING || (io->state == SND_PCM_STATE_PREPARED && io->stream == SND_PCM_STREAM_CAPTURE)) { - avail = snd_pcm_avail_update(io->pcm); - if (avail >= 0 && avail < jack->min_avail) { + avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr); + if (avail < jack->min_avail) { while (read(io->poll_fd, &buf, sizeof(buf)) == sizeof(buf)) ; return 1; @@ -81,11 +81,11 @@ static int pcm_poll_block_check(snd_pcm_ioplug_t *io) static int pcm_poll_unblock_check(snd_pcm_ioplug_t *io) { static char buf[1]; - snd_pcm_sframes_t avail; + snd_pcm_uframes_t avail; snd_pcm_jack_t *jack = io->private_data;
- avail = snd_pcm_avail_update(io->pcm); - if (avail < 0 || avail >= jack->min_avail) { + avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr); + if (avail >= jack->min_avail) { write(jack->fd, &buf, 1); return 1; }
On Tue, 03 Jul 2018 14:23:59 +0200, twischer@de.adit-jv.com wrote:
From: Timo Wischer twischer@de.adit-jv.com
snd_pcm_avail_update() can call snd_pcm_jack_stop() but snd_pcm_jack_stop() should not be called by the JACK thread. It should only be called by the thread how has called snd_pcm_jack_start().
In addition the execution of snd_pcm_avail_update() can take a while. Therefore it should not be called by the JACK thread to not block this thread.
Signed-off-by: Timo Wischer twischer@de.adit-jv.com
Please put "v2" into the subject prefix, and give a brief information what changed from v1 to v2. And, don't attach to the old mail thread. As of this post, it's utterly confusing which patch is which version!
thanks,
Takashi
diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c index e3df4d2..5a57e13 100644 --- a/jack/pcm_jack.c +++ b/jack/pcm_jack.c @@ -62,13 +62,13 @@ static int snd_pcm_jack_stop(snd_pcm_ioplug_t *io); static int pcm_poll_block_check(snd_pcm_ioplug_t *io) { static char buf[32];
- snd_pcm_sframes_t avail;
snd_pcm_uframes_t avail; snd_pcm_jack_t *jack = io->private_data;
if (io->state == SND_PCM_STATE_RUNNING || (io->state == SND_PCM_STATE_PREPARED && io->stream == SND_PCM_STREAM_CAPTURE)) {
avail = snd_pcm_avail_update(io->pcm);if (avail >= 0 && avail < jack->min_avail) {
avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr);if (avail < jack->min_avail) { while (read(io->poll_fd, &buf, sizeof(buf)) == sizeof(buf)) ; return 1;@@ -81,11 +81,11 @@ static int pcm_poll_block_check(snd_pcm_ioplug_t *io) static int pcm_poll_unblock_check(snd_pcm_ioplug_t *io) { static char buf[1];
- snd_pcm_sframes_t avail;
- snd_pcm_uframes_t avail; snd_pcm_jack_t *jack = io->private_data;
- avail = snd_pcm_avail_update(io->pcm);
- if (avail < 0 || avail >= jack->min_avail) {
- avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr);
- if (avail >= jack->min_avail) { write(jack->fd, &buf, 1); return 1; }
-- 2.7.4
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
From: Timo Wischer twischer@de.adit-jv.com
Signed-off-by: Timo Wischer twischer@de.adit-jv.com
diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c index b643d93..0d1ccb9 100644 --- a/jack/pcm_jack.c +++ b/jack/pcm_jack.c @@ -66,6 +66,7 @@ static int pcm_poll_block_check(snd_pcm_ioplug_t *io) snd_pcm_jack_t *jack = io->private_data;
if (io->state == SND_PCM_STATE_RUNNING || + io->state == SND_PCM_STATE_DRAINING || (io->state == SND_PCM_STATE_PREPARED && io->stream == SND_PCM_STREAM_CAPTURE)) { avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr); if (avail >= 0 && avail < jack->min_avail) { @@ -85,7 +86,13 @@ static int pcm_poll_unblock_check(snd_pcm_ioplug_t *io) snd_pcm_jack_t *jack = io->private_data;
avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr); - if (avail < 0 || avail >= jack->min_avail) { + /* In draining state poll_fd is used to wait + * till all pending frames are played. + * Therefore it has to be guarantee that a poll event is also generated + * if the buffer contains less than min_avail frames + */ + if (avail < 0 || avail >= jack->min_avail || + io->state == SND_PCM_STATE_DRAINING) { write(jack->fd, &buf, 1); return 1; } @@ -161,7 +168,8 @@ snd_pcm_jack_process_cb(jack_nframes_t nframes, snd_pcm_ioplug_t *io) jack->areas[channel].step = jack->sample_bits; }
- if (io->state == SND_PCM_STATE_RUNNING) { + if (io->state == SND_PCM_STATE_RUNNING || + io->state == SND_PCM_STATE_DRAINING) { snd_pcm_uframes_t hw_ptr = jack->hw_ptr; const snd_pcm_uframes_t hw_avail = snd_pcm_ioplug_hw_avail(io, hw_ptr, io->appl_ptr);
From: Timo Wischer twischer@de.adit-jv.com
Signed-off-by: Timo Wischer twischer@de.adit-jv.com
diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c index 5a57e13..98a6f7e 100644 --- a/jack/pcm_jack.c +++ b/jack/pcm_jack.c @@ -66,6 +66,7 @@ static int pcm_poll_block_check(snd_pcm_ioplug_t *io) snd_pcm_jack_t *jack = io->private_data;
if (io->state == SND_PCM_STATE_RUNNING || + io->state == SND_PCM_STATE_DRAINING || (io->state == SND_PCM_STATE_PREPARED && io->stream == SND_PCM_STREAM_CAPTURE)) { avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr); if (avail < jack->min_avail) { @@ -85,7 +86,12 @@ static int pcm_poll_unblock_check(snd_pcm_ioplug_t *io) snd_pcm_jack_t *jack = io->private_data;
avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr); - if (avail >= jack->min_avail) { + /* In draining state poll_fd is used to wait + * till all pending frames are played. + * Therefore it has to be guarantee that a poll event is also generated + * if the buffer contains less than min_avail frames + */ + if (avail >= jack->min_avail || io->state == SND_PCM_STATE_DRAINING) { write(jack->fd, &buf, 1); return 1; } @@ -161,7 +167,8 @@ snd_pcm_jack_process_cb(jack_nframes_t nframes, snd_pcm_ioplug_t *io) jack->areas[channel].step = jack->sample_bits; }
- if (io->state == SND_PCM_STATE_RUNNING) { + if (io->state == SND_PCM_STATE_RUNNING || + io->state == SND_PCM_STATE_DRAINING) { snd_pcm_uframes_t hw_ptr = jack->hw_ptr; const snd_pcm_uframes_t hw_avail = snd_pcm_ioplug_hw_avail(io, hw_ptr, io->appl_ptr);
On Tue, 03 Jul 2018 14:26:54 +0200, twischer@de.adit-jv.com wrote:
From: Timo Wischer twischer@de.adit-jv.com
Signed-off-by: Timo Wischer twischer@de.adit-jv.com
Please give some change log text to explain why the patch is needed and what it does. The empty change log indicates already something wrong unless it's a really trivial change.
thanks,
Takashi
diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c index 5a57e13..98a6f7e 100644 --- a/jack/pcm_jack.c +++ b/jack/pcm_jack.c @@ -66,6 +66,7 @@ static int pcm_poll_block_check(snd_pcm_ioplug_t *io) snd_pcm_jack_t *jack = io->private_data;
if (io->state == SND_PCM_STATE_RUNNING ||
io->state == SND_PCM_STATE_DRAINING || (io->state == SND_PCM_STATE_PREPARED && io->stream == SND_PCM_STREAM_CAPTURE)) {@@ -85,7 +86,12 @@ static int pcm_poll_unblock_check(snd_pcm_ioplug_t *io) snd_pcm_jack_t *jack = io->private_data;
avail = snd_pcm_ioplug_avail(io, jack->hw_ptr, io->appl_ptr);
- if (avail >= jack->min_avail) {
- /* In draining state poll_fd is used to wait
* till all pending frames are played.* Therefore it has to be guarantee that a poll event is also generated* if the buffer contains less than min_avail frames*/- if (avail >= jack->min_avail || io->state == SND_PCM_STATE_DRAINING) { write(jack->fd, &buf, 1); return 1; }
@@ -161,7 +167,8 @@ snd_pcm_jack_process_cb(jack_nframes_t nframes, snd_pcm_ioplug_t *io) jack->areas[channel].step = jack->sample_bits; }
- if (io->state == SND_PCM_STATE_RUNNING) {
- if (io->state == SND_PCM_STATE_RUNNING ||
io->state == SND_PCM_STATE_DRAINING) {-- 2.7.4
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (2)
-
Takashi Iwai -
twischer＠de.adit-jv.com