From 49a964705bd5064550f6c6e7133b740b97f5c926 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Sat, 2 Jul 2022 14:04:21 -0600 Subject: [PATCH] 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. --- CHANGELOG.md | 1 + .../org/oxycblt/auxio/image/BitmapProvider.kt | 2 + .../playback/state/PlaybackStateManager.kt | 13 ++- .../playback/system/MediaSessionComponent.kt | 81 +++++++++++++------ .../playback/system/NotificationComponent.kt | 63 +++++---------- .../auxio/playback/system/PlaybackService.kt | 59 ++++---------- 6 files changed, 99 insertions(+), 120 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2bdbbc79..32039e432 100644 --- 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 diff --git a/app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt b/app/src/main/java/org/oxycblt/auxio/image/BitmapProvider.kt index ebc0c18e9..7ff8be92a 100644 --- 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 diff --git a/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt b/app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt index 7e8785e2b..5477904a4 100644 --- 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) } /** diff --git a/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt b/app/src/main/java/org/oxycblt/auxio/playback/system/MediaSessionComponent.kt index f087c76e8..d7013ec3a 100644 --- 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, 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 diff --git a/app/src/main/java/org/oxycblt/auxio/playback/system/NotificationComponent.kt b/app/src/main/java/org/oxycblt/auxio/playback/system/NotificationComponent.kt index 36a0f902c..6f33fa69c 100644 --- 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 --- diff --git a/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt b/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt index 961dec995..d573ae82d 100644 --- 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() - } } }