androidx / media

Jetpack Media3 support libraries for media use cases, including ExoPlayer, an extensible media player for Android
https://developer.android.com/media/media3
Apache License 2.0
1.55k stars 373 forks source link

MediaLibraryService throws ForegroundServiceDidNotStartInTimeException #167

Open ItsBenyaamin opened 1 year ago

ItsBenyaamin commented 1 year ago

Media3 Version

1.0.0-beta02

Devices that reproduce the issue

Firebase crashlytics report these devices:

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

It's happened on some devices. I think It has related to MediaController not properly handling the service.

Expected result

not crash?

Actual result

Fatal Exception: android.app.ForegroundServiceDidNotStartInTimeException: Context.startForegroundService() did not then call Service.startForeground(): ServiceRecord{ac0645b u0 player.PlaybackService}
       at android.app.ActivityThread.throwRemoteServiceException(ActivityThread.java:2147)
       at android.app.ActivityThread.access$2900(ActivityThread.java:310)
       at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2376)
       at android.os.Handler.dispatchMessage(Handler.java:106)
       at android.os.Looper.loopOnce(Looper.java:226)
       at android.os.Looper.loop(Looper.java:313)
       at android.app.ActivityThread.main(ActivityThread.java:8669)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:571)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1135)

Media

-

Bug Report

jackyhieu1211 commented 1 year ago

@ziem

my code has been edited to fit my project. If you use demo, you can try it

  @SuppressLint("MissingPermission") // TODO: b/280766358 - Request this permission at runtime.
  override fun onForegroundServiceStartNotAllowedException() {
  val notificationManagerCompat = NotificationManagerCompat.from(this@PlaybackService)
  ensureNotificationChannel(notificationManagerCompat)
  val pendingIntent =
    TaskStackBuilder.create(this@PlaybackService).run {
      addNextIntent(Intent(this@PlaybackService, MainActivity::class.java))
      getPendingIntent(0, immutableFlag or FLAG_UPDATE_CURRENT)
    }
  val builder =
    NotificationCompat.Builder(this@PlaybackService, CHANNEL_ID)
      .setContentIntent(pendingIntent)
      .setSmallIcon(R.drawable.media3_notification_small_icon)
      .setContentTitle(getString(R.string.notification_content_title))
      .setStyle(
        NotificationCompat.BigTextStyle().bigText(getString(R.string.notification_content_text))
      )
      .setPriority(NotificationCompat.PRIORITY_DEFAULT)
      .setAutoCancel(true)

       if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
        val foregroundServiceBehavior = Notification.FOREGROUND_SERVICE_IMMEDIATE
        builder.foregroundServiceBehavior = foregroundServiceBehavior
    }
  notificationManagerCompat.notify(NOTIFICATION_ID, builder.build())
 }
 }
ziem commented 1 year ago

Thanks. I'm not sure why, but your fix doesn't work for me.

jackyhieu1211 commented 1 year ago

@ziem I haven't released my app yet. Because I need to add some more features. So I can't confirm this error correction code is 100% correct.

But. Can you share your code?

jackyhieu1211 commented 1 year ago

@ziem Have you granted POST NOTIFICATION PERMISSION permission to your device. ( For android 13)

<uses-permission android:name="android.permission.POST_NOTIFICATIONS" />

In YourActivity.

     internal val requestPermissionLauncher =
        registerForActivityResult(ActivityResultContracts.RequestPermission()) {  granted ->

        }

    requestPermissionLauncher.launch(Manifest.permission.POST_NOTIFICATIONS)

if the device has not been granted this permission, the error still occurs

ziem commented 1 year ago

The PlaybackService I use is almost identical to the one in the demo project. Yes, I've tested both cases (with granted & denied post notification permission). I'm still investigating this issue.

jackyhieu1211 commented 1 year ago

@ziem Next week. I will release my app. If OK. I will share the code with you

ziem commented 1 year ago

Yes, please. Thank you @jackyhieu1211

yebonkim commented 1 year ago

Hi @jackyhieu1211 I have same crash. Did you fix your crash by above code?

agnihotriayush commented 1 year ago

For fixing crash: If you are using MediaSessionService, try overiding onUpdateNotification and using startInForegroundRequired always true****

override fun onUpdateNotification(session: MediaSession, startInForegroundRequired: Boolean) { super.onUpdateNotification(session, true) }

fixes the crash.

I think the reason is that, if you start your media and keep it is pause state and move your app in background and then kill the app. The service will be created but startForegroundService() will not be called until you play the media or it will remove your service from foreground state but will not remove the notification. I tried the same on Sample Media Session app, and issue was reproducible with the same steps.

