Fix more CPU hang-up bugs in dmix

- Fix CPU hang-up during snd_pcm_drain()
  A new internal function snd_pcm_wait_nocheck() to force to call poll().

- Clean up, fix status() callback of dmix

diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index 03ba6413f20086e06ffb1fee30981abc1f87c0df..7a3a5b77526b06110a3cc13055410d64f73b6cc0 100644 (file)
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -2188,10 +2188,6 @@ int snd_pcm_open_slave(snd_pcm_t **pcmp, snd_config_t *root,
  */
 int snd_pcm_wait(snd_pcm_t *pcm, int timeout)
 {
-       struct pollfd pfd;
-       unsigned short revents;
-       int err, err_poll;
-       
        if (snd_pcm_mmap_avail(pcm) >= pcm->avail_min) {
                /* check more precisely */
                switch (snd_pcm_state(pcm)) {
@@ -2205,6 +2201,21 @@ int snd_pcm_wait(snd_pcm_t *pcm, int timeout)
                        return 1;
                }
        }
+       return snd_pcm_wait_nocheck(pcm, timeout);
+}
+
+#ifndef DOC_HIDDEN
+/* 
+ * like snd_pcm_wait() but doesn't check mmap_avail before calling poll()
+ *
+ * used in drain code in some plugins
+ */
+int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout)
+{
+       struct pollfd pfd;
+       unsigned short revents;
+       int err, err_poll;
+       
        err = snd_pcm_poll_descriptors(pcm, &pfd, 1);
        if (err < 0)
                return err;
@@ -2247,6 +2258,7 @@ int snd_pcm_wait(snd_pcm_t *pcm, int timeout)
 #endif
        return err_poll > 0 ? 1 : 0;
 }
+#endif
 
 /**
  * \brief Return number of frames ready to be read/written

diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c
index 8d6ba82757a8913052eb455a1e95c7e06ae9b175..6bded40fed613dee054e76e325cd3578eb3921c1 100644 (file)
--- a/src/pcm/pcm_direct.c
+++ b/src/pcm/pcm_direct.c
@@ -416,7 +416,7 @@ int snd_pcm_direct_async(snd_pcm_t *pcm, int sig, pid_t pid)
 }
 
 /* empty the timer read queue */
