Skip to content

Commit

Permalink
Interstitial fragment loading and buffering fixes (#7002)
Browse files Browse the repository at this point in the history
* Interstitial fragment loading and buffering fixes:
- Fix streaming primary content over queued assets
- Fix "Main forward buffer length" warning on "seeked" for asset players
- Fix audio streaming gaps in asset players created after buffer is populated (handles incomplete fragment tracker)
- Fix incorrect interstitial buffering state after flush
- Fix advancing interstitial buffering advancement with append-in-place
- Allow append-in-place at live edge with single playlist media selection (not allowed with alt-audio) (#6995)
- Run skip buffer hole when stalling at a buffered range that ends prior to live playlist window start (gap-controller)
- Add safety around asset player getters that throw when destroyed
- Fix interstitial start/end events at edge (playing last schedule item transitions with new interstitial) (interstitials-controller)
- Fix asset player queue leaks (interstitials-controller `clearAssetPlayer` exits early resulting in `playerQueue` buildup)

Resolves #6995
  • Loading branch information
robwalch authored Feb 7, 2025
1 parent 6442629 commit 78e50de
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 47 deletions.
2 changes: 2 additions & 0 deletions api-extractor/report/hls.js.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,8 @@ export class BaseStreamController extends TaskLoop implements NetworkComponentAP
// (undocumented)
protected getFwdBufferInfo(bufferable: Bufferable | null, type: PlaylistLevelType): BufferInfo | null;
// (undocumented)
protected getFwdBufferInfoAtPos(bufferable: Bufferable | null, pos: number, type: PlaylistLevelType, maxBufferHole: number): BufferInfo | null;
// (undocumented)
protected getInitialLiveFragment(levelDetails: LevelDetails, fragments: MediaFragment[]): MediaFragment | null;
// (undocumented)
getLevelDetails(): LevelDetails | undefined;
Expand Down
41 changes: 22 additions & 19 deletions src/controller/audio-stream-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,35 +416,38 @@ class AudioStreamController
}

// Request audio segments up to one fragment ahead of main stream-controller
const mainFragLoading = this.mainFragLoading?.frag;
let mainFragLoading = this.mainFragLoading?.frag || null;
if (
!this.audioOnly &&
this.startFragRequested &&
mainFragLoading &&
isMediaFragment(mainFragLoading) &&
isMediaFragment(frag) &&
!frag.endList &&
(!trackDetails.live ||
(!this.loadingParts && targetBufferTime < this.hls.liveSyncPosition!))
) {
let mainFrag = mainFragLoading;
if (frag.start > mainFrag.end) {
// Get buffered frag at target position from tracker (loaded out of sequence)
const mainFragAtPos = this.fragmentTracker.getFragAtPos(
targetBufferTime,
PlaylistLevelType.MAIN,
);
if (mainFragAtPos && mainFragAtPos.end > mainFragLoading.end) {
mainFrag = mainFragAtPos;
this.mainFragLoading = {
frag: mainFragAtPos,
targetBufferTime: null,
};
}
if (this.fragmentTracker.getState(mainFragLoading) === FragmentState.OK) {
this.mainFragLoading = mainFragLoading = null;
}
const atBufferSyncLimit = frag.start > mainFrag.end;
if (atBufferSyncLimit) {
return;
if (mainFragLoading && isMediaFragment(mainFragLoading)) {
if (frag.start > mainFragLoading.end) {
// Get buffered frag at target position from tracker (loaded out of sequence)
const mainFragAtPos = this.fragmentTracker.getFragAtPos(
targetBufferTime,
PlaylistLevelType.MAIN,
);
if (mainFragAtPos && mainFragAtPos.end > mainFragLoading.end) {
mainFragLoading = mainFragAtPos;
this.mainFragLoading = {
frag: mainFragAtPos,
targetBufferTime: null,
};
}
}
const atBufferSyncLimit = frag.start > mainFragLoading.end;
if (atBufferSyncLimit) {
return;
}
}
}

Expand Down
16 changes: 15 additions & 1 deletion src/controller/base-stream-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,20 @@ export default class BaseStreamController
}
}
}
// Skip loading of fragments that overlap completely with appendInPlace interstitals
const playerQueue = interstitials?.playerQueue;
if (playerQueue) {
for (let i = playerQueue.length; i--; ) {
const interstitial = playerQueue[i].interstitial;
if (
interstitial.appendInPlace &&
frag.start >= interstitial.startTime &&
frag.end <= interstitial.resumeTime
) {
return;
}
}
}
}
this.startFragRequested = true;
this._loadFragForPlayback(frag, level, targetBufferTime);
Expand Down Expand Up @@ -1201,7 +1215,7 @@ export default class BaseStreamController
return this.getFwdBufferInfoAtPos(bufferable, pos, type, maxBufferHole);
}

