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

Adding ascending sort of TrackGroup #7715

Closed
wants to merge 1 commit into from
Closed

Adding ascending sort of TrackGroup #7715

wants to merge 1 commit into from

Conversation

yoobi
Copy link
Contributor

@yoobi yoobi commented Aug 5, 2020

This enhancement match with this issue #7709

This is my first time contributing to a project, feel free to tell me if I did something wrong.

I'm comparing the bitrate of the TrackGroup as it works for both Audio and Video.

I've also notice that the original TrackGroup from trackGroups.get(groupIndex); is always 0, maybe I could delete the first loop over the groupIndex

@google-cla
Copy link

google-cla bot commented Aug 5, 2020

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Aug 5, 2020
@yoobi
Copy link
Contributor Author

yoobi commented Aug 5, 2020

@googlebot I signed it!

@google-cla google-cla bot added cla: yes and removed cla: no labels Aug 5, 2020
@yoobi
Copy link
Contributor Author

yoobi commented Aug 5, 2020

It seems I've made a mistake, when selecting the format (ie: 250p) it is not switching to it but to another one.

@ojw28
Copy link
Contributor

ojw28 commented Aug 10, 2020

In terms of making this API generally useful, could you design it so that the caller can pass any Comparator<Format>, rather than just providing a boolean option to enable one specific type of sorting? This would also be useful for other use cases, for example sorting text tracks based on their language.

@ojw28 ojw28 self-assigned this Aug 10, 2020
@yoobi
Copy link
Contributor Author

yoobi commented Aug 19, 2020

You're right, I'll use a Comparator<Format> instead, problem is I don't see where to do the sort.
My previous problem was when I changed the order in TrackSelectionView the array was sorted and then displayed in the UI, but behind when I was clicking on a Format it was using the old array.
Or Should I pass the Comparator to MappedTrackInfo and make the sorting there ?

@yoobi
Copy link
Contributor Author

yoobi commented Aug 21, 2020

I've been doing the sorting (inside MappingTrackSelector) but the issue is once it is all sorted, it no longer goes into onTracksChanged, and I've got no error. Do you have any idea why?

@ojw28
Copy link
Contributor

ojw28 commented Aug 24, 2020

Obsoleted by #7798

@ojw28 ojw28 closed this Aug 24, 2020
@google google locked and limited conversation to collaborators Oct 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants