fix: ensure all timelineregionenter events are fired

[littlespex]

We aren't sure if this is the best solution, because we don't entirely understand the original reason for using startTimeOfLoad, the wall clock time that the load started, instead of startTime, the playhead time offset passed to player.load(). We are looking for some clarity and this is our initial fix. This issue results in DAI impressions being missed, especially on lower powered and network constrained devices.

Resolves #6711

[shaka-bot]

Incremental code coverage: 100.00%

[joeyparrish]

I could use more explanation in the PR description. Why is startTime correct and startTimeOfLoad wrong? What's the difference?

Might also be worth a comment here in the code.

[littlespex]

There are extensive notes in the associated issue #6711

[littlespex]

We don't understand why startTimeOfLoad is being used, especially for VOD. The startTimeOfLoad argument is used to set the startsPastZero flag in RegionObserver. The comments there seem to indicate that it is playhead related:

shaka-player/lib/media/region_observer.js

Lines 39 to 44 in 30ac8c0

	* 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 makes sense. If the user called player.load() with a non-zero start time, the observer should wait until the initial seek has completed. startTimeOfLoad is the wall clock time of when the load operation started. How does that play into how the region observer operates? Before the commit mentioned in the issue, startTimeOfLoad was alway NaN at the time createPlayheadObserversForMSE_, which coincidentally resulted in the correct behavior for VOD.

[littlespex]

@theodab Can you provide any insight here? The comments in #5361 state:

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.

Unfortunately, after the @avelad's state engine refactor, startTimeOfLoad is always greater than zero, which means startsPastZero is always true, regardless of live/vod.

This comments also mention:

This does not happen every time; it's likely based on the exact time it takes the player to start playing.

With startTimeOfLoad being a timestamp, it doesn't seem like the correct value to use here. It suggests a delta like Date.now() / 1000 - startTimeOfLoad. That said, when we test on versions <= 4.5.0, we see startTimeOfLoad always comes in as NaN, so really only this.isLive() has any effect on the observer's startsPastZero flag.

@joeyparrish This fact led us to look into what values makes the most sense to use here, and we determined that load()'s startTime was the answer. When playing live, wait for a the playhead to update before dispatching timeline region events. When playing VOD, the player should also wait for any initial seek operations before tracking timeline region changes.

[theodab]

Yeah, I think the original code using startTimeOfLoad was a mistake. It's supposed to check for if we were loading partway through the presentation, so that we won't fire region events for regions that the player loads in after the end of. The startTime variable would be the actual variable we want for this.

Honestly, we should probably just rename the startTImeOfLoad variable. It's confusingly similar to startTime, despite the two variables having much different meanings and effects in the loading process.

[theodab]

Yeah, I think the original code using startTimeOfLoad was a mistake. It's supposed to check for if we were loading partway through the presentation, so that we won't fire region events for regions that the player loads in after the end of. The startTime variable would be the actual variable we want for this.

Honestly, we should probably just rename the startTImeOfLoad variable. It's confusingly similar to startTime, despite the two variables having much different meanings and effects in the loading process.