From b77e6096b8ed26b8fee24a18820c0592eeee87b4 Mon Sep 17 00:00:00 2001 From: Chris Robinson Date: Mon, 17 Sep 2018 22:49:52 -0700 Subject: Fix some potential race conditions with OpenSL For playback, increment the ring buffer's write pointer before queueing audio, to handle cases where the callback is invoked, advancing the read pointer, before the write pointer is advanced. For capture, limit the number of re-queued chunks to the number of fully read chunks. --- Alc/backends/opensl.c | 104 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 42 deletions(-) (limited to 'Alc/backends') diff --git a/Alc/backends/opensl.c b/Alc/backends/opensl.c index 50fe77d8..9815b390 100644 --- a/Alc/backends/opensl.c +++ b/Alc/backends/opensl.c @@ -206,6 +206,9 @@ static void ALCopenslPlayback_Destruct(ALCopenslPlayback* self) self->mEngineObj = NULL; self->mEngine = NULL; + ll_ringbuffer_free(self->mRing); + self->mRing = NULL; + alsem_destroy(&self->mSem); ALCbackend_Destruct(STATIC_CAST(ALCbackend, self)); @@ -251,19 +254,16 @@ static int ALCopenslPlayback_mixerProc(void *arg) result = VCALL(self->mBufferQueueObj,GetInterface)(SL_IID_PLAY, &player); PRINTERR(result, "bufferQueue->GetInterface SL_IID_PLAY"); } + + ALCopenslPlayback_lock(self); if(SL_RESULT_SUCCESS != result) - { - ALCopenslPlayback_lock(self); aluHandleDisconnect(device, "Failed to get playback buffer: 0x%08x", result); - ALCopenslPlayback_unlock(self); - return 1; - } - ALCopenslPlayback_lock(self); - while(!ATOMIC_LOAD(&self->mKillNow, almemory_order_acquire) && + while(SL_RESULT_SUCCESS == result && + !ATOMIC_LOAD(&self->mKillNow, almemory_order_acquire) && ATOMIC_LOAD(&device->Connected, almemory_order_acquire)) { - size_t todo, len0, len1; + size_t todo; if(ll_ringbuffer_write_space(self->mRing) == 0) { @@ -292,34 +292,33 @@ static int ALCopenslPlayback_mixerProc(void *arg) } ll_ringbuffer_get_write_vector(self->mRing, data); - todo = data[0].len+data[1].len; - - len0 = minu(todo, data[0].len); - len1 = minu(todo-len0, data[1].len); - aluMixData(device, data[0].buf, len0*device->UpdateSize); - for(size_t i = 0;i < len0;i++) - { - result = VCALL(bufferQueue,Enqueue)(data[0].buf, device->UpdateSize*self->mFrameSize); - PRINTERR(result, "bufferQueue->Enqueue"); - if(SL_RESULT_SUCCESS == result) - ll_ringbuffer_write_advance(self->mRing, 1); + aluMixData(device, data[0].buf, data[0].len*device->UpdateSize); + if(data[1].len > 0) + aluMixData(device, data[1].buf, data[1].len*device->UpdateSize); - data[0].buf += device->UpdateSize*self->mFrameSize; - } + todo = data[0].len+data[1].len; + ll_ringbuffer_write_advance(self->mRing, todo); - if(len1 > 0) + for(size_t i = 0;i < todo;i++) { - aluMixData(device, data[1].buf, len1*device->UpdateSize); - for(size_t i = 0;i < len1;i++) + if(!data[0].len) { - result = VCALL(bufferQueue,Enqueue)(data[1].buf, device->UpdateSize*self->mFrameSize); - PRINTERR(result, "bufferQueue->Enqueue"); - if(SL_RESULT_SUCCESS == result) - ll_ringbuffer_write_advance(self->mRing, 1); + data[0] = data[1]; + data[1].buf = NULL; + data[1].len = 0; + } - data[1].buf += device->UpdateSize*self->mFrameSize; + result = VCALL(bufferQueue,Enqueue)(data[0].buf, device->UpdateSize*self->mFrameSize); + PRINTERR(result, "bufferQueue->Enqueue"); + if(SL_RESULT_SUCCESS != result) + { + aluHandleDisconnect(device, "Failed to queue audio: 0x%08x", result); + break; } + + data[0].len--; + data[0].buf += device->UpdateSize*self->mFrameSize; } } ALCopenslPlayback_unlock(self); @@ -398,6 +397,9 @@ static ALCboolean ALCopenslPlayback_reset(ALCopenslPlayback *self) VCALL0(self->mBufferQueueObj,Destroy)(); self->mBufferQueueObj = NULL; + ll_ringbuffer_free(self->mRing); + self->mRing = NULL; + sampleRate = device->Frequency; if(!(device->Flags&DEVICE_FREQUENCY_REQUEST) && (env=Android_GetJNIEnv()) != NULL) { @@ -546,6 +548,18 @@ static ALCboolean ALCopenslPlayback_reset(ALCopenslPlayback *self) result = VCALL(self->mBufferQueueObj,Realize)(SL_BOOLEAN_FALSE); PRINTERR(result, "bufferQueue->Realize"); } + if(SL_RESULT_SUCCESS == result) + { + self->mRing = ll_ringbuffer_create(device->NumUpdates, + self->mFrameSize*device->UpdateSize, true + ); + if(!self->mRing) + { + ERR("Out of memory allocating ring buffer %ux%u %u\n", device->UpdateSize, + device->NumUpdates, self->mFrameSize); + result = SL_RESULT_MEMORY_FAILURE; + } + } if(SL_RESULT_SUCCESS != result) { @@ -561,13 +575,10 @@ static ALCboolean ALCopenslPlayback_reset(ALCopenslPlayback *self) static ALCboolean ALCopenslPlayback_start(ALCopenslPlayback *self) { - ALCdevice *device = STATIC_CAST(ALCbackend,self)->mDevice; SLAndroidSimpleBufferQueueItf bufferQueue; SLresult result; - ll_ringbuffer_free(self->mRing); - self->mRing = ll_ringbuffer_create(device->NumUpdates, self->mFrameSize*device->UpdateSize, - true); + ll_ringbuffer_reset(self->mRing); result = VCALL(self->mBufferQueueObj,GetInterface)(SL_IID_ANDROIDSIMPLEBUFFERQUEUE, &bufferQueue); @@ -634,9 +645,6 @@ static void ALCopenslPlayback_stop(ALCopenslPlayback *self) } while(SL_RESULT_SUCCESS == result && state.count > 0); PRINTERR(result, "bufferQueue->GetState"); } - - ll_ringbuffer_free(self->mRing); - self->mRing = NULL; } static ClockLatency ALCopenslPlayback_getClockLatency(ALCopenslPlayback *self) @@ -713,9 +721,6 @@ static void ALCopenslCapture_Construct(ALCopenslCapture *self, ALCdevice *device static void ALCopenslCapture_Destruct(ALCopenslCapture *self) { - ll_ringbuffer_free(self->mRing); - self->mRing = NULL; - if(self->mRecordObj != NULL) VCALL0(self->mRecordObj,Destroy)(); self->mRecordObj = NULL; @@ -725,6 +730,9 @@ static void ALCopenslCapture_Destruct(ALCopenslCapture *self) self->mEngineObj = NULL; self->mEngine = NULL; + ll_ringbuffer_free(self->mRing); + self->mRing = NULL; + ALCbackend_Destruct(STATIC_CAST(ALCbackend, self)); } @@ -843,8 +851,9 @@ static ALCenum ALCopenslCapture_open(ALCopenslCapture *self, const ALCchar *name if(SL_RESULT_SUCCESS == result) { - self->mRing = ll_ringbuffer_create(device->NumUpdates, 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); @@ -972,14 +981,25 @@ static ALCenum ALCopenslCapture_captureSamples(ALCopenslCapture *self, ALCvoid * i += rem; } + if(!advance) + return ALC_NO_ERROR; ll_ringbuffer_read_advance(self->mRing, advance); result = VCALL(self->mRecordObj,GetInterface)(SL_IID_ANDROIDSIMPLEBUFFERQUEUE, &bufferQueue); PRINTERR(result, "recordObj->GetInterface"); - /* Enqueue any newly-writable chunks in the ring buffer. */ + /* Enqueue any newly-writable chunks in the ring buffer. Limit the number + * of enqueued chunks to the number of fully read chunks. + */ ll_ringbuffer_get_write_vector(self->mRing, data); + if(data[0].len > advance) + { + data[0].len = advance; + data[1].len = 0; + } + else if(data[1].len > advance-data[0].len) + data[1].len = advance-data[0].len; for(i = 0;i < data[0].len && SL_RESULT_SUCCESS == result;i++) { result = VCALL(bufferQueue,Enqueue)(data[0].buf + chunk_size*i, chunk_size); -- cgit v1.2.3