Feature: PlayerSelector for Individual Players

[thiagoricieri]

Problem

There is no clear way of how to play single a video on demand in the RecyclerView using Toro.

Solution

To create a new class that implements PlayerSelector to wrap the logic to execute players on demand.

Usage

1. Set Container’s player selector to NONE

myContainer.setPlayerSelector(PlayerSelector.NONE);

2. Create a SinglePlayerSelector on demand and assign it to myContainer. It will wrap ToroPlayer and allow Container to manage the player. Example:

// Holder.java pseudo-code
public void myClickListener(ToroPlayer player) {
  SinglePlayerSelector = singlePlayerSelector = new SinglePlayerSelector(player);
  myContainer.setPlayerSelector(singlePlayerSelector);
}

[eneim]

@thiagoricieri You want the SinglePlayer to be one of ViewHolder? Or a separated one? Please give me a scenario to imagine this use-case if possible. Right now there is one problem with your "select" implementation. Understanding your use-case will help me to support. Thanks :D.

[thiagoricieri]

@eneim Thanks for getting back. The problem I want to solve here is to make Toro Player as flexible as possible: yesterday I invested some time reading the code and there was no clear way to make the Recycler View not play videos automatically, but play it on demand. I explain:

Although I understand the main idea behind Toro is to make a list of automatically playable videos, as soon as the user scrolls down the list, I think it would be a good idea to support that moment in which the developer would like to have a list in which the View Holder had a play button and that play button started the ExoPlayer. Meaning, it is not automatically, it would be manual and on user demand to play the video.

Since Player Manager is not accessible through the Container interface, I was able to workaround this feature by creating a PlayerSelector of one ToroPlayer that gets set when user clicks the View Holder's button. It integrates well with Toro as it will get recycled when the users scrolls up and down, automatically stopping the video.

Let me know if this is clear enough, I will update the PR description later on, I created it late last night and felt kinda sleepy 😴

Keep up with the good work and thanks for open-sourcing this library!

[eneim]

@thiagoricieri Got your point :D. In fact, you were right that a custom PlayerSelector is need. But your implementation has two downsides:

(1) It tracks the ToroPlayer instance but not the order of that player, which is incorrect. The reason is ToroPlayer is always a ViewHolder, which is supposed to be reused and will be recycled eventually. So your ToroPlayer will be triggered for different Videos, not a fixed one. What should be tracked is actually the order of that Video in your list.

(2) It disregards the candidate players (the parameter of PlayerSelector's select method). The design of PlayerSelector is to be the bridge between Container and Application: when the scroll happens, the visible Players will change. Container observes this change, re-calculate the list of Players those can start the playback (a.k.a playback candidates), and then use PlayerSelector to pass this candidate list to Application. What you should do is to select the Player from that list, rather than supply a specified one (it may or may not be contained in the candidate list).

My suggestion for this case is:

(1) The approach of using custom PlayerSelector is the best I can say. I think it should track the adapter position (also the Player order) of the player. And this order can be turned on/off based on User action. In detail: store a NUMBER as the active order in PlayerSelector, should be negative by default (--> state of no Player is activated/clicked). When user clicks "play button", change the NUMBER to the order of that Player. Trigger an "IDLE scroll" to start the playback instantly. PlayerSelector would scan though the candidates and return the Player whose player order is equal to that NUMBER, and empty list otherwise.

(2) The data may change, and this is expected behavior. Following the implementation from (1), your PlayerSelector should care about the case that Adapter change the data set and reset its NUMBER as well. What we can do is to pass the PlayerSelector into the Adapter and trigger the update along with data change, or you can have the Adapter to implement the PlayerSelector as well, which is quite neat.

P/S: If you are using SimpleExoPlayerView, the "click to play button" event may not be obvious to listen. But there is a good news here: there is a thing called "ControlDispatcher". SimpleExoPlayerView comes with a default ControlDispatcher, but also allow user to provide custom ControlDispatcher. Using this can help you to hook into its behavior and update your selector.

Please let me know your opinion about my suggestion.

[thiagoricieri]

@eneim thanks for sharing your thoughts. You have a better view of the design of the library and definitely it has helped me to get a clearer picture why Toro behaves the way it does.

I see where I made the mistake, I forgot to take into account the second argument items. The player I want to execute is within that list. You are right, passing the player's reference (View Holder) is incorrect and a bad idea!

Please don't take this as a critic, but rather like an open discussion: I think using lists of positions not a good developer experience. If the items list is a list of candidates, and they are implementations of ToroPlayer interface, how about introducing a new method like isEligible()?

If the Toro Player interface implemented such method, the Container could disregard any view holder that is not eligible and then play the first candidate in the list.

If I want Toro to behave as originally designed (playing automatically), I set every instance of ToroPlayer to have isEligible() = true. If I want to control that myself, on demand, I make every View Holder defaults to false and turn to true only that one that I want to execute at the moment, say as a callback to an event, like a click.

Please let me know your idea. I am open to change the PR to enhance the library with this feature. Take care and happy Thursday!

[eneim]

Hi @thiagoricieri , sorry for the wait. I will get back to this soon. FYI I just release 3.3.0 that also separates ExoPlayer from Toro. Your commit is now stay in separated toro-ext-exoplayer.