startForegroundService makes an implicit promise that the Service will call startForeground(int, android.app.Notification) once it begins running otherwise this ANR will be triggered.

On reading the internal implementation, using this flag true here works because:

Screenshot 2023-08-08 at 1 52 31 AM

Yours media controller is set and and playback state might be ready, but video is not playing so getPlayWhenReady() will be false. Hence, this will remove your service from foreground state but will not remove notification if you don't send startInForegroundRequired to true.

Screenshot 2023-08-08 at 1 55 17 AM

However, I think media3 should provide a proper fix for this case. Let's wait for @marcbaechinger 's inputs on this.

jackyhieu1211 commented 1 year ago

@yebonkim @s6joui @ziem

I tried solution of @agnihotriayush . It worked good. I'm testing more. Thanks @agnihotriayush

override fun onUpdateNotification(session: MediaSession, startInForegroundRequired: Boolean) {
        super.onUpdateNotification(session, true)
    }
jackyhieu1211 commented 1 year ago

@yebonkim @s6joui @ziem

I tried solution of @agnihotriayush . It worked good. I'm testing more. Thanks @agnihotriayush

override fun onUpdateNotification(session: MediaSession, startInForegroundRequired: Boolean) {
        super.onUpdateNotification(session, true)
    }

After release. This solution reduces crash rate ~10%

yebonkim commented 1 year ago

@jackyhieu1211 Thank you for your information. I will release new version with that solution and share the result here

junseokseo9306 commented 1 year ago

same as @jackyhieu1211 our product crach rate has been decreased about 10%ish after using @agnihotriayush 's solution. Thanks for sharing solution!

agnihotriayush commented 1 year ago

@marcbaechinger @tianyif We are still waiting for an input on this, to completely fix this issue. Initially I thought that my solution will keep the service in foreground and we will not experience this issue. We still have some users experiencing the issue. What should be the proper fix?

yebonkim commented 1 year ago

for 2days, although we didn't release hotfix version fully, Our service don't have this crash! Thanks @agnihotriayush

For additional information, our product use mediaSession with exoplayer and don't use mediaController

marcbaechinger commented 1 year ago

@jackyhieu1211 Thanks for your report and the evidence of problems on some devices. Very useful.

According to this doc, a notification of a foreground service that is having a foregroundServiceType of mediaPlayback is always posted immediately. So setting the foreground service behaviour on the notification seems not to be required.

An app needs to set this in it's manifest, Media3 can't do this for an app. The session demo app has set it here.

However, according to your report this is not sufficent on some devices. We will add setting the foreground service behaviour accordingly in DefaultMediaNotificationProvider. This shouldn't harm in any case and if we can do this for users on the library side we should. The change will land on the main branch soon.

Out of curiousity: Did you set the foreground service type to mediaPlayback in the manifest when you've seen this problem? And did you actually use DefaultMediaNotifciationProvider?

marcbaechinger commented 1 year ago

@agnihotriayush

I think the reason is that, if you start your media and keep it is pause state and move your app in background and then kill the app.

Can you describe in some more detail how you start the service in this scenario?

MediaSessionService or MediaLibraryService both have to be started by building a MediaController or a MediaBrowser from an app that is in the foreground. This starts the service in bound state.

The service will be created but startForegroundService() will not be called until you play the media or it will remove your service from foreground state but will not remove the notification.

As long as a controller is bound to the service, the service is not stopped by the system and, as you mentioned in your post, the service is not started in the foreground at this point.

The service is started in the foreground only, when the player is prepared and has started playing at least once.

For me to understand your use case and the behavior you are seeing, I would be interested to know how you start the service.

I understand you are seeing a ForegroundServiceDidNotStartInTimeException. Is this correct?

EDIT: ignore ~I'm asking because in the scenario you are describing I wonder who is calling play. Because calling play would attempt to put the service into the foreground which is only allowed if you call play from an app in the foreground, or from somewhere that gets an exemption to start from the background (like notification and Bluetooth).~

Your scenario as far a I understand is:

  1. Start the service by building a MediaController/MediaBrowser.
  2. The player and the session is built by your service.
  3. The player is prepared with at least one media item but play is NOT called.

With this scenario, the service wasn't started in the foreground yet. Accordingly, there shouldn't be a ForegroundServiceDidNotStartInTimeException, but instead the service be stopped by the system when the last bound client unbinds..

I tried the same on Sample Media Session app, and issue was reproducible with the same steps.