-static void snd_pcm_direct_clear_timer_queue(snd_pcm_direct_t *dmix)
+void snd_pcm_direct_clear_timer_queue(snd_pcm_direct_t *dmix)
 {
        /* rbuf might be overwriten by multiple plugins */
        /* we don't need the value */
@@ -440,15 +440,17 @@ int snd_pcm_direct_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsigned in
        assert(pfds && nfds == 1 && revents);
        events = pfds[0].revents;
        if (events & POLLIN) {
+               snd_pcm_uframes_t avail;
                int empty = 0;
                snd_pcm_avail_update(pcm);
                if (pcm->stream == SND_PCM_STREAM_PLAYBACK) {
                        events |= POLLOUT;
                        events &= ~POLLIN;
-                       empty = snd_pcm_mmap_playback_avail(pcm) < pcm->avail_min;
+                       avail = snd_pcm_mmap_playback_avail(pcm);
                } else {
-                       empty = snd_pcm_mmap_capture_avail(pcm) < pcm->avail_min;
+                       avail = snd_pcm_mmap_capture_avail(pcm);
                }
+               empty = avail < pcm->avail_min;
                if (empty)
                        snd_pcm_direct_clear_timer_queue(dmix);
                if (empty)

diff --git a/src/pcm/pcm_direct.h b/src/pcm/pcm_direct.h
index 706e86b8fdafcafa349089b5c709caf4008b2939..126bacdb4c5ae67fe398e9719c08e16d9ce2ba95 100644 (file)
--- a/src/pcm/pcm_direct.h
+++ b/src/pcm/pcm_direct.h
@@ -145,6 +145,7 @@ int snd_pcm_direct_channel_info(snd_pcm_t *pcm, snd_pcm_channel_info_t * info);
 int snd_pcm_direct_mmap(snd_pcm_t *pcm);
 int snd_pcm_direct_munmap(snd_pcm_t *pcm);
 int snd_pcm_direct_timer_stop(snd_pcm_direct_t *dmix);
+void snd_pcm_direct_clear_timer_queue(snd_pcm_direct_t *dmix);
 
 int snd_timer_async(snd_timer_t *timer, int sig, pid_t pid);
 struct timespec snd_pcm_hw_fast_tstamp(snd_pcm_t *pcm);

diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
index e0979638109ff5790010dd2e268fb88f8b387eea..924607ae5d5b6809f947938a6216546984230a94 100644 (file)
--- a/src/pcm/pcm_dmix.c
+++ b/src/pcm/pcm_dmix.c
@@ -204,6 +204,18 @@ static void mix_areas(snd_pcm_direct_t *dmix,
        }
 }
 
+/*
+ * if no concurrent access is allowed in the mixing routines, we need to protect
+ * the area via semaphore
+ */
+#ifdef NO_CONCURRENT_ACCESS
+#define dmix_down_sem(dmix) snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT)
+#define dmix_up_sem(dmix) snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT)
+#else
+#define dmix_down_sem(dmix)
+#define dmix_up_sem(dmix)
+#endif
+
 /*
  *  synchronize shm ring buffer with hardware
  */
@@ -224,27 +236,23 @@ static void snd_pcm_dmix_sync_area(snd_pcm_t *pcm, snd_pcm_uframes_t size)
        slave_appl_ptr = dmix->slave_appl_ptr % dmix->shmptr->s.buffer_size;
        dmix->slave_appl_ptr += size;
        dmix->slave_appl_ptr %= dmix->shmptr->s.boundary;
-       while (size > 0) {
+       dmix_down_sem(dmix);
+       for (;;) {
                transfer = appl_ptr + size > pcm->buffer_size ? pcm->buffer_size - appl_ptr : size;
                if (slave_appl_ptr + transfer > dmix->shmptr->s.buffer_size)
                        transfer = dmix->shmptr->s.buffer_size - slave_appl_ptr;
-               if (transfer) {
-#ifdef NO_CONCURRENT_ACCESS
-                       snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT);
-#endif
-                       mix_areas(dmix, src_areas, dst_areas, appl_ptr, slave_appl_ptr, transfer);
-#ifdef NO_CONCURRENT_ACCESS
-                       snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);
-#endif
-               }
+               if (! transfer)
+                       break;
+               mix_areas(dmix, src_areas, dst_areas, appl_ptr, slave_appl_ptr, transfer);
                if (transfer >= size)
-                       return;
+                       break;
                size -= transfer;
                slave_appl_ptr += transfer;
                slave_appl_ptr %= dmix->shmptr->s.buffer_size;
                appl_ptr += transfer;
                appl_ptr %= pcm->buffer_size;
        }
+       dmix_up_sem(dmix);
 }
 
 /*
@@ -270,8 +278,8 @@ static int _snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm, int do_slave_sync)
        diff = slave_hw_ptr - old_slave_hw_ptr;
        if (diff == 0)          /* fast path */
                return 0;
-       if (dmix->state != SNDRV_PCM_STATE_RUNNING &&
-           dmix->state != SNDRV_PCM_STATE_DRAINING)
+       if (dmix->state != SND_PCM_STATE_RUNNING &&
+           dmix->state != SND_PCM_STATE_DRAINING)
                /* not really started yet - don't update hw_ptr */
                return 0;
        if (diff < 0) {
@@ -282,18 +290,21 @@ static int _snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm, int do_slave_sync)
        dmix->hw_ptr %= pcm->boundary;
        if (pcm->stop_threshold >= pcm->boundary)       /* don't care */
                return 0;
-       if ((avail = snd_pcm_mmap_playback_avail(pcm)) >= pcm->stop_threshold) {
+       avail = snd_pcm_mmap_playback_avail(pcm);
+       if (avail > dmix->avail_max)
+               dmix->avail_max = avail;
+       if (avail >= pcm->stop_threshold) {
                struct timeval tv;
                snd_pcm_direct_timer_stop(dmix);
                gettimeofday(&tv, 0);
                dmix->trigger_tstamp.tv_sec = tv.tv_sec;
                dmix->trigger_tstamp.tv_nsec = tv.tv_usec * 1000L;
-               dmix->state = SND_PCM_STATE_XRUN;
-               dmix->avail_max = avail;
-               return -EPIPE;
+               if (dmix->state == SND_PCM_STATE_RUNNING) {
+                       dmix->state = SND_PCM_STATE_XRUN;
+                       return -EPIPE;
+               }
+               dmix->state = SND_PCM_STATE_SETUP;
        }
-       if (avail > dmix->avail_max)
-               dmix->avail_max = avail;
        return 0;
 }
 
