-
Notifications
You must be signed in to change notification settings - Fork 428
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
fix(sync-controller): add a default sync point when currentTimeline is -1 #500
Conversation
Would be good to add a unit test for the behavior of #464 that shows this sync point being returned properly when a rendition switch happens before the first segment download finishes |
I can try looking into making a test for it. #464 is a bit more than just one rendition change, though, there's a bunch of them. |
I don't think you need multiple rendition switches, only the initial + 1. Can't you listen to |
I can try doing the mediachange, but when I updated the example from #464 to wait for all the renditions to be available and then switch, it didn't repro, but it's possible it happened too early/too late at that point. |
type: 'application/vnd.apple.mpegurl' | ||
}); | ||
this.clock.tick(1); | ||
this.player.play(); |
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.
calling play or using autoplay causes us to be in a has played state by the time we switch the playlist, meaning the playlist won't get a syncinfo.
const levels = this.player.qualityLevels(); | ||
|
||
levels.one('change', () => { | ||
this.player.tech_.hls.masterPlaylistController_.mainSegmentLoader_.fillBuffer_(); |
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.
fillBuffer sets up the syncPoint if it can.
this.player.tech_.hls.masterPlaylistController_.mainSegmentLoader_.fillBuffer_(); | ||
assert.notEqual(this.player.tech_.hls.masterPlaylistController_.mainSegmentLoader_.syncPoint_, null); | ||
// ignore warnings | ||
this.env.log.warn.reset(); |
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.
we don't respond to some manifests that are requests and so a warning is logged. Responding to them seems to cause this test to timeout but don't have time to spend figuring out why, instead, ignore these warnings.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
…oaded (#700) * Fix live startup failures when play happens before playlist is downloaded When joining a live stream, VHS starts playback at a time of 0, regardless of how long the stream has been playing. This means that the playlist will start with sync info of time 0 for the first media index. However, if the stream "played" (either via API, autoplay, etc.) before a playlist was downloaded, then after the playlist downloaded the loader would reset the sync info, erasing the assumed time 0 for first media index, and the player would request segments ad infinitum, never able to place them in the timeline and get a new sync point. This separates the notion of played between play initiation and playback of content (progress on the timeline), in order to ensure that the initial sync info is maintained. * Use segment loader's state to determine when to update sync info While using hasPlayedContent helped to alleviate most issues around when to update the sync info in segment loader when updating a live playlist, there still remained potential issues when a segment was requested and the sync info was changed for the in-flight segment. This change uses the segment loader's INIT state to determine if the sync info should be updated, meaning in-flight segment requests keep their sync info fixed. Fixes #464, closes #496, closes #500.
Fixed as part of #700. Release soon. |
This also fixes #464.