Skip to content

Commit

Permalink
fix: Fix video progress events accuracy (#7654)
Browse files Browse the repository at this point in the history
Due do rounding errors the progress events were fired way too early (especially the complete event on longer videos). This changes use float comparison to mitigate the issue.
Further improvements (video with start time, seek handling) will be added in follow up PRs.

Co-authored-by: Álvaro Velad Galván <[email protected]>
  • Loading branch information
2 people authored and joeyparrish committed Dec 11, 2024
1 parent ccae978 commit 576daba
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 35 deletions.
1 change: 1 addition & 0 deletions build/types/core
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
+../../lib/util/multi_map.js
+../../lib/util/mutex.js
+../../lib/util/networking.js
+../../lib/util/number_utils.js
+../../lib/util/object_utils.js
+../../lib/util/operation_manager.js
+../../lib/util/periods.js
Expand Down
76 changes: 41 additions & 35 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ goog.require('shaka.util.ManifestParserUtils');
goog.require('shaka.util.MediaReadyState');
goog.require('shaka.util.MimeUtils');
goog.require('shaka.util.Mutex');
goog.require('shaka.util.NumberUtils');
goog.require('shaka.util.ObjectUtils');
goog.require('shaka.util.Platform');
goog.require('shaka.util.PlayerConfiguration');
Expand Down Expand Up @@ -705,7 +706,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
this.externalSrcEqualsThumbnailsStreams_ = [];

/** @private {number} */
this.completionPercent_ = NaN;
this.completionPercent_ = -1;

/** @private {?shaka.extern.PlayerConfiguration} */
this.config_ = this.defaultConfig_();
Expand Down Expand Up @@ -1457,7 +1458,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {

this.externalSrcEqualsThumbnailsStreams_ = [];

this.completionPercent_ = NaN;
this.completionPercent_ = -1;

// Make sure that the app knows of the new buffering state.
this.updateBufferState_();
Expand Down Expand Up @@ -6370,41 +6371,46 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
if (!this.video_) {
return;
}
let hasNewCompletionPercent = false;
const completionRatio = this.video_.currentTime / this.video_.duration;
if (!isNaN(completionRatio)) {
const percent = Math.round(100 * completionRatio);
if (isNaN(this.completionPercent_)) {
this.completionPercent_ = percent;
hasNewCompletionPercent = true;
} else {
const newCompletionPercent = Math.max(this.completionPercent_, percent);
if (this.completionPercent_ != newCompletionPercent) {
this.completionPercent_ = newCompletionPercent;
hasNewCompletionPercent = true;
}

const isQuartile = (quartilePercent, currentPercent) => {
const NumberUtils = shaka.util.NumberUtils;

if ((NumberUtils.isFloatEqual(quartilePercent, currentPercent) ||
currentPercent > quartilePercent) &&
this.completionPercent_ < quartilePercent) {
this.completionPercent_ = quartilePercent;
return true;
}
return false;
};

const completionRatio = this.video_.currentTime / this.video_.duration;

if (isNaN(completionRatio)) {
return;
}
if (hasNewCompletionPercent) {
let event;
if (this.completionPercent_ == 0) {
event = shaka.Player.makeEvent_(shaka.util.FakeEvent.EventName.Started);
} else if (this.completionPercent_ == 25) {
event = shaka.Player.makeEvent_(
shaka.util.FakeEvent.EventName.FirstQuartile);
} else if (this.completionPercent_ == 50) {
event = shaka.Player.makeEvent_(
shaka.util.FakeEvent.EventName.Midpoint);
} else if (this.completionPercent_ == 75) {
event = shaka.Player.makeEvent_(
shaka.util.FakeEvent.EventName.ThirdQuartile);
} else if (this.completionPercent_ == 100) {
event = shaka.Player.makeEvent_(
shaka.util.FakeEvent.EventName.Complete);
}
if (event) {
this.dispatchEvent(event);
}

const percent = completionRatio * 100;

let event;
if (isQuartile(0, percent)) {
event = shaka.Player.makeEvent_(shaka.util.FakeEvent.EventName.Started);
} else if (isQuartile(25, percent)) {
event = shaka.Player.makeEvent_(
shaka.util.FakeEvent.EventName.FirstQuartile);
} else if (isQuartile(50, percent)) {
event = shaka.Player.makeEvent_(
shaka.util.FakeEvent.EventName.Midpoint);
} else if (isQuartile(75, percent)) {
event = shaka.Player.makeEvent_(
shaka.util.FakeEvent.EventName.ThirdQuartile);
} else if (isQuartile(100, percent)) {
event = shaka.Player.makeEvent_(
shaka.util.FakeEvent.EventName.Complete);
}

if (event) {
this.dispatchEvent(event);
}
}

Expand Down
37 changes: 37 additions & 0 deletions lib/util/number_utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*! @license
* Shaka Player
* Copyright 2016 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

goog.provide('shaka.util.NumberUtils');


shaka.util.NumberUtils = class {
/**
* Compare two float numbers, taking a configurable tolerance margin into
* account.
*
* @param {number} a
* @param {number} b
* @param {number=} tolerance
* @return {boolean}
*/
static isFloatEqual(a, b, tolerance = Number.EPSILON) {
if (a === b) {
return true;
}

const error = Math.abs(a - b);

if (error <= tolerance) {
return true;
}

if (tolerance !== Number.EPSILON) {
return Math.abs(error - tolerance) <= Number.EPSILON;
}

return false;
}
};
20 changes: 20 additions & 0 deletions test/util/number_utils_unit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*! @license
* Shaka Player
* Copyright 2016 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

describe('NumberUtils', () => {
const NumberUtils = shaka.util.NumberUtils;

it('compares float', () => {
expect(NumberUtils.isFloatEqual(0.1 + 0.2, 0.3)).toBe(true);
expect(NumberUtils.isFloatEqual(0.4 - 0.1, 0.3)).toBe(true);
expect(NumberUtils.isFloatEqual(0.0004, 0.0003)).toBe(false);
});

it('respects provided tolerance margin', () => {
expect(NumberUtils.isFloatEqual(1.5, 1.4)).toBe(false);
expect(NumberUtils.isFloatEqual(1.5, 1.4, 0.1)).toBe(true);
});
});

0 comments on commit 576daba

Please sign in to comment.