Can you repro this with the session app and then do a bug report and upload here? A short explanation on how you have modified the demo app to repro would be helpful as well.

jackyhieu1211 commented 1 year ago

@marcbaechinger Thank you for your interest in this issue.

I will share my code. Pls check. if you want to know something more. I'm ready to share

   <service
        android:name=".presentation.service.PlaybackService"
        android:exported="false"
        android:foregroundServiceType="mediaPlayback">
        <intent-filter>
            <action android:name="androidx.media3.session.MediaSessionService" />
            <action android:name="android.media.browse.MediaBrowserService" />
            <action android:name="android.intent.action.MEDIA_BUTTON" />
        </intent-filter>
    </service>

PlaybackService.kt

 class PlaybackService : MediaLibraryService() {

 companion object {
    const val NOTIFICATION_PLAYER_ID = DefaultMediaNotificationProvider.DEFAULT_NOTIFICATION_ID 
    const val EXTRA_VIDEO_SERVICE = "EXTRA_VIDEO_SERVICE"
    private const val EXTRA_ACTION_KEY = "EXTRA_ACTION_KEY"
    const val CUSTOM_COMMAND_TOGGLE_REPEAT_MODE_ON = "android.media3.session.REPEAT_ON"
    const val CUSTOM_COMMAND_TOGGLE_REPEAT_MODE_OFF = "android.media3.session.REPEAT_OFF"

    const val CUSTOM_COMMAND_TOGGLE_NEXT = "android.media3.session.NEXT"
    const val CUSTOM_COMMAND_TOGGLE_PREVIOUS = "android.media3.session.PREVIOUS"
}

private val librarySessionCallback = CustomMediaLibrarySessionCallback()

private lateinit var player: ExoPlayer
private lateinit var mediaLibrarySession: MediaLibrarySession
private var customCommands: List<CommandButton>? = null

private var customLayout = ImmutableList.of<CommandButton>()
private val compositeDisposable = CompositeDisposable()
private val appPreference: AppPreference by inject()

override fun onCreate() {
    super.onCreate()
    initializeSessionAndPlayer()
    setListener(MediaSessionServiceListener())
}

override fun onGetSession(controllerInfo: MediaSession.ControllerInfo): MediaLibrarySession {
    return mediaLibrarySession
}

private inner class CustomMediaLibrarySessionCallback : MediaLibrarySession.Callback {

    override fun onConnect(
        session: MediaSession, controller: MediaSession.ControllerInfo
    ): MediaSession.ConnectionResult {
        val connectionResult = super.onConnect(session, controller)
        val availableSessionCommands = connectionResult.availableSessionCommands.buildUpon()
        customCommands?.forEach { commandButton ->
            // Add custom command to available session commands.
            commandButton.sessionCommand?.let { availableSessionCommands.add(it) }
        }

        return MediaSession.ConnectionResult.accept(
            availableSessionCommands.build(), connectionResult.availablePlayerCommands
        )
    }

    override fun onPostConnect(session: MediaSession, controller: MediaSession.ControllerInfo) {
        /*  if (!customLayout.isEmpty() && controller.controllerVersion != 0) {
              ignoreFuture(mediaLibrarySession.setCustomLayout(controller, customLayout))
          }*/
    }

    override fun onCustomCommand(
        session: MediaSession,
        controller: MediaSession.ControllerInfo,
        customCommand: SessionCommand,
        args: Bundle
    ): ListenableFuture<SessionResult> {
        return Futures.immediateFuture(SessionResult(SessionResult.RESULT_SUCCESS))
    }

    override fun onAddMediaItems(
        mediaSession: MediaSession,
        controller: MediaSession.ControllerInfo,
        mediaItems: List<MediaItem>
    ): ListenableFuture<List<MediaItem>> {

        val updatedMediaItems: List<MediaItem> = mediaItems.map { mediaItem ->
            val resultMedia = mediaItem.buildUpon().setUri(mediaItem.requestMetadata.mediaUri)
                .setMediaId(mediaItem.mediaId)
                .setSubtitleConfigurations(mediaItem.requestMetadata.getSubtitles())
                .setMimeType(MimeTypes.APPLICATION_M3U8).build()

            resultMedia
        }
        return Futures.immediateFuture(updatedMediaItems)
    }
}

private fun initializeSessionAndPlayer() {
    val defaultRenderersFactory =
        DefaultRenderersFactory(applicationContext).setEnableDecoderFallback(true)
    player = ExoPlayer.Builder(this, defaultRenderersFactory)
        .setAudioAttributes(AudioAttributes.DEFAULT, true).build()

    val notificationProvider = MediaNotificationBuilder(context = applicationContext)
    notificationProvider.setSmallIcon(R.mipmap.icon_player_notification)
    this.setMediaNotificationProvider(notificationProvider)
    val intent = MainActivity.createIntent(this).apply {
        putExtra(
            EXTRA_VIDEO_SERVICE, true
        )
    }

    val sessionActivityPendingIntent = PendingIntent.getActivity(
        this@PlaybackService,
        0,
        intent,
        PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE
    )

    mediaLibrarySession =
        MediaLibrarySession.Builder(this, player, librarySessionCallback)
            .setSessionActivity(sessionActivityPendingIntent).build()
}

private inner class MediaSessionServiceListener : Listener {

    /**
     * This method is only required to be implemented on Android 12 or above when an attempt is made
     * by a media controller to resume playback when the {@link MediaSessionService} is in the
     * background.
     */
    override fun onForegroundServiceStartNotAllowedException() {
        player.prepare()
        player.play()
        onUpdateNotification(mediaLibrarySession, false)
    }
}

override fun onUpdateNotification(session: MediaSession, startInForegroundRequired: Boolean) {
    super.onUpdateNotification(session, true)
}

override fun onTaskRemoved(rootIntent: Intent?) {
    super.onTaskRemoved(rootIntent)
    player.stop()
    if (player.playWhenReady.not()) {
        stopSelf()
    }

    compositeDisposable.clear()
    NotificationManagerCompat.from(this).cancel(NOTIFICATION_PLAYER_ID)
}

override fun onDestroy() {
    mediaLibrarySession.run {
        // player.removeListener(playerListener)
        player.release()
        release()
    }
    clearListener()
    super.onDestroy()
}

internal class MediaNotificationBuilder(val context: Context) :
    DefaultMediaNotificationProvider(context) {

    override fun addNotificationActions(
        mediaSession: MediaSession,
        mediaButtons: ImmutableList<CommandButton>,
        builder: NotificationCompat.Builder,
        actionFactory: MediaNotification.ActionFactory
    ): IntArray {

        val skipPreviousCommandButton =
            CommandButton.Builder().setPlayerCommand(Player.COMMAND_SEEK_TO_PREVIOUS)
                .setEnabled(true).setIconResId(R.drawable.exo_ic_skip_previous)
                .setExtras(Bundle().apply {
                    putInt(
                        "androidx.media3.session.command.COMPACT_VIEW_INDEX", 0
                    )
                    putString(EXTRA_ACTION_KEY, CUSTOM_COMMAND_TOGGLE_PREVIOUS)

                }).build()

        val playCommandButton =
            CommandButton.Builder().setPlayerCommand(Player.COMMAND_PLAY_PAUSE).setEnabled(true)
                .setIconResId(if (mediaSession.player.isPlaying) R.drawable.exo_icon_pause else R.drawable.exo_icon_play)
                .setExtras(Bundle().apply {
                    putInt(
                        "androidx.media3.session.command.COMPACT_VIEW_INDEX", 1
                    )
                }).build()

        val skipNextCommandButton =
            CommandButton.Builder().setPlayerCommand(Player.COMMAND_SEEK_TO_NEXT)
                .setEnabled(true).setIconResId(R.drawable.exo_ic_skip_next)
                .setExtras(Bundle().apply {
                    putInt(
                        "androidx.media3.session.command.COMPACT_VIEW_INDEX", 2
                    )
                    putString(EXTRA_ACTION_KEY, CUSTOM_COMMAND_TOGGLE_NEXT)
                }).build()

        val isRepeatOne = mediaSession.player.repeatMode == Player.REPEAT_MODE_ONE
        val actionShuffle =
            if (isRepeatOne) CUSTOM_COMMAND_TOGGLE_REPEAT_MODE_ON else CUSTOM_COMMAND_TOGGLE_REPEAT_MODE_OFF
        val shuffleCommandButton = getShuffleCommandButton(
            SessionCommand(actionShuffle, Bundle.EMPTY)
        )

        val stopCommandButton =
            CommandButton.Builder().setPlayerCommand(Player.COMMAND_STOP).setEnabled(true)
                .setIconResId(R.drawable.exo_icon_stop).build()

        val customLayout = ImmutableList.of(
            skipPreviousCommandButton, playCommandButton, skipNextCommandButton,
            // shuffleCommandButton,
            stopCommandButton
        )
        return super.addNotificationActions(mediaSession, customLayout, builder, actionFactory)
    }

    private fun getShuffleCommandButton(sessionCommand: SessionCommand): CommandButton {

        val isOn = sessionCommand.customAction == CUSTOM_COMMAND_TOGGLE_REPEAT_MODE_ON
        return CommandButton.Builder().setDisplayName(if (isOn) "ON" else "OFF")
            .setSessionCommand(sessionCommand)
            .setIconResId(if (isOn) R.drawable.exo_icon_repeat_one else R.drawable.exo_icon_repeat_off)
            .build()
    }
}
}
jackyhieu1211 commented 1 year ago

