ExoPlayer manipulate notification based on Playerstate

[gowthamgts]

Issue description

How can I make the exoplayer notification dismissable when the player is paused and then make it a foreground service when it it resumed? Here are my questions:

1. https://github.com/google/ExoPlayer/issues/4256#issuecomment-388936681: How can I start a foreground service by using startForeground inside onPlayerStateChanged? Is there a way to retrieve notification object that's already constructed?
2. https://github.com/google/ExoPlayer/issues/4482#issuecomment-407088270: Can we please have the code base on the demo app changed so that we can refer that?

Reproduction steps

IO18 audio app
https://github.com/google/ExoPlayer/tree/io18

Version of ExoPlayer being used

latest

Device(s) and version(s) of Android being used

Nexus 6p. API level 28

[gowthamgts]

Looks like I can do this by constructing the notification myself. Is there any way PlayerNotificationManager supports this? It would be great if there are some samples.

[0xcaff]

I want to implement the same behavior also. I plan to fork + add an api to PlayerNotificationManager which exposes the notification every time it changes.

[0xcaff]

Related to #5117, #4988, #4989, #4482, #4269, #4256, #3559

I believe the issues on flickering notifications are coming from people using startForeground with a stale notification while trying to implement this behavior.

It looks like #4269 could have been a potential solution, but the PR stalled.

[tonihei]

Thanks for pull request! The proposal looks sensible to me but I'm not an expert for notifications. @marcbaechinger Can you take a closer look?

[marcbaechinger]

You need to do the following to make the notification dissmisible:

1. The PlayerNotificationManager must have set the ongoing flag to true
2. When playWhenReady transitions to false you need to call stopForeground(false) on your service.
3. You may want call playerNotificationManager.setStopAction(null) or playerNotificationManager.setStopAction("your-stop-action") to avoid the manager is handling the stop action itself.
4. If you use your own stop action you need to use a CustomActionReceiver to handle your cutom stop action.

[gowthamgts]

Thanks for the reply @marcbaechinger. I was able to achieve the play to pause flow, but unable to do the other way around.

Inorder to start the service in foreground again, I may need to pass the notification object isn't it? Constructing a notification object manually defeats the purpose of using a PlayerNotificationManager IMO. @0xcaff proposed a PR for this problem - please verify and let us know whether that's the best way moving forward.

[0xcaff]

Yes, this is exactly the purpose of the PR

[marcbaechinger]

Thanks for the explanation @gowthamgts I see the issue.

We already have the PlayerNotificationManager.NotificationListener, which passes the notification when it is initially started. Would it be sufficient if we add a method similar to

void onNotificationUpdated(int notificationId, Notification notification);
which is called each time the notification is updated?

This way you have access to the last notification which has been set and you can us for calling startForeground(notificationId, notification). Would that work?

@0xcaff Please let me know your thoughts too. We could do that without adding a new interface but just add that method to NotificationListener?

[gowthamgts]

Yes, this can be a solution. I could have stored the notification object when the service starts but had no way to have an updated object whenever the information changes. This looks good for me. It would be an added bonus if that function gets called even for play/pause state changes.

On another note, is there a way to customize the UI? I prefer having the following two UI changes in my application:

1. Adding a close button to the top right of notification to cancel the playback. This is similar to stop functionality as per exoplayer convention, but using the stop button (because of it's placement in the notification) doesn't feel like best UX in my opinion.
2. When the notification is in collapsed state, I want to show rewind, forward and play buttons as opposed to the default behavior showing only play button.

Are these two implementations feasible? If yes, can you point me in the right direction?

[marcbaechinger]

The added bonus is already given. :) The notification is updated when a relevant player state change happens which changes the appearance of the notification. Like when playing the pause button needs to be included and when paused the play button needs to be included. In this case the notification is already updated and will be passed to the onNotificationUpdated method. onNotificationUpdate is called each time something relevant changes (pause, seek, skipNext/Prev, et al.)

For 1) I don't think this is allowed by the framework. The only way to have something clickable (besides the entire notification) is adding actions to the notification which then fire a broadcast event. There is no way to determine how this is rendered. When you are able to remove the service from foreground, the user also can swipe the notification away. There is no more customization of interactive UI possible in media style notifications.

For 2) you can override PlayerNotificationManager.getActionIndicesForCompactView which returns an array of indices which determines which actions (max is 3) are shown in compact mode (collapsed state as you call it).

[0xcaff]

I think the NotificationDispatcher makes the most sense because otherwise onNotificationStarted and onNotificationUpdated have overlap. The library user can easily implement onNotificationStarted just using onNotificationUpdated and a boolean. I think your solution is more confusing.

[0xcaff]

Here's what my implementation of NotificationDispatcher looks like.

