feat: Add liveSync configuration to catch up on live streams

[avelad]

No description provided.

[github-actions]

Incremental code coverage: 28.57%

[joeyparrish]

These are two different units fed into Math.max: a playback rate and a duration in seconds. What is the intent here?

[avelad]

The problem is that the timeupdate event is not fired every frame, instead it takes a bit of time, sometimes even one second. The idea here is to avoid reaching the live edge in less than a second. If the playbackrate is 3 for example, and we have 3 seconds left for the live point, it would take us a second to reach it, so we want to avoid this case.

[avelad]

Another option is put the min in 0, but I'd prefer the current option

[joeyparrish]

Your explanation makes sense, but I don't find the code clear enough on its own without this conversation. And I will definitely forget.

Here's a comment that I think might help, to replace the one above:

// In src= mode, the seek range isn't updated frequently enough, so we need to fudge
// the latency number with an offset.  The playback rate is used as an offset, since that
// is the amount we catch up 1 second of accelerated playback.

I still don't understand the other duration, though. Why do you subtract bufferedEnd - seekRange.end? That should be negative if we've fallen behind, right?

-------|---------------|-------------|--------------|------
seekRange.start   currentTime   bufferedEnd   seekRange.end

[avelad]

bufferedEnd can greater than seekRange.end when the current time is near to live edge.

I’ll add your comment tomorrow. Thanks!

[joeyparrish]

This Math.max involving the bufferedEnd - seekRange.end parameter still doesn't make sense to me.

Here's a concrete scenario. Let's say your latency goal is 3s. You want to be within 3s of the end of the seek range.

currentTime = 10
seekRange.end = 15
bufferedEnd = 18
liveSyncPlaybackRate = 1.1

latency = seekRange.end - currentTime = 5

offset = Math.max(liveSyncPlaybackRate, bufferedEnd - seekRange.end)
offset = Math.max(1.1, 18 - 15)
offset = 3

latency - offset = 2, which is less than our target of 3. So even though we're 5s behind the end of the seek range, we don't accelerate playback. And because we've buffered past the end of the seek range, we could accelerate playback very safely.

So it seems to me that in the cases where bufferedEnd > seekRange.end, the offset value has the opposite effect of what it should do.

Am I crazy?

[avelad]

You're right, but if we take into account Theodore's comment, the event should trigger at most every 250ms, so it would still be fine. (https://developer.mozilla.org/en-US/docs/Web/API/HTMMLediaElement/timeupdate_event)

Note: this is for valid, because the src= for live should only be used in Safari.

[theodab]

4Hz is the minimum frequency, not the maximum. The maximum is 66Hz, which would be one call every 15ms.

[avelad]

In the case of the maximum, we have no problem :)

[avelad]

@joeyparrish can you review it again? Thanks!

[joeyparrish]

Looks okay otherwise

[joeyparrish]

Your explanation makes sense, but I don't find the code clear enough on its own without this conversation. And I will definitely forget.

Here's a comment that I think might help, to replace the one above:

// In src= mode, the seek range isn't updated frequently enough, so we need to fudge
// the latency number with an offset.  The playback rate is used as an offset, since that
// is the amount we catch up 1 second of accelerated playback.

I still don't understand the other duration, though. Why do you subtract bufferedEnd - seekRange.end? That should be negative if we've fallen behind, right?

-------|---------------|-------------|--------------|------
seekRange.start   currentTime   bufferedEnd   seekRange.end

[theodab]

According to MDN, the lowest frequency you can expect for the timeupdate event is around 4 hz.
So taking an entire second's worth of fast-playback as an offset might be overkill, if the expectation is that this is being called at least 4 times a second. Even if you want to be safe and assume that system load is high, liveSyncPlaybackRate / 2 might still work.

[avelad]

In my tests this does not always work on SmartTV, I prefer to be more conservative in this case.

[joeyparrish]

This Math.max involving the bufferedEnd - seekRange.end parameter still doesn't make sense to me.

Here's a concrete scenario. Let's say your latency goal is 3s. You want to be within 3s of the end of the seek range.

currentTime = 10
seekRange.end = 15
bufferedEnd = 18
liveSyncPlaybackRate = 1.1

latency = seekRange.end - currentTime = 5

offset = Math.max(liveSyncPlaybackRate, bufferedEnd - seekRange.end)
offset = Math.max(1.1, 18 - 15)
offset = 3

latency - offset = 2, which is less than our target of 3. So even though we're 5s behind the end of the seek range, we don't accelerate playback. And because we've buffered past the end of the seek range, we could accelerate playback very safely.

So it seems to me that in the cases where bufferedEnd > seekRange.end, the offset value has the opposite effect of what it should do.

Am I crazy?

Reviewed by Theodore