From 3ac63cc59d7976748c86de0d30487e2480a9b166 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 22 Feb 2022 13:45:02 -0500 Subject: [PATCH] Implement new feature flag strategy for AEC selection. --- .../webrtc/AudioProcessingMethodSelector.kt | 46 ++++++++++++++----- .../securesms/util/FeatureFlags.java | 41 ++++++++++++----- ...rocessingMethodSelectorTest_modelInList.kt | 39 ++++++++++++++++ 3 files changed, 103 insertions(+), 23 deletions(-) create mode 100644 app/src/test/java/org/thoughtcrime/securesms/service/webrtc/AudioProcessingMethodSelectorTest_modelInList.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/service/webrtc/AudioProcessingMethodSelector.kt b/app/src/main/java/org/thoughtcrime/securesms/service/webrtc/AudioProcessingMethodSelector.kt index 3ba4207f87..9e15812b4e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/service/webrtc/AudioProcessingMethodSelector.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/service/webrtc/AudioProcessingMethodSelector.kt @@ -1,6 +1,7 @@ package org.thoughtcrime.securesms.service.webrtc import android.os.Build +import androidx.annotation.VisibleForTesting import org.signal.ringrtc.CallManager.AudioProcessingMethod import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.util.FeatureFlags @@ -10,24 +11,47 @@ import org.thoughtcrime.securesms.util.FeatureFlags */ object AudioProcessingMethodSelector { - private val hardwareModels: Set by lazy { - FeatureFlags.hardwareAecModels() - .split(",") - .map { it.trim() } - .filter { it.isNotEmpty() } - .toSet() - } - @JvmStatic fun get(): AudioProcessingMethod { if (SignalStore.internalValues().audioProcessingMethod() != AudioProcessingMethod.Default) { return SignalStore.internalValues().audioProcessingMethod() } + val useAec3: Boolean = FeatureFlags.useAec3() + return when { - FeatureFlags.forceDefaultAec() -> AudioProcessingMethod.Default - hardwareModels.contains(Build.MODEL) -> AudioProcessingMethod.ForceHardware - else -> AudioProcessingMethod.ForceSoftwareAecM + isHardwareBlocklisted() && useAec3 -> AudioProcessingMethod.ForceSoftwareAec3 + isHardwareBlocklisted() -> AudioProcessingMethod.ForceSoftwareAecM + isSoftwareBlocklisted() -> AudioProcessingMethod.ForceHardware + Build.VERSION.SDK_INT < 29 && FeatureFlags.useHardwareAecIfOlderThanApi29() -> AudioProcessingMethod.ForceHardware + Build.VERSION.SDK_INT < 29 && useAec3 -> AudioProcessingMethod.ForceSoftwareAec3 + Build.VERSION.SDK_INT < 29 -> AudioProcessingMethod.ForceSoftwareAecM + else -> AudioProcessingMethod.ForceHardware } } + + private fun isHardwareBlocklisted(): Boolean { + return modelInList(Build.MODEL, FeatureFlags.hardwareAecBlocklistModels()) + } + + private fun isSoftwareBlocklisted(): Boolean { + return modelInList(Build.MODEL, FeatureFlags.softwareAecBlocklistModels()) + } + + @VisibleForTesting + fun modelInList(model: String, serializedList: String): Boolean { + val items: List = serializedList + .split(",") + .map { it.trim() } + .filter { it.isNotEmpty() } + .toList() + + val exactMatches = items.filter { it.last() != '*' } + val prefixMatches = items.filter { it.last() == '*' } + + return exactMatches.contains(model) || + prefixMatches + .map { it.substring(0, it.length - 1) } + .any { model.startsWith(it) } + } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java index 1cb32e07bd..77261c9c8f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java @@ -88,11 +88,13 @@ public final class FeatureFlags { private static final String DONOR_BADGES = "android.donorBadges.6"; private static final String DONOR_BADGES_DISPLAY = "android.donorBadges.display.4"; private static final String CDSH = "android.cdsh"; - private static final String HARDWARE_AEC_MODELS = "android.calling.hardwareAecModels"; - private static final String FORCE_DEFAULT_AEC = "android.calling.forceDefaultAec"; private static final String STORIES = "android.stories"; private static final String STORIES_TEXT_FUNCTIONS = "android.stories.text.functions"; private static final String STORIES_TEXT_POSTS = "android.stories.text.posts"; + private static final String HARDWARE_AEC_BLOCKLIST_MODELS = "android.calling.hardwareAecBlockList"; + private static final String SOFTWARE_AEC_BLOCKLIST_MODELS = "android.calling.softwareAecBlockList"; + private static final String USE_HARDWARE_AEC_IF_OLD = "android.calling.useHardwareAecIfOlderThanApi29"; + private static final String USE_AEC3 = "android.calling.useAec3"; /** * We will only store remote values for flags in this set. If you want a flag to be controllable @@ -133,11 +135,13 @@ public final class FeatureFlags { SENDER_KEY_MAX_AGE, DONOR_BADGES, DONOR_BADGES_DISPLAY, - HARDWARE_AEC_MODELS, - FORCE_DEFAULT_AEC, STORIES, STORIES_TEXT_FUNCTIONS, - STORIES_TEXT_POSTS + STORIES_TEXT_POSTS, + HARDWARE_AEC_BLOCKLIST_MODELS, + SOFTWARE_AEC_BLOCKLIST_MODELS, + USE_HARDWARE_AEC_IF_OLD, + USE_AEC3 ); @VisibleForTesting @@ -192,7 +196,10 @@ public final class FeatureFlags { SENDER_KEY_MAX_AGE, DONOR_BADGES_DISPLAY, DONATE_MEGAPHONE, - FORCE_DEFAULT_AEC + HARDWARE_AEC_BLOCKLIST_MODELS, + SOFTWARE_AEC_BLOCKLIST_MODELS, + USE_HARDWARE_AEC_IF_OLD, + USE_AEC3 ); /** @@ -454,14 +461,24 @@ public final class FeatureFlags { return Environment.IS_STAGING && getBoolean(CDSH, false); } - /** A comma-separated list of models that should use hardware AEC for calling. */ - public static @NonNull String hardwareAecModels() { - return getString(HARDWARE_AEC_MODELS, ""); + /** A comma-separated list of models that should *not* use hardware AEC for calling. */ + public static @NonNull String hardwareAecBlocklistModels() { + return getString(HARDWARE_AEC_BLOCKLIST_MODELS, ""); } - /** Whether or not all devices should be forced into using default AEC for calling. */ - public static boolean forceDefaultAec() { - return getBoolean(FORCE_DEFAULT_AEC, false); + /** A comma-separated list of models that should *not* use software AEC for calling. */ + public static @NonNull String softwareAecBlocklistModels() { + return getString(SOFTWARE_AEC_BLOCKLIST_MODELS, ""); + } + + /** Whether or not hardware AEC should be used for calling on devices older than API 29. */ + public static boolean useHardwareAecIfOlderThanApi29() { + return getBoolean(USE_HARDWARE_AEC_IF_OLD, false); + } + + /** Whether or not {@link org.signal.ringrtc.CallManager.AudioProcessingMethod#ForceSoftwareAec3} can be used */ + public static boolean useAec3() { + return getBoolean(USE_AEC3, true); } /** Only for rendering debug info. */ diff --git a/app/src/test/java/org/thoughtcrime/securesms/service/webrtc/AudioProcessingMethodSelectorTest_modelInList.kt b/app/src/test/java/org/thoughtcrime/securesms/service/webrtc/AudioProcessingMethodSelectorTest_modelInList.kt new file mode 100644 index 0000000000..70c16feef2 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/service/webrtc/AudioProcessingMethodSelectorTest_modelInList.kt @@ -0,0 +1,39 @@ +package org.thoughtcrime.securesms.service.webrtc + +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized + +@RunWith(Parameterized::class) +class AudioProcessingMethodSelectorTest_modelInList( + private val model: String, + private val serializedList: String, + private val expected: Boolean +) { + + @Test + fun testModelInList() { + val actual = AudioProcessingMethodSelector.modelInList(model, serializedList) + assertEquals(expected, actual) + } + + companion object { + @JvmStatic + @Parameterized.Parameters(name = "{index}: modelInList(model={0}, list={1})={2}") + fun data(): List> { + return listOf>( + arrayOf("a", "a", true), + arrayOf("a", "a,b", true), + arrayOf("a", "c,a,b", true), + arrayOf("ab", "a*", true), + arrayOf("ab", "c,a*,b", true), + arrayOf("abc", "c,ab*,b", true), + + arrayOf("a", "b", false), + arrayOf("a", "abc", false), + arrayOf("b", "a*", false), + ).toList() + } + } +}