Return undefined from getCurrentTime() when it is unknown

[tjenkinson]

📺 What

Currently getCurrentTime() has a default value of 0 in several cases. This is not ideal because it makes it impossible to know if it truly is time 0, or if the time is not known yet.

It also sometimes returns the wrong value.

An example where this is an issue is if you start a live stream from the live point. Currently you will get back the incorrect time whilst the stream loads, and then it will jump to the correct value.

With this PR when the strategy is still loading (hasn't changed the state to something other than WAITING) getCurrentTime() will now always return

• the value of the latest setCurrentTime request or
• initialPlaybackTime or
• undefined

I chose to put this logic in 'bigscreenplayer.js' instead of the individual strategies because I think this is something that benefits them all.

This also means a call to getCurrentTime() in the same stack as setCurrentTime() is guaranteed to be what you expect even if the update happens asynchronously in the strategy.

This PR also contains a fix in the msestrategy getCurrentTime() to prevent it returning incorrect values when the media element isn't ready. This change isn't strictly needed given the other change in 'bigscreenplayer.js' prevents it causing an issue, but it's probably good to fix anyway.

IPLAYERTVV1-10907

🛠 How

• Introduce a SeekState enum which can be NONE, QUEUED or IN_FLIGHT. When a seek is QUEUED or IN_FLIGHT we now return the time we're seeking to from getCurrentTime() instead of calling through to the strategy.
• In msestrategy return undefined instead of 0 from getCurrentTime when the time is unknown.
• In msestrategy return undefined when the media element readyState is at least HAVE_METADATA. Before this dashjs has not set the current time to near the live point meaning it would return the wrong value.
• Update/add tests

✅ Acceptance criteria

• getCurrentTime() returns undefined whilst a stream is loading with no initialPlaybackTime using the mse strategy
• getCurrentTime() always returns the time it was set to immediately after setCurrentTime()
• getCurrentTime() initially returns the value of initialPlaybackTime.
• getCurrentTime() returns the expected time when the media state is not WAITING

[tjenkinson]

Setting mockVideoElement.readyState does not work if it's a real element

[jlks]

I'd argue this is a little overkill for the time being. We should be trying to be close to normal web behaviour when providing the current time. Can we not just check whether or not there is a timeCorrection & time is 0 then return 0?

Alternatively I'd see something like this spike as more of a long term 'fix' (don't say the player is fully initialised when is isn't) https://github.com/bbc/bigscreen-player/compare/emit-loaded-spike

Note the above branch doesn't deal with the CEHTML strategy (its an idea, not a broad fix!) and also doesn't clean up errors before playback...

Also, I like the general idea around this PR: We absolutely should be returning something like null (or a runtime error!) when the player is not initialised from many of these functions.

Thoughts welcome!

[tjenkinson]

We should be trying to be close to normal web behaviour when providing the current time

Returning undefined instead of 0 is not sticking to that, but I think making getCurrentTime() return the queued seek and be correct immediately after setting it is. The queued seek time looks to me like the default-playback-start-position.

Also I reckon when that spec was first written live streams where there might not be a time 0 weren't really a thing for a native media element, and therefore defaulting to 0 made a lot of sense because if no api was invoked, 0 would be the correct time. If the api was being designed today with live streams and MSE being a thing I wonder if it would be different? ¯\(ツ)/¯

Can we not just check whether or not there is a timeCorrection & time is 0 then return 0

But then if getCurrentTime() returns 0, is it because you are at time 0 in the video/stream, or is it because it's not loaded yet and jumped to near the live point?

Alternatively I'd see something like this spike as more of a long term 'fix' (don't say the player is fully initialised when is isn't) https://github.com/bbc/bigscreen-player/compare/emit-loaded-spike

Delaying the point when the player is considered initialised sounds ok to me, but I still think getCurrentTime() should return the queued seek (ie default-playback-start-position), and it should be possible for the user to call setCurrentTime() before the player is considered loaded (just like it is with a standard media element). If we want to return 0 instead of undefined it needs to be possible on the api to query if the player is loaded yet. Maybe a isInitialised()?