@marcbaechinger -Recently. I try custom DefaultMediaNotificationProvider . i want to change foregroundServiceBehavior . But my app is not released yet. so I can't announce the results yet Link 1

  public NotificationCompat.Builder getNotificationBuilder(String channelId) {
    return new NotificationCompat.Builder(context, channelId);
  }

  @Override
  public final MediaNotification createNotification(
      MediaSession mediaSession,
    ensureNotificationChannel();
    Player player = mediaSession.getPlayer();
    NotificationCompat.Builder builder = getNotificationBuilder(channelId);
    int notificationId = notificationIdProvider.getNotificationId(mediaSession);

Link 2

   class MediaNotificationBuilderProvider(private val context: Context) : DefaultMediaNotificationProvider(context) {
    @SuppressLint("WrongConstant")
    override fun getNotificationBuilder(channelId: String): NotificationCompat.Builder {
        val builder = NotificationCompat.Builder(context, channelId)
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
            builder.foregroundServiceBehavior =  NotificationCompat.FOREGROUND_SERVICE_IMMEDIATE
            builder.priority = NotificationCompat.PRIORITY_MAX
        }
        builder.setChannelId(channelId)
        return builder
    }
whitipet commented 1 year ago

I have already tried the solution that @jackyhieu1211 has suggested here. But it doesn't seem to have helped and I still get crashes on Samsungs. @marcbaechinger, it seems to me that the solution from the last commit is quite similar to the one proposed by @jackyhieu1211. Or am I wrong?

jackyhieu1211 commented 1 year ago

@whitipet Have you granted notification permission?. this is required for android 13

jackyhieu1211 commented 1 year ago

@marcbaechinger Please release new version. I will submit store and check this solution. thank you

whitipet commented 1 year ago

@jackyhieu1211 In my case, there is no good place for the permission request in the app before playback starts. According to the documentation, if the user does not grant notification permission, the notification will be displayed in Task Manager. Also, on the previous playback implementation (ExoPlayer before switching to media3) without MediaSessionService(), everything worked fine without notification permission and there were no crashes.

jackyhieu1211 commented 1 year ago

@whitipet I have a test device that is Xiaomi 11T Pro android 13. I tested 2 cases

  1. Don't allow POST Notification Permission ==> App crash with log : "Fatal Exception: android.app.RemoteServiceException$ForegroundServiceDidNotStartInTimeException: Context.startForegroundService() did not then call Service.startForeground()"
  2. Allow POST Notification Permission ==> App not crash.

==> I have tested many times. and have the same result

whitipet commented 1 year ago

@jackyhieu1211 Strangely, this does not match the documentation. Either the documentation is wrong, or some manufacturers have decided to change something. I can't reproduce this crash on my Pixel 7 Pro android 13, but according to Crashlytics, 99% of users are Samsung. In any case, I will wait for the release with the fix and try it. If it doesn't work, I'll add a request for notification permission.

agnihotriayush commented 1 year ago

@whitipet What is your targetSDK ?

jackyhieu1211 commented 1 year ago

@whitipet I have reproduced the problem. If you want to try, follow the instructions here. LINK HERE

whitipet commented 1 year ago

@whitipet What is your targetSDK ?

33

fourofspades commented 1 year ago

Seeing the same issue here. 100% Android 13, and mostly Samsung devices.

image

tosam144 commented 1 year ago

Exactly the same issue for me. Android 13, mostly Samsung.

image image
jackyhieu1211 commented 1 year ago
Screen Shot 2023-09-04 at 11 37 19 Screen Shot 2023-09-04 at 11 33 17

@marcbaechinger Hi Marcbaechinger My app. Before apply solution: 7000 user crash/day After apply solution: 2000 user crash/day

The above solution worked well. My application has reduced the "Crash free-users" rate from 90% -> ~98%. How wonderful. Although the error still exists. but I think 2% user error rate is an acceptable rate. So please release new version Media. Thank you

marcbaechinger commented 1 year ago

Do I understand correctly, that part of your solution is not exporting the service?

jackyhieu1211 commented 1 year ago

@marcbaechinger are you referring to this? This is my code

    <service
        android:name="my.service.PlaybackService"
        android:exported="false"
        android:foregroundServiceType="mediaPlayback">
        <intent-filter>
            <action android:name="androidx.media3.session.MediaSessionService" />
            <action android:name="android.media.browse.MediaBrowserService" />
        </intent-filter>
    </service>
marcbaechinger commented 1 year ago

Yes. That's interesting.

If you don't export the service, then no other app than your own app sees the service. I'm not sure for users with system-privilege (like System UI), but other external apps would not see the service in the PackageManager nor would they be able to crash your app with a ForegroundServiceDidNotStartInTimeException in such a case. So given the reason for your crashes were external apps, it seems evident that not exporting the service works, the question is if the implications with this are intended and also, why external apps were trying to start your service without you knowing it and giving consent.

"Crash free-users" rate from 90% -> ~98%. How wonderful.

That's amazing. I'm excited for you as well. If your app has a lower crash-rate by not exporting the service, then is it likely that those users that produced the crash before, where trying to access your service from external apps?

If you don't have such external users, then I would definitely not export the service. You need to test whether System UI still works though. I would guess the notification works without a problem because the notification only uses the MediaControllerCompat. If you are using the resumption notification of System UI starting with API 30 that is using MediaBrowserCompat, then I would test this thoroughly on all API level when not exporting the service.

If you not export the service, you can probably also remove

<action android:name="android.media.browse.MediaBrowserService" />

because internally I'd guess you have Media3 controllers only.

Unfortunately, we can't be sure because your fix seems to do several things. This change around not exporting the service though, is either not required (as in not fixing anything), or then the actual reason for the crashes you are seeing were external apps.

marcbaechinger commented 1 year ago

After looking at the two commits that helped you. One change makes the service always run in the foreground. The other does not export the service anymore.

From this, the crashes could be from external apps that start your service, which makes the service create a session that is returned by MediaSessionService.Calback.onGetSession(controllerInfo) . This then calls onUpdateNotification(session, boolean startInForeground) that you override by always passing true some further.

If my reasoning is correct (which isn't sure with the available inormation), then the reason for your crashes can be an external app that starts the service but does not call play when starting. This, by design, leads to a ForegroundServiceDidNotStartInTimeException.

Both of these commits prevent this. Not exporting prevents external apps from seeing and starting your service. Always passing true into super.onUpdateNotification(session, true) prevents the service to not be put into the foreground when started with something else than play.

jackyhieu1211 commented 1 year ago

@marcbaechinger Thanks for your detailed answer and explanation.

fourofspades commented 1 year ago

Hi, wondering what the release schedule looks like for seeing there fixes in an official release?

marcbaechinger commented 1 year ago

Which fix are you referring to? I don't see a change that is fixing something for which you need a release. Everything that was mentioned here can be done without a release. But I may have missed something in this thread which has grown quite a bit.

Can you let me know which changes you mean?

jackyhieu1211 commented 1 year ago

@fourofspades @marcbaechinger Hi I need this commit. Media 1.1.1 does not have this commit yet.

Set notification foreground service behaviour starting with API 31

I'm not sure this commit if this helps my app crash rate. Because I am using all 3 commits

  1. android:exported="false"
  2. super.onUpdateNotification(session, true)
  3. Set notification foreground service behavior starting with API 31

I have forked and custom media library have three this commit if you want to do a test without 3rd commit . I'll do it and post the results later

Screen Shot 2023-09-04 at 11 37 19
whitipet commented 1 year ago

@marcbaechinger in my case, both solutions (android:exported="false" and super.onUpdateNotification(session, true)) did not help.

marcelpallares commented 1 year ago

Same, we tried android:exported="false" and still facing the crash. We don't override the notification behavior, so we don't have access to super.onUpdateNotification(session, true)) at the moment, but if the only solution is to do that we might need to override it?

fourofspades commented 1 year ago

Also, it's really important to mention, if you want to support things like Android Auto, you need to export your service, and set up the intent filters for MediaBrowserService and MediaLibraryService so these proposed changed are of little value.

marcbaechinger commented 1 year ago

@marcelpallares

Same, we tried android:exported="false" and still facing the crash.

Who is attempting to starting the service? Do you have a stack trace of this?

@fourofspades Sure. Not exporting the service isn't a solution. Exporting the service is the actual purpose of the service.

marcelpallares commented 1 year ago

@marcbaechinger

This is the stack trace (we shared it a few months ago and have been following this issue since hoping for a solution).

Fatal Exception: android.app.RemoteServiceException$ForegroundServiceDidNotStartInTimeException: Context.startForegroundService() did not then call Service.startForeground(): ServiceRecord{6b77eb u0 com.reveri.reverihealth/com.reveri.core.player.service.PlayerService}
       at android.app.ActivityThread.generateForegroundServiceDidNotStartInTimeException(ActivityThread.java:2245)
       at android.app.ActivityThread.throwRemoteServiceException(ActivityThread.java:2216)
       at android.app.ActivityThread.-$$Nest$mthrowRemoteServiceException()
       at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2508)
       at android.os.Handler.dispatchMessage(Handler.java:106)
       at android.os.Looper.loopOnce(Looper.java:226)
       at android.os.Looper.loop(Looper.java:313)
       at android.app.ActivityThread.main(ActivityThread.java:8757)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:571)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1067)

