This repository has been archived by the owner on Jan 12, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 795
use last segment duration + 2*targetDuration for safe live point instead of 3 segments #1271
Merged
Merged
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
0918559
use 3*targetDuration for safe live point instead of 3 segments
mjneil ee2e4fc
cleanup/comments
mjneil 8050a02
break up tests
mjneil 3917852
use the new spec requirement of last segment duration + 2 target dura…
mjneil b781765
oops
mjneil 54ea94f
update comment
mjneil 3881c2e
do not let back buffer trimming remove within target duration of curr…
mjneil 2e31dd6
add unit test
mjneil ab60fd7
increase threshold for stuck playlist checking
mjneil fb9fbc9
clarifying comment
mjneil 3a77d68
review fixes
mjneil 94d9d91
refactor some things
mjneil 1b69a71
move some comments
mjneil File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,6 +73,46 @@ export const illegalMediaSwitch = (loaderType, startingMedia, newSegmentMedia) = | |
return null; | ||
}; | ||
|
||
/** | ||
* Calculates a time value that is safe to remove from the back buffer without interupting | ||
* playback. | ||
* | ||
* @param {TimeRange} seekable | ||
* The current seekable range | ||
* @param {Number} currentTime | ||
* The current time of the player | ||
* @param {Number} targetDuration | ||
* The target duration of the current playlist | ||
* @return {Number} | ||
* Time that is safe to remove from the back buffer without interupting playback | ||
*/ | ||
export const safeBackBufferTrimTime = (seekable, currentTime, targetDuration) => { | ||
// Don't allow removing from the buffer within target duration of current time | ||
// to avoid the possibility of removing the GOP currently being played which could | ||
// cause playback stalls. | ||
const safeRemoveToTimeLimit = currentTime - targetDuration; | ||
|
||
// Chrome has a hard limit of 150MB of | ||
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. This comment should probably go in the |
||
// buffer and a very conservative "garbage collector" | ||
// We manually clear out the old buffer to ensure | ||
// we don't trigger the QuotaExceeded error | ||
// on the source buffer during subsequent appends | ||
|
||
let removeToTime; | ||
|
||
if (seekable.length && | ||
seekable.start(0) > 0 && | ||
seekable.start(0) < currentTime) { | ||
// If we have a seekable range use that as the limit for what can be removed safely | ||
removeToTime = seekable.start(0); | ||
} else { | ||
// otherwise remove anything older than 30 seconds before the current play head | ||
removeToTime = currentTime - 30; | ||
} | ||
|
||
return Math.min(removeToTime, safeRemoveToTimeLimit); | ||
}; | ||
|
||
/** | ||
* An object that manages segment loading and appending. | ||
* | ||
|
@@ -906,33 +946,9 @@ export default class SegmentLoader extends videojs.EventTarget { | |
* @param {Object} segmentInfo - the current segment | ||
*/ | ||
trimBackBuffer_(segmentInfo) { | ||
const seekable = this.seekable_(); | ||
const currentTime = this.currentTime_(); | ||
const targetDuration = this.playlist_.targetDuration || 10; | ||
|
||
// Don't allow removing from the buffer within target duration of current time | ||
// to avoid the possibility of removing the GOP currently being played which could | ||
// cause playback stalls. | ||
const safeRemoveToTimeLimit = currentTime - targetDuration; | ||
let removeToTime = 0; | ||
|
||
// Chrome has a hard limit of 150MB of | ||
// buffer and a very conservative "garbage collector" | ||
// We manually clear out the old buffer to ensure | ||
// we don't trigger the QuotaExceeded error | ||
// on the source buffer during subsequent appends | ||
|
||
// If we have a seekable range use that as the limit for what can be removed safely | ||
// otherwise remove anything older than 30 seconds before the current play head | ||
if (seekable.length && | ||
seekable.start(0) > 0 && | ||
seekable.start(0) < currentTime) { | ||
removeToTime = seekable.start(0); | ||
} else { | ||
removeToTime = currentTime - 30; | ||
} | ||
|
||
removeToTime = Math.min(removeToTime, safeRemoveToTimeLimit); | ||
const removeToTime = safeBackBufferTrimTime(this.seekable_(), | ||
this.currentTime_(), | ||
this.playlist_.targetDuration || 10); | ||
|
||
if (removeToTime > 0) { | ||
this.remove(0, removeToTime); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 line and comment might be better just as part of the
Math.min
return. We can remove the variable since it just obfuscates the meaning (since it is just a subtraction of two numbers).