-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add liveSync configuration to catch up on live streams #5304
Changes from all commits
d64060a
000e98f
a8bc358
ef70a61
b1e42c3
a56bd4d
5c4e099
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2275,6 +2275,11 @@ shaka.Player = class extends shaka.util.FakeEventTarget { | |
'arbitrary language initially'); | ||
} | ||
|
||
if (this.isLive() && this.config_.streaming.liveSync) { | ||
const onTimeUpdate = () => this.onTimeUpdate_(); | ||
this.loadEventManager_.listen(mediaElement, 'timeupdate', onTimeUpdate); | ||
} | ||
|
||
this.fullyLoaded_ = true; | ||
|
||
// Wait for the 'loadedmetadata' event to measure load() latency. | ||
|
@@ -2627,6 +2632,11 @@ shaka.Player = class extends shaka.util.FakeEventTarget { | |
fullyLoaded.reject(abortedError); | ||
return Promise.resolve(); // Abort complete. | ||
}).chain(() => { | ||
if (this.isLive() && this.config_.streaming.liveSync) { | ||
const onTimeUpdate = () => this.onTimeUpdate_(); | ||
this.loadEventManager_.listen(mediaElement, 'timeupdate', onTimeUpdate); | ||
} | ||
|
||
this.fullyLoaded_ = true; | ||
}); | ||
} | ||
|
@@ -5692,6 +5702,51 @@ shaka.Player = class extends shaka.util.FakeEventTarget { | |
} | ||
} | ||
|
||
/** | ||
* Callback for liveSync | ||
* | ||
* @private | ||
*/ | ||
onTimeUpdate_() { | ||
// If the live stream has reached its end, do not sync. | ||
if (!this.isLive()) { | ||
return; | ||
} | ||
const seekRange = this.seekRange(); | ||
if (!Number.isFinite(seekRange.end)) { | ||
return; | ||
} | ||
const currentTime = this.video_.currentTime; | ||
if (currentTime < seekRange.start) { | ||
// Bad stream? | ||
return; | ||
} | ||
const liveSyncMaxLatency = this.config_.streaming.liveSyncMaxLatency; | ||
const liveSyncPlaybackRate = this.config_.streaming.liveSyncPlaybackRate; | ||
const playbackRate = this.video_.playbackRate; | ||
const latency = seekRange.end - this.video_.currentTime; | ||
let offset = 0; | ||
// 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. | ||
if (this.loadMode_ == shaka.Player.LoadMode.SRC_EQUALS) { | ||
const buffered = this.video_.buffered; | ||
if (buffered.length > 0) { | ||
const bufferedEnd = buffered.end(buffered.length - 1); | ||
offset = Math.max(liveSyncPlaybackRate, bufferedEnd - seekRange.end); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to MDN, the lowest frequency you can expect for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my tests this does not always work on SmartTV, I prefer to be more conservative in this case. |
||
} | ||
} | ||
|
||
if ((latency - offset) > liveSyncMaxLatency) { | ||
if (playbackRate != liveSyncPlaybackRate) { | ||
this.trickPlay(liveSyncPlaybackRate); | ||
} | ||
} else if (playbackRate !== 1 && playbackRate !== 0) { | ||
this.cancelTrickPlay(); | ||
} | ||
} | ||
|
||
/** | ||
* Callback from Playhead. | ||
* | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are two different units fed into Math.max: a playback rate and a duration in seconds. What is the intent here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option is put the min in 0, but I'd prefer the current option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bufferedEnd can greater than seekRange.end when the current time is near to live edge.
I’ll add your comment tomorrow. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4Hz is the minimum frequency, not the maximum. The maximum is 66Hz, which would be one call every 15ms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the case of the maximum, we have no problem :)