class NotificationDispatcher(
    private val notificationId: Int,
    private val service: Service,
    private val player: Player
) {
    private val notificationManager = NotificationManagerCompat.from(service)

    private var foregrounded = false

    fun notify(notification: Notification) {
        if (player.playWhenReady && !foregrounded) {
            Util.startForegroundService(
                service,
                Intent(service, PlaybackService::class.java)
            )

            service.startForeground(notificationId, notification)
            foregrounded = true
        } else {
            notificationManager.notify(notificationId, notification)
        }

        if (!player.playWhenReady && foregrounded) {
            service.stopForeground(false)
            foregrounded = false
        }
    }

    fun cancel() {
        notificationManager.cancel(notificationId)
        service.stopForeground(true)
        service.stopSelf()
        foregrounded = false
    }
}

I don't see a place where on onNotificationStarted makes sense. Also, this isn't really a notification listener, it handles dispatching (and displaying) notifications with startForeground or NotificationManager depending on the current state.

[marcbaechinger]

Many thanks @0xcaff for your inputs! You pointed out several things we need to fix/improve. We will work on this. Really appreciated.

Beside the foreground service topic, we have a couple of other issues and enhancements around the PlayerNotificationManager which we look into. I update this issue to let you both know when we have these changes on the dev-2 branch to get your feedback.

Regarding foreground services it's roughly:

• default stop action button can not be hidden without disabling the stop action on dismissal of the notification
• document/reconsider behaviour of default stop action calling controlDispatcher.stop()
• fix NotificationListener behaviour/semantic
• foreground service needs most recent notification to put service to foreground
• document how a custom action can be set as stop action and be used at dismissal without being shown as a button

[gowthamgts]

@marcbaechinger: When can we expect these changes to land? Do you have any ballpark estimate?

[HighA]

I'm interested in this update too

[marcbaechinger]

We are working on this. I can't give you an exact time of arrival but I think it will land on the dev-2 branch next week.

[marcbaechinger]

@gowthamgts @0xcaff @HighA

We pushed a change regarding the PlayerNotificationManager to the dev-2 branch.

Roughly, the PlayerNotificationManager.NotificationListener has been changed to have a better semantic and receive every notification. Besides we separated stop action (optional button) and dismiss action (always attached to notification). Hence player.stop() is only called when the stop button is enabled.

There are a few other changes included. Would be great if you can look into this and give us feedback whether this fits your needs better now.

Many thanks in advance.

[HighA]

I get an error :

abstract method "java.lang.String com.google.android.exoplayer2.ui.PlayerNotificationManager$MediaDescriptionAdapter.getCurrentSubText(com.google.android.exoplayer2.Player)

I don't know why maybe because my player plays a stream so it has no info on tracks played but it's supposed to be nullable and no error on getCurrentContentText or title so I don't know. Also I don't get this error on emulator or if I run my app on my phone directly from the avd manager, only if I manually create an apk and install it on my phone. I won't have time to investigate on it until sunday or monday I think sadly, I tried a few things but didn't manage to make it work.

[marcbaechinger]

That's odd. The method has been added to the interface as a default implementation with Java 8, so it should not affect you at all. This is supported for all minSdks with Android.

Have you turned on Java 8 as mentioned here: https://google.github.io/ExoPlayer/guide.html#turn-on-java-8-support?

If you this does not help, can you send me a bug report so I can look into this?

[HighA]

ok i tried it I have a little problem with the way it works : in my app, I stop the player when it is paused, because I play a live stream and I get the tracks infos from another server, so if I let the user pause and resume the stream a few seconds later the infos will be desynchronized and it's not really user friendly. Plus I don't want a user to stop the stream to resume it for the next song, and get back to the same song he didn't want to hear on resume... But when I stop the player, it automatically dismiss the notification now, which is not the way I would like it to work.

Apart from that I still can't dismiss the notification when the player is paused, even if I manually set ongoing to false. I still have to set
stopForeground(false); playerNotificationManager.setOngoing(false);
in order to be able to dismiss the notification, but if I do so, when I resume the player, I still can't startForeground again. So basically I'm still at the same level with the same problems then before, except that now stopping the player also stops the notification, so it's even worse than before

notificationListener has also been deprecated so is there a replacement or is it not used anymore ? cause that's where this foreground problem could be easily fixed, like you said before by adding a onNotificationUpdate or something like that

[marcbaechinger]

Thanks for looking into that and giving feedback!

The NotificationListener is not deprecated, but we added two new methods which have a default implementation. The other methods are deprecated though. I recommend migrating to the new ones which should be easy. Please note that default methods of Java 8 interfaces need to be overridden specifically, because there is no need to implement them as there is a default. It might be not obvious to spot these.

The new onNotificationPosted(int, Notification) is called each time a notification is posted, so when you implement this method you get the most recent notification which you can use for restarting the service in foreground.

If you call stop on the player the player transitions to the IDLE state in which case the PlayerNotificationManager does not deliver a notification anymore and removes the notification. You can override createNotification and provide return your own notification for the idel state:

