Audio ducking now supported during external notifications (#1009)

Co-authored-by: Matt Chowning <[email protected]>
Automattic · Jul 24, 2023 · 814ca7e · 814ca7e
1 parent a89c30f
commit 814ca7e
Show file tree

Hide file tree

Showing 11 changed files with 119 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,10 @@
+7.45
+-----
+*   Bug Fixes:
+    *   Added audio ducking as an option when playing over notifications
+        ([#1009](https://github.com/Automattic/pocket-casts-android/pull/1009))
+
+
 7.44
 -----
 *   New Feature:

diff --git a/...gs/src/main/java/au/com/shiftyjelly/pocketcasts/settings/NotificationsSettingsFragment.kt b/...gs/src/main/java/au/com/shiftyjelly/pocketcasts/settings/NotificationsSettingsFragment.kt
@@ -16,6 +16,7 @@ import androidx.preference.PreferenceScreen
 import androidx.preference.SwitchPreference
 import au.com.shiftyjelly.pocketcasts.analytics.AnalyticsEvent
 import au.com.shiftyjelly.pocketcasts.analytics.AnalyticsTrackerWrapper
+import au.com.shiftyjelly.pocketcasts.preferences.PlayOverNotificationSetting
 import au.com.shiftyjelly.pocketcasts.preferences.Settings
 import au.com.shiftyjelly.pocketcasts.repositories.notification.NewEpisodeNotificationAction
 import au.com.shiftyjelly.pocketcasts.repositories.notification.NotificationHelper
@@ -71,6 +72,7 @@ class NotificationsSettingsFragment :
     private var enabledPreference: SwitchPreference? = null
     private var systemSettingsPreference: Preference? = null
     private var notificationActions: PreferenceScreen? = null
+    private var playOverNotificationPreference: ListPreference? = null
 
     private val toolbar
         get() = view?.findViewById<Toolbar>(R.id.toolbar)
@@ -95,6 +97,7 @@ class NotificationsSettingsFragment :
         vibratePreference = manager.findPreference("notificationVibrate")
         notificationActions = manager.findPreference("notificationActions")
         systemSettingsPreference = manager.findPreference("openSystemSettings")
+        playOverNotificationPreference = manager.findPreference("overrideNotificationAudio")
 
         // turn preferences off by default, because they are enable async, we don't want this view to remove them from the screen after it loads as it looks jarring
         enabledPreferences(false)
@@ -111,13 +114,6 @@ class NotificationsSettingsFragment :
         updateNotificationsEnabled()
 
         manager.run {
-            findPreference<SwitchPreference>(Settings.PREFERENCE_OVERRIDE_AUDIO)?.setOnPreferenceChangeListener { _, newValue ->
-                analyticsTracker.track(
-                    AnalyticsEvent.SETTINGS_NOTIFICATIONS_PLAY_OVER_NOTIFICATIONS_TOGGLED,
-                    mapOf("enabled" to newValue as Boolean)
-                )
-                true
-            }
             findPreference<SwitchPreference>(Settings.PREFERENCE_HIDE_NOTIFICATION_ON_PAUSE)?.setOnPreferenceChangeListener { _, newValue ->
                 analyticsTracker.track(
                     AnalyticsEvent.SETTINGS_NOTIFICATIONS_HIDE_PLAYBACK_NOTIFICATION_ON_PAUSE,
@@ -140,6 +136,21 @@ class NotificationsSettingsFragment :
             )
             true
         }
+        playOverNotificationPreference?.setOnPreferenceChangeListener { _, newValue ->
+            val playOverNotificationSetting = (newValue as? String)
+                ?.let { PlayOverNotificationSetting.fromPreferenceString(it) }
+                ?: throw IllegalStateException("Invalid value for play over notification preference: $newValue")
+
+            analyticsTracker.track(
+                AnalyticsEvent.SETTINGS_NOTIFICATIONS_PLAY_OVER_NOTIFICATIONS_TOGGLED,
+                mapOf(
+                    "enabled" to (playOverNotificationSetting != PlayOverNotificationSetting.NEVER),
+                    "value" to playOverNotificationSetting.analyticsString,
+                ),
+            )
+
+            true
+        }
     }
 
     private fun updateNotificationsEnabled() {
@@ -386,6 +397,7 @@ class NotificationsSettingsFragment :
     override fun onResume() {
         super.onResume()
         setupNotificationVibrate()
+        setupPlayOverNotifications()
         changePodcastsSummary()
         preferenceScreen.sharedPreferences?.registerOnSharedPreferenceChangeListener(this)
     }
@@ -398,6 +410,8 @@ class NotificationsSettingsFragment :
     override fun onSharedPreferenceChanged(sharedPreferences: SharedPreferences, key: String) {
         if (Settings.PREFERENCE_NOTIFICATION_VIBRATE == key) {
             changeVibrateSummary()
+        } else if (Settings.PREFERENCE_OVERRIDE_NOTIFICATION_AUDIO == key) {
+            changePlayOverNotificationSummary()
         }
     }
 
@@ -452,6 +466,24 @@ class NotificationsSettingsFragment :
         }
     }
 
+    private fun setupPlayOverNotifications() {
+        playOverNotificationPreference?.apply {
+            val options = listOf(
+                PlayOverNotificationSetting.NEVER,
+                PlayOverNotificationSetting.DUCK,
+                PlayOverNotificationSetting.ALWAYS,
+            )
+            entries = options.map { getString(it.titleRes) }.toTypedArray()
+            entryValues = options.map { it.preferenceInt.toString() }.toTypedArray()
+            value = settings.getPlayOverNotification().preferenceInt.toString()
+        }
+        changePlayOverNotificationSummary()
+    }
+
+    private fun changePlayOverNotificationSummary() {
+        playOverNotificationPreference?.summary = getString(settings.getPlayOverNotification().titleRes)
+    }
+
     override fun getBackstackCount(): Int {
         return childFragmentManager.backStackEntryCount
     }

diff --git a/modules/features/settings/src/main/res/xml/preferences_notifications.xml b/modules/features/settings/src/main/res/xml/preferences_notifications.xml
@@ -42,12 +42,11 @@
     </PreferenceCategory>
 
     <PreferenceCategory android:title="@string/settings" >
-        <SwitchPreference
-            android:defaultValue="false"
-            android:key="overrideAudioInterruption"
-            android:persistent="true"
-            android:summary="@string/settings_notification_play_over_summary"
-            android:title="@string/settings_notification_play_over" />
+        <ListPreference
+            android:key="overrideNotificationAudio"
+            android:title="@string/settings_notification_play_over"
+            android:persistent="true"/>
+
         <SwitchPreference
             android:key="hideNotificationOnPause"
             android:persistent="true"

diff --git a/modules/services/localization/src/main/res/values/strings.xml b/modules/services/localization/src/main/res/values/strings.xml
@@ -1136,7 +1136,9 @@
     <string name="settings_notification_hide_on_pause">Hide playback notification on pause</string>
     <string name="settings_notification_notify_me">Notify me</string>
     <string name="settings_notification_play_over">Play over notifications</string>
-    <string name="settings_notification_play_over_summary">Keep playing even when other apps, like notifications or navigation, play sounds.</string>
+    <string name="settings_notification_play_over_never">Never</string>
+    <string name="settings_notification_play_over_duck">Duck volume</string>
+    <string name="settings_notification_play_over_always">Always</string>
     <string name="settings_notification_sound">Notification sound</string>
     <string name="settings_notification_sound_summary">Notification sound played.</string>
     <string name="settings_notification_actions_title">Select 3 actions</string>

diff --git a/...s/src/main/java/au/com/shiftyjelly/pocketcasts/preferences/PlayOverNotificationSetting.kt b/...s/src/main/java/au/com/shiftyjelly/pocketcasts/preferences/PlayOverNotificationSetting.kt
@@ -0,0 +1,37 @@
+package au.com.shiftyjelly.pocketcasts.preferences
+
+import androidx.annotation.StringRes
+import au.com.shiftyjelly.pocketcasts.localization.R as LR
+
+enum class PlayOverNotificationSetting(
+    val preferenceInt: Int,
+    @StringRes val titleRes: Int,
+    val analyticsString: String,
+) {
+    NEVER(
+        titleRes = LR.string.settings_notification_play_over_never,
+        preferenceInt = 2,
+        analyticsString = "never"
+    ),
+    DUCK(
+        titleRes = LR.string.settings_notification_play_over_duck,
+        preferenceInt = 1,
+        analyticsString = "duck"
+    ),
+    ALWAYS(
+        titleRes = LR.string.settings_notification_play_over_always,
+        preferenceInt = 0,
+        analyticsString = "always"
+    );
+
+    companion object {
+        fun fromPreferenceString(stringValue: String): PlayOverNotificationSetting {
+            try {
+                val intValue = stringValue.toInt()
+                return values().first { it.preferenceInt == intValue }
+            } catch (e: Exception) {
+                throw IllegalStateException("Unknown play over notification setting: $stringValue")
+            }
+        }
+    }
+}
diff --git a/...services/preferences/src/main/java/au/com/shiftyjelly/pocketcasts/preferences/Settings.kt b/...services/preferences/src/main/java/au/com/shiftyjelly/pocketcasts/preferences/Settings.kt
@@ -83,7 +83,8 @@ interface Settings {
         const val PREFERENCE_SELECT_PODCAST_LIBRARY_SORT = "selectPodcastLibrarySort"
         const val PREFERENCE_WARN_WHEN_NOT_ON_WIFI = "warnWhenNotOnWifi"
         const val PREFERENCE_SYNC_ON_METERED = "SyncWhenOnMetered"
-        const val PREFERENCE_OVERRIDE_AUDIO = "overrideAudioInterruption"
+        const val PREFERENCE_OVERRIDE_AUDIO_LEGACY = "overrideAudioInterruption"
+        const val PREFERENCE_OVERRIDE_NOTIFICATION_AUDIO = "overrideNotificationAudio"
         const val PREFERENCE_USE_EMBEDDED_ARTWORK = "useEmbeddedArtwork"
         const val PREFERENCE_LAST_MODIFIED = "lastModified"
         const val PREFERENCE_FIRST_SYNC_RUN = "firstSyncRun"
@@ -396,7 +397,7 @@ interface Settings {
 
     fun setPopularPodcastCountryCode(code: String)
 
-    fun canDuckAudioWithNotifications(): Boolean
+    fun getPlayOverNotification(): PlayOverNotificationSetting
 
     fun hasBlockAlreadyRun(label: String): Boolean
     fun setBlockAlreadyRun(label: String, hasRun: Boolean)

diff --git a/...ices/preferences/src/main/java/au/com/shiftyjelly/pocketcasts/preferences/SettingsImpl.kt b/...ices/preferences/src/main/java/au/com/shiftyjelly/pocketcasts/preferences/SettingsImpl.kt
@@ -504,8 +504,17 @@ class SettingsImpl @Inject constructor(
         return getBoolean(Settings.PREFERENCE_AUTO_SHOW_PLAYED, false)
     }
 
-    override fun canDuckAudioWithNotifications(): Boolean {
-        return sharedPreferences.getBoolean(Settings.PREFERENCE_OVERRIDE_AUDIO, false)
+    override fun getPlayOverNotification(): PlayOverNotificationSetting {
+        val value = sharedPreferences.getString(Settings.PREFERENCE_OVERRIDE_NOTIFICATION_AUDIO, null) ?: legacyPlayOverNotification()
+        return PlayOverNotificationSetting.fromPreferenceString(value)
+    }
+
+    private fun legacyPlayOverNotification(): String {
+        if (sharedPreferences.getBoolean(Settings.PREFERENCE_OVERRIDE_AUDIO_LEGACY, false)) {
+            return PlayOverNotificationSetting.ALWAYS.preferenceInt.toString()
+        }
+
+        return PlayOverNotificationSetting.NEVER.preferenceInt.toString()
     }
 
     override fun hasBlockAlreadyRun(label: String): Boolean {

diff --git a/...tories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/FocusManager.kt b/...tories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/FocusManager.kt
@@ -7,6 +7,7 @@ import android.media.AudioManager
 import androidx.media.AudioAttributesCompat
 import androidx.media.AudioFocusRequestCompat
 import androidx.media.AudioManagerCompat
+import au.com.shiftyjelly.pocketcasts.preferences.PlayOverNotificationSetting
 import au.com.shiftyjelly.pocketcasts.preferences.Settings
 import au.com.shiftyjelly.pocketcasts.utils.log.LogBuffer
 import timber.log.Timber
@@ -150,17 +151,14 @@ open class FocusManager(private val settings: Settings, context: Context?) : Aud
         }
     }
 
-    fun canDuck(): Boolean {
-        return audioFocus == AUDIO_NO_FOCUS_CAN_DUCK_TRANSIENT && hasUserAllowedDucking()
-    }
-
-    protected open fun hasUserAllowedDucking(): Boolean {
-        return settings.canDuckAudioWithNotifications()
+    private fun canDuck(): PlayOverNotificationSetting {
+        if (audioFocus != AUDIO_NO_FOCUS_CAN_DUCK_TRANSIENT) return PlayOverNotificationSetting.NEVER
+        return settings.getPlayOverNotification()
     }
 
     interface FocusChangeListener {
         fun onFocusGain(shouldResume: Boolean)
-        fun onFocusLoss(mayDuck: Boolean, transientLoss: Boolean)
+        fun onFocusLoss(playOverNotification: PlayOverNotificationSetting, transientLoss: Boolean)
         fun onFocusRequestFailed()
     }
 

diff --git a/...itories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/LocalPlayer.kt b/...itories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/LocalPlayer.kt
@@ -14,7 +14,7 @@ abstract class LocalPlayer(override val onPlayerEvent: (Player, PlayerEvent) ->
 
     companion object {
         // The volume we set the media player to seekToTimeMswhen we lose audio focus, but are allowed to reduce the volume instead of stopping playback.
-        const val VOLUME_DUCK = 1.0f // We don't actually duck the volume
+        const val VOLUME_DUCK = 0.5f
         // The volume we set the media player when we have audio focus.
         const val VOLUME_NORMAL = 1.0f
     }

diff --git a/...ies/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/PlaybackManager.kt b/...ies/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/PlaybackManager.kt
@@ -30,6 +30,7 @@ import au.com.shiftyjelly.pocketcasts.models.to.PodcastGrouping
 import au.com.shiftyjelly.pocketcasts.models.type.EpisodePlayingStatus
 import au.com.shiftyjelly.pocketcasts.models.type.EpisodeStatusEnum
 import au.com.shiftyjelly.pocketcasts.models.type.UserEpisodeServerStatus
+import au.com.shiftyjelly.pocketcasts.preferences.PlayOverNotificationSetting
 import au.com.shiftyjelly.pocketcasts.preferences.Settings
 import au.com.shiftyjelly.pocketcasts.repositories.R
 import au.com.shiftyjelly.pocketcasts.repositories.chromecast.CastManager
@@ -1337,14 +1338,14 @@ open class PlaybackManager @Inject constructor(
         focusWasPlaying = null
     }
 
-    override fun onFocusLoss(mayDuck: Boolean, transientLoss: Boolean) {
+    override fun onFocusLoss(playOverNotification: PlayOverNotificationSetting, transientLoss: Boolean) {
         val player = player
         if (player == null || player.isRemote) {
             return
         }
         // if we are playing but can't just reduce the volume then play when focus gained
         val playing = isPlaying()
-        if (!mayDuck && playing) {
+        if ((playOverNotification == PlayOverNotificationSetting.NEVER) && playing) {
             LogBuffer.i(LogBuffer.TAG_PLAYBACK, "Focus lost while playing")
             focusWasPlaying = Date()
 
@@ -1355,9 +1356,12 @@ open class PlaybackManager @Inject constructor(
         }
 
         // check if we need to reduce the volume
-        if (focusManager.canDuck()) {
+        if (playOverNotification == PlayOverNotificationSetting.DUCK) {
             player.setVolume(VOLUME_DUCK)
+            return
         }
+
+        player.setVolume(VOLUME_NORMAL)
     }
 
     override fun onFocusRequestFailed() {

diff --git a/...repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/support/Support.kt b/...repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/support/Support.kt
@@ -406,7 +406,7 @@ class Support @Inject constructor(
                 output.append(eol)
 
                 output.append("Notifications").append(eol)
-                output.append("Play over notifications? ").append(if (settings.canDuckAudioWithNotifications()) "yes" else "no").append(eol)
+                output.append("Play over notifications? ").append(settings.getPlayOverNotification().analyticsString).append(eol)
                 output.append("Hide notification on pause? ").append(if (settings.hideNotificationOnPause()) "yes" else "no").append(eol)
                 output.append(eol)