Skip to content

Commit

Permalink
revert: perf: Cache currentTime and buffered from Flash (#3705)
Browse files Browse the repository at this point in the history
This reverts commit 45ffa81.
  • Loading branch information
gkatsev committed Nov 14, 2016
1 parent 252bcee commit 4c80e2a
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 137 deletions.
31 changes: 0 additions & 31 deletions src/js/tech/flash-cache.js

This file was deleted.

45 changes: 16 additions & 29 deletions src/js/tech/flash.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import * as Dom from '../utils/dom.js';
import * as Url from '../utils/url.js';
import { createTimeRange } from '../utils/time-ranges.js';
import FlashRtmpDecorator from './flash-rtmp';
import timeExpiringCache from './flash-cache.js';
import Component from '../component';
import window from 'global/window';
import assign from 'object.assign';
Expand Down Expand Up @@ -60,34 +59,6 @@ class Flash extends Tech {
this.on('seeked', function() {
this.lastSeekTarget_ = undefined;
});

// calling into the SWF can be expensive, especially if Flash is
// busy rendering video frames
// automatically cache commonly used properties for a short period
// of time so that multiple calls within a short time period don't
// all pay a big performance penalty for properties that change
// relatively slowly over time
const getCurrentTimeCached = timeExpiringCache(() => {
return this.el_.vjs_getProperty('currentTime');
}, 100);

this.currentTime = (time) => {
// when seeking make the reported time keep up with the requested time
// by reading the time we're seeking to
if (this.seeking()) {
return this.lastSeekTarget_ || 0;
}

return getCurrentTimeCached();
};
this.buffered = timeExpiringCache(() => {
const ranges = this.el_.vjs_getProperty('buffered');

if (ranges.length === 0) {
return createTimeRange();
}
return createTimeRange(ranges[0][0], ranges[0][1]);
}, 100);
}

/**
Expand Down Expand Up @@ -242,6 +213,14 @@ class Flash extends Tech {
* @return {Number} Current time
* @method currentTime
*/
currentTime(time) {
// when seeking make the reported time keep up with the requested time
// by reading the time we're seeking to
if (this.seeking()) {
return this.lastSeekTarget_ || 0;
}
return this.el_.vjs_getProperty('currentTime');
}

/**
* Get current source
Expand Down Expand Up @@ -315,6 +294,14 @@ class Flash extends Tech {
* @return {TimeRangeObject}
* @method buffered
*/
buffered() {
const ranges = this.el_.vjs_getProperty('buffered');

if (ranges.length === 0) {
return createTimeRange();
}
return createTimeRange(ranges[0][0], ranges[0][1]);
}

/**
* Get fullscreen support -
Expand Down
50 changes: 0 additions & 50 deletions test/unit/tech/flash-cache.test.js

This file was deleted.

57 changes: 30 additions & 27 deletions test/unit/tech/flash.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,50 +31,53 @@ QUnit.test('Flash.canPlaySource', function(assert) {
});

QUnit.test('currentTime', function(assert) {
const mockFlash = new MockFlash();
const getCurrentTime = Flash.prototype.currentTime;
const setCurrentTime = Flash.prototype.setCurrentTime;
let seekingCount = 0;
let seeking = false;
let setPropVal;
let getPropVal;
let result;

// Mock out a Flash instance to avoid creating the swf object
mockFlash.el_ = {
/* eslint-disable camelcase */
vjs_setProperty(prop, val) {
setPropVal = val;
const mockFlash = {
el_: {
/* eslint-disable camelcase */
vjs_setProperty(prop, val) {
setPropVal = val;
},
vjs_getProperty() {
return getPropVal;
}
/* eslint-enable camelcase */
},
vjs_getProperty() {
return getPropVal;
}
/* eslint-enable camelcase */
};
mockFlash.seekable = function() {
return createTimeRange(5, 1000);
};
mockFlash.trigger = function(event) {
if (event === 'seeking') {
seekingCount++;
seekable() {
return createTimeRange(5, 1000);
},
trigger(event) {
if (event === 'seeking') {
seekingCount++;
}
},
seeking() {
return seeking;
}
};
mockFlash.seeking = function() {
return seeking;
};

// Test the currentTime getter
getPropVal = 3;
result = mockFlash.currentTime();
result = getCurrentTime.call(mockFlash);
assert.equal(result, 3, 'currentTime is retreived from the swf element');

// Test the currentTime setter
mockFlash.setCurrentTime(10);
setCurrentTime.call(mockFlash, 10);
assert.equal(setPropVal, 10, 'currentTime is set on the swf element');
assert.equal(seekingCount, 1, 'triggered seeking');

// Test current time while seeking
mockFlash.setCurrentTime(20);
setCurrentTime.call(mockFlash, 20);
seeking = true;
result = mockFlash.currentTime();
result = getCurrentTime.call(mockFlash);
assert.equal(result,
20,
'currentTime is retrieved from the lastSeekTarget while seeking');
Expand All @@ -84,13 +87,13 @@ QUnit.test('currentTime', function(assert) {
assert.equal(seekingCount, 2, 'triggered seeking');

// clamp seeks to seekable
mockFlash.setCurrentTime(1001);
result = mockFlash.currentTime();
setCurrentTime.call(mockFlash, 1001);
result = getCurrentTime.call(mockFlash);
assert.equal(result, mockFlash.seekable().end(0), 'clamped to the seekable end');
assert.equal(seekingCount, 3, 'triggered seeking');

mockFlash.setCurrentTime(1);
result = mockFlash.currentTime();
setCurrentTime.call(mockFlash, 1);
result = getCurrentTime.call(mockFlash);
assert.equal(result, mockFlash.seekable().start(0), 'clamped to the seekable start');
assert.equal(seekingCount, 4, 'triggered seeking');
});
Expand Down

0 comments on commit 4c80e2a

Please sign in to comment.