Skip to content

Commit

Permalink
feat: Greater text track precision using requestVideoFrameCallback (#…
Browse files Browse the repository at this point in the history 
…7633)
  • Loading branch information
@mister-ben
mister-ben committed Mar 2, 2022
1 parent 64e55f5 commit 1179826
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 17 deletions.
33 changes: 33 additions & 0 deletions src/js/tech/html5.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,32 @@ class Html5 extends Tech {
return this.el_.requestPictureInPicture();
}

/**
* Native requestVideoFrameCallback if supported by browser/tech, or fallback
*
* @param {function} cb function to call
* @return {number} id of request
*/
requestVideoFrameCallback(cb) {
if (this.featuresVideoFrameCallback) {
return this.el_.requestVideoFrameCallback(cb);
}
return super.requestVideoFrameCallback(cb);
}

/**
* Native or fallback requestVideoFrameCallback
*
* @param {number} id request id to cancel
*/
cancelVideoFrameCallback(id) {
if (this.featuresVideoFrameCallback) {
this.el_.cancelVideoFrameCallback(id);
} else {
super.cancelVideoFrameCallback(id);
}
}

/**
* A getter/setter for the `Html5` Tech's source object.
* > Note: Please use {@link Html5#setSource}
Expand Down Expand Up @@ -1299,6 +1325,13 @@ Html5.prototype.featuresProgressEvents = true;
*/
Html5.prototype.featuresTimeupdateEvents = true;

/**
* Whether the HTML5 el supports `requestVideoFrameCallback`
*
* @type {boolean}
*/
Html5.prototype.featuresVideoFrameCallback = !!(Html5.TEST_VID && Html5.TEST_VID.requestVideoFrameCallback);

// HTML5 Feature detection and Device Fixes --------------------------------- //
let canPlayType;

Expand Down
48 changes: 48 additions & 0 deletions src/js/tech/tech.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {isPlain} from '../utils/obj';
import * as TRACK_TYPES from '../tracks/track-types';
import {toTitleCase, toLowerCase} from '../utils/string-cases.js';
import vtt from 'videojs-vtt.js';
import * as Guid from '../utils/guid.js';

/**
* An Object containing a structure like: `{src: 'url', type: 'mimetype'}` or string
Expand Down Expand Up @@ -103,6 +104,8 @@ class Tech extends Component {
this.stopTrackingCurrentTime_ = (e) => this.stopTrackingCurrentTime(e);
this.disposeSourceHandler_ = (e) => this.disposeSourceHandler(e);

this.queuedHanders_ = new Set();

// keep track of whether the current source has played at all to
// implement a very limited played()
this.hasStarted_ = false;
Expand Down Expand Up @@ -857,6 +860,43 @@ class Tech extends Component {
*/
setDisablePictureInPicture() {}

/**
* A fallback implementation of requestVideoFrameCallback using requestAnimationFrame
*
* @param {function} cb
* @return {number} request id
*/
requestVideoFrameCallback(cb) {
const id = Guid.newGUID();

if (this.paused()) {
this.queuedHanders_.add(id);
this.one('playing', () => {
if (this.queuedHanders_.has(id)) {
this.queuedHanders_.delete(id);
cb();
}
});
} else {
this.requestNamedAnimationFrame(id, cb);
}

return id;
}

/**
* A fallback implementation of cancelVideoFrameCallback
*
* @param {number} id id of callback to be cancelled
*/
cancelVideoFrameCallback(id) {
if (this.queuedHanders_.has(id)) {
this.queuedHanders_.delete(id);
} else {
this.cancelNamedAnimationFrame(id);
}
}

/**
* A method to set a poster from a `Tech`.
*
Expand Down Expand Up @@ -1171,6 +1211,14 @@ Tech.prototype.featuresTimeupdateEvents = false;
*/
Tech.prototype.featuresNativeTextTracks = false;

/**
* Boolean indicating whether the `Tech` supports `requestVideoFrameCallback`.
*
* @type {boolean}
* @default
*/
Tech.prototype.featuresVideoFrameCallback = false;

