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

Tech 2.0 additional fixes #2166

Closed
wants to merge 11 commits into from
34 changes: 8 additions & 26 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as browser from './utils/browser.js';
import log from './utils/log.js';
import toTitleCase from './utils/to-title-case.js';
import { createTimeRange } from './utils/time-ranges.js';
import { bufferedPercent } from './utils/buffer.js';
import FullscreenApi from './fullscreen-api.js';
import MediaError from './media-error.js';
import Options from './options.js';
Expand Down Expand Up @@ -415,7 +416,12 @@ class Player extends Component {
var techOptions = assign({
'source': source,
'playerId': this.id(),
'textTracks': this.textTracks_
'techId': `${this.id()}_${techName}_api`,
'textTracks': this.textTracks_,
'autoplay': this.options_.autoplay,
'preload': this.options_.preload,
'loop': this.options_.loop,
'muted': this.options_.muted
Copy link
Member

Choose a reason for hiding this comment

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

Very nice. For #2033 I want to try to move away from using options like this and instead use the player properties (e.g. this.autoplay()), but there's still more work to do to make that even possible. Not something for this PR but just wanted to call it out as something that may change in the near future.

}, this.options_[techName.toLowerCase()]);

if (this.tag) {
Expand Down Expand Up @@ -510,9 +516,6 @@ class Player extends Component {
this.on(this.tech, 'touchmove', this.handleTechTouchMove);
this.on(this.tech, 'touchend', this.handleTechTouchEnd);

// Turn on component tap events
this.tech.emitTapEvents();

// The tap listener needs to come after the touchend listener because the tap
// listener cancels out any reportedUserActivity when setting userActive(false)
this.on(this.tech, 'tap', this.handleTechTap);
Expand Down Expand Up @@ -1148,28 +1151,7 @@ class Player extends Component {
* @return {Number} A decimal between 0 and 1 representing the percent
*/
bufferedPercent() {
var duration = this.duration(),
buffered = this.buffered(),
bufferedDuration = 0,
start, end;

if (!duration) {
return 0;
}

for (var i=0; i<buffered.length; i++){
start = buffered.start(i);
end = buffered.end(i);

// buffered end can be bigger than duration by a very small fraction
if (end > duration) {
end = duration;
}

bufferedDuration += end - start;
}

return bufferedDuration / duration;
return bufferedPercent(this.buffered(), this.duration());
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/js/tech/flash.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class Flash extends Tech {
let { source, parentEl } = options;

// Generate ID for swf object
let objId = options.playerId+'_flash_api';
let objId = options.techId;

// Merge default flashvars with ones passed in to init
let flashVars = assign({
Expand Down
2 changes: 1 addition & 1 deletion src/js/tech/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class Html5 extends Tech {

Dom.setElementAttributes(el,
assign(attributes, {
id: this.options_.playerId + '_html5_api',
id: this.options_.techId,
class: 'vjs-tech'
})
);
Expand Down
59 changes: 13 additions & 46 deletions src/js/tech/tech.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import TextTrackList from '../tracks/text-track-list';
import * as Fn from '../utils/fn.js';
import log from '../utils/log.js';
import { createTimeRange } from '../utils/time-ranges.js';
import { bufferedPercent } from '../utils/buffer.js';
import window from 'global/window';
import document from 'global/document';

Expand All @@ -21,7 +22,6 @@ import document from 'global/document';
class Tech extends Component {

constructor(options={}, ready=function(){}){
options = options || {};
// we don't want the tech to report user activity automatically.
// This is done manually in addControlsListeners
options.reportTouchActivity = false;
Expand Down Expand Up @@ -50,6 +50,9 @@ class Tech extends Component {
}

this.initTextTrackListeners();

// Turn on component tap events
this.emitTapEvents();
}

/**
Expand Down Expand Up @@ -109,15 +112,15 @@ class Tech extends Component {
this.progressInterval = this.setInterval(Fn.bind(this, function(){
// Don't trigger unless buffered amount is greater than last time

let bufferedPercent = this.bufferedPercent();
let numBufferedPercent = this.bufferedPercent();

if (this.bufferedPercent_ !== bufferedPercent) {
if (this.bufferedPercent_ !== numBufferedPercent) {
this.trigger('progress');
}

this.bufferedPercent_ = bufferedPercent;
this.bufferedPercent_ = numBufferedPercent;

if (bufferedPercent === 1) {
if (numBufferedPercent === 1) {
this.stopTrackingProgress();
}
}), 500);
Expand All @@ -127,33 +130,12 @@ class Tech extends Component {
this.duration_ = this.duration();
}

bufferedPercent() {
let bufferedDuration = 0,
start, end;

if (!this.duration_) {
return 0;
}

let buffered = this.buffered();

if (!buffered || !buffered.length) {
buffered = createTimeRange(0,0);
}

for (var i=0; i<buffered.length; i++){
start = buffered.start(i);
end = buffered.end(i);

// buffered end can be bigger than duration by a very small fraction
if (end > this.duration_) {
end = this.duration_;
}

bufferedDuration += end - start;
}
buffered() {
return createTimeRange(0, 0);
}

return bufferedDuration / this.duration_;
bufferedPercent() {
return bufferedPercent(this.buffered(), this.duration_);
}

stopTrackingProgress() {
Expand All @@ -166,21 +148,6 @@ class Tech extends Component {

this.on('play', this.trackCurrentTime);
this.on('pause', this.stopTrackingCurrentTime);
// timeupdate is also called by .currentTime whenever current time is set

// Watch for native timeupdate event only
var onTimeUpdate = function(e){
if (e.manuallyTriggered) return;

this.off('timeupdate', onTimeUpdate);

// Update known progress support for this playback technology
this.featuresTimeupdateEvents = true;
// Turn off manual progress tracking
this.manualTimeUpdatesOff();
};

this.on('timeupdate', onTimeUpdate);
}

manualTimeUpdatesOff() {
Expand Down
35 changes: 35 additions & 0 deletions src/js/utils/buffer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { createTimeRange } from './time-ranges.js';

/**
* Compute how much your video has been buffered
* @param {Object} Buffered object
* @param {Number} Total duration
* @return {Number} Percent buffered of the total duration
* @private
*/
export function bufferedPercent(buffered, duration) {
var bufferedDuration = 0,
start, end;

if (!duration) {
return 0;
}

if (!buffered || !buffered.length) {
buffered = createTimeRange(0, 0);
}

for (let i = 0; i < buffered.length; i++){
start = buffered.start(i);
end = buffered.end(i);

// buffered end can be bigger than duration by a very small fraction
if (end > duration) {
end = duration;
}

bufferedDuration += end - start;
}

return bufferedDuration / duration;
}
17 changes: 0 additions & 17 deletions test/unit/tech/tech.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,6 @@ test('should synthesize timeupdate events by default', function() {
equal(timeupdates, 1, 'triggered at least one timeupdate');
});

test('stops timeupdates if the tech produces them natively', function() {
var timeupdates = 0, tech, expected;
tech = new Tech();
tech.on('timeupdate', function() {
timeupdates++;
});

tech.trigger('play');

// simulate a native timeupdate event
tech.trigger('timeupdate');

expected = timeupdates;
this.clock.tick(10 * 1000);
equal(timeupdates, expected, 'did not simulate timeupdates');
});

test('stops manual timeupdates while paused', function() {
var timeupdates = 0, tech, expected;
tech = new Tech();
Expand Down