- Notifications
You must be signed in to change notification settings - Fork2.7k
Fix Low-Latency HLS VTT subtitle part loading#7626
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 ourterms of service andprivacy 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?
Uh oh!
There was an error while loading.Please reload this page.
Changes from1 commit
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,4 @@ | ||
| import BaseStreamController, { State } from './base-stream-controller'; | ||
| import { FragmentState } from './fragment-tracker'; | ||
| import { ErrorDetails, ErrorTypes } from '../errors'; | ||
| import { Events } from '../events'; | ||
| @@ -48,7 +47,7 @@ export class SubtitleStreamController | ||
| implements NetworkComponentAPI | ||
| { | ||
| private currentTrackId: number = -1; | ||
| private tracksBuffered: Array<TimeRange[] | undefined> = []; | ||
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. I don't see anywhere where we are appending an undefined object to tracksBuffered? Do we suspect we need this? CollaboratorAuthor 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. We don't append 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. Got it, I think this is fine for now. And agree maybe something to look into in the future. | ||
| private mainDetails: LevelDetails | null = null; | ||
| constructor( | ||
| @@ -128,13 +127,7 @@ export class SubtitleStreamController | ||
| event: Events.SUBTITLE_FRAG_PROCESSED, | ||
| data: SubtitleFragProcessed, | ||
| ) { | ||
| const { frag, part, success } = data; | ||
| if (!success) { | ||
| return; | ||
| } | ||
| @@ -147,28 +140,32 @@ export class SubtitleStreamController | ||
| // Create/update a buffered array matching the interface used by BufferHelper.bufferedInfo | ||
| // so we can re-use the logic used to detect how much has been buffered | ||
| let timeRange: TimeRange | undefined; | ||
| conststart =(part ||frag).start; | ||
| for (let i = 0; i < buffered.length; i++) { | ||
| if (start >= buffered[i].start &&start <= buffered[i].end) { | ||
| timeRange = buffered[i]; | ||
| break; | ||
| } | ||
| } | ||
| constend = start +(part ||frag).duration; | ||
| if (timeRange) { | ||
| timeRange.end =end; | ||
| } else { | ||
| timeRange = { start, end }; | ||
| buffered.push(timeRange); | ||
| } | ||
| if (!part || end >= frag.end) { | ||
| this.fragmentTracker.fragBuffered(frag as MediaFragment); | ||
| if (!this.fragContextChanged(frag)) { | ||
| if (isMediaFragment(frag)) { | ||
| this.fragPrevious = frag; | ||
| } | ||
| } | ||
| this.fragBufferedComplete(frag, part); | ||
| if (this.media) { | ||
| this.tickImmediate(); | ||
| } | ||
| } | ||
| } | ||
| @@ -184,6 +181,7 @@ export class SubtitleStreamController | ||
| } | ||
| data.endOffsetSubtitles = Math.max(0, endOffsetSubtitles); | ||
| this.tracksBuffered.forEach((buffered) => { | ||
| if (!buffered) return; | ||
| for (let i = 0; i < buffered.length; ) { | ||
| if (buffered[i].end <= endOffsetSubtitles) { | ||
| buffered.shift(); | ||
| @@ -259,13 +257,13 @@ export class SubtitleStreamController | ||
| } | ||
| // Check if track has the necessary details to load fragments | ||
| const currentTrack = this.levels[this.currentTrackId] as Level | undefined; | ||
| if (!currentTrack?.details) { | ||
| this.mediaBuffer = null; | ||
| return; | ||
| } | ||
| this.mediaBuffer = this.mediaBufferTimeRanges; | ||
| if (this.state !== State.STOPPED) { | ||
| this.setInterval(TICK_INTERVAL); | ||
| } | ||
| } | ||
| @@ -281,7 +279,7 @@ export class SubtitleStreamController | ||
| this.warn(`Subtitle tracks were reset while loading level ${trackId}`); | ||
| return; | ||
| } | ||
| const track= levels[trackId] as Level | undefined; | ||
| if (trackId >= levels.length || !track) { | ||
| return; | ||
| } | ||
| @@ -346,26 +344,7 @@ export class SubtitleStreamController | ||
| }); | ||
| // trigger handler right now | ||
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 logic for streaming subtitles in LL-HLS has become much more lenient 👍 CollaboratorAuthor 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. Additional changes in#7482 will help to ensure proper playlist alignment. When combined with these changes, audio and subtitle segment loading will follow more of the same logic. | ||
| this.tickImmediate(); | ||
| } | ||
| _handleFragmentLoadComplete(fragLoadedData: FragLoadedData) { | ||
| @@ -423,18 +402,23 @@ export class SubtitleStreamController | ||
| } | ||
| doTick() { | ||
| if (this.state === State.IDLE) { | ||
| if ( | ||
| !this.media && | ||
| !this.primaryPrefetch && | ||
| (this.startFragRequested || !this.config.startFragPrefetch) | ||
| ) { | ||
| return; | ||
| } | ||
| const { currentTrackId, levels } = this; | ||
| const track = levels?.[currentTrackId]; | ||
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. Before there was a check to see if track is undefined, now it's removed here. I believe this may cause issues. We may need to keep what was here previously: CollaboratorAuthor 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. We're covered by 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. Got it. that's true that the original code had that: | ||
| const trackDetails = track?.details; | ||
| if ( | ||
| !trackDetails || | ||
| this.waitForLive(track) || | ||
| this.waitForCdnTuneIn(trackDetails) | ||
| ) { | ||
| this.startFragRequested = false; | ||
| return; | ||
| } | ||
| const { config } = this; | ||
| @@ -445,62 +429,32 @@ export class SubtitleStreamController | ||
| config.maxBufferHole, | ||
| ); | ||
| const { end: targetBufferTime, len: bufferLen } = bufferedInfo; | ||
| const maxBufLen = | ||
| this.hls.maxBufferLength + trackDetails.levelTargetDuration; | ||
| if (bufferLen > maxBufLen || (bufferLen && !this.buffering)) { | ||
| return; | ||
| } | ||
| let frag = this.getNextFragment(currentTime, trackDetails); | ||
| if (!frag) { | ||
| return; | ||
| } | ||
| // Load earlier fragment in same discontinuity to make up for misaligned playlists and cues that extend beyond end of segment | ||
| if (isMediaFragment(frag)) { | ||
| const curSNIdx = frag.sn - trackDetails.startSN; | ||
| const prevFrag = trackDetails.fragments[curSNIdx - 1] as | ||
| | MediaFragment | ||
| | undefined; | ||
| if ( | ||
| prevFrag && | ||
| prevFrag.cc === frag.cc && | ||
| !trackDetails.partList?.length && | ||
| this.fragmentTracker.getState(prevFrag) === FragmentState.NOT_LOADED | ||
| ) { | ||
| frag = prevFrag; | ||
| } | ||
| } | ||
| this.loadFragment(frag, track, targetBufferTime); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.