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 29

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

@@ -228,4 +228,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="true"
+        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;
+        }
+
+        /**
+         * Test programs that want to reproduce bugs in AAudio or OpenSL ES
+         * that have workarounds in Oboe, can disable them by setting this false.\\\\
+         * @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.
  *

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,71 @@
 
 #include <oboe/AudioStreamBuilder.h>
 #include <oboe/Oboe.h>
+
 #include "QuirksManager.h"
 
 using namespace oboe;
 
+int32_t QuirksManager::DeviceQuirks::clipBufferSize(AudioStream &stream,
+            int32_t requestedSize) {
+    // We are mainly concerned with Exclusive MMAP streams because they are using a timing
+    // model of the DSP to control IO instead of direct synchronization.
+    bool clippingEnabled = (OboeGlobals::areWorkaroundsEnabled()
+            && stream.getSharingMode() == SharingMode::Exclusive
+            && isMMapUsed(stream));
+    if (!clippingEnabled) {
+        return requestedSize;
+    }
+
+    int32_t burst = stream.getFramesPerBurst();
+    int32_t minSize = getBottomMarginInBursts() * burst;
+    int32_t adjustedSize = requestedSize;
+    if (adjustedSize < minSize ) {
+        adjustedSize = minSize;
+    } else {
+        int32_t maxSize = stream.getBufferCapacityInFrames() - (getTopMarginInBursts() * 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 getBottomMarginInBursts() const override {
+        // TODO Make this conditional on build version when MMAP timing improves.
+        return isExynos ? kBottomMarginExynos : kBottomMarginOther;
+    }
+
+    int32_t getTopMarginInBursts() 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 +115,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;
         }

src/common/QuirksManager.h

@@ -19,10 +19,13 @@
 
 #include <memory>
 #include <oboe/AudioStreamBuilder.h>
+#include <aaudio/AudioStreamAAudio.h>
 
 namespace oboe {
 
 /**
+ * INTERNAL USE ONLY.
+ *
  * Based on manufacturer, model and Android version number
  * decide whether data conversion needs to occur.
  *
@@ -33,19 +36,60 @@ class QuirksManager {
 public:
 
     static QuirksManager &getInstance() {
-        static QuirksManager instance;
+        static QuirksManager instance; // singleton
         return instance;
     }
 
+    QuirksManager();
+    virtual ~QuirksManager() = default;
+
     /**
+     * Do we need to do channel, format or rate conversion to provide a low latency
+     * stream for this builder? If so then provide a builder for the native child stream
+     * that will be used to get low latency.
      *
      * @param builder builder provided by application
      * @param childBuilder modified builder appropriate for the underlying device
      * @return true if conversion is needed
      */
     bool isConversionNeeded(const AudioStreamBuilder &builder, AudioStreamBuilder &childBuilder);
 
+    static bool isMMapUsed(AudioStream &stream) {
+        bool answer = false;
+        if (stream.getAudioApi() == AudioApi::AAudio) {
+            AudioStreamAAudio *streamAAudio =
+                    reinterpret_cast<AudioStreamAAudio *>(&stream);
+            answer = streamAAudio->isMMapUsed();
+        }
+        return answer;
+    }
+
+    virtual int32_t clipBufferSize(AudioStream &stream, int32_t bufferSize) {
+        return mDeviceQuirks->clipBufferSize(stream, bufferSize);
+    }
+
+    class DeviceQuirks {
+    public:
+        virtual ~DeviceQuirks() = default;
+
+        /**
+         * Restrict buffer size. This is mainly to avoid glitches caused by MMAP
+         * timestamp inaccuracies.
+         * @param stream
+         * @param requestedSize
+         * @return
+         */
+        int32_t clipBufferSize(AudioStream &stream, int32_t requestedSize);
+
+        virtual int32_t getBottomMarginInBursts() const { return 0; }
+
+        virtual int32_t getTopMarginInBursts() const { return 0; }
+    };
+
 private:
+
+    std::unique_ptr<DeviceQuirks> mDeviceQuirks{};
+
 };
 
 }