PlayerNotificationManager: support custom queue navigation

[nilskassube]

[REQUIRED] Use case description

I have a custom queue navigator not based on a TimelineQueueNavigator. Inside a media session it works fine, however integration with the PlayerNotificationManager is more cumbersome than necessary. I tried to create a subclass from PlayerNotificationManager but the next() and previous() methods are private and most of the instance variables, so additional work is required. Copy and paste of the PlayerNotificationManager to create a custom version should not be easier than reusing the existing one. It does almost everything I want.

Proposed solution

Enable delegation for queue actions to a custom QueueNavigator (same one as in the mediasession extension) so the onSkipToPrevious and onSkipToNext can be reused. In general is would be awesome to have less objects "private" and be able to supply custom versions easier.

Alternatives considered

Complete custom management of notifications based on the media session.

[marcbaechinger]

Thanks for your request.

Do I understand correctly that you never have more than a single window in the timeline? Meaning you are not using ConcatenatingMediaSource but you always prepare with a new source when onSkipToNext/Previous is called?

If this is the case I think just making next(Player) and previous(Player) protected, so you can override them, is not enough. For instance, the getActions(Player) only puts the next action in case there is actually a next window in the timeline. So you would have at least to override getActions as well. If so, you have to provide the logic whether to show or hide certain actions at a given player state yourself (like for instance play/pause according to player.getPlayWhenReady()). That is not too difficult. My point is rather that if you need to override getActions(Player), then you could also just disable all built-in actions and provide/receive your own custom actions with a CustomActionReceiver.

Disabling all built in actions and using a CustomActionReceiver is as far as I understand the way, with which you can achieve what you want with the current version of the PlayerNotificationManager. No need to subclass it or waiting for a change of ours to land in the repo. I acknowledge that you have to do the logic of hiding/displaying certain actions for a given player state yourself, but that should be doable and is a solution, which you can do right now.

Can you confirm my assumption about how you do onSkipToNext/Previous, so I can think about whether there is a sensible change, which would make it easier for you?

Aside: all actions except next/previous are delegated to a ControlDispatcher for which a custom version can be set by apps using setControlDispatcher(ControlDispatcher). So besides these navigation methods all actions can be intercepted already. So it's just about displaying/hiding these navigation actions which depend to the timeline.

[nilskassube]

Thanks for your response! Yes, there is only a single window in the timeline. It's a customer requirement to only start buffering when the user wants to play the stream and to stop buffering when the users taps on pause. Due to this we can only prepare the source after the user interaction.

Yes, it would be feasible to use custom actions for the use case. I have to admit I was just looking for an easier way, i.e. to directly overwrite the play logic methods next() and previous() because everything else would be the same.

We already use a custom ControlDispatcher elsewhere so we also reuse the object here and the queue navigator.

[kingargyle]

Similar to this request, it would be nice, to be able to not just hide or show the navigation actions, but be able to controll these during the notification manager creation. Our use case, is that we don't want to provide a previous option, or the ability to seek, but do want to provide the option to skip, play, and pause. Currently I have to disable the navigation all together. Which isn't ideal.

Custom actions might work, which I'll investigate, but it would be nice to beable to toggle these individually within the notification manager setup.

[ojw28]

@kingargyle - I think you can do that by extending PlayerNotificationManager and overriding getActions.

[kingargyle]

@kingargyle - I think you can do that by extending PlayerNotificationManager and overriding getActions.

Thanks, this looks like a possible solution. The only issue is that the PlayerNotificastionManager itself doesn't make it easy to extend with the number of Static methods that are included to create the notificaton.

I'll mess around with this and see what I can get working thought.

[ojw28]

I agree it's not that easy in the general case, but for your specific case I think you can just call super.getActions and then just remove the ones you don't want.

[kingargyle]

@ojw28 yea, I managed to get it to work, would still be nice to have setters to toggle them directly without having to override getActions.

[lwld]

@ojw28 in 2.14.0, the PlayerNotificationManager constructors are deprecated and PlayerNotificationManager.Builder should be used instead. But when using the builder, it is not possible to override getActions. For now, it is still possible to use PlayerNotificationManager directly without the builder by using the deprecated constructor.

However this only works as long as these constructors exist. Is it planned to keep the constructors without builder around for public access, or would it be possible to add an option to override getActions to the builder (maybe by setting an optional ActionsProvider) before the constructors are removed?

[lwld]

The suggested method doesn't work any more in 2.15.0. Since the constructors have become private, the class can't be extended.

We have a customized notification with default and custom actions, with the available actions toggled on or off depending on the played stream (live, video on demand) and playback state (some streams have cue points, the cue point buttons are only visible if there is a cue point to skip to, etc.).

For this reason, we currently override getActions and getActionIndicesForCompactView, only returning the available actions. However, extending PlayerNotificationManager is no longer possible in 2.15.0 as the constructors are private. Is there any other suggested way how to achieve this that we missed?

[marcbaechinger]

Thanks for reporting. You are right, that this was broken in 2.15.0.

There is a fix for this on dev-v2: 2f09ece

[lwld]

Great, thanks for the update!