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

Fix for #1150 and improvements for #1143 #1162

Merged
merged 2 commits into from
Aug 1, 2018

Conversation

ashnfb
Copy link
Contributor

@ashnfb ashnfb commented Aug 1, 2018

@cobarx @ahmetabdi
Fixes #1150
Improves #1143 (onLoad -> textTracks returns only valid sideloaded vtt)

@cobarx
Copy link
Contributor

cobarx commented Aug 1, 2018

@ashnfb Thanks for all your contributions!

Did you by any chance figure out how to disable sideloaded tracks? I'm still waiting to hear back from my contact.

@cobarx cobarx merged commit bf59742 into TheWidlarzGroup:master Aug 1, 2018
@ashnfb
Copy link
Contributor Author

ashnfb commented Aug 1, 2018

@cobarx I gave disabling tracks a try. I'm a little frustrated with the lack of AVPlayer docs. I'll try and spend a bit more time on it.

@cobarx
Copy link
Contributor

cobarx commented Aug 2, 2018

@ashnfb Yup, AVPlayer is great when it works, when it doesn't you're just out of luck.

I was thinking about this last night and I don't think we want the fallback code if it can't find the requested track. The way that code works right now is it will only use the fallback if the system Captions preference is enabled, so the behavior will be inconsistent. I think it's totally reasonable to say that if you request an invalid track nothing happens. Really it should throw an error, but there's no framework for it (onError has already been documented as only being for load errors). Also, we don't use the fallback code on Android, so the behaviors are different. Do you have an objection to me reverting that part of the code?

@ashnfb
Copy link
Contributor Author

ashnfb commented Aug 2, 2018

@cobarx no objection to reverting that code!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants