Skip to content

Commit

Permalink
fix(text): Only compare cues when necessary
Browse files Browse the repository at this point in the history
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
  • Loading branch information
michellezhuogg committed Dec 21, 2020
1 parent 3ad70b4 commit 927c020
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
12 changes: 11 additions & 1 deletion lib/text/cue.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion lib/text/ui_text_displayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 927c020

Please sign in to comment.