From 03274a5b95146675c05b5b6a0340f45a7b122c50 Mon Sep 17 00:00:00 2001 From: Chris Robinson Date: Fri, 2 Mar 2018 12:46:31 -0800 Subject: [PATCH] Ensure at least the specified ringbuffer size is writable Previously, all but one of the specified size could be written (so for a size of n, only n-1 was guaranteed writable). All users pretty much compensated for this, but it makes more sense to fix it at the source. --- Alc/backends/alsa.c | 2 +- Alc/backends/coreaudio.c | 2 +- Alc/backends/dsound.c | 2 +- Alc/backends/jack.c | 10 +++------- Alc/backends/mmdevapi.c | 2 +- Alc/backends/opensl.c | 12 ++++-------- Alc/backends/oss.c | 2 +- Alc/backends/winmm.c | 2 +- Alc/ringbuffer.c | 34 ++++++++-------------------------- Alc/ringbuffer.h | 48 ++++++++++++++++++++++++++++++++++++++++++++---- 10 files changed, 65 insertions(+), 51 deletions(-) diff --git a/Alc/backends/alsa.c b/Alc/backends/alsa.c index d0f0e24e..9fc36582 100644 --- a/Alc/backends/alsa.c +++ b/Alc/backends/alsa.c @@ -1123,7 +1123,7 @@ static ALCenum ALCcaptureAlsa_open(ALCcaptureAlsa *self, const ALCchar *name) if(needring) { self->ring = ll_ringbuffer_create( - device->UpdateSize*device->NumUpdates + 1, + device->UpdateSize*device->NumUpdates, FrameSizeFromDevFmt(device->FmtChans, device->FmtType, device->AmbiOrder), false ); diff --git a/Alc/backends/coreaudio.c b/Alc/backends/coreaudio.c index caa01167..b2545c47 100644 --- a/Alc/backends/coreaudio.c +++ b/Alc/backends/coreaudio.c @@ -663,7 +663,7 @@ static ALCenum ALCcoreAudioCapture_open(ALCcoreAudioCapture *self, const ALCchar goto error; self->ring = ll_ringbuffer_create( - device->UpdateSize*self->sampleRateRatio*device->NumUpdates + 1, + (size_t)ceil(device->UpdateSize*self->sampleRateRatio*device->NumUpdates), self->frameSize, false ); if(!self->ring) goto error; diff --git a/Alc/backends/dsound.c b/Alc/backends/dsound.c index 4c52e0f8..6bab641c 100644 --- a/Alc/backends/dsound.c +++ b/Alc/backends/dsound.c @@ -856,7 +856,7 @@ static ALCenum ALCdsoundCapture_open(ALCdsoundCapture *self, const ALCchar *devi hr = IDirectSoundCapture_CreateCaptureBuffer(self->DSC, &DSCBDescription, &self->DSCbuffer, NULL); if(SUCCEEDED(hr)) { - self->Ring = ll_ringbuffer_create(device->UpdateSize*device->NumUpdates + 1, + self->Ring = ll_ringbuffer_create(device->UpdateSize*device->NumUpdates, InputType.Format.nBlockAlign, false); if(self->Ring == NULL) hr = DSERR_OUTOFMEMORY; diff --git a/Alc/backends/jack.c b/Alc/backends/jack.c index 003877a4..67e3c106 100644 --- a/Alc/backends/jack.c +++ b/Alc/backends/jack.c @@ -229,8 +229,7 @@ static int ALCjackPlayback_bufferSizeNotify(jack_nframes_t numframes, void *arg) bufsize = device->UpdateSize; if(ConfigValueUInt(alstr_get_cstr(device->DeviceName), "jack", "buffer-size", &bufsize)) bufsize = maxu(NextPowerOf2(bufsize), device->UpdateSize); - bufsize += device->UpdateSize; - device->NumUpdates = bufsize / device->UpdateSize; + device->NumUpdates = (bufsize+device->UpdateSize) / device->UpdateSize; TRACE("%u update size x%u\n", device->UpdateSize, device->NumUpdates); @@ -391,9 +390,7 @@ static ALCboolean ALCjackPlayback_reset(ALCjackPlayback *self) } /* Ignore the requested buffer metrics and just keep one JACK-sized buffer - * ready for when requested. Note that one period's worth of audio in the - * ring buffer will always be left unfilled because one element of the ring - * buffer will not be writeable, and we only write in period-sized chunks. + * ready for when requested. */ device->Frequency = jack_get_sample_rate(self->Client); device->UpdateSize = jack_get_buffer_size(self->Client); @@ -402,8 +399,7 @@ static ALCboolean ALCjackPlayback_reset(ALCjackPlayback *self) bufsize = device->UpdateSize; if(ConfigValueUInt(alstr_get_cstr(device->DeviceName), "jack", "buffer-size", &bufsize)) bufsize = maxu(NextPowerOf2(bufsize), device->UpdateSize); - bufsize += device->UpdateSize; - device->NumUpdates = bufsize / device->UpdateSize; + device->NumUpdates = (bufsize+device->UpdateSize) / device->UpdateSize; /* Force 32-bit float output. */ device->FmtType = DevFmtFloat; diff --git a/Alc/backends/mmdevapi.c b/Alc/backends/mmdevapi.c index 34dcf468..961cba52 100644 --- a/Alc/backends/mmdevapi.c +++ b/Alc/backends/mmdevapi.c @@ -1810,7 +1810,7 @@ static HRESULT ALCmmdevCapture_resetProxy(ALCmmdevCapture *self) return hr; } - buffer_len = maxu(device->UpdateSize*device->NumUpdates + 1, buffer_len); + buffer_len = maxu(device->UpdateSize*device->NumUpdates, buffer_len); ll_ringbuffer_free(self->Ring); self->Ring = ll_ringbuffer_create(buffer_len, FrameSizeFromDevFmt(device->FmtChans, device->FmtType, device->AmbiOrder), diff --git a/Alc/backends/opensl.c b/Alc/backends/opensl.c index d930526d..a5ad3b09 100644 --- a/Alc/backends/opensl.c +++ b/Alc/backends/opensl.c @@ -566,12 +566,8 @@ static ALCboolean ALCopenslPlayback_start(ALCopenslPlayback *self) SLresult result; ll_ringbuffer_free(self->mRing); - /* NOTE: Add an extra update since one period's worth of audio in the ring - * buffer will always be left unfilled because one element of the ring - * buffer will not be writeable, and we only write in period-sized chunks. - */ - self->mRing = ll_ringbuffer_create(device->NumUpdates + 1, - self->mFrameSize*device->UpdateSize, true); + self->mRing = ll_ringbuffer_create(device->NumUpdates, self->mFrameSize*device->UpdateSize, + true); result = VCALL(self->mBufferQueueObj,GetInterface)(SL_IID_ANDROIDSIMPLEBUFFERQUEUE, &bufferQueue); @@ -847,8 +843,8 @@ static ALCenum ALCopenslCapture_open(ALCopenslCapture *self, const ALCchar *name if(SL_RESULT_SUCCESS == result) { - self->mRing = ll_ringbuffer_create(device->NumUpdates + 1, - device->UpdateSize * self->mFrameSize, false); + self->mRing = ll_ringbuffer_create(device->NumUpdates, device->UpdateSize*self->mFrameSize, + false); result = VCALL(self->mRecordObj,GetInterface)(SL_IID_ANDROIDSIMPLEBUFFERQUEUE, &bufferQueue); diff --git a/Alc/backends/oss.c b/Alc/backends/oss.c index 61d25476..c0c98c43 100644 --- a/Alc/backends/oss.c +++ b/Alc/backends/oss.c @@ -729,7 +729,7 @@ static ALCenum ALCcaptureOSS_open(ALCcaptureOSS *self, const ALCchar *name) return ALC_INVALID_VALUE; } - self->ring = ll_ringbuffer_create(device->UpdateSize*device->NumUpdates + 1, frameSize, false); + self->ring = ll_ringbuffer_create(device->UpdateSize*device->NumUpdates, frameSize, false); if(!self->ring) { ERR("Ring buffer create failed\n"); diff --git a/Alc/backends/winmm.c b/Alc/backends/winmm.c index 787ba8e4..2f4c65df 100644 --- a/Alc/backends/winmm.c +++ b/Alc/backends/winmm.c @@ -626,7 +626,7 @@ static ALCenum ALCwinmmCapture_open(ALCwinmmCapture *self, const ALCchar *name) if(CapturedDataSize < (self->Format.nSamplesPerSec / 10)) CapturedDataSize = self->Format.nSamplesPerSec / 10; - self->Ring = ll_ringbuffer_create(CapturedDataSize+1, self->Format.nBlockAlign, false); + self->Ring = ll_ringbuffer_create(CapturedDataSize, self->Format.nBlockAlign, false); if(!self->Ring) goto failure; InitRef(&self->WaveBuffersCommitted, 0); diff --git a/Alc/ringbuffer.c b/Alc/ringbuffer.c index 02fa857c..6c419cf8 100644 --- a/Alc/ringbuffer.c +++ b/Alc/ringbuffer.c @@ -46,8 +46,6 @@ struct ll_ringbuffer { alignas(16) char buf[]; }; -/* Create a new ringbuffer to hold at least `sz' elements of `elem_sz' bytes. - * The number of elements is rounded up to the next power of two. */ ll_ringbuffer_t *ll_ringbuffer_create(size_t sz, size_t elem_sz, int limit_writes) { ll_ringbuffer_t *rb; @@ -55,7 +53,7 @@ ll_ringbuffer_t *ll_ringbuffer_create(size_t sz, size_t elem_sz, int limit_write if(sz > 0) { - power_of_two = sz - 1; + power_of_two = sz; power_of_two |= power_of_two>>1; power_of_two |= power_of_two>>2; power_of_two |= power_of_two>>4; @@ -79,13 +77,11 @@ ll_ringbuffer_t *ll_ringbuffer_create(size_t sz, size_t elem_sz, int limit_write return rb; } -/* Free all data associated with the ringbuffer `rb'. */ void ll_ringbuffer_free(ll_ringbuffer_t *rb) { al_free(rb); } -/* Reset the read and write pointers to zero. This is not thread safe. */ void ll_ringbuffer_reset(ll_ringbuffer_t *rb) { ATOMIC_STORE(&rb->write_ptr, 0, almemory_order_release); @@ -93,16 +89,14 @@ void ll_ringbuffer_reset(ll_ringbuffer_t *rb) memset(rb->buf, 0, (rb->size_mask+1)*rb->elem_size); } -/* Return the number of elements available for reading. This is the number of - * elements in front of the read pointer and behind the write pointer. */ + size_t ll_ringbuffer_read_space(const ll_ringbuffer_t *rb) { size_t w = ATOMIC_LOAD(&CONST_CAST(ll_ringbuffer_t*,rb)->write_ptr, almemory_order_acquire); size_t r = ATOMIC_LOAD(&CONST_CAST(ll_ringbuffer_t*,rb)->read_ptr, almemory_order_acquire); return (w-r) & rb->size_mask; } -/* Return the number of elements available for writing. This is the number of - * elements in front of the write pointer and behind the read pointer. */ + size_t ll_ringbuffer_write_space(const ll_ringbuffer_t *rb) { size_t w = ATOMIC_LOAD(&CONST_CAST(ll_ringbuffer_t*,rb)->write_ptr, almemory_order_acquire); @@ -111,8 +105,7 @@ size_t ll_ringbuffer_write_space(const ll_ringbuffer_t *rb) return (w > rb->size) ? rb->size : w; } -/* The copying data reader. Copy at most `cnt' elements from `rb' to `dest'. - * Returns the actual number of elements copied. */ + size_t ll_ringbuffer_read(ll_ringbuffer_t *rb, char *dest, size_t cnt) { size_t read_ptr; @@ -151,9 +144,6 @@ size_t ll_ringbuffer_read(ll_ringbuffer_t *rb, char *dest, size_t cnt) return to_read; } -/* The copying data reader w/o read pointer advance. Copy at most `cnt' - * elements from `rb' to `dest'. Returns the actual number of elements copied. - */ size_t ll_ringbuffer_peek(ll_ringbuffer_t *rb, char *dest, size_t cnt) { size_t free_cnt; @@ -190,8 +180,6 @@ size_t ll_ringbuffer_peek(ll_ringbuffer_t *rb, char *dest, size_t cnt) return to_read; } -/* The copying data writer. Copy at most `cnt' elements to `rb' from `src'. - * Returns the actual number of elements copied. */ size_t ll_ringbuffer_write(ll_ringbuffer_t *rb, const char *src, size_t cnt) { size_t write_ptr; @@ -230,22 +218,19 @@ size_t ll_ringbuffer_write(ll_ringbuffer_t *rb, const char *src, size_t cnt) return to_write; } -/* Advance the read pointer `cnt' places. */ + void ll_ringbuffer_read_advance(ll_ringbuffer_t *rb, size_t cnt) { ATOMIC_ADD(&rb->read_ptr, cnt, almemory_order_acq_rel); } -/* Advance the write pointer `cnt' places. */ void ll_ringbuffer_write_advance(ll_ringbuffer_t *rb, size_t cnt) { ATOMIC_ADD(&rb->write_ptr, cnt, almemory_order_acq_rel); } -/* The non-copying data reader. `vec' is an array of two places. Set the values - * at `vec' to hold the current readable data at `rb'. If the readable data is - * in one segment the second segment has zero length. */ -void ll_ringbuffer_get_read_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_data_t * vec) + +void ll_ringbuffer_get_read_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_data_t vec[2]) { size_t free_cnt; size_t cnt2; @@ -277,10 +262,7 @@ void ll_ringbuffer_get_read_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_data } } -/* The non-copying data writer. `vec' is an array of two places. Set the values - * at `vec' to hold the current writeable data at `rb'. If the writeable data - * is in one segment the second segment has zero length. */ -void ll_ringbuffer_get_write_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_data_t *vec) +void ll_ringbuffer_get_write_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_data_t vec[2]) { size_t free_cnt; size_t cnt2; diff --git a/Alc/ringbuffer.h b/Alc/ringbuffer.h index c1a7a6fa..bcf67374 100644 --- a/Alc/ringbuffer.h +++ b/Alc/ringbuffer.h @@ -10,20 +10,60 @@ typedef struct ll_ringbuffer_data { size_t len; } ll_ringbuffer_data_t; + +/** + * Create a new ringbuffer to hold at least `sz' elements of `elem_sz' bytes. + * The number of elements is rounded up to the next power of two (even if it is + * already a power of two, to ensure the requested amount can be written). + */ ll_ringbuffer_t *ll_ringbuffer_create(size_t sz, size_t elem_sz, int limit_writes); +/** Free all data associated with the ringbuffer `rb'. */ void ll_ringbuffer_free(ll_ringbuffer_t *rb); +/** Reset the read and write pointers to zero. This is not thread safe. */ void ll_ringbuffer_reset(ll_ringbuffer_t *rb); -void ll_ringbuffer_get_read_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_data_t *vec); -void ll_ringbuffer_get_write_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_data_t *vec); +/** + * The non-copying data reader. `vec' is an array of two places. Set the values + * at `vec' to hold the current readable data at `rb'. If the readable data is + * in one segment the second segment has zero length. + */ +void ll_ringbuffer_get_read_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_data_t vec[2]); +/** + * The non-copying data writer. `vec' is an array of two places. Set the values + * at `vec' to hold the current writeable data at `rb'. If the writeable data + * is in one segment the second segment has zero length. + */ +void ll_ringbuffer_get_write_vector(const ll_ringbuffer_t *rb, ll_ringbuffer_data_t vec[2]); +/** + * Return the number of elements available for reading. This is the number of + * elements in front of the read pointer and behind the write pointer. + */ +size_t ll_ringbuffer_read_space(const ll_ringbuffer_t *rb); +/** + * The copying data reader. Copy at most `cnt' elements from `rb' to `dest'. + * Returns the actual number of elements copied. + */ size_t ll_ringbuffer_read(ll_ringbuffer_t *rb, char *dest, size_t cnt); +/** + * The copying data reader w/o read pointer advance. Copy at most `cnt' + * elements from `rb' to `dest'. Returns the actual number of elements copied. + */ size_t ll_ringbuffer_peek(ll_ringbuffer_t *rb, char *dest, size_t cnt); +/** Advance the read pointer `cnt' places. */ void ll_ringbuffer_read_advance(ll_ringbuffer_t *rb, size_t cnt); -size_t ll_ringbuffer_read_space(const ll_ringbuffer_t *rb); +/** + * Return the number of elements available for writing. This is the number of + * elements in front of the write pointer and behind the read pointer. + */ +size_t ll_ringbuffer_write_space(const ll_ringbuffer_t *rb); +/** + * The copying data writer. Copy at most `cnt' elements to `rb' from `src'. + * Returns the actual number of elements copied. + */ size_t ll_ringbuffer_write(ll_ringbuffer_t *rb, const char *src, size_t cnt); +/** Advance the write pointer `cnt' places. */ void ll_ringbuffer_write_advance(ll_ringbuffer_t *rb, size_t cnt); -size_t ll_ringbuffer_write_space(const ll_ringbuffer_t *rb); #endif /* RINGBUFFER_H */ -- 2.11.4.GIT