As with everyone else, most of the devices reported are Samsung (96%) and Android 13/14 only.

marcbaechinger commented 1 year ago

@marcelpallares

Sorry for wild questions, just trying to learn a few things when doing some tests on Samsung. Trying to have your setup as closely as possible.

How is your app designed to stop the service (with stopSelf())? Are you using onTaskRemoved() to stop the service conditionally or unconditionally?

If your app decides to stop the service, what APIs methods (Media3 session and service) does your app call and where (for instance in onTaskRemoved() vs. onDestroy())?

From one of the life-cycle methods onTaskRemoved/onDestroy() do you call any asynchronous methods that return later and stop/destroy the service?

Do you override onStartCommand() or use a custom MediaButtonReceiver?

marcelpallares commented 1 year ago

@marcbaechinger

Not at all! I'm happy to provide as many insights as I can to get this resolved. Even if needed, happy to schedule a quick call to walk you through the codebase. We have never been able to reproduce the issue ourselves btw.

How is your app designed to stop the service (with stopSelf())? Are you using onTaskRemoved() to stop the service conditionally or unconditionally? If your app decides to stop the service, what APIs methods (Media3 session and service) does your app call and where (for instance in onTaskRemoved() vs. onDestroy())?

We have use stopSelf() to stop the service. We also call player.stop() inside the onTaskRemoved() method. We don't manually stop the service in any other way at any point, we call controller.stop() using the MediaController when an audio session is finished, and if we need to start another session we set up the new mediaItems and call prepare and play. So the service is always around unless the app is killed. Is this a bad practice?

    /**
     * Takes care of stopping the player and destroying the service [stopSelf] when the app
     * is dismissed from recent apps.
     */
    override fun onTaskRemoved(rootIntent: Intent?) {
        super.onTaskRemoved(rootIntent)
        player.stop()
        stopSelf()
    }

