From d06ae2236e0e1618db4bd3aadc5fa71f53374d61 Mon Sep 17 00:00:00 2001
From: mister-ben <git@misterben.me>
Date: Fri, 9 Sep 2022 19:50:21 +0200
Subject: [PATCH] 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).
---
 src/js/tracks/text-track.js         | 14 +++++++---
 test/unit/tracks/text-track.test.js | 41 ++++++++++++++++++++++-------
 2 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/src/js/tracks/text-track.js b/src/js/tracks/text-track.js
index 1b3ffdf9f6..caf766c32b 100644
--- a/src/js/tracks/text-track.js
+++ b/src/js/tracks/text-track.js
@@ -184,13 +184,15 @@ class TextTrack extends Track {
     const activeCues = new TextTrackCueList(this.activeCues_);
     let changed = false;
 
-    this.timeupdateHandler = Fn.bind_(this, function() {
+    this.timeupdateHandler = Fn.bind_(this, function(event = {}) {
       if (this.tech_.isDisposed()) {
         return;
       }
 
       if (!this.tech_.isReady_) {
-        this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);
+        if (event.type !== 'timeupdate') {
+          this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);
+        }
 
         return;
       }
@@ -205,7 +207,9 @@ class TextTrack extends Track {
         changed = false;
       }
 
-      this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);
+      if (event.type !== 'timeupdate') {
+        this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);
+      }
 
     });
 
@@ -368,7 +372,10 @@ class TextTrack extends Track {
   }
 
   startTracking() {
+    // More precise cues based on requestVideoFrameCallback with a requestAnimationFram fallback
     this.rvf_ = this.tech_.requestVideoFrameCallback(this.timeupdateHandler);
+    // Also listen to timeupdate in case rVFC/rAF stops (window in background, audio in video el)
+    this.tech_.on('timeupdate', this.timeupdateHandler);
   }
 
   stopTracking() {
@@ -376,6 +383,7 @@ class TextTrack extends Track {
       this.tech_.cancelVideoFrameCallback(this.rvf_);
       this.rvf_ = undefined;
     }
+    this.tech_.off('timeupdate', this.timeupdateHandler);
   }
 
   /**
diff --git a/test/unit/tracks/text-track.test.js b/test/unit/tracks/text-track.test.js
index 11abcf9da7..571e3fc4bf 100644
--- a/test/unit/tracks/text-track.test.js
+++ b/test/unit/tracks/text-track.test.js
@@ -279,7 +279,9 @@ QUnit.test('does not fire cuechange before Tech is ready', function(assert) {
     return 0;
   };
 
+  // `playing` would trigger rvfc or raf, `timeupdate` for fallback
   player.tech_.trigger('playing');
+  player.tech_.trigger('timeupdate');
   assert.equal(changes, 0, 'a cuechange event is not triggered');
 
   player.tech_.on('ready', function() {
@@ -292,6 +294,11 @@ QUnit.test('does not fire cuechange before Tech is ready', function(assert) {
 
     assert.equal(changes, 2, 'a cuechange event trigger addEventListener and oncuechange');
 
+    player.tech_.trigger('timeupdate');
+    clock.tick(1);
+
+    assert.equal(changes, 2, 'a cuechange event trigger not duplicated by timeupdate');
+
     tt.off();
     player.dispose();
     clock.restore();
@@ -311,31 +318,45 @@ QUnit.test('fires cuechange when cues become active and inactive', function(asse
   const cuechangeHandler = function() {
     changes++;
   };
+  let fakeCurrentTime = 0;
+
+  player.tech_.currentTime = function() {
+    return fakeCurrentTime;
+  };
 
   tt.addCue({
     id: '1',
     startTime: 1,
     endTime: 5
   });
+  tt.addCue({
+    id: '2',
+    startTime: 11,
+    endTime: 14
+  });
 
   tt.oncuechange = cuechangeHandler;
   tt.addEventListener('cuechange', cuechangeHandler);
 
-  player.tech_.currentTime = function() {
-    return 2;
-  };
+  fakeCurrentTime = 2;
+  player.tech_.trigger('playing');
 
+  assert.equal(changes, 2, 'a cuechange event trigger addEventListener and oncuechange (rvfc/raf)');
+
+  fakeCurrentTime = 7;
   player.tech_.trigger('playing');
 
-  assert.equal(changes, 2, 'a cuechange event trigger addEventListener and oncuechange');
+  assert.equal(changes, 4, 'a cuechange event trigger addEventListener and oncuechange (rvfc/raf)');
 
-  player.tech_.currentTime = function() {
-    return 7;
-  };
+  fakeCurrentTime = 12;
+  player.tech_.trigger('timeupdate');
 
-  player.tech_.trigger('playing');
+  assert.equal(changes, 6, 'a cuechange event trigger addEventListener and oncuechange (timeupdate)');
+
+  fakeCurrentTime = 17;
+  player.tech_.trigger('timeupdate');
 
-  assert.equal(changes, 4, 'a cuechange event trigger addEventListener and oncuechange');
+  assert.equal(changes, 8, 'a cuechange event trigger addEventListener and oncuechange (timeupdate)');
 
   tt.off();
   player.dispose();
@@ -365,6 +386,7 @@ QUnit.test('enabled and disabled cuechange handler when changing mode to hidden'
     return 2;
   };
   player.tech_.trigger('playing');
+  player.tech_.trigger('timeupdate');
 
   assert.equal(changes, 1, 'a cuechange event trigger');
 
@@ -376,6 +398,7 @@ QUnit.test('enabled and disabled cuechange handler when changing mode to hidden'
     return 7;
   };
   player.tech_.trigger('playing');
+  player.tech_.trigger('timeupdate');
 
   assert.equal(changes, 0, 'NO cuechange event trigger');