commit eab3e3a Author: Mikhail Teterin Date: Tue Dec 16 19:34:02 2014 -0800 Bug 1130155 - Avoid assert failures when consuming only part of buffer. --- media/libcubeb/src/cubeb_alsa.c | 112 ++++++++++++++++++++++++++++++---------- 1 file changed, 85 insertions(+), 27 deletions(-) diff --git media/libcubeb/src/cubeb_alsa.c media/libcubeb/src/cubeb_alsa.c index 9bbc129..e72944a 100644 --- media/libcubeb/src/cubeb_alsa.c +++ media/libcubeb/src/cubeb_alsa.c @@ -14,6 +14,8 @@ #include #include #include +#include +#include #include #include #include "cubeb/cubeb.h" @@ -45,6 +47,7 @@ MAKE_TYPEDEF(snd_pcm_avail_update); MAKE_TYPEDEF(snd_pcm_close); MAKE_TYPEDEF(snd_pcm_delay); MAKE_TYPEDEF(snd_pcm_drain); +MAKE_TYPEDEF(snd_pcm_forward); MAKE_TYPEDEF(snd_pcm_frames_to_bytes); MAKE_TYPEDEF(snd_pcm_get_params); /* snd_pcm_hw_params_alloca is actually a macro */ @@ -305,32 +308,35 @@ alsa_refill_stream(cubeb_stream * stm) long got; void * p; int draining; + unsigned pipefailures, againfailures; draining = 0; pthread_mutex_lock(&stm->mutex); - r = WRAP(snd_pcm_poll_descriptors_revents)(stm->pcm, stm->fds, stm->nfds, &revents); - if (r < 0 || revents != POLLOUT) { - /* This should be a stream error; it makes no sense for poll(2) to wake - for this stream and then have the stream report that it's not ready. - Unfortunately, this does happen, so just bail out and try again. */ - pthread_mutex_unlock(&stm->mutex); - return RUNNING; - } + for (pipefailures = 0;;) { + r = WRAP(snd_pcm_poll_descriptors_revents)(stm->pcm, stm->fds, stm->nfds, &revents); + if (r < 0 || revents != POLLOUT || + (avail = WRAP(snd_pcm_avail_update)(stm->pcm)) == 0) { + /* This should be a stream error; it makes no sense for poll(2) to wake + for this stream and then have the stream report that it's not ready. + Unfortunately, this does happen, so just bail out and try again. */ + pthread_mutex_unlock(&stm->mutex); + return RUNNING; + } - avail = WRAP(snd_pcm_avail_update)(stm->pcm); - if (avail == -EPIPE) { + if (avail > 0) + break; + if (pipefailures++ > 11) { + fprintf(stderr, "%s: repeated failures from snd_pcm_avail_update, " + "giving up\n", __func__); + pthread_mutex_unlock(&stm->mutex); + stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_ERROR); + return ERROR; + } WRAP(snd_pcm_recover)(stm->pcm, avail, 1); - avail = WRAP(snd_pcm_avail_update)(stm->pcm); - } - - /* Failed to recover from an xrun, this stream must be broken. */ - if (avail < 0) { - pthread_mutex_unlock(&stm->mutex); - stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_ERROR); - return ERROR; } + pipefailures = againfailures = 0; /* This should never happen. */ if ((unsigned int) avail > stm->buffer_size) { @@ -359,10 +365,11 @@ alsa_refill_stream(cubeb_stream * stm) if (got < 0) { pthread_mutex_unlock(&stm->mutex); stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_ERROR); + free(p); return ERROR; } if (got > 0) { - snd_pcm_sframes_t wrote; + snd_pcm_sframes_t wrote, towrite = got; if (stm->params.format == CUBEB_SAMPLE_FLOAT32NE) { float * b = (float *) p; @@ -375,14 +382,62 @@ alsa_refill_stream(cubeb_stream * stm) b[i] *= stm->volume; } } - wrote = WRAP(snd_pcm_writei)(stm->pcm, p, got); - if (wrote == -EPIPE) { - WRAP(snd_pcm_recover)(stm->pcm, wrote, 1); - wrote = WRAP(snd_pcm_writei)(stm->pcm, p, got); + for (;;) { + wrote = WRAP(snd_pcm_writei)(stm->pcm, p, + towrite > avail ? avail : towrite); + switch(wrote) { + case -EPIPE: + if (pipefailures++ > 3) { + fprintf(stderr, "%s: Too many underflows, giving up\n", __func__); + stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_ERROR); + pthread_mutex_unlock(&stm->mutex); + free(p); + return ERROR; + } + WRAP(snd_pcm_recover)(stm->pcm, wrote, 1); + continue; + case -EAGAIN: + if (againfailures++ > 3) { + fprintf(stderr, "%s: Too many -EAGAIN errors from snd_pcm_writei, " + "giving up\n", __func__); + stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_ERROR); + pthread_mutex_unlock(&stm->mutex); + free(p); + return ERROR; + } + continue; + case -EBADFD: + fprintf(stderr, "%s: snc_pcm_writei returned -%s, giving up\n", + __func__, "EBADFD"); + free(p); + stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_ERROR); + pthread_mutex_unlock(&stm->mutex); + return ERROR; + } + if (wrote < 0) { + fprintf(stderr, "%s: snc_pcm_writei returned unexpected error %lld, " + "giving up\n", __func__, (long long)wrote); + free(p); + stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_ERROR); + pthread_mutex_unlock(&stm->mutex); + return ERROR; + } + pipefailures = againfailures = 0; + stm->write_position += wrote; + gettimeofday(&stm->last_activity, NULL); + if (wrote > towrite) { + fprintf(stderr, "%s: snc_pcm_writei wrote %lld frames, which was more " + "than we requested (%lld). This should not happen, giving up\n", + __func__, (long long)wrote, (long long)towrite); + free(p); + stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_ERROR); + pthread_mutex_unlock(&stm->mutex); + return ERROR; + } + if (towrite == wrote) + break; + towrite -= wrote; } - assert(wrote >= 0 && wrote == got); - stm->write_position += wrote; - gettimeofday(&stm->last_activity, NULL); } if (got != avail) { long buffer_fill = stm->buffer_size - (avail - got); @@ -1177,7 +1232,10 @@ alsa_stream_get_position(cubeb_stream * stm, uint64_t * position) return CUBEB_OK; } - assert(delay >= 0); + if (delay < 0) { + WRAP(snd_pcm_forward)(stm->pcm, -delay); + delay = 0; + } *position = 0; if (stm->write_position >= (snd_pcm_uframes_t) delay) {