From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?U2VyZ2V5?= Subject: Re: =?utf-8?q?=5BPATCH=5D_alsa-plugins=3A_fix_polling_an?= =?utf-8?q?d_recovering_in_alsa-jack_plugin?= Date: Tue, 13 May 2014 03:20:00 +0400 Message-ID: <1399936800.902465971@f362.i.mail.ru> References: <1399056393.47049129@f181.i.mail.ru> <1399593650.837833135@f94.i.mail.ru> Reply-To: =?UTF-8?B?U2VyZ2V5?= Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from f362.i.mail.ru (f362.i.mail.ru [217.69.141.4]) by alsa0.perex.cz (Postfix) with ESMTP id E39D7265114 for ; Tue, 13 May 2014 01:20:01 +0200 (CEST) Received: from mail by f362.i.mail.ru with local (envelope-from ) id 1WjzVo-0007Vw-W0 for alsa-devel@alsa-project.org; Tue, 13 May 2014 03:20:01 +0400 In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: =?UTF-8?B?YWxzYS1kZXZlbA==?= List-Id: alsa-devel@alsa-project.org Fri 09 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. >>> 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()? > > A bit better, but not really. You can give a comment to each function > to explain what they do. Then you might find better names while > explaining functionality to yourself. pcm_poll_block() function blocks pcm polling if pcm is not ready to send data (capture pcm) or receive data (playback pcm). And pcm_poll_unblock() unblocks poll() if pcm is ready to send/receive data. But block_poll_if_pcm_not_ready() looks too cumbersome. Maybe pcm_poll_block_check() and pcm_poll_unblock_check()? >> Playback pcm accepts writes since _prepare(), so poll() is unblocked there. > > Then do it only for the playback. For capture, the poll shouldn't > return at prepare state. I thought that socket is created empty, so poll() is blocked by default. But you're right, I forgot about XRUN recovery, when _prepare() can be called with unblocked poll. It could be blocked by the next _poll_revents(), but it's probably better to have it explicitly blocked in _prepare(). I modified _block function and added its call to _prepare(). >> 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. > > You can declare a function. Done. Patch updated. --- diff --git a/jack/pcm_jack.c b/jack/pcm_jack.c index 7a8b24d..36fbdc9 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 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_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) { + while (read(io->poll_fd, &buf, sizeof(buf)) == sizeof(buf)); + return 1; + } + } + + return 0; +} + +static int pcm_poll_unblock_check(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 && !pcm_poll_block_check(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 */ + pcm_poll_unblock_check(io); /* unblock socket for polling if needed */ return 0; } @@ -154,9 +185,26 @@ 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); + + if (io->stream == SND_PCM_STREAM_PLAYBACK) + pcm_poll_unblock_check(io); /* playback pcm initially accepts writes */ + else + pcm_poll_block_check(io); /* block capture pcm if that's XRUN recovery */ + if (jack->ports) return 0; --