We also use onDestory() to release the player and mediaSession.

    override fun onDestroy() {
        player.release()
        mediaSession.release()
        super.onDestroy()
    }

From one of the life-cycle methods onTaskRemoved/onDestroy() do you call any asynchronous methods that return later and stop/destroy the service?

No, we don't. See above both methods.

Do you override onStartCommand() or use a custom MediaButtonReceiver?

None of them.

marcbaechinger commented 1 year ago

Thanks everyone for the provided information. @marcelpallares Looks like your app is doing the right thing.

I can repro this with the demo app on a Samsung Galaxy A51.

21:36:45.832 25471-25471 AndroidRuntime      E  FATAL EXCEPTION: main
Process: androidx.media3.demo.session, PID: 25471
android.app.RemoteServiceException$ForegroundServiceDidNotStartInTimeException: Context.startForegroundService() did not then call Service.startForeground(): ServiceRecord{1b1ee80 u0 androidx.media3.demo.session/.PlaybackService}
    at android.app.ActivityThread.generateForegroundServiceDidNotStartInTimeException(ActivityThread.java:2245)
    at android.app.ActivityThread.throwRemoteServiceException(ActivityThread.java:2216)
    at android.app.ActivityThread.-$$Nest$mthrowRemoteServiceException(Unknown Source:0)
    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2508)
    at android.os.Handler.dispatchMessage(Handler.java:106)
    at android.os.Looper.loopOnce(Looper.java:226)
    at android.os.Looper.loop(Looper.java:313)
    at android.app.ActivityThread.main(ActivityThread.java:8757)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:571)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1067)

