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() - } } }