(Documentation & demo bug) removing task in STATE_ENDED causes dead notification

[nift4]

Version

Media3 1.3.0

More version details

No response

Devices that reproduce the issue

• Android 5.1 emulator
• Android 7.1 emulator
• Android 13 Pixel 7a
• Android 14 emulator

Devices that do not reproduce the issue

N/A

Reproducible in the demo app?

Yes

Reproduction steps

Example:

1. Add one song to playlist in demo app
2. Play the song
3. When the song ended and player state is STATE_ENDED, remove the app from recents
4. Notice stale notification

Expected result

No stale notification (like if you pause instead of waiting for ended)

Actual result

Stale notification

This is caused by

media/demos/session_service/src/main/java/androidx/media3/demo/session/DemoPlaybackService.kt

Lines 93 to 96 in d13a0f4

	override fun onTaskRemoved(rootIntent: Intent?) {
	val player = mediaLibrarySession.player
	if (!player.playWhenReady || player.mediaItemCount == 0) {
	stopSelf()

not checking for STATE_ENDED. As there is a song in the player and playWhenReady happens to stay true, the service does not terminate. Adding player.playbackState == Player.STATE_ENDED || to the condition has helped fix this in my app.

I believe https://developer.android.com/media/media3/session/background-playback should be updated as well.

Thank you.

Media

N/A

Bug Report

• You will email the zip file produced by adb bugreport to android-media-github@google.com after filing this issue.

[marcbaechinger]

Thanks for your report!

The intended behavior in the case of STATE_ENDED is to keep the notification and have the play button active. The user can either press the play button to resume playback at the start of the last item in the playlist, or (starting with API 29) use the timebar to seek back to a position in the last item to resume playback.

I think this would be the desired behavior when the user removes the app from recents as well, but as you report this is not the case. We will look into this and update this issue if we have some news.

player.playbackState == Player.STATE_ENDED || to the condition has helped fix this in my app.

Agreed this is a workaround that fixed the behavior. I'd keep this as a last resort in the case the above scenario isn't working.

[marcbaechinger]

Thanks again for your report.

Adding player.playbackState == Player.STATE_ENDED || to the condition has helped fix this in my app.

After investigating a bit more, I think this is indeed the correct fix.

When doing the repro steps above, I end up dismissing the app from the recents when the player is paused and with this, the player is not in the foreground. This means that the service needs to be stopped. Playback can then be resumed for instance with the playback resumption notification.

I think that behaves as it should. When in STATE_ENDED the service is taken off the foreground. If the user dismissed the app in this state, the service needs to be stopped or then the system restarts the service assuming it has crashed because it wasn't properly stopped after being in STARTED state. This results in this log statement that I can repro:

2024-03-26 21:58:46.133   649-1272  ActivityManager         system_process                       W  Scheduling restart of crashed service androidx.media3.demo.session/.PlaybackService in 1000ms for start-requested

So the proper fix is indeed to check for STATE_ENDED also as you suggest. I will change the documentation accordinlgy:

  override fun onTaskRemoved(rootIntent: Intent?) {
    val player = mediaLibrarySession.player
    if (!player.playWhenReady || player.mediaItemCount == 0 || player.playbackState == STATE_ENDED) {
      stopSelf()
    }
  }

However, this check becomes a bit cumbersome and I think we should create an API that an app can use.

[nift4]

I agree, this is basically inversing the check here:

media/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java

Lines 188 to 195 in d13a0f4

	/* package */ boolean shouldRunInForeground(
	MediaSession session, boolean startInForegroundWhenPaused) {
	@Nullable MediaController controller = getConnectedControllerForSession(session);
	return controller != null
	&& (controller.getPlayWhenReady() || startInForegroundWhenPaused)
	&& (controller.getPlaybackState() == Player.STATE_READY
	|| controller.getPlaybackState() == Player.STATE_BUFFERING);
	}

[marcbaechinger]

We fixed the documentation here, so that it works with the current version.

For the next version we have included API support to make this a bit easier for app.

There are two options an app is having:

1. Keep the service running in the background as a foreground service when playback is ongoing

For this we have added a default implementation of onTaskRemoved() in MediaSessionService that looks like this:

@Override
public void onTaskRemoved(@Nullable Intent rootIntent) {
  if (!isPlaybackOngoing()) {
    // The service needs to be stopped when playback is not ongoing and the service is not in the
    // foreground.
    stopSelf();
  }
}

The default implementation makes sure that we don't see crashes of the service in case an app doesn't implement onTaskRemoved().

2. Always stop the service when the task is removed

The default from above can be overriden as follows by using the new convenience method:

override fun onTaskRemoved(rootIntent: Intent?) {
  // Stop the service event when playback is ongoing.
  pauseAllPlayersAndStopSelf()
}

the method pauseAllPlayersAndStopSelf() pauses the player of every session that is managed by the service and then calls stopSelf.

The whole change in MediaSessionService is based on these methods. We also added JavaDoc to explain the default behaviour and the options an app has based on the foreground service restrictions imposed by the system.

I'm closing this issue. Please file a new issue if required.

[nift4]

Thank you!