/**
* A functional mixin for techs that want to use the Source Handler pattern.
* Source handlers are scripts for handling specific formats.
Expand Down
33 changes: 27 additions & 6 deletions src/js/tracks/text-track.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,18 @@ class TextTrack extends Track {
const cues = new TextTrackCueList(this.cues_);
const activeCues = new TextTrackCueList(this.activeCues_);
let changed = false;
const timeupdateHandler = Fn.bind(this, function() {
if (!this.tech_.isReady_ || this.tech_.isDisposed()) {

this.timeupdateHandler = Fn.bind(this, function() {
if (this.tech_.isDisposed()) {
return;
}

if (!this.tech_.isReady_) {
this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);

return;
}

// Accessing this.activeCues for the side-effects of updating itself
// due to its nature as a getter function. Do not remove or cues will
// stop updating!
Expand All @@ -197,15 +204,18 @@ class TextTrack extends Track {
this.trigger('cuechange');
changed = false;
}

this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);

});

const disposeHandler = () => {
this.tech_.off('timeupdate', timeupdateHandler);
this.stopTracking();
};

this.tech_.one('dispose', disposeHandler);
if (mode !== 'disabled') {
this.tech_.on('timeupdate', timeupdateHandler);
this.startTracking();
}

Object.defineProperties(this, {
Expand Down Expand Up @@ -251,10 +261,10 @@ class TextTrack extends Track {
// On-demand load.
loadTrack(this.src, this);
}
this.tech_.off('timeupdate', timeupdateHandler);
this.stopTracking();

if (mode !== 'disabled') {
this.tech_.on('timeupdate', timeupdateHandler);
this.startTracking();
}
/**
* An event that fires when mode changes on this track. This allows
Expand Down Expand Up @@ -357,6 +367,17 @@ class TextTrack extends Track {
}
}

startTracking() {
this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);
}

stopTracking() {
if (this.rvf_) {
this.tech_.cancelVideoFrameCallback(this.rvf_);
this.rvf_ = undefined;
}
}

/**
* Add a cue to the internal list of cues.
*
Expand Down
24 changes: 16 additions & 8 deletions test/unit/tracks/text-track.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ QUnit.test('can only remove one cue at a time', function(assert) {

QUnit.test('does not fire cuechange before Tech is ready', function(assert) {
const done = assert.async();
const clock = sinon.useFakeTimers();
const player = TestHelpers.makePlayer({techfaker: {autoReady: false}});
let changes = 0;
const tt = new TextTrack({
Expand All @@ -278,23 +279,26 @@ QUnit.test('does not fire cuechange before Tech is ready', function(assert) {
return 0;
};

player.tech_.trigger('timeupdate');
player.tech_.trigger('playing');
assert.equal(changes, 0, 'a cuechange event is not triggered');

player.tech_.on('ready', function() {
player.tech_.currentTime = function() {
return 0.2;
};

player.tech_.trigger('timeupdate');
player.tech_.trigger('playing');
clock.tick(1);

assert.equal(changes, 2, 'a cuechange event trigger addEventListener and oncuechange');

tt.off();
player.dispose();
clock.restore();
done();
});
player.tech_.triggerReady();
clock.tick(1);
});

QUnit.test('fires cuechange when cues become active and inactive', function(assert) {
Expand All @@ -321,15 +325,15 @@ QUnit.test('fires cuechange when cues become active and inactive', function(asse
return 2;
};

player.tech_.trigger('timeupdate');
player.tech_.trigger('playing');

assert.equal(changes, 2, 'a cuechange event trigger addEventListener and oncuechange');

player.tech_.currentTime = function() {
return 7;
};

player.tech_.trigger('timeupdate');
player.tech_.trigger('playing');

assert.equal(changes, 4, 'a cuechange event trigger addEventListener and oncuechange');

Expand Down Expand Up @@ -360,17 +364,18 @@ QUnit.test('enabled and disabled cuechange handler when changing mode to hidden'
player.tech_.currentTime = function() {
return 2;
};
player.tech_.trigger('timeupdate');
player.tech_.trigger('playing');

assert.equal(changes, 1, 'a cuechange event trigger');

changes = 0;
// debugger;
tt.mode = 'disabled';

player.tech_.currentTime = function() {
return 7;
};
player.tech_.trigger('timeupdate');
player.tech_.trigger('playing');

assert.equal(changes, 0, 'NO cuechange event trigger');

Expand All @@ -379,6 +384,7 @@ QUnit.test('enabled and disabled cuechange handler when changing mode to hidden'
});

QUnit.test('enabled and disabled cuechange handler when changing mode to showing', function(assert) {
const clock = sinon.useFakeTimers();
const player = TestHelpers.makePlayer();
let changes = 0;
const tt = new TextTrack({
Expand All @@ -401,7 +407,8 @@ QUnit.test('enabled and disabled cuechange handler when changing mode to showing
player.tech_.currentTime = function() {
return 2;
};
player.tech_.trigger('timeupdate');
player.tech_.trigger('playing');
clock.tick(10);

assert.equal(changes, 1, 'a cuechange event trigger');

Expand All @@ -411,12 +418,13 @@ QUnit.test('enabled and disabled cuechange handler when changing mode to showing
player.tech_.currentTime = function() {
return 7;
};
player.tech_.trigger('timeupdate');
player.tech_.trigger('playing');

assert.equal(changes, 0, 'NO cuechange event trigger');

tt.off();
player.dispose();
clock.restore();
});

QUnit.test('if preloadTextTracks is false, default tracks are not parsed until mode is showing', function(assert) {
Expand Down
6 changes: 3 additions & 3 deletions test/unit/tracks/text-tracks.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ QUnit.test('no lang attribute on cue elements if one is provided', function(asse
player.tech_.textTracks().addTrack(tt);

player.currentTime(2);
player.trigger('timeupdate');
player.tech_.trigger('playing');

assert.notOk(tt.activeCues[0].displayState.hasAttribute('lang'), 'no lang attribute should be set');

Expand All @@ -361,7 +361,7 @@ QUnit.test('set lang attribute on cue elements if one is provided', function(ass
player.tech_.textTracks().addTrack(tt);

player.currentTime(2);
player.trigger('timeupdate');
player.tech_.trigger('playing');

assert.equal(tt.activeCues[0].displayState.getAttribute('lang'), 'en', 'the lang should be set to en');

Expand Down Expand Up @@ -404,7 +404,7 @@ QUnit.test('removes cuechange event when text track is hidden for emulated track
player.tech_.currentTime = function() {
return 3;
};
player.tech_.trigger('timeupdate');
player.tech_.trigger('playing');
assert.equal(
numTextTrackChanges, 3,
'texttrackchange should be triggered once for the cuechange'
Expand Down

0 comments on commit 1179826

Please sign in to comment.