Repository navigation
onAudioReady() is called simultaneously from two threads on OpenSL ES backend #1567
Description
Android version(s): 10 and 11
Android device(s): Multiple, see below
Oboe version: 1.6.1
App name used for testing: -
Short description
When calling requestStart() for an OpenSL ES output stream, onAudioReady() is called from the thread that called requestStart(), and simultaneously from an AudioTrack thread.
Steps to reproduce
The issue may be hard to reproduce. It occurs in our game within 5 minutes of starting the game.
Call requestStart() for an OpenSL ES output stream, and observe that onAudioReady() is called from two threads simultaneously.
Expected behavior
onAudioReady() is not called from multiple threads simultaneously.
Actual behavior
onAudioReady() is called from two threads simultaneously.
Device
- Nokia 9 PureView (Android 10)
- Samsung Galaxy A12 (Android 11)
Any additional context
Stack trace of the thread that called requestStart():
> libNativeGame.so!OboeChannelCallback::onAudioReady(oboe::AudioStream*, void*, int)(OboeChannelCallback * this, oboe::AudioStream * stream, void * buffer, int32_t frame_count) Line 37 c++14 liboboe.so!non-virtual thunk to oboe::AudioSourceCaller::onProcessFixedBlock(unsigned char*, int)() unknown liboboe.so!FixedBlockReader::read(unsigned char*, int)() unknown liboboe.so!oboe::SourceFloatCaller::onProcess(int)() unknown liboboe.so!oboe::flowgraph::FlowGraphPortFloatOutput::pullData(long, int)() unknown liboboe.so!oboe::flowgraph::SampleRateConverter::onProcess(int)() unknown liboboe.so!oboe::flowgraph::FlowGraphPortFloatOutput::pullData(long, int)() unknown liboboe.so!oboe::flowgraph::FlowGraphSink::pullData(int)() unknown liboboe.so!oboe::flowgraph::SinkFloat::read(void*, int)() unknown liboboe.so!non-virtual thunk to oboe::FilterAudioStream::onAudioReady(oboe::AudioStream*, void*, int)() unknown liboboe.so!oboe::AudioStream::fireDataCallback(void*, int)() unknown liboboe.so!oboe::AudioStreamOpenSLES::processBufferCallback(SLAndroidSimpleBufferQueueItf_ const* const*)() unknown liboboe.so!oboe::AudioOutputStreamOpenSLES::requestStart()() unknown
Stack trace of AudioTrack thread:
> libNativeGame.so!OboeChannelCallback::onAudioReady(oboe::AudioStream*, void*, int)(OboeChannelCallback * this, oboe::AudioStream * stream, void * buffer, int32_t frame_count) Line 37 c++14 liboboe.so!non-virtual thunk to oboe::AudioSourceCaller::onProcessFixedBlock(unsigned char*, int)() unknown liboboe.so!FixedBlockReader::read(unsigned char*, int)() unknown liboboe.so!oboe::SourceFloatCaller::onProcess(int)() unknown liboboe.so!oboe::flowgraph::FlowGraphPortFloatOutput::pullData(long, int)() unknown liboboe.so!oboe::flowgraph::SampleRateConverter::onProcess(int)() unknown liboboe.so!oboe::flowgraph::FlowGraphPortFloatOutput::pullData(long, int)() unknown liboboe.so!oboe::flowgraph::FlowGraphSink::pullData(int)() unknown liboboe.so!oboe::flowgraph::SinkFloat::read(void*, int)() unknown liboboe.so!non-virtual thunk to oboe::FilterAudioStream::onAudioReady(oboe::AudioStream*, void*, int)() unknown liboboe.so!oboe::AudioStream::fireDataCallback(void*, int)() unknown liboboe.so!oboe::AudioStreamOpenSLES::processBufferCallback(SLAndroidSimpleBufferQueueItf_ const* const*)() unknown libwilhelm.so!audioTrack_callBack_pullFromBuffQueue(int, void*, void*)() unknown libaudioclient.so!android::AudioTrack::processAudioBuffer()() unknown libaudioclient.so!android::AudioTrack::AudioTrackThread::threadLoop()() unknown libutils.so!android::Thread::_threadLoop(void*)() unknown libandroid_runtime.so!android::AndroidRuntime::javaThreadShell(void*)() unknown libutils.so!thread_data_t::trampoline(thread_data_t const*)() unknown libc.so!__pthread_start(void*)() unknown libc.so!__start_thread() unknown
Activity
Devenec commented on Jun 29, 2022
It seems that calling processBufferCallback() in AudioOutputStreamOpenSLES::requestStart() before calling setPlayState_l(SL_PLAYSTATE_PLAYING) fixes the issue.
From:
setState(StreamState::Starting); Result result = setPlayState_l(SL_PLAYSTATE_PLAYING); if (result == Result::OK) { setState(StreamState::Started); mLock.unlock(); if (getBufferDepth(mSimpleBufferQueueInterface) == 0) { // Enqueue the first buffer if needed to start the streaming. // This might call requestStop() so try to avoid a recursive lock. processBufferCallback(mSimpleBufferQueueInterface); } } else {to:
setState(StreamState::Starting); mLock.unlock(); if (getBufferDepth(mSimpleBufferQueueInterface) == 0) { // Enqueue the first buffer if needed to start the streaming. // This might call requestStop() so try to avoid a recursive lock. processBufferCallback(mSimpleBufferQueueInterface); } mLock.lock(); Result result = setPlayState_l(SL_PLAYSTATE_PLAYING); if (result == Result::OK) { setState(StreamState::Started); mLock.unlock(); } else {Could this change cause any issues?
robertwu1 commented on Jun 29, 2022
Thanks for finding this issue!
Moving processBufferCallback up seems like the right thing to do. However, I don't think we should release the lock. I opened #1570 to change how processBufferCallback instead.
Can you help verify whether my set of changes works for you?
philburk commented on Jun 29, 2022
Great! I think there must be a race condition if there is something in the queue when setting the PLAYSTATE. It is possible the callbacks start immediately and drain the queue and then the processBufferCallback gets called.
If we call getBufferDepth() BEFORE we call setPlayState_l(SL_PLAYSTATE_PLAYING)
then we can avoid the race.
robertwu1 commented on Jun 30, 2022
Per Phil's suggestion, the most recent version of #1570 saves the earlier buffer depth instead. @Devenec can you verify whether this works for you?
setState(StreamState::Starting); int bufferDepthBeforeSettingPlayState = getBufferDepth(mSimpleBufferQueueInterface); Result result = setPlayState_l(SL_PLAYSTATE_PLAYING); robertwu1 marked this conversation as resolved. Show resolved if (result == Result::OK) { setState(StreamState::Started); mLock.unlock(); if (bufferDepthBeforeSettingPlayState == 0) { // Enqueue the first buffer if needed to start the streaming. // This might call requestStop() so try to avoid a recursive lock. processBufferCallback(mSimpleBufferQueueInterface);Devenec commented on Jun 30, 2022
This doesn't seem to work.
However the changes in the first commit of #1570 work, after removing the lock from AudioOutputStreamOpenSLES::requestStop_l() and adding conditional requestStop() call to AudioStreamOpenSLES::bqCallbackGlue().
I'm leaving for summer holiday today and will be back in August. I'll go with the first commit for now.
robertwu1 commented on Jun 30, 2022
Thanks for the feedback! I reverted the PR back to the first commit and made the necessary fixes to it.
Have a great vacation!