@Nullable
@Override
protected Notification createNotification(Player player, @Nullable Bitmap largeIcon) {
  if (player.getPlaybackState() == Player.STATE_IDLE) {
    return createYouCustomNotificationForTheIdleState(); 
  } else {
    return super.createNotification(player, largeIcon);
  }
}

[HighA]

Ok thanks I found the new methods, tried it out and yes they do the trick for ongoing and foreground issues 👍 anyway I still have a few questions :

About what you said about the Java 8 methods I'm a little confused. How do I spot these methods ? will they mess with my app and create bugs or unexpected behavior so I can spot them ?

And the most important : about the createNotification I have to override, I assume I have to create a custom class that extends PlayerNotificationManager and override createNotification in it ? but I don't really know how to do so. I get a "no default constructor" error on my custom class, obviously because I really don't know how I should create it, but don't really know what to do or where to look for answers. do you know where I could find resources to understand what I'm supposed to do ? or should I modify PlayerNotificationManager directly in the exoplayer-dev2 files ?

Sorry for the noob questions and thanks already

[marcbaechinger]

Good to hear the ongoing and foreground service management works for you now!

Regarding default methods of Java 8 interfaces, they do not do any harm to your app :). We are using these to extend an interface with new methods without breaking the code of everyone who uses these interfaces already. The default implementation are no-ops and if you need to you can override them. That's the difference: while normal interface methods are implemented, default methods need to be overridden. So in Android Studio you need to choose "Code > Override methods" from the menu instead of "Code > Implement methods".

Regarding overriding createNotification you are right that you need to subclass the PlayerNotificationManager and then override createNotification. When you do this you need to call a constructor of the super class. Because the PlayerNotificationManager does not have a default constructor (a constructor without any arguments) you need to do this yourself. It looks roughly like t

public class MyCustomerPlayerNotificationManager {
   public MyCustomerPlayerNotificationManager(
      Context context,
      String channelId,
      int notificationId,
      MediaDescriptionAdapter mediaDescriptionAdapter,
      @Nullable CustomActionReceiver customActionReceiver) {
      super(context, channelId, notificationId, mediaDescriptionAdapter, customActionReceiver);
      // do your thing....
   }
}

[gowthamgts]

@marcbaechinger: Sorry, I was caught up on work and didn't had the time to test this. I will pickup some time this weekend and will let you know how this goes.

[HighA]

@marcbaechinger Ok thanks I'll try to do so for now I did a workaround to match my needs.
I was about to ask you something else, then I checked this "code>implement methods" thing.. I didn't know there was a way to see all methods within a class until now. Thanks a lot for that, I feel dumb but I learned a precious tip today. And I got the answer to my question doing so. So now I managed to match all my needs with this new way of handling notifications. When will these new methods be available in the release version ?

[gowthamgts]

@ojw28 @marcbaechinger: I did tried to use the dev-v2 branch locally, but couldn't get it to work because of androidx problems. I followed the steps as mentioned on https://github.com/google/ExoPlayer#locally and I also have the following two lines in my gradle.properties:

android.useAndroidX=true
android.enableJetifier=true

So, when I set android.enableJetifier=true, I'm getting a lot of package: android.support.annotation does not exist and when I set it to false I'm getting Manifest merger failed error.

I also looked at the issues but none of the solutions helped me. Do I need to manually create a gradle.properties for each module? Let me know what I'm doing wrong.

[ojw28]

Unfortunately, I don't think Jetifier works when you include ExoPlayer as source (I think it only applies to dependencies pulled in as aars). We need to take a serious look at how we can migrate properly to androidx (we have some internal build complexities that make this non-trivial to do).

[gowthamgts]

@ojw28: So, is there a way to have the exoplayer dev2 branch work with my app locally? As I mentioned before, I unable to do so. Should I wait till the changes land in release-v2?

[ojw28]

@ojw28: So, is there a way to have the exoplayer dev2 branch work with my app locally? As I mentioned before, I unable to do so. Should I wait till the changes land in release-v2?

The four options are

1. Wait until a release so you can depend on ExoPlayer as a gradle dependency
2. Use something like JitPack to depend on the dev-v2 as a gradle dependency, which is discussed a bit in https://github.com/google/ExoPlayer/issues/3859
3. Use dev-v2 as source locally, but you'll have to migrate it to androidx to get it to work. This basically means you need to update all the support library imports in the ExoPlayer source tree to use the androidx imports instead
4. Wait for us to do (3) ourselves. We're accelerating this effort, which is now being tracked by https://github.com/google/ExoPlayer/issues/5489. I intend to take another look at this this week (it's somewhat complicated by some internal build complexities we have, but it's probably not as hard as was initially thought)

[gowthamgts]

@ojw28: Thanks for the prompt reply. Jitpack seems to be the promising solution here. I'll try that.

[ojw28]

@gowthamgts: If you sync dev-v2 tip of tree, you should find that it's now using androidx.

[gowthamgts]

@ojw28: Thanks, I'm able to build now.

[gowthamgts]

@marcbaechinger @ojw28: any eta on when these changes will land on release-v2 branch?