Skip to content

Commit

Permalink
fix(playback-watcher): ignore subtitles (#980)
Browse files Browse the repository at this point in the history 
It's possible that a subtitle track will have segments without any text
in them depending on how the subtitles and captions were created.
Therefor, we should ignore it in the playback-watcher where we're
checking for excessive downloads.
  • Loading branch information
@gkatsev
gkatsev authored Oct 30, 2020
1 parent 3afd29f commit 8987194
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 53 deletions.
9 changes: 0 additions & 9 deletions src/playback-watcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import window from 'global/window';
import * as Ranges from './ranges';
import logger from './util/logger';
import videojs from 'video.js';

// Set of events that reset the playback-watcher time check logic and clear the timeout
const timerCancelEvents = [
Expand Down Expand Up @@ -220,14 +219,6 @@ export default class PlaybackWatcher {
this.tech_.trigger({type: 'usage', name: `vhs-${type}-download-exclusion`});

if (type === 'subtitle') {
// TODO: Is there anything else that we can do here?
// removing the track and disabling could have accesiblity implications.
const track = loader.track();
const label = track.label || track.language || 'Unknown';

videojs.log.warn(`Text track "${label}" is not working correctly. It will be disabled and excluded.`);
track.mode = 'disabled';
this.tech_.textTracks().removeTrack(track);
return;
}

Expand Down
88 changes: 44 additions & 44 deletions test/playback-watcher.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -933,64 +933,64 @@ QUnit.module('PlaybackWatcher download detection', {
});

loaderTypes.forEach(function(type) {
QUnit.test(`detects ${type} appends without buffer changes and excludes`, function(assert) {
this.setup();
const loader = this.mpc[`${type}SegmentLoader_`];
const track = {label: 'foobar', mode: 'showing'};
if (type !== 'subtitle') {
QUnit.test(`detects ${type} appends without buffer changes and excludes`, function(assert) {
this.setup();
const loader = this.mpc[`${type}SegmentLoader_`];
const track = {label: 'foobar', mode: 'showing'};

if (type === 'subtitle') {
loader.track = () => track;
sinon.stub(this.player.tech_.textTracks(), 'removeTrack');
}
if (type === 'subtitle') {
loader.track = () => track;
sinon.stub(this.player.tech_.textTracks(), 'removeTrack');
}

this.setBuffered(videojs.createTimeRanges([[0, 30]]));
this.setBuffered(videojs.createTimeRanges([[0, 30]]));

for (let i = 0; i <= EXCLUDE_APPEND_COUNT; i++) {
loader.trigger('appendsdone');
if (i === EXCLUDE_APPEND_COUNT) {
assert.equal(this.playbackWatcher[`${type}StalledDownloads_`], 0, `append #${i} resets stalled downloads to 0`);
} else {
assert.equal(this.playbackWatcher[`${type}StalledDownloads_`], i, `append #${i + 1} ${i} stalled downloads`);
for (let i = 0; i <= EXCLUDE_APPEND_COUNT; i++) {
loader.trigger('appendsdone');
if (i === EXCLUDE_APPEND_COUNT) {
assert.equal(this.playbackWatcher[`${type}StalledDownloads_`], 0, `append #${i} resets stalled downloads to 0`);
} else {
assert.equal(this.playbackWatcher[`${type}StalledDownloads_`], i, `append #${i + 1} ${i} stalled downloads`);
}
}
}

const expectedUsage = {};
const expectedUsage = {};

expectedUsage[`vhs-${type}-download-exclusion`] = 1;
expectedUsage[`vhs-${type}-download-exclusion`] = 1;

if (type !== 'subtitle') {
expectedUsage['vhs-rendition-blacklisted'] = 1;
expectedUsage['hls-rendition-blacklisted'] = 1;
}
if (type !== 'subtitle') {
expectedUsage['vhs-rendition-blacklisted'] = 1;
expectedUsage['hls-rendition-blacklisted'] = 1;
}

assert.deepEqual(this.usageEvents, expectedUsage, 'usage as expected');
assert.deepEqual(this.usageEvents, expectedUsage, 'usage as expected');

if (type !== 'subtitle') {
const message = 'Playback cannot continue. No available working or supported playlists.';
if (type !== 'subtitle') {
const message = 'Playback cannot continue. No available working or supported playlists.';

assert.equal(this.mpcErrors, 1, 'one mpc error');
assert.equal(this.mpc.error, message, 'mpc error set');
assert.equal(this.player.error().message, message, 'player error set');
assert.equal(this.env.log.error.callCount, 1, 'player error logged');
assert.equal(this.env.log.error.args[0][1], message, 'error message as expected');
assert.equal(this.mpcErrors, 1, 'one mpc error');
assert.equal(this.mpc.error, message, 'mpc error set');
assert.equal(this.player.error().message, message, 'player error set');
assert.equal(this.env.log.error.callCount, 1, 'player error logged');
assert.equal(this.env.log.error.args[0][1], message, 'error message as expected');

this.env.log.error.resetHistory();
} else {
const message = 'Text track "foobar" is not working correctly. It will be disabled and excluded.';
this.env.log.error.resetHistory();
} else {
const message = 'Text track "foobar" is not working correctly. It will be disabled and excluded.';

assert.equal(this.mpcErrors, 0, 'no mpc error set');
assert.notOk(this.player.error(), 'no player error set');
assert.equal(this.player.textTracks().removeTrack.callCount, 1, 'text track remove called');
assert.equal(this.player.textTracks().removeTrack.args[0][0], track, 'text track remove called with expected');
assert.equal(track.mode, 'disabled', 'mode set to disabled now');
assert.equal(this.env.log.warn.callCount, 1, 'warning logged');
assert.equal(this.env.log.warn.args[0][0], message, 'warning message as expected');
assert.equal(this.mpcErrors, 0, 'no mpc error set');
assert.notOk(this.player.error(), 'no player error set');
assert.equal(this.player.textTracks().removeTrack.callCount, 1, 'text track remove called');
assert.equal(this.player.textTracks().removeTrack.args[0][0], track, 'text track remove called with expected');
assert.equal(track.mode, 'disabled', 'mode set to disabled now');
assert.equal(this.env.log.warn.callCount, 1, 'warning logged');
assert.equal(this.env.log.warn.args[0][0], message, 'warning message as expected');

this.env.log.warn.resetHistory();
}
});
this.env.log.warn.resetHistory();
}
});

if (type !== 'subtitle') {
QUnit.test(`detects ${type} appends without buffer changes and excludes many playlists`, function(assert) {
this.setup({src: 'multipleAudioGroupsCombinedMain.m3u8', type: 'application/vnd.apple.mpegurl'});

Expand Down

0 comments on commit 8987194

Please sign in to comment.