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

feat: Utilize option to override native on tech #76

Merged
merged 6 commits into from
Jun 6, 2018
Merged

feat: Utilize option to override native on tech #76

merged 6 commits into from
Jun 6, 2018

Conversation

OshinKaramian
Copy link
Contributor

@OshinKaramian OshinKaramian commented Apr 4, 2018

Description

Utilize the addition of overrideNativeAudioTracks and overrideNativeVideoTracks as added in videojs/video.js#5074.

I'm in the process of adding tests, but would like eyes on this and the above noted PR before getting too far along.

Specific Changes proposed

This PR removes the need to set nativeAudioTracks and nativeVideoTracks to false when enabling overrideNative.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
  • Reviewed by Two Core Contributors

(tech.featuresNativeVideoTracks || tech.featuresNativeAudioTracks)) {
throw new Error('Overriding native HLS requires emulated tracks. ' +
'See https://git.io/vMpjB');
if (this.options_.overrideNative) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to fall back to the old behavior if the APIs aren't available (older video.js 6)

@forbesjo
Copy link
Contributor

This feature is now in video.js 7.0.0-rc.1, this PR can either bump up the video.js version to pass the tests or we can mock that functionality and test both with and without that API. I think we should do the latter since this PR has backwards compatible changes.

@forbesjo forbesjo removed the blocked label Jun 6, 2018
@forbesjo
Copy link
Contributor

forbesjo commented Jun 6, 2018

#102 has been merged so this PR does not need to update the package files

@forbesjo forbesjo changed the title Utilize option to override native on tech feat: Utilize option to override native on tech Jun 6, 2018
@forbesjo
Copy link
Contributor

forbesjo commented Jun 6, 2018

@forbesjo
Copy link
Contributor

forbesjo commented Jun 6, 2018

I think the REAME code snippet should show how to set it up with both the player constructor options and the video source options

@forbesjo forbesjo merged commit 5c7ab4c into videojs:master Jun 6, 2018
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