Skip to content

Commit

Permalink
fix(media): Fix region checking in livestreams (#5361)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
theodab authored and joeyparrish committed Jul 20, 2023
1 parent e6f69fb commit b77a947
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 5 deletions.
20 changes: 19 additions & 1 deletion lib/media/region_observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 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
// 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);
Expand Down
11 changes: 8 additions & 3 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -2191,7 +2191,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
Expand Down Expand Up @@ -2918,17 +2919,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} */
Expand Down
3 changes: 2 additions & 1 deletion test/media/region_observer_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
});
Expand Down

0 comments on commit b77a947

Please sign in to comment.