[alsa-devel] [PATCH] alsa-plugins: fix polling and recovering in alsa-jack plugin
Sergey
sergemp at mail.ru
Fri May 9 02:00:50 CEST 2014
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,
--
More information about the Alsa-devel
mailing list