@@ -307,10 +318,27 @@ static int snd_pcm_dmix_sync_ptr(snd_pcm_t *pcm)
  *  plugin implementation
  */
 
-static int snd_pcm_dmix_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
+static snd_pcm_state_t snd_pcm_dmix_state(snd_pcm_t *pcm)
 {
        snd_pcm_direct_t *dmix = pcm->private_data;
        snd_pcm_state_t state;
+       state = snd_pcm_state(dmix->spcm);
+       switch (state) {
+       case SND_PCM_STATE_SUSPENDED:
+               return state;
+       case SND_PCM_STATE_DISCONNECTED:
+               return state;
+       default:
+               break;
+       }
+       if (dmix->state == STATE_RUN_PENDING)
+               return SNDRV_PCM_STATE_RUNNING;
+       return dmix->state;
+}
+
+static int snd_pcm_dmix_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
+{
+       snd_pcm_direct_t *dmix = pcm->private_data;
 
        switch (dmix->state) {
        case SNDRV_PCM_STATE_DRAINING:
@@ -321,15 +349,7 @@ static int snd_pcm_dmix_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
                break;
        }
        memset(status, 0, sizeof(*status));
-       if (dmix->state == STATE_RUN_PENDING)
-               status->state = SNDRV_PCM_STATE_RUNNING;
-       else {
-               state = snd_pcm_state(dmix->spcm);
-               if (state == SNDRV_PCM_STATE_RUNNING)
-                       status->state = dmix->state;
-               else
-                       status->state = state;
-       }
+       status->state = snd_pcm_dmix_state(pcm);
        status->trigger_tstamp = dmix->trigger_tstamp;
        status->tstamp = snd_pcm_hw_fast_tstamp(dmix->spcm);
        status->avail = snd_pcm_mmap_playback_avail(pcm);
@@ -338,23 +358,6 @@ static int snd_pcm_dmix_status(snd_pcm_t *pcm, snd_pcm_status_t * status)
        return 0;
 }
 
-static snd_pcm_state_t snd_pcm_dmix_state(snd_pcm_t *pcm)
-{
-       snd_pcm_direct_t *dmix = pcm->private_data;
-       switch (snd_pcm_state(dmix->spcm)) {
-       case SND_PCM_STATE_SUSPENDED:
-               return SND_PCM_STATE_SUSPENDED;
-       case SND_PCM_STATE_DISCONNECTED:
-               dmix->state = SND_PCM_STATE_DISCONNECTED;
-               return -ENOTTY;
-       default:
-               break;
-       }
-       if (dmix->state == STATE_RUN_PENDING)
-               return SNDRV_PCM_STATE_RUNNING;
-       return dmix->state;
-}
-
 static int snd_pcm_dmix_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp)
 {
        snd_pcm_direct_t *dmix = pcm->private_data;
@@ -366,6 +369,7 @@ static int snd_pcm_dmix_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp)
                err = snd_pcm_dmix_sync_ptr(pcm);
                if (err < 0)
                        return err;
+               /* fallthru */
        case SNDRV_PCM_STATE_PREPARED:
        case SNDRV_PCM_STATE_SUSPENDED:
        case STATE_RUN_PENDING:
@@ -481,22 +485,28 @@ static int snd_pcm_dmix_drain(snd_pcm_t *pcm)
 
        if (dmix->state == SND_PCM_STATE_OPEN)
                return -EBADFD;
-       stop_threshold = pcm->stop_threshold;
-       if (pcm->stop_threshold > pcm->buffer_size)
-               pcm->stop_threshold = pcm->buffer_size;
+       if (pcm->mode & SND_PCM_NONBLOCK)
+               return -EAGAIN;
        if (dmix->state == SND_PCM_STATE_PREPARED &&
            snd_pcm_mmap_playback_hw_avail(pcm) > 0)
                snd_pcm_dmix_start(pcm);
