playback: sync mediasession and notif

Make notification updates entirely reliant on the MediaSession. Android 11 and onwards automatically populate the notification with the MediaSession state. This apparently conflicts with updating the notification in some cases, resulting in the incorrect song being shown. Fix this by not populating the notification from Android 11 onward and only posting it when the MediaSession state was set. Resolves #179.
2022-07-02 14:04:21 -06:00 · 2022-07-02 14:04:21 -06:00 · 49a964705b
commit 49a964705b
parent 3663396b97
6 changed files with 99 additions and 120 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -26,6 +26,7 @@ finished saving
 - Fixed issue where the search filter menu would not display the correct mode
 - Fixed crash when search filter mode was changed
 - Fixed shuffle button appearing below playback bar on Android 10 and lower
+- Fixed incorrect song being shown in the notification in some cases [#179]

 #### What's Changed
 - Reworked typography and iconography to be more aligned with material design guidelines
--- a/app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt
+++ b/app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt
@ -25,6 +25,7 @@ import coil.request.Disposable
 import coil.request.ImageRequest
 import coil.size.Size
 import org.oxycblt.auxio.music.Song
+import org.oxycblt.auxio.util.logD

 /**
 * A utility to provide bitmaps in a manner less prone to race conditions.
@ -53,6 +54,7 @@ class BitmapProvider(private val context: Context) {
        // Increment the generation value so that previous requests are invalidated.
        // This is a second safeguard to mitigate instruction-by-instruction race conditions.
        val generation = synchronized(this) { ++currentGeneration }
+        logD("new generation for ${song.rawName}: $generation $currentGeneration")

        currentRequest?.run { disposable.dispose() }
        currentRequest = null
--- a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt
@ -122,7 +122,6 @@ class PlaybackStateManager private constructor() {
                controller.loadSong(song)
                controller.seekTo(positionMs)
                controller.onPlayingChanged(isPlaying)
-                controller.onRepeatChanged(repeatMode)
                controller.onPlayingChanged(isPlaying)
            }

@ -468,14 +467,12 @@ class PlaybackStateManager private constructor() {
    }

    private fun notifyRepeatModeChanged() {
-        controller?.onRepeatChanged(repeatMode)
        for (callback in callbacks) {
            callback.onRepeatChanged(repeatMode)
        }
    }

    private fun notifyShuffledChanged() {
-        controller?.onShuffledChanged(isShuffled)
        for (callback in callbacks) {
            callback.onShuffledChanged(isShuffled)
        }
@ -495,11 +492,11 @@ class PlaybackStateManager private constructor() {
        /** Called when the playing state is changed. */
        fun onPlayingChanged(isPlaying: Boolean)

-        /** Called when the repeat mode is changed. */
-        fun onRepeatChanged(repeatMode: RepeatMode)
-
-        /** Called when the shuffled state is changed. */
-        fun onShuffledChanged(isShuffled: Boolean)
+        //        /** Called when the repeat mode is changed. */
+        //        fun onRepeatChanged(repeatMode: RepeatMode)
+        //
+        //        /** Called when the shuffled state is changed. */
+        //        fun onShuffledChanged(isShuffled: Boolean)
    }

    /**
--- a/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt
@ -26,6 +26,7 @@ import android.support.v4.media.session.MediaSessionCompat
 import android.support.v4.media.session.PlaybackStateCompat
 import androidx.media.session.MediaButtonReceiver
 import com.google.android.exoplayer2.Player
+import org.oxycblt.auxio.BuildConfig
 import org.oxycblt.auxio.R
 import org.oxycblt.auxio.image.BitmapProvider
 import org.oxycblt.auxio.music.MusicParent
@ -43,20 +44,25 @@ import org.oxycblt.auxio.util.logD
 *
 * @author OxygenCobalt
 */