Repro steps with session demo app:

  1. User starts playback of a media item.
  2. user pauses playback and removes app from recent tasks.
  3. In onTaskRemoved() the app stops the service with stopSelf() which triggers onDestroy() where resources are released.
  4. System stops the process. All fine.
  5. User opens the notification drawer and selects Media output
  6. System shows a panel appears with device ouput selction and a mini player with a play button for the session demo (see screenshot at the end)
  7. User presses play
  8. Media3 tries playback resumption but app doesn't have implemented MediaSession.Callback.onPlaybackResumption(). Playback doesn't start and service doesn't go to the foreground.
  9. App crashed after a while with stack trace from above.

--

The interesting part is what shows up on this device when looking at the sysdump after the app was terminated.

# adb shell dumpsys media_session
[...]
Media key listener: null
  Media key listener package: 
  OnMediaKeyEventDispatchedListener: added 0 listener(s)
  OnMediaKeyEventSessionChangedListener: added 3 listener(s)
    from com.sec.android.app.bluetoothagent
    from com.android.systemui
    from com.android.systemui
  Last MediaButtonReceiver: MBR {pi=PendingIntent{21872e3: PendingIntentRecord{5fc44e0 androidx.media3.demo.session startForegroundService}}, componentName=ComponentInfo{androidx.media3.demo.session/androidx.media3.demo.session.PlaybackService}, type=3, pkg=androidx.media3.demo.session}
  High priority mediakey receiver: null
  Volume key long-press receiver: null
  Media button session is null
  Sessions Stack - have 0 sessions:
