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

Overrides not populated by default in TrackSelectionDialogBuilder #10429

Closed
1 task
TheBeastLT opened this issue Jul 12, 2022 · 2 comments
Closed
1 task

Overrides not populated by default in TrackSelectionDialogBuilder #10429

TheBeastLT opened this issue Jul 12, 2022 · 2 comments
Assignees

Comments

@TheBeastLT
Copy link

ExoPlayer Version

2.18.0

Devices that reproduce the issue

Any

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Yes

Reproduction steps

When building TrackSelectionDialogBuilder using constructor with the Player argument, builder does not take into account existing overrides and by default the dialog will always select Auto option.

Expected result

In the TrackSelectionDialogBuilder constructor it could already take available overrides from the player instance via player.getTrackSelectionParameters().getOverrides() instead of having empty map:


So that you wouldn't need to explore the source code to understand why the previous selection in the dialog is not pre-selected when opening the dialog again.

Actual result

When constructing TrackSelectionDialogBuilder the overrides are left empty and you have to know that you need to provide them manually via .setOverrides(player.getTrackSelectionParameters().getOverrides())

Media

Not applicable

Bug Report

@marcbaechinger
Copy link
Contributor

marcbaechinger commented Jul 13, 2022

Thanks for reporting!

To achieve what you want I think you can do

TrackSelectionDialogBuilder builder =
          new TrackSelectionDialogBuilder(this, "Select tracks", player, C.TRACK_TYPE_VIDEO)
          .setOverrides(player.getTrackSelectionParameters().overrides);

I'm not sure about whether not setting the overrides in the constructor is the intended behaviour. It's meant to present the dialog without a pre-selection. The selection then can be done by using the setter TrackSelectionDialogBuilder.setOverrides(Map).

Agreed that it could be the other way around and then to prevent a preselection, an app can do TrackSelectionDialogBuilder.setOverrides(Collections.emptyMap()).

We need to discuss internally whether we want to change this to default to a pre-selection when the player is passed. Do you agree I change this from bug to enhancement? As in, by using the setter you can achieve what you want?

@TheBeastLT
Copy link
Author

To achieve what you want I think you can do

TrackSelectionDialogBuilder builder =
          new TrackSelectionDialogBuilder(this, "Select tracks", player, C.TRACK_TYPE_VIDEO)
      .setOverrides(player.getTrackSelectionParameters().overrides);

Yes, that's what I do now, just that I had to dig into code a bit to understand why the previous selection was not saved and would always reset. So that waa a bit confusing and unexpected, if you have to use this dialog.

Do you agree I change this from bug to enhancement?

Sure, that's fine.

rohitjoins pushed a commit to androidx/media that referenced this issue Jul 21, 2022
rohitjoins pushed a commit that referenced this issue Jul 21, 2022
tonihei pushed a commit to androidx/media that referenced this issue Aug 8, 2022
…anges

#minor-release

PiperOrigin-RevId: 462391856
@google google locked and limited conversation to collaborators Sep 20, 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