From 0d9759835418952357b3fbb48ebc7cc13acdec47 Mon Sep 17 00:00:00 2001 From: Theodore Abshire Date: Tue, 13 Jun 2023 18:08:47 -0700 Subject: [PATCH 1/3] fix(media): Fix region checking in livestreams In some cases, in a livestream situation, the region observer's poll method ends up being called before the media time is set to the live edge. This does not happen every time; it's likely based on the exact time it takes the player to start playing. When this does happen, it leads to the region observer "skipping" over every region between time=0 and the live head, as it erroneously believes that it has actually played through all of that time. This could result in confusion for applications that are relying on region data. This change modifiers the region observer so that it does not start examining regions until the time gets past 0 seconds, except in the case of VOD. Closes #5213 --- lib/media/region_observer.js | 20 +++++++++++++++++++- lib/player.js | 11 ++++++++--- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/media/region_observer.js b/lib/media/region_observer.js index e4666d42f8..11e4301d2d 100644 --- a/lib/media/region_observer.js +++ b/lib/media/region_observer.js @@ -27,13 +27,24 @@ shaka.media.RegionObserver = class extends shaka.util.FakeEventTarget { * destroy the timeline. * * @param {!shaka.media.RegionTimeline} timeline + * @param {boolean} startsPastZero */ - constructor(timeline) { + constructor(timeline, startsPastZero) { super(); /** @private {shaka.media.RegionTimeline} */ this.timeline_ = timeline; + /** + * Whether the asset is expected to start at a time beyond 0 seconds. + * For example, if the asset is a live stream. + * If true, we will not start polling for regions until the playhead has + * moved past 0 seconds, to avoid bad behaviors where the current time is + * briefly 0 before we have enough data to play. + * @private {boolean} + */ + this.startsPastZero_ = startsPastZero; + /** * A mapping between a region and where we previously were relative to it. * When the value here differs from what we calculate, it means we moved and @@ -121,6 +132,13 @@ shaka.media.RegionObserver = class extends shaka.util.FakeEventTarget { /** @override */ poll(positionInSeconds, wasSeeking) { const RegionObserver = shaka.media.RegionObserver; + if (this.startsPastZero_ && positionInSeconds == 0) { + // Don't stack checking regions until the timeline has begun moving. + return; + } + // Now that we have seen the playhead go past 0, it's okay if it goes + // back there (e.g. seeking back to the start). + this.startsPastZero_ = false; for (const region of this.timeline_.regions()) { const previousPosition = this.oldPosition_.get(region); diff --git a/lib/player.js b/lib/player.js index 82b061aaf9..5ea855516d 100644 --- a/lib/player.js +++ b/lib/player.js @@ -2188,7 +2188,8 @@ shaka.Player = class extends shaka.util.FakeEventTarget { } this.playhead_ = this.createPlayhead(has.startTime); - this.playheadObservers_ = this.createPlayheadObserversForMSE_(); + this.playheadObservers_ = + this.createPlayheadObserversForMSE_(has.startTimeOfLoad); // We need to start the buffer management code near the end because it will // set the initial buffering state and that depends on other components @@ -2930,17 +2931,21 @@ shaka.Player = class extends shaka.util.FakeEventTarget { * Create the observers for MSE playback. These observers are responsible for * notifying the app and player of specific events during MSE playback. * + * @param {number} startTimeOfLoad * @return {!shaka.media.PlayheadObserverManager} * @private */ - createPlayheadObserversForMSE_() { + createPlayheadObserversForMSE_(startTimeOfLoad) { goog.asserts.assert(this.manifest_, 'Must have manifest'); goog.asserts.assert(this.regionTimeline_, 'Must have region timeline'); goog.asserts.assert(this.video_, 'Must have video element'); + const startsPastZero = this.isLive() || startTimeOfLoad > 0; + // Create the region observer. This will allow us to notify the app when we // move in and out of timeline regions. - const regionObserver = new shaka.media.RegionObserver(this.regionTimeline_); + const regionObserver = new shaka.media.RegionObserver( + this.regionTimeline_, startsPastZero); regionObserver.addEventListener('enter', (event) => { /** @type {shaka.extern.TimelineRegionInfo} */ From 9d012e43d6d74f2e2f6ae87b99a05d096dfa146a Mon Sep 17 00:00:00 2001 From: Theodore Abshire Date: Tue, 27 Jun 2023 03:23:45 -0700 Subject: [PATCH 2/3] Fix not-updated test. --- test/media/region_observer_unit.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/media/region_observer_unit.js b/test/media/region_observer_unit.js index 6d49f38a43..b41dac7a1e 100644 --- a/test/media/region_observer_unit.js +++ b/test/media/region_observer_unit.js @@ -31,7 +31,8 @@ describe('RegionObserver', () => { timeline = new shaka.media.RegionTimeline( shaka.test.Util.spyFunc(onSeekRange)); - observer = new shaka.media.RegionObserver(timeline); + observer = new shaka.media.RegionObserver( + timeline, /* startsPastZero= */ false); observer.addEventListener('enter', (event) => { shaka.test.Util.spyFunc(onEnterRegion)(event['region'], event['seeking']); }); From f2aaf4742e3c43600ddf9c132365644bf18d8829 Mon Sep 17 00:00:00 2001 From: Theodore Abshire Date: Tue, 27 Jun 2023 11:49:14 -0700 Subject: [PATCH 3/3] Fix typo. --- lib/media/region_observer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/media/region_observer.js b/lib/media/region_observer.js index 11e4301d2d..f7f19f94eb 100644 --- a/lib/media/region_observer.js +++ b/lib/media/region_observer.js @@ -133,7 +133,7 @@ shaka.media.RegionObserver = class extends shaka.util.FakeEventTarget { poll(positionInSeconds, wasSeeking) { const RegionObserver = shaka.media.RegionObserver; if (this.startsPastZero_ && positionInSeconds == 0) { - // Don't stack checking regions until the timeline has begun moving. + // Don't start checking regions until the timeline has begun moving. return; } // Now that we have seen the playhead go past 0, it's okay if it goes