[...]

While on a Pixel 6 API 33 it shows:

# adb shell dumpsys media_session
[...]
  Media key listener: null
  Media key listener package: 
  OnMediaKeyEventDispatchedListener: added 0 listener(s)
  OnMediaKeyEventSessionChangedListener: added 2 listener(s)
    from com.android.bluetooth
    from com.android.bluetooth
  Last MediaButtonReceiver: null
  Media button session is null
  Sessions Stack - have 4 sessions:

[...]

The latter is the expected output as per these lines in MediaSessionLegacyStub of Media3 that I verified is executed on both devices before the player is released and the app terminates.

Looks like a different behavior than expected after setting the MBR to null. We will investigate some further.

Fixing

The fix is to achieve that the Last MediaButtonReceiver is set to null on all devices and all API levels as intended by Media3. Mdeia3 must make sure this works. We investigate on our end and file a bug and talk to Samsung if required for a solution.

I wouldn't recommend other fixes suggested above. It may remove the stack traces from the statistics, but the user sees the same play button that doesn't work.

Besides the unfortunate fact that there is this play button that doesn't work, this should not affect the proper functioning of your app in other areas. It is also restricted to these devices, although this is certainly an important case to take care of for this important vendor.

Available workaround

The only workaround I know for devices that exhibit this behavior with the current released Media3 version is implementing MediaSession.Callback.onPlaybackResumption() to return at least one playable item. Then playback starts, the service goes into the foreground and the exception is not thrown.

Screenshot of the button from the notification drawer:

image

svenoaks commented 1 year ago

@marcbaechinger I repro this on a Tab S7 with the demo-session app and in my MediaSessionService and SimpleBasePlayer app if I clear the playlist in the app before doing the procedure, so it has nothing to play from their mini-player.

However, in the normal case where my app restores persisted playlist and last playing item before any other commands are allowed to be issued to the player, the crash does not happen and the last item starts playing. That's even adding waiting at least 2 mins before playing or force stopping the app. That's without implementing MediaSession.Callback.onPlaybackResumption(), so maybe there are other ways.

On another note I wonder where this mini-player is pulling the title and artwork from because my app shows "No Title", despite my notifications showing everything correctly with something similar to DefaultNotificationProvider.

svenoaks commented 1 year ago

I still get these crashes while rolling out to beta channel slowly, despite verifying @marcbaechinger repro case is not happening for me and implementing all the things mentioned in this thread as workarounds. It's by far the most popular crash, does anyone know if it's going to count towards "User-perceived crash rate" in the Android vitals?

marcbaechinger commented 1 year ago

I repro this on a Tab S7 with the demo-session app and in my MediaSessionService and SimpleBasePlayer app if I clear the playlist in the app before doing the procedure, so it has nothing to play from their mini-player.

Thanks for reporting.

Have you rolled out after building from source with the fix referenced above?

If so, can you give me the repro steps you are doing so I can try to repro myself? Then please go through the repro steps and do a bug report and upload it here. This way we get all the information required in the repro step.