| Message ID | 1399056393.47049129@f181.i.mail.ru (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Takashi Iwai |
| Headers | show |
At Fri, 02 May 2014 22:46:33 +0400, Sergey wrote: > > Hello. > > 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. > > --- > diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c > index 7a8b24d..a39fea5 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,41 @@ typedef struct { > jack_client_t *client; > } snd_pcm_jack_t; > > +static inline int poll_blocked(snd_pcm_ioplug_t *io) You don't need inline in general. Compilers should be smart enough nowadays. > +{ > + static char buf[65536]; > + snd_pcm_sframes_t avail; > + snd_pcm_jack_t *jack = io->private_data; > + > + if (io->state == SND_PCM_STATE_RUNNING) > + { Please follow the coding style in that file. The brace is placed in the same line as if (). > + avail = snd_pcm_avail_update(io->pcm); > + if (avail >= 0 && avail < jack->min_avail) > + { > + read(io->poll_fd, &buf, sizeof buf); 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. > + return 1; > + } > + } > + > + return 0; > +} > + > +static inline int poll_unblocked(snd_pcm_ioplug_t *io) The function names are a bit too ambiguous and hard to understand what they are really doing only from the names. > +{ > + 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 +117,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 && !poll_blocked(io)) > *revents |= (io->stream == SND_PCM_STREAM_PLAYBACK) ? POLLOUT : POLLIN; > return 0; > } > @@ -105,7 +137,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,7 +176,7 @@ snd_pcm_jack_process_cb(jack_nframes_t nframes, snd_pcm_ioplug_t *io) > xfer += frames; > } > > - write(jack->fd, buf, 1); /* for polling */ > + poll_unblocked(io); /* unblock socket for polling if needed */ > > return 0; > } > @@ -154,9 +185,25 @@ 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); > + } > + > + poll_unblocked(io); /* unblock socket for initial polling if needed */ Is this really needed? > + > + if (jack->activated) { > + jack_deactivate(jack->client); > + jack->activated = 0; > + } This is same as just calling snd_pcm_jack_stop(). thanks, Takashi
diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c index 7a8b24d..a39fea5 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,41 @@ typedef struct { jack_client_t *client; } snd_pcm_jack_t; +static inline int poll_blocked(snd_pcm_ioplug_t *io) +{ + static char buf[65536]; + 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) + { + read(io->poll_fd, &buf, sizeof buf); + return 1; + } + } + + return 0; +} + +static inline int poll_unblocked(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 +117,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 && !poll_blocked(io)) *revents |= (io->stream == SND_PCM_STREAM_PLAYBACK) ? POLLOUT : POLLIN; return 0; } @@ -105,7 +137,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,7 +176,7 @@ snd_pcm_jack_process_cb(jack_nframes_t nframes, snd_pcm_ioplug_t *io) xfer += frames; } - write(jack->fd, buf, 1); /* for polling */ + poll_unblocked(io); /* unblock socket for polling if needed */ return 0; } @@ -154,9 +185,25 @@ 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); + } + + poll_unblocked(io); /* unblock socket for initial polling if needed */ + + if (jack->activated) { + jack_deactivate(jack->client); + jack->activated = 0; + } + if (jack->ports) return 0;