Repository navigation
Mark fragments as gaps on SourceBuffer errors #8045
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
base: master
Are you sure you want to change the base?
Changes from all commits
ff85808
722a9d8
f13adb5
798f04c
f4ea147
b77946e
ae98eba
1844c97
acb4866
65b7da6
8703bb3
41f2e10
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 |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import { SOURCE_BUFFER_ERROR_NAME } from './buffer-controller'; | ||
| import { ErrorActionFlags, NetworkErrorAction } from './error-controller'; | ||
| import { | ||
| findFragmentByPDT, | ||
|
|
@@ -368,7 +369,11 @@ export default class BaseStreamController | |
| return; | ||
| } | ||
| this.loadingParts = false; | ||
| this.fragmentTracker.removeAllFragments(); | ||
| // Keep tracked gaps to skip segments given up on after re-attach (`recoverMediaError()`) | ||
| const { fragmentTracker } = this; | ||
| const gaps = fragmentTracker.gapFragments(); | ||
| fragmentTracker.removeAllFragments(); | ||
| gaps.forEach((frag) => fragmentTracker.addAsGap(frag)); | ||
| this.stopLoad(); | ||
| } | ||
|
|
||
|
|
@@ -438,13 +443,19 @@ export default class BaseStreamController | |
| } | ||
|
|
||
| if (media) { | ||
| // Remove gap fragments | ||
| this.fragmentTracker.removeFragmentsInRange( | ||
| currentTime, | ||
| Infinity, | ||
| this.playlistType, | ||
| true, | ||
| ); | ||
| if ( | ||
| this.hls?.hasEnoughToStart && | ||
| !fowardBuffer && | ||
| currentTime !== this.startPosition | ||
| ) { | ||
| // Remove gap fragments on a seek out of the buffer, after `seekToStartPos` | ||
| this.fragmentTracker.removeFragmentsInRange( | ||
| currentTime, | ||
| Infinity, | ||
| this.playlistType, | ||
| true, | ||
| ); | ||
| } | ||
|
|
||
| // Don't set lastCurrentTime with backward seeks (allows for frag selection with strict tolerances) | ||
| const lastCurrentTime = this.lastCurrentTime; | ||
|
|
@@ -1625,7 +1636,7 @@ export default class BaseStreamController | |
| return true; | ||
| } | ||
| if (trackerState === FragmentState.PARTIAL) { | ||
| if (frag.gap) { | ||
| if (frag.gap || this.fragmentTracker.isGap(frag)) { | ||
| return true; | ||
| } | ||
| if (frag.stats.retry > 1) { | ||
|
|
@@ -1758,7 +1769,8 @@ export default class BaseStreamController | |
| if (nextPart > -1 && targetBufferTime < part.start) { | ||
| break; | ||
| } | ||
| const loaded = part.loaded || part.gap; | ||
| const loaded = | ||
| part.loaded || part.gap || this.fragmentTracker.isGap(part.fragment); | ||
| if (loaded) { | ||
| nextPart = -1; | ||
| } else if ( | ||
|
|
@@ -1878,7 +1890,7 @@ export default class BaseStreamController | |
| ): MediaFragment | null { | ||
| const { config } = this; | ||
| let { fragPrevious } = this; | ||
| let { fragments, endSN } = levelDetails; | ||
| let { fragments } = levelDetails; | ||
| const { fragmentHint } = levelDetails; | ||
| const { maxFragLookUpTolerance } = config; | ||
| const partList = levelDetails.partList; | ||
|
|
@@ -1895,7 +1907,6 @@ export default class BaseStreamController | |
| ) { | ||
| // Include incomplete fragment with parts at end | ||
| fragments = fragments.concat(fragmentHint); | ||
| endSN = fragmentHint.sn; | ||
| } | ||
|
|
||
| // Clear fragPrevious if removed from buffer / tracker so that is picked again | ||
|
|
@@ -1937,19 +1948,24 @@ export default class BaseStreamController | |
| const fragState = this.fragmentTracker.getState(frag); | ||
| if ( | ||
| fragState === FragmentState.OK || | ||
| (fragState === FragmentState.PARTIAL && frag.gap) | ||
| (fragState === FragmentState.PARTIAL && | ||
| (frag.gap || this.fragmentTracker.isGap(frag))) | ||
| ) { | ||
| fragPrevious = frag; | ||
| } | ||
| if ( | ||
| mediaFragmentsAreEqual(frag, fragPrevious) && | ||
| (!loadingParts || | ||
| partList[0].fragment.sn > frag.sn || | ||
| !levelDetails.live) | ||
| !levelDetails.live || | ||
| this.fragmentTracker.isGap(frag)) | ||
| ) { | ||
| const nextFrag = fragments[curSNIdx + 1]; | ||
| let nextFrag = fragments[curSNIdx + 1]; | ||
| while (nextFrag && this.fragmentTracker.isGap(nextFrag)) { | ||
| nextFrag = fragments[nextFrag.sn - levelDetails.startSN + 1]; | ||
| } | ||
| if ( | ||
| frag.sn < endSN && | ||
| nextFrag && | ||
| this.fragmentTracker.getState(nextFrag) !== FragmentState.OK | ||
| ) { | ||
| frag = nextFrag; | ||
|
|
@@ -2211,6 +2227,49 @@ export default class BaseStreamController | |
| this.tickImmediate(); | ||
| } | ||
|
|
||
| protected onSourceBufferError( | ||
| filterType: PlaylistLevelType, | ||
| data: ErrorData, | ||
| ) { | ||
| const { frag, part, sourceBufferName } = data; | ||
| const details = frag && this.levels?.[frag.level]?.details; | ||
| if ( | ||
| data.error.name !== SOURCE_BUFFER_ERROR_NAME || | ||
| frag?.type !== filterType || | ||
| !isMediaFragment(frag) || | ||
| !details?.live | ||
| ) { | ||
| return; | ||
| } | ||
| const refused = part ? (part.index === 0 ? part : null) : frag; | ||
| const start = | ||
| refused && sourceBufferName | ||
| ? refused.elementaryStreams[sourceBufferName]?.startPTS | ||
| : undefined; | ||
| this.markSourceBufferErrorGap(frag); | ||
| if (start === undefined) { | ||
| return; | ||
| } | ||
| const previous = getFragmentWithSN(details, frag.sn - 1); | ||
| if ( | ||
| previous && | ||
| this.fragmentTracker.getState(previous) !== FragmentState.NOT_LOADED && | ||
| !BufferHelper.isBuffered( | ||
|
Comment on lines
+2256
to
+2257
Collaborator
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. The previous fragment is not buffered but also not-not_loaded?
Collaborator
Author
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. Yes, a truncated tail. The previous fragment was appended without error, but the buffer ends before the refused fragment starts, so it would be reloaded and refused again. In 7 of 24 truncated runs the tracker state was OK, because the hole was under its 0.2 s padding, so the buffered check is needed. Removed the comment. |
||
| this.mediaBuffer || this.media, | ||
| start - this.config.maxBufferHole, | ||
| ) | ||
| ) { | ||
| this.markSourceBufferErrorGap(previous); | ||
| } | ||
| } | ||
|
|
||
| private markSourceBufferErrorGap(frag: MediaFragment) { | ||
| this.warn( | ||
| `Marking fragment ${frag.sn} of ${this.playlistLabel()} ${frag.level} as a gap after a SourceBuffer error`, | ||
| ); | ||
| this.fragmentTracker.addAsGap(frag); | ||
| } | ||
|
|
||
| protected checkRetryDate() { | ||
| const now = self.performance.now(); | ||
| const retryDate = this.retryDate; | ||
|
|
||
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.
Can you explain this change? Do source buffer errors on parts result in the whole fragment getting marked as gap?
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.
Yes. The refused append is often not the damaged one: a truncated part is accepted and the next append fails. Marking only the reported part reset repeatedly in 3 of 5 runs.
The check skips the parts
loadPartrejects (frag.gap || part.gap). It reads the tracker because after a refreshpart.fragmentis a new object.