At Mon, 02 Feb 2009 15:34:57 +0100, I wrote: > > At Mon, 02 Feb 2009 08:02:29 +0100, > I wrote: > > > > At Mon, 2 Feb 2009 03:50:22 +0100, > > Lennart Poettering wrote: > > > > > > Heya! > > > > > > If we look into the pcm example how snd_pcm_poll_descriptors_revents() > > > is used then we can see that the revents parameter apparently is > > > supposed to be a single integer. (which makes a lot of sense to me) > > > > > > http://www.alsa-project.org/alsa-doc/alsa-lib/_2test_2pcm_8c-example.html#a33 > > > > > > However, snd_pcm_wait_nocheck() calls the same function and assumes it > > > is a complete array! > > > > > > http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/pcm/pcm.c;h=74d1d1a4bd6083cd461b6d793c0ae41cca912f16;hb=HEAD#l2368 > > > > > > So what's it now? It makes more sense to me if it would be a single > > > fd. > > > > No, it's a bug in test/pcm.c. As documented, the API converts (fixes) > > each poll_fd in the given array. > > Hmm.. just looking through the whole tree again, this thing seems > mixed up weirdly right now. > At least, the latest pcm_hw.c assumes the single revent. Meanwhile, > the old pcm_multi.c assumed the multiple revents. > > As you notice the work "revents", the function itself supposes the > multiple events. However, obviously, handling the single revent is > much easier for applications. > > So I believe we should go to the current pcm_hw.c implementation way, > i.e. demangling to a single revent. But, yes, more investigations and > fixes are needed. And the patch is below. If no one has objection, I'll apply it later. thanks, Takashi == >From fdd79369b24d2e9405d4ec5f1bdfc09dda127ed4 Mon Sep 17 00:00:00 2001 From: Takashi Iwai <tiwai@xxxxxxx> Date: Mon, 2 Feb 2009 15:43:48 +0100 Subject: [PATCH] Fix handling of revents in snd_pcm_poll_descriptors_revents() The revents parameter is ambiguously defined whether it's a pointer to a single event or an arary. While the recent pcm_hw.c assumes it being a single event, the old multi plugin supposed it as an array. Similarly snd_pcm_wait_nocheck() checks the revents as an arary. This patch defines the behavior of revents more strictly. It's a pointer of a single event. Fixed snd_pcm_wait_nocheck() to follow that. Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> --- src/pcm/pcm.c | 42 +++++++++++++++++++----------------------- 1 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c index 74d1d1a..0203720 100644 --- a/src/pcm/pcm.c +++ b/src/pcm/pcm.c @@ -1430,7 +1430,7 @@ int snd_pcm_poll_descriptors(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int s * \param pcm PCM handle * \param pfds array of poll descriptors * \param nfds count of poll descriptors - * \param revents returned events + * \param revents pointer to the returned (single) event * \return zero if success, otherwise a negative error code * * This function does "demangling" of the revents mask returned from @@ -1440,6 +1440,9 @@ int snd_pcm_poll_descriptors(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int s * syscall returned that some events are waiting, this function might * return empty set of events. In this case, application should * do next event waiting using poll() or select(). + * + * Note: Even if multiple poll descriptors are used (i.e. pfds > 1), + * this function returns only a single event. */ int snd_pcm_poll_descriptors_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int nfds, unsigned short *revents) { @@ -2338,8 +2341,8 @@ int snd_pcm_wait(snd_pcm_t *pcm, int timeout) int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout) { struct pollfd *pfd; - unsigned short *revents; - int i, npfds, pollio, err, err_poll; + unsigned short revents = 0; + int i, npfds, err, err_poll; npfds = snd_pcm_poll_descriptors_count(pcm); if (npfds <= 0 || npfds >= 16) { @@ -2347,7 +2350,6 @@ int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout) return -EIO; } pfd = alloca(sizeof(*pfd) * npfds); - revents = alloca(sizeof(*revents) * npfds); err = snd_pcm_poll_descriptors(pcm, pfd, npfds); if (err < 0) return err; @@ -2356,7 +2358,6 @@ int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout) return -EIO; } do { - pollio = 0; err_poll = poll(pfd, npfds, timeout); if (err_poll < 0) { if (errno == EINTR) @@ -2365,28 +2366,23 @@ int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout) } if (! err_poll) break; - err = snd_pcm_poll_descriptors_revents(pcm, pfd, npfds, revents); + err = snd_pcm_poll_descriptors_revents(pcm, pfd, npfds, &revents); if (err < 0) return err; - for (i = 0; i < npfds; i++) { - if (revents[i] & (POLLERR | POLLNVAL)) { - /* check more precisely */ - switch (snd_pcm_state(pcm)) { - case SND_PCM_STATE_XRUN: - return -EPIPE; - case SND_PCM_STATE_SUSPENDED: - return -ESTRPIPE; - case SND_PCM_STATE_DISCONNECTED: - return -ENODEV; - default: - return -EIO; - } + if (revents & (POLLERR | POLLNVAL)) { + /* check more precisely */ + switch (snd_pcm_state(pcm)) { + case SND_PCM_STATE_XRUN: + return -EPIPE; + case SND_PCM_STATE_SUSPENDED: + return -ESTRPIPE; + case SND_PCM_STATE_DISCONNECTED: + return -ENODEV; + default: + return -EIO; } - if ((revents[i] & (POLLIN | POLLOUT)) == 0) - continue; - pollio++; } - } while (! pollio); + } while (!(revents & (POLLIN | POLLOUT))); #if 0 /* very useful code to test poll related problems */ { snd_pcm_sframes_t avail_update; -- 1.6.1.2 _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
