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

Add option to override native audio and video to html5 tech #5074

Merged
merged 17 commits into from
Apr 19, 2018
39 changes: 39 additions & 0 deletions src/js/tech/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,43 @@ class Html5 extends Tech {
});
}

/**
* Attempt to force override of native audiio/video tracks.
Copy link
Contributor

Choose a reason for hiding this comment

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

audiio -> audio

*
* @param {Boolean} override - If set to true native audio/video will be overridden,
* otherwise native audio/video will potentially be used.
*/
overrideNativeTracks(override) {
Copy link
Member

Choose a reason for hiding this comment

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

might be good to make this a no-op if we aren't changing the override.

Copy link
Member

Choose a reason for hiding this comment

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

we should add either an empty method or a default implementation in tech.js for this.

// If there is no behavioral change don't add/remove listeners
if (override !== (this.featuresNativeAudioTracks && this.featuresNativeAudioTracks)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

copy/paste

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That took me a minute to spot, good 👀 s

return;
}

if (this.audioTracksListeners_) {
Object.keys(this.audioTracksListeners_).forEach((eventName) => {
const elTracks = this.el().audioTracks;

elTracks.removeEventListener(eventName, this.audioTracksListeners_[eventName]);
});
}

if (this.videoTracksListeners_) {
Object.keys(this.videoTracksListeners_).forEach((eventName) => {
const elTracks = this.el().videoTracks;

elTracks.removeEventListener(eventName, this.videoTracksListeners_[eventName]);
});
}

this.featuresNativeVideoTracks = !override;
this.featuresNativeAudioTracks = !override;

this.audioTracksListeners_ = [];
Copy link
Member

Choose a reason for hiding this comment

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

should be set to null probably.

this.videoTracksListeners_ = [];

this.proxyNativeTracks_();
Copy link
Member

Choose a reason for hiding this comment

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

so basically, everything above will remove listeners and then we call proxyNativeTracks_ so that if we turn native back on it'll run but if we turned it off, proxyNativeTracks_ will exit early and not run?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds right to me.

}

/**
* Proxy all native track list events to our track lists if the browser we are playing
* in supports that type of track list.
Expand Down Expand Up @@ -256,6 +293,8 @@ class Html5 extends Tech {
}
};

this[props.getterName + 'Listeners_'] = listeners;

Object.keys(listeners).forEach((eventName) => {
const listener = listeners[eventName];

Expand Down
12 changes: 11 additions & 1 deletion src/js/tech/tech.js
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,16 @@ class Tech extends Component {
*/
setPlaysinline() {}

/**
* Attempt to force override of native audio.video tracks.
*
* @param {Boolean} override - If set to true native audio/video will be overridden,
* otherwise native audio/video will potentially be used.
*
* @abstract
*/
overrideNativeTracks() {}

/*
* Check if the tech can support the given mime-type.
*
Expand Down Expand Up @@ -1218,7 +1228,7 @@ Tech.withSourceHandlers = function(_Tech) {
if (_Tech.nativeSourceHandler) {
sh = _Tech.nativeSourceHandler;
} else {
log.error('No source hander found for the current source.');
log.error('No source handler found for the current source.');
}
}

Expand Down
113 changes: 113 additions & 0 deletions test/unit/tech/html5.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,62 @@ if (Html5.supportsNativeAudioTracks()) {
assert.equal(adds[1][0], rems[1][0], 'addtrack event handler removed');
assert.equal(adds[2][0], rems[2][0], 'removetrack event handler removed');
});

QUnit.test('should use overrideNativeTracks on audio correctly', function(assert) {
assert.expect(8);

const adds = [];
const rems = [];
const at = {
length: 0,
addEventListener: (type, fn) => {
adds.push({ type, fn });
},
removeEventListener: (type, fn) => {
rems.push({ type, fn });
}
};
const vt = {
length: 0,
addEventListener: (type, fn) => null,
removeEventListener: (type, fn) => null
};

const el = document.createElement('div');

el.audioTracks = at;
el.videoTracks = vt;

const htmlTech = new Html5({el});

assert.equal(adds.length, 3,
'should have added change, remove, add listeners');
assert.equal(rems.length, 0,
'no listeners should be removed');

htmlTech.overrideNativeTracks(true);

assert.equal(adds.length, 3,
'should not have added additional listeners');
assert.equal(rems.length, 3,
'should have removed previous three listeners');

htmlTech.overrideNativeTracks(true);

assert.equal(adds.length, 3,
'no state change so do not add listeners');
assert.equal(rems.length, 3,
'no state change so do not remove listeners');

htmlTech.overrideNativeTracks(false);

assert.equal(adds.length, 6,
'should add listeners because native tracks should be proxied');
assert.equal(rems.length, 3,
'should not remove listeners because there where none added on previous state');

htmlTech.dispose();
});
}

if (Html5.supportsNativeVideoTracks()) {
Expand Down Expand Up @@ -603,6 +659,63 @@ if (Html5.supportsNativeVideoTracks()) {
assert.equal(adds[1][0], rems[1][0], 'addtrack event handler removed');
assert.equal(adds[2][0], rems[2][0], 'removetrack event handler removed');
});

QUnit.test('should use overrideNativeTracks on video correctly', function(assert) {
assert.expect(8);

const adds = [];
const rems = [];
const vt = {
length: 0,
addEventListener: (type, fn) => {
adds.push({ type, fn });
},
removeEventListener: (type, fn) => {
rems.push({ type, fn });
}
};

const at = {
length: 0,
addEventListener: (type, fn) => null,
removeEventListener: (type, fn) => null
};

const el = document.createElement('div');

el.audioTracks = at;
el.videoTracks = vt;

const htmlTech = new Html5({el});

assert.equal(adds.length, 3,
'should have added change, remove, add listeners');
assert.equal(rems.length, 0,
'no listeners should be removed');

htmlTech.overrideNativeTracks(true);

assert.equal(adds.length, 3,
'should not have added additional listeners');
assert.equal(rems.length, 3,
'should have removed previous three listeners');

htmlTech.overrideNativeTracks(true);

assert.equal(adds.length, 3,
'no state change so do not add listeners');
assert.equal(rems.length, 3,
'no state change so do not remove listeners');

htmlTech.overrideNativeTracks(false);

assert.equal(adds.length, 6,
'should add listeners because native tracks should be proxied');
assert.equal(rems.length, 3,
'should not remove listeners because there where none added on previous state');

htmlTech.dispose();
});
}

QUnit.test('should always return currentSource_ if set', function(assert) {
Expand Down