At Mon 05 May 2014 Takashi Iwai wrote:
This patch fixes polling in alsa-to-jack plugin. It makes poll()+_poll_revents() return correct values whenever there's something to read/write.
Additionally jack_deactivate() code makes it survive snd_pcm_recover().
It works for the code example I posted before, aplay, arecord, mozilla, etc.
Comments and suggestions are welcome.
Thanks for the patch. The basic idea looks good, but some suggestions below.
Thank you for your response and comments.
You don't need inline in general. Compilers should be smart enough nowadays.
Uninlined.
Please follow the coding style in that file. The brace is placed in the same line as if ().
Missed that, sorry. Fixed.
So, it's intended to clear all pending writes, right? If so, do reads in loop. Then you don't have to use such a big buffer. The size of 32 or such should be enough.
Done.
The function names are a bit too ambiguous and hard to understand what they are really doing only from the names.
How about pcm_poll_block() and pcm_poll_unblock()?
- poll_unblocked(io); /* unblock socket for initial polling if needed */
Is this really needed?
Playback pcm accepts writes since _prepare(), so poll() is unblocked there. Without it playback pcm gets poll() blocked by default and a code like: _open(); _set_params(); while (still_playing) { if (poll() > 0 && _revents() == 0 && revents&POLLOUT) _write(); } never starts writing.
- if (jack->activated) {
jack_deactivate(jack->client);jack->activated = 0;- }
This is same as just calling snd_pcm_jack_stop().
Right. To put snd_pcm_jack_stop() call in snd_pcm_jack_prepare() I have to move snd_pcm_jack_prepare() below snd_pcm_jack_stop(), that makes patch larger.
Patch updated and ready for more comments.
--- diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c index 7a8b24d..837e15c 100644 --- a/jack/pcm_jack.c +++ b/jack/pcm_jack.c @@ -42,6 +42,7 @@ typedef struct { unsigned int num_ports; unsigned int hw_ptr; unsigned int sample_bits; + snd_pcm_uframes_t min_avail;
unsigned int channels; snd_pcm_channel_area_t *areas; @@ -50,6 +51,38 @@ typedef struct { jack_client_t *client; } snd_pcm_jack_t;
+static int pcm_poll_block(snd_pcm_ioplug_t *io) +{ + static char buf[32]; + snd_pcm_sframes_t avail; + snd_pcm_jack_t *jack = io->private_data; + + if (io->state == SND_PCM_STATE_RUNNING) { + avail = snd_pcm_avail_update(io->pcm); + if (avail >= 0 && avail < jack->min_avail) { + while (read(io->poll_fd, &buf, sizeof(buf)) == sizeof(buf)); + return 1; + } + } + + return 0; +} + +static int pcm_poll_unblock(snd_pcm_ioplug_t *io) +{ + static char buf[1]; + snd_pcm_sframes_t avail; + snd_pcm_jack_t *jack = io->private_data; + + avail = snd_pcm_avail_update(io->pcm); + if (avail < 0 || avail >= jack->min_avail) { + write(jack->fd, &buf, 1); + return 1; + } + + return 0; +} + static void snd_pcm_jack_free(snd_pcm_jack_t *jack) { if (jack) { @@ -81,14 +114,10 @@ static int snd_pcm_jack_poll_revents(snd_pcm_ioplug_t *io, struct pollfd *pfds, unsigned int nfds, unsigned short *revents) { - static char buf[1]; - assert(pfds && nfds == 1 && revents);
- read(pfds[0].fd, buf, 1); - *revents = pfds[0].revents & ~(POLLIN | POLLOUT); - if (pfds[0].revents & POLLIN) + if (pfds[0].revents & POLLIN && !pcm_poll_block(io)) *revents |= (io->stream == SND_PCM_STREAM_PLAYBACK) ? POLLOUT : POLLIN; return 0; } @@ -105,7 +134,6 @@ snd_pcm_jack_process_cb(jack_nframes_t nframes, snd_pcm_ioplug_t *io) snd_pcm_jack_t *jack = io->private_data; const snd_pcm_channel_area_t *areas; snd_pcm_uframes_t xfer = 0; - static char buf[1]; unsigned int channel; for (channel = 0; channel < io->channels; channel++) { @@ -145,44 +173,11 @@ snd_pcm_jack_process_cb(jack_nframes_t nframes, snd_pcm_ioplug_t *io) xfer += frames; }
- write(jack->fd, buf, 1); /* for polling */ + pcm_poll_unblock(io); /* unblock socket for polling if needed */
return 0; }
-static int snd_pcm_jack_prepare(snd_pcm_ioplug_t *io) -{ - snd_pcm_jack_t *jack = io->private_data; - unsigned int i; - - jack->hw_ptr = 0; - - if (jack->ports) - return 0; - - jack->ports = calloc(io->channels, sizeof(jack_port_t*)); - - for (i = 0; i < io->channels; i++) { - char port_name[32]; - if (io->stream == SND_PCM_STREAM_PLAYBACK) { - - sprintf(port_name, "out_%03d", i); - jack->ports[i] = jack_port_register(jack->client, port_name, - JACK_DEFAULT_AUDIO_TYPE, - JackPortIsOutput, 0); - } else { - sprintf(port_name, "in_%03d", i); - jack->ports[i] = jack_port_register(jack->client, port_name, - JACK_DEFAULT_AUDIO_TYPE, - JackPortIsInput, 0); - } - } - - jack_set_process_callback(jack->client, - (JackProcessCallback)snd_pcm_jack_process_cb, io); - return 0; -} - static int snd_pcm_jack_start(snd_pcm_ioplug_t *io) { snd_pcm_jack_t *jack = io->private_data; @@ -233,6 +228,54 @@ static int snd_pcm_jack_stop(snd_pcm_ioplug_t *io) return 0; }
+static int snd_pcm_jack_prepare(snd_pcm_ioplug_t *io) +{ + snd_pcm_jack_t *jack = io->private_data; + unsigned int i; + snd_pcm_sw_params_t *swparams; + int err; + + jack->hw_ptr = 0; + + jack->min_avail = io->period_size; + snd_pcm_sw_params_alloca(&swparams); + err = snd_pcm_sw_params_current(io->pcm, swparams); + if (err == 0) { + snd_pcm_sw_params_get_avail_min(swparams, &jack->min_avail); + } + + /* deactivate jack connections if this is XRUN recovery */ + snd_pcm_jack_stop(io); + + /* playback pcm initially accepts writes, poll must be unblocked */ + pcm_poll_unblock(io); + + if (jack->ports) + return 0; + + jack->ports = calloc(io->channels, sizeof(jack_port_t*)); + + for (i = 0; i < io->channels; i++) { + char port_name[32]; + if (io->stream == SND_PCM_STREAM_PLAYBACK) { + + sprintf(port_name, "out_%03d", i); + jack->ports[i] = jack_port_register(jack->client, port_name, + JACK_DEFAULT_AUDIO_TYPE, + JackPortIsOutput, 0); + } else { + sprintf(port_name, "in_%03d", i); + jack->ports[i] = jack_port_register(jack->client, port_name, + JACK_DEFAULT_AUDIO_TYPE, + JackPortIsInput, 0); + } + } + + jack_set_process_callback(jack->client, + (JackProcessCallback)snd_pcm_jack_process_cb, io); + return 0; +} + static snd_pcm_ioplug_callback_t jack_pcm_callback = { .close = snd_pcm_jack_close, .start = snd_pcm_jack_start, --