Skip to content

Commit 9b81afe

Browse files
authored
fix: Use timeupdate as well as rvfc/raf for cues (#7918)
Use the timeupdate event as well as the rvfc and raf callbacks to check cues. This is a bit overkill for "usual" playback but avoids edge cases. If the more preceise callback trigger first the cue will update but the timeupdate event should catch any that were missed, notwithstanding that timeupdate was always somewhat unpredictable. Fixes #7910 (audio in video els and Samsung being weird) and fixes #7902 (no updates off screen).
1 parent 2810507 commit 9b81afe

File tree

2 files changed

+43
-12
lines changed

2 files changed

+43
-12
lines changed

src/js/tracks/text-track.js

+11-3
Original file line numberDiff line numberDiff line change
@@ -184,13 +184,15 @@ class TextTrack extends Track {
184184
const activeCues = new TextTrackCueList(this.activeCues_);
185185
let changed = false;
186186

187-
this.timeupdateHandler = Fn.bind(this, function() {
187+
this.timeupdateHandler = Fn.bind(this, function(event = {}) {
188188
if (this.tech_.isDisposed()) {
189189
return;
190190
}
191191

192192
if (!this.tech_.isReady_) {
193-
this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);
193+
if (event.type !== 'timeupdate') {
194+
this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);
195+
}
194196

195197
return;
196198
}
@@ -205,7 +207,9 @@ class TextTrack extends Track {
205207
changed = false;
206208
}
207209

208-
this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);
210+
if (event.type !== 'timeupdate') {
211+
this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);
212+
}
209213

210214
});
211215

@@ -368,14 +372,18 @@ class TextTrack extends Track {
368372
}
369373

370374
startTracking() {
375+
// More precise cues based on requestVideoFrameCallback with a requestAnimationFram fallback
371376
this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);
377+
// Also listen to timeupdate in case rVFC/rAF stops (window in background, audio in video el)
378+
this.tech_.on('timeupdate', this.timeupdateHandler);
372379
}
373380

374381
stopTracking() {
375382
if (this.rvf_) {
376383
this.tech_.cancelVideoFrameCallback(this.rvf_);
377384
this.rvf_ = undefined;
378385
}
386+
this.tech_.off('timeupdate', this.timeupdateHandler);
379387
}
380388

381389
/**

test/unit/tracks/text-track.test.js

+32-9
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,9 @@ QUnit.test('does not fire cuechange before Tech is ready', function(assert) {
279279
return 0;
280280
};
281281

282+
// `playing` would trigger rvfc or raf, `timeupdate` for fallback
282283
player.tech_.trigger('playing');
284+
player.tech_.trigger('timeupdate');
283285
assert.equal(changes, 0, 'a cuechange event is not triggered');
284286

285287
player.tech_.on('ready', function() {
@@ -292,6 +294,11 @@ QUnit.test('does not fire cuechange before Tech is ready', function(assert) {
292294

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

297+
player.tech_.trigger('timeupdate');
298+
clock.tick(1);
299+
300+
assert.equal(changes, 2, 'a cuechange event trigger not duplicated by timeupdate');
301+
295302
tt.off();
296303
player.dispose();
297304
clock.restore();
@@ -311,31 +318,45 @@ QUnit.test('fires cuechange when cues become active and inactive', function(asse
311318
const cuechangeHandler = function() {
312319
changes++;
313320
};
321+
let fakeCurrentTime = 0;
322+
323+
player.tech_.currentTime = function() {
324+
return fakeCurrentTime;
325+
};
314326

315327
tt.addCue({
316328
id: '1',
317329
startTime: 1,
318330
endTime: 5
319331
});
332+
tt.addCue({
333+
id: '2',
334+
startTime: 11,
335+
endTime: 14
336+
});
320337

321338
tt.oncuechange = cuechangeHandler;
322339
tt.addEventListener('cuechange', cuechangeHandler);
323340

324-
player.tech_.currentTime = function() {
325-
return 2;
326-
};
341+
fakeCurrentTime = 2;
342+
player.tech_.trigger('playing');
327343

344+
assert.equal(changes, 2, 'a cuechange event trigger addEventListener and oncuechange (rvfc/raf)');
345+
346+
fakeCurrentTime = 7;
328347
player.tech_.trigger('playing');
329348

330-
assert.equal(changes, 2, 'a cuechange event trigger addEventListener and oncuechange');
349+
assert.equal(changes, 4, 'a cuechange event trigger addEventListener and oncuechange (rvfc/raf)');
331350

332-
player.tech_.currentTime = function() {
333-
return 7;
334-
};
351+
fakeCurrentTime = 12;
352+
player.tech_.trigger('timeupdate');
335353

336-
player.tech_.trigger('playing');
354+
assert.equal(changes, 6, 'a cuechange event trigger addEventListener and oncuechange (timeupdate)');
355+
356+
fakeCurrentTime = 17;
357+
player.tech_.trigger('timeupdate');
337358

338-
assert.equal(changes, 4, 'a cuechange event trigger addEventListener and oncuechange');
359+
assert.equal(changes, 8, 'a cuechange event trigger addEventListener and oncuechange (timeupdate)');
339360

340361
tt.off();
341362
player.dispose();
@@ -365,6 +386,7 @@ QUnit.test('enabled and disabled cuechange handler when changing mode to hidden'
365386
return 2;
366387
};
367388
player.tech_.trigger('playing');
389+
player.tech_.trigger('timeupdate');
368390

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

@@ -376,6 +398,7 @@ QUnit.test('enabled and disabled cuechange handler when changing mode to hidden'
376398
return 7;
377399
};
378400
player.tech_.trigger('playing');
401+
player.tech_.trigger('timeupdate');
379402

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

0 commit comments

Comments
 (0)