Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change in behavior of selection override in 2.16.0 #9665

Closed
moneytoo opened this issue Nov 8, 2021 · 6 comments
Closed

Change in behavior of selection override in 2.16.0 #9665

moneytoo opened this issue Nov 8, 2021 · 6 comments

Comments

@moneytoo
Copy link
Contributor

moneytoo commented Nov 8, 2021

Follow up on #9649. (@tonihei)

I applied eb678a7 on top of r2.16.0 but I still have issues working with selection overrides. The same code which works on 2.15.1 doesn't work on 2.16.0. (Is there possibly more to this issue?)

for (int rendererIndex = 0; rendererIndex < mappedTrackInfo.getRendererCount(); rendererIndex++) {
    if (mappedTrackInfo.getRendererType(rendererIndex) == C.TRACK_TYPE_TEXT) {
        final TrackGroupArray trackGroups = mappedTrackInfo.getTrackGroups(rendererIndex);
        final DefaultTrackSelector.SelectionOverride selectionOverride =
                trackSelector.getParameters().getSelectionOverride(rendererIndex, trackGroups);
        DefaultTrackSelector.Parameters parameters = trackSelector.getParameters();

        if (parameters.getRendererDisabled(rendererIndex)) {
            Log.d("Test", "Disabled");
        } else if (selectionOverride == null) {
            Log.d("Test", "No selection override");
        } else {
            Log.d("Test", "Index=" + selectionOverride.groupIndex);
        }
    }
}

On 2.15.1, I can use this to detect subtitle index. (Testing with the same video file as in the linked issue)

On 2.16.0, the selection override is always null.

I noticed that methods like DefaultTrackSelector.ParametersBuilder.setSelectionOverride() etc. are deprecated but nothing from the code above(getSelectionOverride). (I would still expect the code to work the same though).

  • ExoPlayer version number: 2.16.0
  • Android version: Android 10
  • Android device: OnePlus 7
@tonihei
Copy link
Collaborator

tonihei commented Nov 9, 2021

I believe this is working as intended, although I can see how it might not be obvious why.

ExoPlayer 2.16.0 introduced new track selection APIs in the Player interface. In particular, TrackSelectionParameters allows to set track overrides. The "old" way of using DefaultTrackSelector still works, but has a different set of (now deprecated) selection overrides, which work in a slightly different way. Your code snippet above checks the old overrides, but StyledPlayerView started setting the new overrides, that's why you can't see them anymore.

In any case, the selection overrides are the input to the track selection algorithm, not the output that tells you which tracks are actually selected. So to check which text track is selected, you should not query the configuration parameters, but the actually selected tracks:

for(TrackGroupInfo groupInfo : player.getCurrentTracksInfo().getTrackGroupInfos()) {
  if (groupInfo.getTrackType() == C.TRACK_TYPE_TEXT && groupInfo.isSelected()) {
    Log.d("Test", "Selected text language: " + groupInfo.getTrackGroup().getFormat(0).language);
  }
}

There are still two things to fix here:

  1. trackSelector.getParameters().getSelectionOverride should be deprecated in line with all the setters so that it would have been clear to you that this code is not up to date.
  2. We are missing some documentation for how to use the new track selection APIs in general.

@moneytoo
Copy link
Contributor Author

What would be the best way to detect user selected "Auto" audio track (in StyledPlayerView)? Right now I'm detecting it by analyzing Format mime type of TrackSelectionOverrides from ExoPlayer.getTrackSelectionParameters(). But that doesn't seem like the best way as I do not filter it by C.TRACK_TYPE_AUDIO...

@tonihei
Copy link
Collaborator

tonihei commented Nov 11, 2021

I think this is the only way to make sure, yes. We have a private method TrackSelectionOverride.getTrackType which returns the track type, seems we could make this public.

tonihei added a commit that referenced this issue Nov 11, 2021
The setters in the Builder are already deprecated and using the
old getter is error-prone as they only return the overrides set
with the deprecated setters.

Issue: #9665
PiperOrigin-RevId: 408817640
tonihei added a commit that referenced this issue Nov 11, 2021
This method is helpful when iterating the list of track overrides
to figure out which type the override applies to.

Issue: #9665
PiperOrigin-RevId: 409108977
tonihei added a commit that referenced this issue Nov 11, 2021
The setters in the Builder are already deprecated and using the
old getter is error-prone as they only return the overrides set
with the deprecated setters.

Issue: #9665
PiperOrigin-RevId: 408817640
tonihei added a commit that referenced this issue Nov 11, 2021
This method is helpful when iterating the list of track overrides
to figure out which type the override applies to.

Issue: #9665
PiperOrigin-RevId: 409108977
@krocard
Copy link
Contributor

krocard commented Nov 11, 2021

What would be the best way to detect user selected "Auto" audio track (in StyledPlayerView)?

The only way right now is to check if any trackGroup in ExoPlayer.getCurrentTracksInfo() is currently overridden. This is the same as the old override.

Just looking for any overrides of a given type doesn't give the expected information because that override could be left from a previous Media. Eg. VideoA has an override, VideoB starts playing, the VideoA override is still present but ignored as it doesn't match any trackGroup from VideoB (the player doesn't remove overrides on track transition).

@tonihei I had considered to change the selected boolean in each TrackInfo to a selectedReason flag. That would allow to specify why each trackGroup was selected (eg, override).
I had

@tonihei
Copy link
Collaborator

tonihei commented Nov 11, 2021

Ah, yes, for playlists you'd also need to check if the track overrides are actually part of the current item.

I had considered to change the selected boolean in each TrackInfo to a selectedReason flag. That would allow to specify why each trackGroup was selected (eg, override).

I wonder if this is confusing because you can also select tracks by specifying a preferred language for example and for someone looking at the selected tracks, this is probably the same case (=a specific parameter preference has caused this selection). But if we include these settings in the reasons then it becomes hard to track which specific setting was responsible for selecting a track.

@tonihei
Copy link
Collaborator

tonihei commented Nov 11, 2021

I'll close this issue for now because all immediate bugs/questions have been addressed I believe.

@tonihei tonihei closed this as completed Nov 11, 2021
icbaker pushed a commit to androidx/media that referenced this issue Nov 19, 2021
The setters in the Builder are already deprecated and using the
old getter is error-prone as they only return the overrides set
with the deprecated setters.

Issue: google/ExoPlayer#9665
PiperOrigin-RevId: 408817640
icbaker pushed a commit to androidx/media that referenced this issue Nov 19, 2021
This method is helpful when iterating the list of track overrides
to figure out which type the override applies to.

Issue: google/ExoPlayer#9665
PiperOrigin-RevId: 409108977
@google google locked and limited conversation to collaborators Jan 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants