Oboe: Add device specific Quirks for Samsung

apps/OboeTester/app/build.gradle

@@ -4,7 +4,7 @@ android {
     compileSdkVersion 28
     defaultConfig {
         applicationId = "com.google.sample.oboe.manualtest"
-        minSdkVersion 25
+        minSdkVersion 23
         targetSdkVersion 28
         // Also update the version in the AndroidManifest.xml file.
         versionCode 30

apps/OboeTester/app/src/main/cpp/jni-bridge.cpp

@@ -97,6 +97,12 @@ Java_com_google_sample_oboe_manualtest_NativeEngine_isMMapExclusiveSupported(JNI
     return AAudioExtensions::getInstance().isMMapExclusiveSupported();
 }
 
+JNIEXPORT void JNICALL
+Java_com_google_sample_oboe_manualtest_NativeEngine_setWorkaroundsEnabled(JNIEnv *env, jclass type,
+                                                                          jboolean enabled) {
+    oboe::OboeGlobals::setWorkaroundsEnabled(enabled);
+}
+
 JNIEXPORT jint JNICALL
 Java_com_google_sample_oboe_manualtest_OboeAudioStream_openNative(
         JNIEnv *env, jobject synth,

apps/OboeTester/app/src/main/java/com/google/sample/oboe/manualtest/MainActivity.java

@@ -100,6 +100,7 @@ public void onNothingSelected(AdapterView<?> adapterView) {
         mBuildTextView = (TextView) findViewById(R.id.text_build_info);
         mBuildTextView.setText(Build.DISPLAY);
 
+
         saveIntentBundleForLaterProcessing(getIntent());
     }
 
@@ -145,6 +146,7 @@ private void processBundleFromIntent() {
     @Override
     public void onResume(){
         super.onResume();
+        NativeEngine.setWorkaroundsEnabled(false);
         processBundleFromIntent();
     }
 
@@ -250,4 +252,9 @@ public void onSetSpeakerphoneOn(View view) {
         myAudioMgr.setSpeakerphoneOn(enabled);
     }
 
+    public void onEnableWorkarounds(View view) {
+        CheckBox checkBox = (CheckBox) view;
+        boolean enabled = checkBox.isChecked();
+        NativeEngine.setWorkaroundsEnabled(enabled);
+    }
 }

apps/OboeTester/app/src/main/java/com/google/sample/oboe/manualtest/NativeEngine.java

@@ -5,4 +5,6 @@ public class NativeEngine {
     static native boolean isMMapSupported();
 
     static native boolean isMMapExclusiveSupported();
+
+    static native void setWorkaroundsEnabled(boolean enabled);
 }

apps/OboeTester/app/src/main/res/layout/activity_main.xml

@@ -154,14 +154,25 @@
         app:layout_constraintStart_toStartOf="@+id/useCallback"
         app:layout_constraintTop_toBottomOf="@+id/useCallback" />
 
+    <CheckBox
+        android:id="@+id/boxEnableWorkarounds"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:layout_marginTop="6dp"
+        android:checked="false"
+        android:onClick="onEnableWorkarounds"
+        android:text="enable Oboe workarounds"
+        app:layout_constraintStart_toStartOf="@+id/setSpeakerphoneOn"
+        app:layout_constraintTop_toBottomOf="@+id/setSpeakerphoneOn" />
+
 
     <TextView
         android:id="@+id/textView2"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:text="Mode:"
         app:layout_constraintBaseline_toBaselineOf="@+id/spinnerAudioMode"
-        app:layout_constraintStart_toStartOf="@+id/setSpeakerphoneOn" />
+        app:layout_constraintStart_toStartOf="@+id/boxEnableWorkarounds" />
 
     <Spinner
         android:id="@+id/spinnerAudioMode"
@@ -170,7 +181,7 @@
         android:entries="@array/audio_modes"
         android:prompt="@string/audio_mode_prompt"
         app:layout_constraintStart_toEndOf="@+id/textView2"
-        app:layout_constraintTop_toBottomOf="@+id/setSpeakerphoneOn" />
+        app:layout_constraintTop_toBottomOf="@+id/boxEnableWorkarounds" />
 
 
     <TextView

include/oboe/Definitions.h

@@ -495,6 +495,25 @@ namespace oboe {
         int64_t timestamp; // in nanoseconds
     };
 
+    class OboeGlobals {
+    public:
+
+        static bool areWorkaroundsEnabled() {
+            return mWorkaroundsEnabled;
+        }
+
+        /**
+         * Disable this when writing tests to reproduce bugs in AAudio or OpenSL ES
+         * that have workarounds in Oboe.
+         * @param enabled
+         */
+        static void setWorkaroundsEnabled(bool enabled) {
+            mWorkaroundsEnabled = enabled;
+        }
+
+    private:
+        static bool mWorkaroundsEnabled;
+    };
 } // namespace oboe
 
 #endif // OBOE_DEFINITIONS_H

include/oboe/Utilities.h

@@ -19,6 +19,7 @@
 
 #include <unistd.h>
 #include <sys/types.h>
+#include <string>
 #include "oboe/Definitions.h"
 
 namespace oboe {
@@ -58,6 +59,19 @@ int32_t convertFormatToSizeInBytes(AudioFormat format);
 template <typename FromType>
 const char * convertToText(FromType input);
 
+/**
+ * @param name
+ * @return the value of a named system property in a string or empty string
+ */
+std::string getPropertyString(const char * name);
+
+/**
+ * @param name
+ * @param defaultValue
+ * @return integer value associated with a property or the default value
+ */
+int getPropertyInteger(const char * name, int defaultValue);
+
 /**
  * Return the version of the SDK that is currently running.
  *

include/oboe/Version.h

@@ -37,7 +37,7 @@
 #define OBOE_VERSION_MINOR 3
 
 // Type: 16-bit unsigned int. Min value: 0 Max value: 65535. See below for description.
-#define OBOE_VERSION_PATCH 2
+#define OBOE_VERSION_PATCH 3
 
 #define OBOE_STRINGIFY(x) #x
 #define OBOE_TOSTRING(x) OBOE_STRINGIFY(x)

src/aaudio/AudioStreamAAudio.cpp

@@ -26,6 +26,8 @@
 
 #ifdef __ANDROID__
 #include <sys/system_properties.h>
+#include <common/QuirksManager.h>
+
 #endif
 
 #ifndef OBOE_FIX_FORCE_STARTING_TO_STARTED
@@ -161,7 +163,8 @@ Result AudioStreamAAudio::open() {
     // does not increase latency.
     int32_t capacity = mBufferCapacityInFrames;
     constexpr int kCapacityRequiredForFastLegacyTrack = 4096; // matches value in AudioFinger
-    if (mDirection == oboe::Direction::Input
+    if (OboeGlobals::areWorkaroundsEnabled()
+            && mDirection == oboe::Direction::Input
             && capacity != oboe::Unspecified
             && capacity < kCapacityRequiredForFastLegacyTrack
             && mPerformanceMode == oboe::PerformanceMode::LowLatency) {
@@ -445,7 +448,8 @@ Result AudioStreamAAudio::waitForStateChange(StreamState currentState,
             break;
         }
 #if OBOE_FIX_FORCE_STARTING_TO_STARTED
-        if (aaudioNextState == static_cast<aaudio_stream_state_t >(StreamState::Starting)) {
+        if (OboeGlobals::areWorkaroundsEnabled()
+            && aaudioNextState == static_cast<aaudio_stream_state_t >(StreamState::Starting)) {
             aaudioNextState = static_cast<aaudio_stream_state_t >(StreamState::Started);
         }
 #endif // OBOE_FIX_FORCE_STARTING_TO_STARTED
@@ -481,11 +485,13 @@ ResultWithValue<int32_t> AudioStreamAAudio::setBufferSizeInFrames(int32_t reques
     AAudioStream *stream = mAAudioStream.load();
 
     if (stream != nullptr) {
-
-        if (requestedFrames > mBufferCapacityInFrames) {
-            requestedFrames = mBufferCapacityInFrames;
+        int32_t adjustedFrames = requestedFrames;
+        if (adjustedFrames > mBufferCapacityInFrames) {
+            adjustedFrames = mBufferCapacityInFrames;
         }
-        int32_t newBufferSize = mLibLoader->stream_setBufferSize(mAAudioStream, requestedFrames);
+        adjustedFrames = QuirksManager::getInstance().clipBufferSize(*this, adjustedFrames);
+
+        int32_t newBufferSize = mLibLoader->stream_setBufferSize(mAAudioStream, adjustedFrames);
 
         // Cache the result if it's valid
         if (newBufferSize > 0) mBufferSizeInFrames = newBufferSize;
@@ -502,7 +508,8 @@ StreamState AudioStreamAAudio::getState() const {
     if (stream != nullptr) {
         aaudio_stream_state_t aaudioState = mLibLoader->stream_getState(stream);
 #if OBOE_FIX_FORCE_STARTING_TO_STARTED
-        if (aaudioState == AAUDIO_STREAM_STATE_STARTING) {
+        if (OboeGlobals::areWorkaroundsEnabled()
+            && aaudioState == AAUDIO_STREAM_STATE_STARTING) {
             aaudioState = AAUDIO_STREAM_STATE_STARTED;
         }
 #endif // OBOE_FIX_FORCE_STARTING_TO_STARTED

src/aaudio/AudioStreamAAudio.h

@@ -91,6 +91,8 @@ class AudioStreamAAudio : public AudioStream {
                                                    void *audioData,
                                                    int32_t numFrames);
 
+    bool                 isMMapUsed();
+
 protected:
     static void internalErrorCallback(
             AAudioStream *stream,
@@ -108,8 +110,6 @@ class AudioStreamAAudio : public AudioStream {
 
 private:
 
-    bool                 isMMapUsed();
-
     std::atomic<bool>    mCallbackThreadEnabled;
 
     // pointer to the underlying AAudio stream, valid if open, null if closed

src/common/AudioStreamBuilder.cpp

@@ -26,6 +26,8 @@
 #include "opensles/AudioStreamOpenSLES.h"
 #include "QuirksManager.h"
 
+bool oboe::OboeGlobals::mWorkaroundsEnabled = true;
+
 namespace oboe {
 
 /**

src/common/QuirksManager.cpp

@@ -16,10 +16,76 @@
 
 #include <oboe/AudioStreamBuilder.h>
 #include <oboe/Oboe.h>
+
 #include "QuirksManager.h"
 
 using namespace oboe;
 
+int32_t QuirksManager::DeviceQuirks::clipBufferSize(AudioStream &stream,
+            int32_t requestedSize) {
+    if (!OboeGlobals::areWorkaroundsEnabled()) {
+        return requestedSize;
+    }
+    int bottomMargin = kDefaultBottomMarginInBursts;
+    int topMargin = kDefaultTopMarginInBursts;
+    if (isMMapUsed(stream)) {
+        if (stream.getSharingMode() == SharingMode::Exclusive) {
+            bottomMargin = getExclusiveBottomMarginInBursts();
+            topMargin = getExclusiveTopMarginInBursts();
+        }
+    } else {
+        bottomMargin = kLegacyBottomMarginInBursts;
+    }
+
+    int32_t burst = stream.getFramesPerBurst();
+    int32_t minSize = bottomMargin * burst;
+    int32_t adjustedSize = requestedSize;
+    if (adjustedSize < minSize ) {
+        adjustedSize = minSize;
+    } else {
+        int32_t maxSize = stream.getBufferCapacityInFrames() - (topMargin * burst);
+        if (adjustedSize > maxSize ) {
+            adjustedSize = maxSize;
+        }
+    }
+    return adjustedSize;
+}
+
+class SamsungDeviceQuirks : public  QuirksManager::DeviceQuirks {
+public:
+    SamsungDeviceQuirks() {
+        std::string arch = getPropertyString("ro.arch");
+        isExynos = (arch.rfind("exynos", 0) == 0); // starts with?
+    }
+
+    virtual ~SamsungDeviceQuirks() = default;
+
+    int32_t getExclusiveBottomMarginInBursts() const override {
+        // TODO Make this conditional on build version when MMAP timing improves.
+        return isExynos ? kBottomMarginExynos : kBottomMarginOther;
+    }
+
+    int32_t getExclusiveTopMarginInBursts() const override {
+        return kTopMargin;
+    }
+
+private:
+    // Stay farther away from DSP position on Exynos devices.
+    static constexpr int32_t kBottomMarginExynos = 2;
+    static constexpr int32_t kBottomMarginOther = 1;
+    static constexpr int32_t kTopMargin = 1;
+    bool isExynos = false;
+};
+
+QuirksManager::QuirksManager() {
+    std::string manufacturer = getPropertyString("ro.product.manufacturer");
+    if (manufacturer == "samsung") {
+        mDeviceQuirks = std::make_unique<SamsungDeviceQuirks>();
+    } else {
+        mDeviceQuirks = std::make_unique<DeviceQuirks>();
+    }
+}
+
 bool QuirksManager::isConversionNeeded(
         const AudioStreamBuilder &builder,
         AudioStreamBuilder &childBuilder) {
@@ -54,11 +120,13 @@ bool QuirksManager::isConversionNeeded(
     // Channel Count
     if (builder.getChannelCount() != oboe::Unspecified
             && builder.isChannelConversionAllowed()) {
-        if (builder.getChannelCount() == 2 // stereo?
+        if (OboeGlobals::areWorkaroundsEnabled()
+                && builder.getChannelCount() == 2 // stereo?
                 && isInput
                 && isLowLatency
                 && (!builder.willUseAAudio() && (getSdkVersion() == __ANDROID_API_O__))) {
-            // workaround for temporary heap size regression, b/66967812
+            // Workaround for heap size regression in O.
+            // b/66967812 AudioRecord does not allow FAST track for stereo capture in O
             childBuilder.setChannelCount(1);
             conversionNeeded = true;
         }