-
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/more llhls fixes #1201
Fix/more llhls fixes #1201
Changes from all commits
1feddea
96d5e9f
b071028
c6144ad
ff350fa
9e74eae
c42b1e1
e5fd656
a3b9f09
1704056
cf340b0
33052bb
b40a28c
5fd0ff3
21463d4
5387192
c7765ba
2927cd8
9b2395b
124fcc9
bd6adce
64d513f
fe2c2a6
feabe07
42fd68c
db2da0f
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 |
---|---|---|
|
@@ -47,8 +47,9 @@ const sumLoaderStat = function(stat) { | |
}; | ||
const shouldSwitchToMedia = function({ | ||
currentPlaylist, | ||
buffered, | ||
currentTime, | ||
nextPlaylist, | ||
forwardBuffer, | ||
bufferLowWaterLine, | ||
bufferHighWaterLine, | ||
duration, | ||
|
@@ -73,15 +74,25 @@ const shouldSwitchToMedia = function({ | |
return false; | ||
} | ||
|
||
// determine if current time is in a buffered range. | ||
const isBuffered = Boolean(Ranges.findRange(buffered, currentTime).length); | ||
|
||
// If the playlist is live, then we want to not take low water line into account. | ||
// This is because in LIVE, the player plays 3 segments from the end of the | ||
// playlist, and if `BUFFER_LOW_WATER_LINE` is greater than the duration availble | ||
// in those segments, a viewer will never experience a rendition upswitch. | ||
if (!currentPlaylist.endList) { | ||
// For LLHLS live streams, don't switch renditions before playback has started, as it almost | ||
// doubles the time to first playback. | ||
if (!isBuffered && typeof currentPlaylist.partTargetDuration === 'number') { | ||
log(`not ${sharedLogLine} as current playlist is live llhls, but currentTime isn't in buffered.`); | ||
return false; | ||
} | ||
log(`${sharedLogLine} as current playlist is live`); | ||
return true; | ||
} | ||
|
||
const forwardBuffer = Ranges.timeAheadOf(buffered, currentTime); | ||
const maxBufferLowWaterLine = experimentalBufferBasedABR ? | ||
Config.EXPERIMENTAL_MAX_BUFFER_LOW_WATER_LINE : Config.MAX_BUFFER_LOW_WATER_LINE; | ||
|
||
|
@@ -732,18 +743,18 @@ export class MasterPlaylistController extends videojs.EventTarget { | |
} | ||
|
||
shouldSwitchToMedia_(nextPlaylist) { | ||
const currentPlaylist = this.masterPlaylistLoader_.media(); | ||
const buffered = this.tech_.buffered(); | ||
const forwardBuffer = buffered.length ? | ||
buffered.end(buffered.length - 1) - this.tech_.currentTime() : 0; | ||
|
||
const currentPlaylist = this.masterPlaylistLoader_.media() || | ||
this.masterPlaylistLoader_.pendingMedia_; | ||
const currentTime = this.tech_.currentTime(); | ||
const bufferLowWaterLine = this.bufferLowWaterLine(); | ||
const bufferHighWaterLine = this.bufferHighWaterLine(); | ||
const buffered = this.tech_.buffered(); | ||
|
||
return shouldSwitchToMedia({ | ||
buffered, | ||
currentTime, | ||
currentPlaylist, | ||
nextPlaylist, | ||
forwardBuffer, | ||
bufferLowWaterLine, | ||
bufferHighWaterLine, | ||
duration: this.duration(), | ||
|
@@ -1434,7 +1445,9 @@ export class MasterPlaylistController extends videojs.EventTarget { | |
onSyncInfoUpdate_() { | ||
let audioSeekable; | ||
|
||
if (!this.masterPlaylistLoader_) { | ||
// If we have two source buffers and only one is created then the seekable range will be incorrect. | ||
// We should wait until all source buffers are created. | ||
if (!this.masterPlaylistLoader_ || this.sourceUpdater_.hasCreatedSourceBuffers()) { | ||
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 was causing us to:
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. Would be worth adding a small comment above stating that. Something like: |
||
return; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -257,7 +257,8 @@ const getAllSegments = function(media) { | |
export const isPlaylistUnchanged = (a, b) => a === b || | ||
(a.segments && b.segments && a.segments.length === b.segments.length && | ||
a.endList === b.endList && | ||
a.mediaSequence === b.mediaSequence); | ||
a.mediaSequence === b.mediaSequence && | ||
a.preloadSegment === b.preloadSegment); | ||
|
||
/** | ||
* Returns a new master playlist that is the result of merging an | ||
|
@@ -516,6 +517,8 @@ export default class PlaylistLoader extends EventTarget { | |
|
||
this.targetDuration = playlist.partTargetDuration || playlist.targetDuration; | ||
|
||
this.pendingMedia_ = null; | ||
|
||
if (update) { | ||
this.master = update; | ||
this.media_ = this.master.playlists[id]; | ||
|
@@ -662,6 +665,8 @@ export default class PlaylistLoader extends EventTarget { | |
this.trigger('mediachanging'); | ||
} | ||
|
||
this.pendingMedia_ = playlist; | ||
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. Added this to prevent switching to the same playlist twice at the start of playback. 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. Where does it get used? 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.request = this.vhs_.xhr({ | ||
uri: playlist.resolvedUri, | ||
withCredentials: this.withCredentials | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,43 @@ import {TIME_FUDGE_FACTOR} from './ranges.js'; | |
|
||
const {createTimeRange} = videojs; | ||
|
||
/** | ||
* Get the duration of a segment, with special cases for | ||
* llhls segments that do not have a duration yet. | ||
* | ||
* @param {Object} playlist | ||
* the playlist that the segment belongs to. | ||
* @param {Object} segment | ||
* the segment to get a duration for. | ||
* | ||
* @return {number} | ||
* the segment duration | ||
*/ | ||
export const segmentDurationWithParts = (playlist, segment) => { | ||
// if this isn't a preload segment | ||
// then we will have a segment duration that is accurate. | ||
if (!segment.preload) { | ||
return segment.duration; | ||
} | ||
|
||
// otherwise we have to add up parts and preload hints | ||
// to get an up to date duration. | ||
let result = 0; | ||
|
||
(segment.parts || []).forEach(function(p) { | ||
result += p.duration; | ||
}); | ||
|
||
// for preload hints we have to use partTargetDuration | ||
// as they won't even have a duration yet. | ||
(segment.preloadHints || []).forEach(function(p) { | ||
if (p.type === 'PART') { | ||
result += playlist.partTargetDuration; | ||
} | ||
}); | ||
|
||
return result; | ||
}; | ||
/** | ||
* A function to get a combined list of parts and segments with durations | ||
* and indexes. | ||
|
@@ -117,7 +154,7 @@ const backwardDuration = function(playlist, endSequence) { | |
return { result: result + segment.end, precise: true }; | ||
} | ||
|
||
result += segment.duration; | ||
result += segmentDurationWithParts(playlist, segment); | ||
|
||
if (typeof segment.start !== 'undefined') { | ||
return { result: result + segment.start, precise: true }; | ||
|
@@ -149,7 +186,7 @@ const forwardDuration = function(playlist, endSequence) { | |
}; | ||
} | ||
|
||
result += segment.duration; | ||
result += segmentDurationWithParts(playlist, segment); | ||
|
||
if (typeof segment.end !== 'undefined') { | ||
return { | ||
|
@@ -321,19 +358,19 @@ export const playlistEnd = function(playlist, expired, useSafeLiveEnd, liveEdgeP | |
|
||
expired = expired || 0; | ||
|
||
let lastSegmentTime = intervalDuration( | ||
let lastSegmentEndTime = intervalDuration( | ||
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. A more descriptive name as it isn't the start time, but the end time we are getting here. |
||
playlist, | ||
playlist.mediaSequence + playlist.segments.length, | ||
expired | ||
); | ||
|
||
if (useSafeLiveEnd) { | ||
liveEdgePadding = typeof liveEdgePadding === 'number' ? liveEdgePadding : liveEdgeDelay(null, playlist); | ||
lastSegmentTime -= liveEdgePadding; | ||
lastSegmentEndTime -= liveEdgePadding; | ||
} | ||
|
||
// don't return a time less than zero | ||
return Math.max(0, lastSegmentTime); | ||
return Math.max(0, lastSegmentEndTime); | ||
}; | ||
|
||
/** | ||
|
@@ -737,5 +774,6 @@ export default { | |
estimateSegmentRequestTime, | ||
isLowestEnabledRendition, | ||
isAudioOnly, | ||
playlistMatch | ||
playlistMatch, | ||
segmentDurationWithParts | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,8 +22,7 @@ import { | |
import { gopsSafeToAlignWith, removeGopBuffer, updateGopBuffer } from './util/gops'; | ||
import shallowEqual from './util/shallow-equal.js'; | ||
import { QUOTA_EXCEEDED_ERR } from './error-codes'; | ||
import { timeRangesToArray } from './ranges'; | ||
import {lastBufferedEnd} from './ranges.js'; | ||
import {timeRangesToArray, lastBufferedEnd, timeAheadOf} from './ranges.js'; | ||
import {getKnownPartCount} from './playlist.js'; | ||
|
||
/** | ||
|
@@ -135,7 +134,7 @@ export const safeBackBufferTrimTime = (seekable, currentTime, targetDuration) => | |
return Math.min(maxTrimTime, trimTime); | ||
}; | ||
|
||
const segmentInfoString = (segmentInfo) => { | ||
export const segmentInfoString = (segmentInfo) => { | ||
const { | ||
startOfSegment, | ||
duration, | ||
|
@@ -160,6 +159,10 @@ const segmentInfoString = (segmentInfo) => { | |
selection = 'getSyncSegmentCandidate (isSyncRequest)'; | ||
} | ||
|
||
if (segmentInfo.independent) { | ||
selection += ` with independent ${segmentInfo.independent}`; | ||
} | ||
|
||
const hasPartIndex = typeof partIndex === 'number'; | ||
const name = segmentInfo.segment.uri ? 'segment' : 'pre-segment'; | ||
const zeroBasedPartCount = hasPartIndex ? getKnownPartCount({preloadSegment: segment}) - 1 : 0; | ||
|
@@ -1024,9 +1027,20 @@ export default class SegmentLoader extends videojs.EventTarget { | |
|
||
if (!oldPlaylist || oldPlaylist.uri !== newPlaylist.uri) { | ||
if (this.mediaIndex !== null) { | ||
// we must "resync" the segment loader when we switch renditions and | ||
// we must reset/resync the segment loader when we switch renditions and | ||
// the segment loader is already synced to the previous rendition | ||
this.resyncLoader(); | ||
|
||
// on playlist changes we want it to be possible to fetch | ||
// at the buffer for vod but not for live. So we use resetLoader | ||
// for live and resyncLoader for vod. We want this because | ||
// if a playlist uses independent and non-independent segments/parts the | ||
// buffer may not accurately reflect the next segment that we should try | ||
// downloading. | ||
if (!newPlaylist.endList) { | ||
this.resetLoader(); | ||
} else { | ||
this.resyncLoader(); | ||
} | ||
} | ||
this.currentMediaInfo_ = void 0; | ||
this.trigger('playlistupdate'); | ||
|
@@ -1366,8 +1380,9 @@ export default class SegmentLoader extends videojs.EventTarget { | |
* @return {Object} a request object that describes the segment/part to load | ||
*/ | ||
chooseNextRequest_() { | ||
const bufferedEnd = lastBufferedEnd(this.buffered_()) || 0; | ||
const bufferedTime = Math.max(0, bufferedEnd - this.currentTime_()); | ||
const buffered = this.buffered_(); | ||
const bufferedEnd = lastBufferedEnd(buffered) || 0; | ||
const bufferedTime = timeAheadOf(buffered, this.currentTime_()); | ||
const preloaded = !this.hasPlayed_() && bufferedTime >= 1; | ||
const haveEnoughBuffer = bufferedTime >= this.goalBufferLength_(); | ||
const segments = this.playlist_.segments; | ||
|
@@ -1420,14 +1435,15 @@ export default class SegmentLoader extends videojs.EventTarget { | |
startTime: this.syncPoint_.time | ||
}); | ||
|
||
next.getMediaInfoForTime = this.fetchAtBuffer_ ? 'bufferedEnd' : 'currentTime'; | ||
next.getMediaInfoForTime = this.fetchAtBuffer_ ? | ||
`bufferedEnd ${bufferedEnd}` : `currentTime ${this.currentTime_()}`; | ||
next.mediaIndex = segmentIndex; | ||
next.startOfSegment = startTime; | ||
next.partIndex = partIndex; | ||
} | ||
|
||
const nextSegment = segments[next.mediaIndex]; | ||
const nextPart = nextSegment && | ||
let nextPart = nextSegment && | ||
typeof next.partIndex === 'number' && | ||
nextSegment.parts && | ||
nextSegment.parts[next.partIndex]; | ||
|
@@ -1442,6 +1458,28 @@ export default class SegmentLoader extends videojs.EventTarget { | |
// Set partIndex to 0 | ||
if (typeof next.partIndex !== 'number' && nextSegment.parts) { | ||
next.partIndex = 0; | ||
nextPart = nextSegment.parts[0]; | ||
} | ||
|
||
// if we have no buffered data then we need to make sure | ||
// that the next part we append is "independent" if possible. | ||
// So we check if the previous part is independent, and request | ||
// it if it is. | ||
if (!bufferedTime && nextPart && !nextPart.independent) { | ||
gkatsev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (next.partIndex === 0) { | ||
const lastSegment = segments[next.mediaIndex - 1]; | ||
const lastSegmentLastPart = lastSegment.parts && lastSegment.parts.length && lastSegment.parts[lastSegment.parts.length - 1]; | ||
|
||
if (lastSegmentLastPart && lastSegmentLastPart.independent) { | ||
next.mediaIndex -= 1; | ||
next.partIndex = lastSegment.parts.length - 1; | ||
next.independent = 'previous segment'; | ||
} | ||
} else if (nextSegment.parts[next.partIndex - 1].independent) { | ||
next.partIndex -= 1; | ||
next.independent = 'previous part'; | ||
} | ||
} | ||
|
||
const ended = this.mediaSource_ && this.mediaSource_.readyState === 'ended'; | ||
|
@@ -1459,6 +1497,7 @@ export default class SegmentLoader extends videojs.EventTarget { | |
|
||
generateSegmentInfo_(options) { | ||
const { | ||
independent, | ||
playlist, | ||
mediaIndex, | ||
startOfSegment, | ||
|
@@ -1499,7 +1538,8 @@ export default class SegmentLoader extends videojs.EventTarget { | |
byteLength: 0, | ||
transmuxer: this.transmuxer_, | ||
// type of getMediaInfoForTime that was used to get this segment | ||
getMediaInfoForTime | ||
getMediaInfoForTime, | ||
independent | ||
}; | ||
|
||
const overrideCheck = | ||
|
@@ -1991,7 +2031,7 @@ export default class SegmentLoader extends videojs.EventTarget { | |
this.setTimeMapping_(segmentInfo.timeline); | ||
|
||
// for tracking overall stats | ||
this.updateMediaSecondsLoaded_(segmentInfo.segment); | ||
this.updateMediaSecondsLoaded_(segmentInfo.part || segmentInfo.segment); | ||
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. stats fix |
||
|
||
// Note that the state isn't changed from loading to appending. This is because abort | ||
// logic may change behavior depending on the state, and changing state too early may | ||
|
@@ -2995,15 +3035,19 @@ export default class SegmentLoader extends videojs.EventTarget { | |
// and attempt to resync when the post-update seekable window and live | ||
// point would mean that this was the perfect segment to fetch | ||
this.trigger('syncinfoupdate'); | ||
|
||
const segment = segmentInfo.segment; | ||
const part = segmentInfo.part; | ||
const badSegmentGuess = segment.end && | ||
this.currentTime_() - segment.end > segmentInfo.playlist.targetDuration * 3; | ||
const badPartGuess = part && | ||
part.end && this.currentTime_() - part.end > segmentInfo.playlist.partTargetDuration * 3; | ||
|
||
// If we previously appended a segment that ends more than 3 targetDurations before | ||
// If we previously appended a segment/part that ends more than 3 part/targetDurations before | ||
// the currentTime_ that means that our conservative guess was too conservative. | ||
// In that case, reset the loader state so that we try to use any information gained | ||
// from the previous request to create a new, more accurate, sync-point. | ||
if (segment.end && | ||
this.currentTime_() - segment.end > segmentInfo.playlist.targetDuration * 3) { | ||
if (badSegmentGuess || badPartGuess) { | ||
this.logger_(`bad ${badSegmentGuess ? 'segment' : 'part'} ${segmentInfoString(segmentInfo)}`); | ||
this.resetEverything(); | ||
return; | ||
} | ||
|
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 should add the comment you made in github below above here.