-class MediaSessionComponent(private val context: Context, private val player: Player) :
+class MediaSessionComponent(
+    private val context: Context,
+    private val player: Player,
+    private val callback: Callback
+) :
    Player.Listener,
    MediaSessionCompat.Callback(),
    PlaybackStateManager.Callback,
    Settings.Callback {
+    interface Callback {
+        fun onPostNotification(notification: NotificationComponent)
+    }
+
    private val playbackManager = PlaybackStateManager.getInstance()
    private val settings = Settings(context, this)
-    private val mediaSession =
-        MediaSessionCompat(context, context.packageName).apply { isActive = true }
+    val mediaSession = MediaSessionCompat(context, context.packageName).apply { isActive = true }
+    private val notification = NotificationComponent(context, mediaSession.sessionToken)
    private val provider = BitmapProvider(context)

-    val token: MediaSessionCompat.Token
-        get() = mediaSession.sessionToken
-
    init {
        player.addListener(this)
        playbackManager.addCallback(this)
@ -81,15 +87,15 @@ class MediaSessionComponent(private val context: Context, private val player: Pl

    // --- PLAYBACKSTATEMANAGER CALLBACKS ---

-    override fun onIndexMoved(index: Int) {
-        updateMediaMetadata(playbackManager.song)
-    }
-
    override fun onNewPlayback(index: Int, queue: List<Song>, parent: MusicParent?) {
-        updateMediaMetadata(playbackManager.song)
+        updateMediaMetadata(playbackManager.song, parent)
    }

-    private fun updateMediaMetadata(song: Song?) {
+    override fun onIndexMoved(index: Int) {
+        updateMediaMetadata(playbackManager.song, playbackManager.parent)
+    }
+
+    private fun updateMediaMetadata(song: Song?, parent: MusicParent?) {
        if (song == null) {
            mediaSession.setMetadata(emptyMetadata)
            return
@ -97,7 +103,7 @@ class MediaSessionComponent(private val context: Context, private val player: Pl

        val title = song.resolveName(context)
        val artist = song.resolveIndividualArtistName(context)
-        val metadata =
+        val builder =
            MediaMetadataCompat.Builder()
                .putText(MediaMetadataCompat.METADATA_KEY_TITLE, title)
                .putText(MediaMetadataCompat.METADATA_KEY_DISPLAY_TITLE, title)
@ -110,18 +116,21 @@ class MediaSessionComponent(private val context: Context, private val player: Pl
                .putText(MediaMetadataCompat.METADATA_KEY_COMPOSER, artist)
                .putText(MediaMetadataCompat.METADATA_KEY_WRITER, artist)
                .putText(MediaMetadataCompat.METADATA_KEY_GENRE, song.genre.resolveName(context))
+                .putText(
+                    METADATA_KEY_PARENT,
+                    parent?.resolveName(context) ?: context.getString(R.string.lbl_all_songs))
                .putLong(MediaMetadataCompat.METADATA_KEY_DURATION, song.durationMs)

        if (song.track != null) {
-            metadata.putLong(MediaMetadataCompat.METADATA_KEY_TRACK_NUMBER, song.track.toLong())
+            builder.putLong(MediaMetadataCompat.METADATA_KEY_TRACK_NUMBER, song.track.toLong())
        }

        if (song.disc != null) {
-            metadata.putLong(MediaMetadataCompat.METADATA_KEY_DISC_NUMBER, song.disc.toLong())
+            builder.putLong(MediaMetadataCompat.METADATA_KEY_DISC_NUMBER, song.disc.toLong())
        }

        if (song.album.year != null) {
-            metadata.putString(MediaMetadataCompat.METADATA_KEY_DATE, song.album.year.toString())
+            builder.putString(MediaMetadataCompat.METADATA_KEY_DATE, song.album.year.toString())
        }

        // Normally, android expects one to provide a URI to the metadata instance instead of
@ -132,25 +141,30 @@ class MediaSessionComponent(private val context: Context, private val player: Pl
            song,
            object : BitmapProvider.Target {
                override fun onCompleted(bitmap: Bitmap?) {
-                    metadata.putBitmap(MediaMetadataCompat.METADATA_KEY_ART, bitmap)
-                    metadata.putBitmap(MediaMetadataCompat.METADATA_KEY_ALBUM_ART, bitmap)
-                    mediaSession.setMetadata(metadata.build())
+                    builder.putBitmap(MediaMetadataCompat.METADATA_KEY_ART, bitmap)
+                    builder.putBitmap(MediaMetadataCompat.METADATA_KEY_ALBUM_ART, bitmap)
+                    val metadata = builder.build()
+                    mediaSession.setMetadata(metadata)
+                    notification.updateMetadata(metadata)
+                    callback.onPostNotification(notification)
                }
            })
    }

    override fun onPlayingChanged(isPlaying: Boolean) {
        invalidateSessionState()
+        invalidateNotificationActions()
    }

    override fun onRepeatChanged(repeatMode: RepeatMode) {
-        // TODO: Add the custom actions for Android 13
        mediaSession.setRepeatMode(
            when (repeatMode) {
                RepeatMode.NONE -> PlaybackStateCompat.REPEAT_MODE_NONE
                RepeatMode.TRACK -> PlaybackStateCompat.REPEAT_MODE_ONE
                RepeatMode.ALL -> PlaybackStateCompat.REPEAT_MODE_ALL
            })
+
+        invalidateNotificationActions()
    }

    override fun onShuffledChanged(isShuffled: Boolean) {
@ -160,14 +174,18 @@ class MediaSessionComponent(private val context: Context, private val player: Pl
            } else {
                PlaybackStateCompat.SHUFFLE_MODE_NONE
            })
+
+        invalidateNotificationActions()
    }

    // --- SETTINGSMANAGER CALLBACKS ---

    override fun onSettingChanged(key: String) {
-        if (key == context.getString(R.string.set_key_show_covers) ||
-            key == context.getString(R.string.set_key_quality_covers)) {
-            updateMediaMetadata(playbackManager.song)
+        when (key) {
+            context.getString(R.string.set_key_show_covers),
+            context.getString(R.string.set_key_quality_covers) ->
+                updateMediaMetadata(playbackManager.song, playbackManager.parent)
+            context.getString(R.string.set_key_alt_notif_action) -> invalidateNotificationActions()
        }
    }

@ -239,6 +257,7 @@ class MediaSessionComponent(private val context: Context, private val player: Pl
        // forces the system to re-poll the position.
        // FIXME: For some reason however, positions just DON'T UPDATE AT ALL when you
        //  change from FROM THE APP ONLY WHEN THE PLAYER IS PAUSED.
+        // TODO: Add the custom actions for Android 13
        val state =
            PlaybackStateCompat.Builder()
                .setActions(ACTIONS)
@ -266,9 +285,25 @@ class MediaSessionComponent(private val context: Context, private val player: Pl
        mediaSession.setPlaybackState(state.build())
    }

+    private fun invalidateNotificationActions() {
+        notification.updatePlaying(playbackManager.isPlaying)
+
+        if (settings.useAltNotifAction) {
+            notification.updateShuffled(playbackManager.isShuffled)
+        } else {
+            notification.updateRepeatMode(playbackManager.repeatMode)
+        }
+
+        if (!provider.isBusy) {
+            callback.onPostNotification(notification)
+        }
+    }
+
    companion object {
        private val emptyMetadata = MediaMetadataCompat.Builder().build()

+        const val METADATA_KEY_PARENT = BuildConfig.APPLICATION_ID + ".metadata.PARENT"
+
        private const val ACTIONS =
            PlaybackStateCompat.ACTION_PLAY or
                PlaybackStateCompat.ACTION_PAUSE or
--- a/app/src/main/java/org/oxycblt/auxio/playback/system/NotificationComponent.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/system/NotificationComponent.kt
@ -21,8 +21,8 @@ import android.annotation.SuppressLint
 import android.app.NotificationChannel
 import android.app.NotificationManager
 import android.content.Context
-import android.graphics.Bitmap
 import android.os.Build
+import android.support.v4.media.MediaMetadataCompat
 import android.support.v4.media.session.MediaSessionCompat
 import androidx.annotation.DrawableRes
 import androidx.core.app.NotificationCompat
@ -31,12 +31,9 @@ import okhttp3.internal.notify
 import org.oxycblt.auxio.BuildConfig
 import org.oxycblt.auxio.IntegerTable
 import org.oxycblt.auxio.R
-import org.oxycblt.auxio.image.BitmapProvider
-import org.oxycblt.auxio.music.MusicParent
 import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.playback.state.RepeatMode
 import org.oxycblt.auxio.util.getSystemServiceSafe
-import org.oxycblt.auxio.util.logD
 import org.oxycblt.auxio.util.newBroadcastPendingIntent
 import org.oxycblt.auxio.util.newMainPendingIntent

@ -47,13 +44,9 @@ import org.oxycblt.auxio.util.newMainPendingIntent
 * @author OxygenCobalt
 */
@SuppressLint("RestrictedApi")
-class NotificationComponent(
-    private val context: Context,
-    private val callback: Callback,
-    sessionToken: MediaSessionCompat.Token
-) : NotificationCompat.Builder(context, CHANNEL_ID) {
+class NotificationComponent(private val context: Context, sessionToken: MediaSessionCompat.Token) :
+    NotificationCompat.Builder(context, CHANNEL_ID) {
    private val notificationManager = context.getSystemServiceSafe(NotificationManager::class)
-    private val provider = BitmapProvider(context)

    init {
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
@ -88,61 +81,41 @@ class NotificationComponent(
        notificationManager.notify(IntegerTable.PLAYBACK_NOTIFICATION_CODE, build())
    }

-    fun release() {
-        provider.release()
-    }
-
    // --- STATE FUNCTIONS ---

-    /** Set the metadata of the notification using [song]. */
-    fun updateMetadata(song: Song, parent: MusicParent?) {
-        provider.load(
-            song,
-            object : BitmapProvider.Target {
-                override fun onCompleted(bitmap: Bitmap?) {
-                    logD("writing ${song.rawName} to notif")
-                    setContentTitle(song.resolveName(context))
-                    setContentText(song.resolveIndividualArtistName(context))
+    fun updateMetadata(metadata: MediaMetadataCompat) {
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
+            // Notification is automatically filled in by media session, ignore
+            return
+        }

-                    // Starting in API 24, the subtext field changed semantics from being below the
-                    // content text to being above the title.
-                    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
-                        setSubText(
-                            parent?.resolveName(context)
-                                ?: context.getString(R.string.lbl_all_songs))
-                    } else {
-                        setSubText(song.album.resolveName(context))
-                    }
+        setContentTitle(metadata.getString(MediaMetadataCompat.METADATA_KEY_TITLE))
+        setContentText(metadata.getText(MediaMetadataCompat.METADATA_KEY_ARTIST))

-                    setLargeIcon(bitmap)
+        // Starting in API 24, the subtext field changed semantics from being below the
+        // content text to being above the title.
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
+            setSubText(metadata.getText(MediaSessionComponent.METADATA_KEY_PARENT))
+        } else {
+            setSubText(metadata.getText(MediaMetadataCompat.METADATA_KEY_ALBUM))
+        }

-                    callback.onNotificationChanged(song, this@NotificationComponent)
-                }
-            })
+        setLargeIcon(metadata.getBitmap(MediaMetadataCompat.METADATA_KEY_ALBUM_ART))
    }

    /** Set the playing icon on the notification */
    fun updatePlaying(isPlaying: Boolean) {
        mActions[2] = buildPlayPauseAction(context, isPlaying)
-        if (!provider.isBusy) {
-            callback.onNotificationChanged(null, this)
-        }
    }

    /** Update the first action to reflect the [repeatMode] given. */
    fun updateRepeatMode(repeatMode: RepeatMode) {
        mActions[0] = buildRepeatAction(context, repeatMode)
-        if (!provider.isBusy) {
-            callback.onNotificationChanged(null, this)
-        }
    }

    /** Update the first action to reflect whether the queue is shuffled or not */
    fun updateShuffled(isShuffled: Boolean) {
        mActions[0] = buildShuffleAction(context, isShuffled)
-        if (!provider.isBusy) {
-            callback.onNotificationChanged(null, this)
-        }
    }

    // --- NOTIFICATION ACTION BUILDERS ---
--- a/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt
@ -24,7 +24,7 @@ import android.content.Intent
 import android.content.IntentFilter
 import android.media.AudioManager
 import android.os.IBinder
-import androidx.core.app.NotificationCompat
+import android.support.v4.media.MediaMetadataCompat
 import androidx.core.app.ServiceCompat
 import com.google.android.exoplayer2.C
 import com.google.android.exoplayer2.ExoPlayer
@ -75,15 +75,14 @@ import org.oxycblt.auxio.widgets.WidgetProvider
 class PlaybackService :
    Service(),
    Player.Listener,
-    NotificationComponent.Callback,
    PlaybackStateManager.Controller,
+    MediaSessionComponent.Callback,
    Settings.Callback {
    // Player components
    private lateinit var player: ExoPlayer
    private lateinit var replayGainProcessor: ReplayGainAudioProcessor

    // System backend components
-    private lateinit var notificationComponent: NotificationComponent
    private lateinit var mediaSessionComponent: MediaSessionComponent
    private lateinit var widgetComponent: WidgetComponent
    private val systemReceiver = PlaybackReceiver()
@ -123,8 +122,7 @@ class PlaybackService :
        // --- SYSTEM SETUP ---

        widgetComponent = WidgetComponent(this)
-        mediaSessionComponent = MediaSessionComponent(this, player)
-        notificationComponent = NotificationComponent(this, this, mediaSessionComponent.token)
+        mediaSessionComponent = MediaSessionComponent(this, player, this)

        IntentFilter().apply {
            addAction(AudioManager.ACTION_AUDIO_BECOMING_NOISY)
@ -177,7 +175,6 @@ class PlaybackService :

        widgetComponent.release()
        mediaSessionComponent.release()
-        notificationComponent.release()
        player.release()

        logD("Service destroyed")
@ -260,7 +257,6 @@ class PlaybackService :
        logD("Loading ${song.rawName}")
        player.setMediaItem(MediaItem.fromUri(song.uri))
        player.prepare()
-        notificationComponent.updateMetadata(song, playbackManager.parent)
    }

    override fun seekTo(positionMs: Long) {
@ -273,18 +269,21 @@ class PlaybackService :

    override fun onPlayingChanged(isPlaying: Boolean) {
        player.playWhenReady = isPlaying
-        notificationComponent.updatePlaying(isPlaying)
    }

-    override fun onRepeatChanged(repeatMode: RepeatMode) {
-        if (!settings.useAltNotifAction) {
-            notificationComponent.updateRepeatMode(repeatMode)
-        }
-    }
+    override fun onPostNotification(notification: NotificationComponent) {
+        if (hasPlayed && playbackManager.song != null) {
+            logD(
+                mediaSessionComponent.mediaSession.controller.metadata.getText(
+                    MediaMetadataCompat.METADATA_KEY_TITLE))

-    override fun onShuffledChanged(isShuffled: Boolean) {
-        if (settings.useAltNotifAction) {
-            notificationComponent.updateShuffled(isShuffled)
+            if (!isForeground) {
+                startForeground(IntegerTable.PLAYBACK_NOTIFICATION_CODE, notification.build())
+                isForeground = true
+            } else {
+                // If we are already in foreground just update the notification
+                notification.renotify()
+            }
        }
    }

@ -295,34 +294,6 @@ class PlaybackService :
            getString(R.string.set_key_replay_gain),
            getString(R.string.set_key_pre_amp_with),
            getString(R.string.set_key_pre_amp_without) -> onTracksChanged(player.currentTracks)
-            getString(R.string.set_key_show_covers),
-            getString(R.string.set_key_quality_covers) ->
-                playbackManager.song?.let { song ->
-                    notificationComponent.updateMetadata(song, playbackManager.parent)
-                }
-            getString(R.string.set_key_alt_notif_action) ->
-                if (settings.useAltNotifAction) {
-                    onShuffledChanged(playbackManager.isShuffled)
-                } else {
-                    onRepeatChanged(playbackManager.repeatMode)
-                }
-        }
-    }
-
-    // --- NOTIFICATION CALLBACKS ---
-
-    override fun onNotificationChanged(song: Song?, component: NotificationComponent) {
-        if (hasPlayed && playbackManager.song != null) {
-            logD("Starting foreground/notifying: ${song?.rawName}")
-            logD(NotificationCompat.getContentTitle(component.build()))
-
-            if (!isForeground) {
-                startForeground(IntegerTable.PLAYBACK_NOTIFICATION_CODE, component.build())
-                isForeground = true
-            } else {
-                // If we are already in foreground just update the notification
-                notificationComponent.renotify()
-            }
        }
    }