oboe: address review comments, better singleton
Use constexpr.
Add new files to CMakeLists.txt
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1a4bfad..1b82a0f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -11,9 +11,13 @@
src/fifo/FifoController.cpp
src/fifo/FifoControllerBase.cpp
src/fifo/FifoControllerIndirect.cpp
+ src/opensles/AudioInputStreamOpenSLES.cpp
+ src/opensles/AudioOutputStreamOpenSLES.cpp
src/opensles/AudioStreamBuffered.cpp
src/opensles/AudioStreamOpenSLES.cpp
+ src/opensles/EngineOpenSLES.cpp
src/opensles/OpenSLESUtilities.cpp
+ src/opensles/OutputMixerOpenSLES.cpp
)
add_library(oboe STATIC ${oboe_sources})
@@ -25,4 +29,4 @@
PRIVATE -Wall
PRIVATE "$<$<CONFIG:DEBUG>:-Werror>") # Only include -Werror when building debug config
-target_link_libraries(oboe PRIVATE log OpenSLES)
\ No newline at end of file
+target_link_libraries(oboe PRIVATE log OpenSLES)
diff --git a/src/opensles/AudioInputStreamOpenSLES.cpp b/src/opensles/AudioInputStreamOpenSLES.cpp
index 23e1c66..3ff10e6 100644
--- a/src/opensles/AudioInputStreamOpenSLES.cpp
+++ b/src/opensles/AudioInputStreamOpenSLES.cpp
@@ -59,7 +59,7 @@
Result oboeResult = AudioStreamOpenSLES::open();
if (Result::OK != oboeResult) return oboeResult;
- SLuint32 bitsPerSample = getBytesPerSample() * OBOE_BITS_PER_BYTE;
+ SLuint32 bitsPerSample = getBytesPerSample() * kBitsPerByte;
// configure audio sink
SLDataLocator_AndroidSimpleBufferQueue loc_bufq = {
@@ -100,7 +100,7 @@
NULL };
SLDataSource audioSrc = {&loc_dev, NULL };
- SLresult result = EngineOpenSLES::getInstance()->createAudioRecorder(&mObjectInterface,
+ SLresult result = EngineOpenSLES::getInstance().createAudioRecorder(&mObjectInterface,
&audioSrc,
&audioSink);
if (SL_RESULT_SUCCESS != result) {
@@ -130,17 +130,17 @@
result = (*mObjectInterface)->GetInterface(mObjectInterface, SL_IID_RECORD, &mRecordInterface);
if (SL_RESULT_SUCCESS != result) {
- LOGE("get recorder interface result:%s", getSLErrStr(result));
+ LOGE("GetInterface RECORD result:%s", getSLErrStr(result));
goto error;
}
result = AudioStreamOpenSLES::registerBufferQueueCallback();
if (SL_RESULT_SUCCESS != result) {
- LOGE("get bufferqueue interface:%p result:%s", mSimpleBufferQueueInterface, getSLErrStr(result));
goto error;
}
return Result::OK;
+
error:
return Result::ErrorInternal; // TODO convert error from SLES to OBOE
}
diff --git a/src/opensles/AudioOutputStreamOpenSLES.cpp b/src/opensles/AudioOutputStreamOpenSLES.cpp
index b31d627..2129766 100644
--- a/src/opensles/AudioOutputStreamOpenSLES.cpp
+++ b/src/opensles/AudioOutputStreamOpenSLES.cpp
@@ -35,15 +35,16 @@
}
// These will wind up in <SLES/OpenSLES_Android.h>
-#define SL_ANDROID_SPEAKER_QUAD (SL_SPEAKER_FRONT_LEFT | SL_SPEAKER_FRONT_RIGHT \
- | SL_SPEAKER_BACK_LEFT | SL_SPEAKER_BACK_RIGHT)
+constexpr int SL_ANDROID_SPEAKER_STEREO = (SL_SPEAKER_FRONT_LEFT | SL_SPEAKER_FRONT_RIGHT);
-#define SL_ANDROID_SPEAKER_5DOT1 (SL_SPEAKER_FRONT_LEFT | SL_SPEAKER_FRONT_RIGHT \
- | SL_SPEAKER_FRONT_CENTER | SL_SPEAKER_LOW_FREQUENCY| SL_SPEAKER_BACK_LEFT \
- | SL_SPEAKER_BACK_RIGHT)
+constexpr int SL_ANDROID_SPEAKER_QUAD = (SL_ANDROID_SPEAKER_STEREO
+ | SL_SPEAKER_BACK_LEFT | SL_SPEAKER_BACK_RIGHT);
-#define SL_ANDROID_SPEAKER_7DOT1 (SL_ANDROID_SPEAKER_5DOT1 | SL_SPEAKER_SIDE_LEFT \
- |SL_SPEAKER_SIDE_RIGHT)
+constexpr int SL_ANDROID_SPEAKER_5DOT1 = (SL_ANDROID_SPEAKER_QUAD
+ | SL_SPEAKER_FRONT_CENTER | SL_SPEAKER_LOW_FREQUENCY);
+
+constexpr int SL_ANDROID_SPEAKER_7DOT1 = (SL_ANDROID_SPEAKER_5DOT1 | SL_SPEAKER_SIDE_LEFT
+ | SL_SPEAKER_SIDE_RIGHT);
int AudioOutputStreamOpenSLES::chanCountToChanMask(int chanCount) {
int channelMask = 0;
@@ -54,7 +55,7 @@
break;
case 2:
- channelMask = SL_SPEAKER_FRONT_LEFT | SL_SPEAKER_FRONT_RIGHT;
+ channelMask = SL_ANDROID_SPEAKER_STEREO;
break;
case 4: // Quad
@@ -76,13 +77,13 @@
Result oboeResult = AudioStreamOpenSLES::open();
if (Result::OK != oboeResult) return oboeResult;
- SLresult result = OutputMixerOpenSL::getInstance()->open();
+ SLresult result = OutputMixerOpenSL::getInstance().open();
if (SL_RESULT_SUCCESS != result) {
AudioStreamOpenSLES::close();
return Result::ErrorInternal;
}
- SLuint32 bitsPerSample = getBytesPerSample() * OBOE_BITS_PER_BYTE;
+ SLuint32 bitsPerSample = getBytesPerSample() * kBitsPerByte;
// configure audio source
SLDataLocator_AndroidSimpleBufferQueue loc_bufq = {
@@ -116,7 +117,7 @@
audioSrc.pFormat = &format_pcm_ex;
}
- result = OutputMixerOpenSL::getInstance()->createAudioPlayer(&mObjectInterface,
+ result = OutputMixerOpenSL::getInstance().createAudioPlayer(&mObjectInterface,
&audioSrc);
if (SL_RESULT_SUCCESS != result) {
LOGE("createAudioPlayer() result:%s", getSLErrStr(result));
@@ -131,13 +132,12 @@
result = (*mObjectInterface)->GetInterface(mObjectInterface, SL_IID_PLAY, &mPlayInterface);
if (SL_RESULT_SUCCESS != result) {
- LOGE("get player interface result:%s", getSLErrStr(result));
+ LOGE("GetInterface PLAY result:%s", getSLErrStr(result));
goto error;
}
result = AudioStreamOpenSLES::registerBufferQueueCallback();
if (SL_RESULT_SUCCESS != result) {
- LOGE("get bufferqueue interface:%p result:%s", mSimpleBufferQueueInterface, getSLErrStr(result));
goto error;
}
@@ -150,7 +150,7 @@
requestPause();
// invalidate any interfaces
mPlayInterface = NULL;
- OutputMixerOpenSL::getInstance()->close();
+ OutputMixerOpenSL::getInstance().close();
return AudioStreamOpenSLES::close();
}
diff --git a/src/opensles/AudioStreamOpenSLES.cpp b/src/opensles/AudioStreamOpenSLES.cpp
index 0a674af..3830eeb 100644
--- a/src/opensles/AudioStreamOpenSLES.cpp
+++ b/src/opensles/AudioStreamOpenSLES.cpp
@@ -65,7 +65,7 @@
return Result::ErrorInvalidFormat;
}
- SLresult result = EngineOpenSLES::getInstance()->open();
+ SLresult result = EngineOpenSLES::getInstance().open();
if (SL_RESULT_SUCCESS != result) {
return Result::ErrorInternal;
}
@@ -119,7 +119,7 @@
}
mSimpleBufferQueueInterface = NULL;
- EngineOpenSLES::getInstance()->close();
+ EngineOpenSLES::getInstance().close();
return Result::OK;
}
@@ -149,7 +149,9 @@
SLresult result = (*mObjectInterface)->GetInterface(mObjectInterface, SL_IID_ANDROIDSIMPLEBUFFERQUEUE,
&mSimpleBufferQueueInterface);
if (SL_RESULT_SUCCESS != result) {
- LOGE("get bufferqueue interface:%p result:%s", mSimpleBufferQueueInterface, getSLErrStr(result));
+ LOGE("get buffer queue interface:%p result:%s",
+ mSimpleBufferQueueInterface,
+ getSLErrStr(result));
} else {
// Register the BufferQueue callback
result = (*mSimpleBufferQueueInterface)->RegisterCallback(mSimpleBufferQueueInterface,
diff --git a/src/opensles/AudioStreamOpenSLES.h b/src/opensles/AudioStreamOpenSLES.h
index 1688c1e..da4a9ad 100644
--- a/src/opensles/AudioStreamOpenSLES.h
+++ b/src/opensles/AudioStreamOpenSLES.h
@@ -26,10 +26,7 @@
namespace oboe {
-#define OBOE_BITS_PER_BYTE 8 // common value TODO modernize
-
-
-
+constexpr int kBitsPerByte = 8;
/**
* INTERNAL USE ONLY
@@ -51,7 +48,6 @@
virtual Result open() override;
virtual Result close() override;
-
/**
* Query the current state, eg. OBOE_STREAM_STATE_PAUSING
*
@@ -61,8 +57,6 @@
int32_t getFramesPerBurst() override;
- static SLuint32 getDefaultByteOrder();
-
virtual int chanCountToChanMask(int chanCount) = 0;
/**
@@ -75,6 +69,8 @@
protected:
+ static SLuint32 getDefaultByteOrder();
+
SLresult registerBufferQueueCallback();
SLresult enqueueCallbackBuffer(SLAndroidSimpleBufferQueueItf bq);
diff --git a/src/opensles/EngineOpenSLES.cpp b/src/opensles/EngineOpenSLES.cpp
index cf87ecc..8df3fed 100644
--- a/src/opensles/EngineOpenSLES.cpp
+++ b/src/opensles/EngineOpenSLES.cpp
@@ -20,9 +20,9 @@
using namespace oboe;
-EngineOpenSLES *EngineOpenSLES::getInstance() {
+EngineOpenSLES &EngineOpenSLES::getInstance() {
static EngineOpenSLES sInstance;
- return &sInstance;
+ return sInstance;
}
SLresult EngineOpenSLES::open() {
diff --git a/src/opensles/EngineOpenSLES.h b/src/opensles/EngineOpenSLES.h
index 20564f9..7658c95 100644
--- a/src/opensles/EngineOpenSLES.h
+++ b/src/opensles/EngineOpenSLES.h
@@ -30,7 +30,7 @@
*/
class EngineOpenSLES {
public:
- static EngineOpenSLES *getInstance();
+ static EngineOpenSLES &getInstance();
SLresult open();
@@ -46,8 +46,15 @@
SLDataSink *audioSink);
private:
+ // Make this a safe Singleton
+ EngineOpenSLES()= default;
+ ~EngineOpenSLES()= default;
+ EngineOpenSLES(const EngineOpenSLES&)= delete;
+ EngineOpenSLES& operator=(const EngineOpenSLES&)= delete;
+
std::mutex mLock;
- std::atomic<int32_t> mOpenCount{0};
+ int32_t mOpenCount = 0;
+
SLObjectItf mEngineObject = 0;
SLEngineItf mEngineInterface;
};
diff --git a/src/opensles/OpenSLESUtilities.cpp b/src/opensles/OpenSLESUtilities.cpp
index c764af3..45f1c18 100644
--- a/src/opensles/OpenSLESUtilities.cpp
+++ b/src/opensles/OpenSLESUtilities.cpp
@@ -61,13 +61,15 @@
SLuint32 OpenSLES_ConvertFormatToRepresentation(AudioFormat format) {
switch(format) {
- case AudioFormat::Invalid:
- case AudioFormat::Unspecified:
- return 0;
case AudioFormat::I16:
return SL_ANDROID_PCM_REPRESENTATION_SIGNED_INT;
case AudioFormat::Float:
return SL_ANDROID_PCM_REPRESENTATION_FLOAT;
+ case AudioFormat::Invalid:
+ case AudioFormat::Unspecified:
+ default:
+ return 0;
}
}
+
} // namespace oboe
\ No newline at end of file
diff --git a/src/opensles/OutputMixerOpenSLES.cpp b/src/opensles/OutputMixerOpenSLES.cpp
index db36ec1..901e9a7 100644
--- a/src/opensles/OutputMixerOpenSLES.cpp
+++ b/src/opensles/OutputMixerOpenSLES.cpp
@@ -22,9 +22,9 @@
using namespace oboe;
-OutputMixerOpenSL *OutputMixerOpenSL::getInstance() {
+OutputMixerOpenSL &OutputMixerOpenSL::getInstance() {
static OutputMixerOpenSL sInstance;
- return &sInstance;
+ return sInstance;
}
SLresult OutputMixerOpenSL::open() {
@@ -33,7 +33,7 @@
SLresult result = SL_RESULT_SUCCESS;
if (mOpenCount++ == 0) {
// get the output mixer
- result = EngineOpenSLES::getInstance()->createOutputMix(&mOutputMixObject);
+ result = EngineOpenSLES::getInstance().createOutputMix(&mOutputMixObject);
if (SL_RESULT_SUCCESS != result) {
LOGE("OutputMixerOpenSL() - createOutputMix() result:%s", getSLErrStr(result));
goto error;
@@ -70,5 +70,5 @@
SLDataSource *audioSource) {
SLDataLocator_OutputMix loc_outmix = {SL_DATALOCATOR_OUTPUTMIX, mOutputMixObject};
SLDataSink audioSink = {&loc_outmix, NULL};
- return EngineOpenSLES::getInstance()->createAudioPlayer(objectItf, audioSource, &audioSink);
+ return EngineOpenSLES::getInstance().createAudioPlayer(objectItf, audioSource, &audioSink);
}
diff --git a/src/opensles/OutputMixerOpenSLES.h b/src/opensles/OutputMixerOpenSLES.h
index 5a414df..1fc77e5 100644
--- a/src/opensles/OutputMixerOpenSLES.h
+++ b/src/opensles/OutputMixerOpenSLES.h
@@ -31,7 +31,7 @@
class OutputMixerOpenSL {
public:
- static OutputMixerOpenSL *getInstance();
+ static OutputMixerOpenSL &getInstance();
SLresult open();
@@ -41,8 +41,15 @@
SLDataSource *audioSource);
private:
+ // Make this a safe Singleton
+ OutputMixerOpenSL()= default;
+ ~OutputMixerOpenSL()= default;
+ OutputMixerOpenSL(const OutputMixerOpenSL&)= delete;
+ OutputMixerOpenSL& operator=(const OutputMixerOpenSL&)= delete;
+
std::mutex mLock;
- std::atomic<int32_t> mOpenCount{0};
+ int32_t mOpenCount = 0;
+
SLObjectItf mOutputMixObject = 0;
};
