From 927c020ee19d99c6fd89222cbb27c67ae41f9d1a Mon Sep 17 00:00:00 2001
From: Michelle Zhuo <michellezhuo@google.com>
Date: Fri, 18 Dec 2020 14:19:21 -0800
Subject: [PATCH] fix(text): Only compare cues when necessary

When a VTT cue spans a segment boundary, the cue will be duplicated
across two segments.
To avoid displaying duplicate cues, we compare the cues to be
appended with the cues already in the list.

1. We can assume the cues to be appended at once have no duplicates.
Make a copy of the current cues list, and compare the new cues only
with the cues in the current cues list.
If we append m new cues to a list of n cues, we can reduce the times to
compare the cues from m*m*n/2 times to m*n times.

2. We compare the cues deeply by comparing their nested cues and
all the elements. We can compare their start time, end time and payload
first, and only compare their nested cues and all other elements when
their start time, end time and payload are the same.
If cue1 and cue2 both have m nested cues, we may not need
to compare the nested cues instead of comparing m times.

Fixes #3018

Change-Id: I9992f0e1834fd16e8aedaf1895b036bc7ca29190
---
 lib/text/cue.js               | 12 +++++++++++-
 lib/text/ui_text_displayer.js |  6 +++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/lib/text/cue.js b/lib/text/cue.js
index c180da70de..22d71c5ed3 100644
--- a/lib/text/cue.js
+++ b/lib/text/cue.js
@@ -262,8 +262,18 @@ shaka.text.Cue = class {
    * @suppress {checkTypes} since we must use [] and "in" with a struct type.
    */
   static equal(cue1, cue2) {
+    // Compare the start time, end time and payload of the cues first for
+    // performance optimization.  We can avoid the more expensive recursive
+    // checks if the top-level properties don't match.
+    // See: https://github.com/google/shaka-player/issues/3018
+    if (cue1.startTime != cue2.startTime || cue1.endTime != cue2.endTime ||
+      cue1.payload != cue2.payload) {
+      return false;
+    }
     for (const k in cue1) {
-      if (k == 'nestedCues') {
+      if (k == 'startTime' || k == 'endTime' || k == 'payload') {
+        // Already compared.
+      } else if (k == 'nestedCues') {
         // This uses shaka.text.Cue.equal rather than just this.equal, since
         // otherwise recursing here will unbox the method and cause "this" to be
         // undefined in deeper recursion.
diff --git a/lib/text/ui_text_displayer.js b/lib/text/ui_text_displayer.js
index 41a0692902..ba18395f68 100644
--- a/lib/text/ui_text_displayer.js
+++ b/lib/text/ui_text_displayer.js
@@ -82,12 +82,16 @@ shaka.text.UITextDisplayer = class {
    * @export
    */
   append(cues) {
+    // Clone the cues list for performace optimization. We can avoid the cues
+    // list growing during the comparisons for duplicate cues.
+    // See: https://github.com/google/shaka-player/issues/3018
+    const cuesList = [...this.cues_];
     for (const cue of cues) {
       // When a VTT cue spans a segment boundary, the cue will be duplicated
       // into two segments.
       // To avoid displaying duplicate cues, if the current cue list already
       // contains the cue, skip it.
-      const containsCue = this.cues_.some(
+      const containsCue = cuesList.some(
           (cueInList) => shaka.text.Cue.equal(cueInList, cue));
       if (!containsCue) {
         this.cues_.push(cue);