private getFwdBufferInfoAtPos(
protected getFwdBufferInfoAtPos(
bufferable: Bufferable | null,
pos: number,
type: PlaylistLevelType,
Expand Down
7 changes: 5 additions & 2 deletions src/controller/gap-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,9 +436,12 @@ export default class GapController extends TaskLoop {
}

const currentTime = media.currentTime;

const levelDetails = this.hls?.latestLevelDetails;
const partial = fragmentTracker.getPartialFragment(currentTime);
if (partial) {
if (
partial ||
(levelDetails?.live && currentTime < levelDetails.fragmentStart)
) {
// Try to skip over the buffer hole caused by a partial fragment
// This method isn't limited by the size of the gap between buffered ranges
const targetTime = this._trySkipBufferHole(partial);
Expand Down
4 changes: 2 additions & 2 deletions src/controller/interstitial-player.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export class HlsAssetPlayer {
}

get media(): HTMLMediaElement | null {
return this.hls.media;
return this.hls?.media || null;
}

get bufferedEnd(): number {
Expand Down Expand Up @@ -114,7 +114,7 @@ export class HlsAssetPlayer {
}

get timelineOffset(): number {
return this.hls.config.timelineOffset || 0;
return this.hls?.config.timelineOffset || 0;
}

set timelineOffset(value: number) {
Expand Down
28 changes: 14 additions & 14 deletions src/controller/interstitials-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -613,11 +613,12 @@ export default class InterstitialsController
// Schedule getters
private get playingLastItem(): boolean {
const playingItem = this.playingItem;
if (!this.playbackStarted || !playingItem) {
const items = this.schedule?.items;
if (!this.playbackStarted || !playingItem || !items) {
return false;
}
const items = this.schedule?.items;
return this.itemsMatch(playingItem, items ? items[items.length - 1] : null);

return this.findItemIndex(playingItem) === items.length - 1;
}

private get playbackStarted(): boolean {
Expand Down Expand Up @@ -677,6 +678,7 @@ export default class InterstitialsController
const appendInPlace = player.interstitial.appendInPlace;
const playerMedia = player.media;
if (appendInPlace && playerMedia === this.primaryMedia) {
this.bufferingAsset = null;
if (
!toSegment ||
(this.isInterstitial(toSegment) && !toSegment.event.appendInPlace)
Expand All @@ -685,14 +687,13 @@ export default class InterstitialsController
// no-op when toSegment is undefined
if (toSegment && playerMedia) {
this.detachedData = { media: playerMedia };
return;
}
return;
}
const attachMediaSourceData = player.transferMedia();
this.log(
`transfer MediaSource from ${player} ${JSON.stringify(attachMediaSourceData)}`,
);
this.bufferingAsset = null;
this.detachedData = attachMediaSourceData;
} else if (toSegment && playerMedia) {
this.shouldPlay ||= !playerMedia.paused;
Expand Down Expand Up @@ -760,7 +761,7 @@ MediaSource ${JSON.stringify(attachMediaSourceData)} from ${logFromSource}`,
dataToAttach.overrides = {
duration: this.schedule.duration,
endOfStream: !isAssetPlayer || isAssetAtEndOfSchedule,
cueRemoval: false,
cueRemoval: !isAssetPlayer,
};
}
player.attachMedia(dataToAttach);
Expand Down Expand Up @@ -1417,7 +1418,7 @@ MediaSource ${JSON.stringify(attachMediaSourceData)} from ${logFromSource}`,
) {
const timelinePos = this.timelinePos;
this.bufferedPos = timelinePos;
this.setBufferingItem(playingItem);
this.checkBuffer();
}
}

Expand Down Expand Up @@ -1582,7 +1583,7 @@ Schedule: ${scheduleItems.map((seg) => segmentToString(seg))}`,
(a.event && b.event && this.eventItemsMatch(a, b)) ||
(!a.event &&
!b.event &&
a.nextEvent?.identifier === b.nextEvent?.identifier))
this.findItemIndex(a) === this.findItemIndex(b)))
);
}

Expand Down Expand Up @@ -1647,9 +1648,11 @@ Schedule: ${scheduleItems.map((seg) => segmentToString(seg))}`,
const nextToBufferIndex = Math.min(bufferingIndex + 1, items.length - 1);
const nextItemToBuffer = items[nextToBufferIndex];
if (
bufferEndIndex === -1 &&
bufferingItem &&
bufferEnd >= bufferingItem.end
(bufferEndIndex === -1 &&
bufferingItem &&
bufferEnd >= bufferingItem.end) ||
(nextItemToBuffer.event?.appendInPlace &&
bufferEnd + 0.01 >= nextItemToBuffer.start)
) {
bufferEndIndex = nextToBufferIndex;
}
Expand Down Expand Up @@ -2136,9 +2139,6 @@ Schedule: ${scheduleItems.map((seg) => segmentToString(seg))}`,
assetId: InterstitialAssetId,
toSegment: InterstitialScheduleItem | null,
) {
if (toSegment === null) {
return;
}
const playerIndex = this.getAssetPlayerQueueIndex(assetId);
if (playerIndex !== -1) {
this.log(
Expand Down
16 changes: 11 additions & 5 deletions src/controller/interstitials-schedule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ export class InterstitialsSchedule extends Logger {
}
// Only return index of a Primary Item
while (index >= 0 && items[index]?.event) {
// If index found is an interstitial it is not a valid result as it should have been matched up top
// decrement until result is negative (not found) or a primary segment
index--;
}
}
Expand Down Expand Up @@ -592,14 +594,18 @@ export class InterstitialsSchedule extends Logger {
);
return false;
}
return !Object.keys(mediaSelection).some((playlistType) => {
const playlists = Object.keys(mediaSelection);
return !playlists.some((playlistType) => {
const details = mediaSelection[playlistType].details;
const playlistEnd = details.edge;
if (resumeTime > playlistEnd) {
this.log(
`"${interstitial.identifier}" resumption ${resumeTime} past ${playlistType} playlist end ${playlistEnd}`,
);
return true;
if (playlists.length > 1) {
this.log(
`"${interstitial.identifier}" resumption ${resumeTime} past ${playlistType} playlist end ${playlistEnd}`,
);
return true;
}
return false;
}
const startFragment = findFragmentByPTS(
null,
Expand Down
18 changes: 14 additions & 4 deletions src/controller/stream-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -573,15 +573,25 @@ export default class StreamController
private onMediaSeeked = () => {
const media = this.media;
const currentTime = media ? media.currentTime : null;
if (Number.isFinite(currentTime)) {
this.log(`Media seeked to ${(currentTime as number).toFixed(3)}`);
if (currentTime === null || !Number.isFinite(currentTime)) {
return;
}

this.log(`Media seeked to ${currentTime.toFixed(3)}`);

// If seeked was issued before buffer was appended do not tick immediately
const bufferInfo = this.getMainFwdBufferInfo();
if (!this.getBufferedFrag(currentTime)) {
return;
}
const bufferInfo = this.getFwdBufferInfoAtPos(
media,
currentTime,
PlaylistLevelType.MAIN,
0,
);
if (bufferInfo === null || bufferInfo.len === 0) {
this.warn(
`Main forward buffer length on "seeked" event ${
`Main forward buffer length at ${currentTime} on "seeked" event ${
bufferInfo ? bufferInfo.len : 'empty'
})`,
);
Expand Down

0 comments on commit 78e50de

Please sign in to comment.