From 576daba0caa17a333a3c77877caf8a8b3c5423e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?/z=C9=92=CC=83ge/?= Date: Tue, 26 Nov 2024 03:01:25 +0100 Subject: [PATCH] fix: Fix video progress events accuracy (#7654) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- build/types/core | 1 + lib/player.js | 76 ++++++++++++++++++---------------- lib/util/number_utils.js | 37 +++++++++++++++++ test/util/number_utils_unit.js | 20 +++++++++ 4 files changed, 99 insertions(+), 35 deletions(-) create mode 100644 lib/util/number_utils.js create mode 100644 test/util/number_utils_unit.js diff --git a/build/types/core b/build/types/core index 23d04a5c8f..5f322c0e7d 100644 --- a/build/types/core +++ b/build/types/core @@ -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 diff --git a/lib/player.js b/lib/player.js index 4639cbf2ad..cf465f4bde 100644 --- a/lib/player.js +++ b/lib/player.js @@ -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'); @@ -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_(); @@ -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_(); @@ -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); } } diff --git a/lib/util/number_utils.js b/lib/util/number_utils.js new file mode 100644 index 0000000000..f3aa3e37c7 --- /dev/null +++ b/lib/util/number_utils.js @@ -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; + } +}; diff --git a/test/util/number_utils_unit.js b/test/util/number_utils_unit.js new file mode 100644 index 0000000000..647dfa3383 --- /dev/null +++ b/test/util/number_utils_unit.js @@ -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); + }); +});