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