-       while (dmix->state == SND_PCM_STATE_RUNNING) {
+       stop_threshold = pcm->stop_threshold;
+       if (pcm->stop_threshold > pcm->buffer_size)
+               pcm->stop_threshold = pcm->buffer_size;
+       dmix->state = SND_PCM_STATE_DRAINING;
+       do {
                err = snd_pcm_dmix_sync_ptr(pcm);
-               if (err < 0)
-                       break;
-               if (pcm->mode & SND_PCM_NONBLOCK)
-                       return -EAGAIN;
-               snd_pcm_wait(pcm, -1);
-       }
+               if (err < 0) {
+                       snd_pcm_dmix_drop(pcm);
+                       return err;
+               }
+               if (dmix->state == SND_PCM_STATE_DRAINING) {
+                       snd_pcm_wait_nocheck(pcm, -1);
+                       snd_pcm_direct_clear_timer_queue(dmix); /* force poll to wait */
+               }
+       } while (dmix->state == SND_PCM_STATE_DRAINING);
        pcm->stop_threshold = stop_threshold;
-       return snd_pcm_dmix_drop(pcm);
+       return 0;
 }
 
 static int snd_pcm_dmix_pause(snd_pcm_t *pcm, int enable)
@@ -603,15 +613,17 @@ static snd_pcm_sframes_t snd_pcm_dmix_mmap_commit(snd_pcm_t *pcm,
        default:
                break;
        }
+       if (! size)
+               return 0;
+       snd_pcm_mmap_appl_forward(pcm, size);
        if (dmix->state == STATE_RUN_PENDING) {
                if ((err = snd_pcm_dmix_start_timer(dmix)) < 0)
                        return err;
-       }
-       snd_pcm_mmap_appl_forward(pcm, size);
-       if (dmix->state == SND_PCM_STATE_RUNNING) {
-               err = snd_pcm_dmix_sync_ptr(pcm);
-               if (err < 0)
-                       return err;
+       } else if (dmix->state == SND_PCM_STATE_RUNNING ||
+                  dmix->state == SND_PCM_STATE_DRAINING)
+               _snd_pcm_dmix_sync_ptr(pcm, 1);
+       if (dmix->state == SND_PCM_STATE_RUNNING ||
+           dmix->state == SND_PCM_STATE_DRAINING) {
                /* ok, we commit the changes after the validation of area */
                /* it's intended, although the result might be crappy */
                snd_pcm_dmix_sync_area(pcm, size);
@@ -622,13 +634,10 @@ static snd_pcm_sframes_t snd_pcm_dmix_mmap_commit(snd_pcm_t *pcm,
 static snd_pcm_sframes_t snd_pcm_dmix_avail_update(snd_pcm_t *pcm ATTRIBUTE_UNUSED)
 {
        snd_pcm_direct_t *dmix = pcm->private_data;
-       int err;
        
-       if (dmix->state == SND_PCM_STATE_RUNNING) {
-               err = snd_pcm_dmix_sync_ptr(pcm);
-               if (err < 0)
-                       return err;
-       }
+       if (dmix->state == SND_PCM_STATE_RUNNING ||
+           dmix->state == SND_PCM_STATE_DRAINING)
+               snd_pcm_dmix_sync_ptr(pcm);
        return snd_pcm_mmap_playback_avail(pcm);
 }
 

diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
index 5d7569bb9b687183d060896c3275cc2c0c3ac5fb..bd8838a5eeef188dd7f409d3531dc98a3bac1304 100644 (file)
--- a/src/pcm/pcm_local.h
+++ b/src/pcm/pcm_local.h
@@ -744,6 +744,8 @@ int snd_pcm_conf_generic_id(const char *id);
 
 int snd_pcm_hw_open_fd(snd_pcm_t **pcmp, const char *name, int fd, int mmap_emulation, int sync_ptr_ioctl);
 
+int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout);
+
 #define SND_PCM_HW_PARBIT_ACCESS       (1U << SND_PCM_HW_PARAM_ACCESS)
 #define SND_PCM_HW_PARBIT_FORMAT       (1U << SND_PCM_HW_PARAM_FORMAT)
 #define SND_PCM_HW_PARBIT_SUBFORMAT    (1U << SND_PCM_HW_PARAM_SUBFORMAT)