From ff858087d738baaa254baec943eb004e34f7feed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=B0=ED=99=8D=EC=A4=80?= Date: Fri, 18 Sep 2026 17:45:48 +0900 Subject: [PATCH 1/9] Skip a fragment the SourceBuffer keeps rejecting An append rejection sets HTMLMediaElement.error, so every later appendBuffer throws until the media is rebuilt. buffer-controller reclassifies that as MEDIA_SOURCE_REQUIRES_RESET, the MediaSource is reset, and the reset puts the playhead back on the same fragment. With no alternate level to switch to, the cycle runs for as long as the stream is watched. #7941 already ends a per-fragment budget in fragmentTracker.addAsGap(), after which fragment-loader refuses the fragment. The budget did not cover the rejection case, and the gap it sets did not survive to the next load. Four places dropped it and masked each other: addAsGap() marked only the object that was appended, frag.stats.retry was incremented after the event that marks the gap had already been handled, initTracks() cleared the per-fragment count on every SourceBuffer rebuild, and mergeDetails lost a gap set on a preload hint that has no duration yet. The only public addition is fragmentTracker.gapFragments(), so onMediaDetaching can re-add gaps around removeAllFragments(). Nothing is written to tagList, no part is marked, and there is no new config, event, error detail, field or error action. Playlist lookups reuse getFragmentWithSN(). The trade-off is that fragment-loader reads frag.stats.retry as "already given up on", and that field is also the network retry counter. FragmentLoader has no reference to FragmentTracker, so no existing signal carries this on its own. Fixes #8035 --- api-extractor/report/hls.js.api.md | 1 + src/controller/base-stream-controller.ts | 13 +- src/controller/buffer-controller.ts | 66 ++++- src/controller/error-controller.ts | 7 +- src/controller/fragment-tracker.ts | 23 +- src/loader/fragment-loader.ts | 8 +- src/utils/level-helper.ts | 13 + .../unit/controller/base-stream-controller.ts | 107 +++++++ .../buffer-controller-operations.ts | 272 ++++++++++++++++++ tests/unit/controller/fragment-tracker.ts | 164 +++++++++++ tests/unit/controller/level-helper.ts | 31 ++ tests/unit/loader/fragment-loader.ts | 25 ++ 12 files changed, 715 insertions(+), 15 deletions(-) diff --git a/api-extractor/report/hls.js.api.md b/api-extractor/report/hls.js.api.md index fe72fbeb7e6..d2c6fc2edb6 100644 --- a/api-extractor/report/hls.js.api.md +++ b/api-extractor/report/hls.js.api.md @@ -2130,6 +2130,7 @@ export class FragmentTracker implements ComponentAPI { detectPartialFragments(data: FragBufferedData): void; // (undocumented) fragBuffered(frag: MediaFragment, force?: true): FragmentEntity | undefined; + gapFragments(): MediaFragment[]; getAppendedFrag(position: number, levelType: PlaylistLevelType): MediaFragment | Part | null; getBackBufferEvictionEnd(beforePosition: number, levelType: PlaylistLevelType, bytesNeeded: number): number; getBufferedFrag(position: number, levelType: PlaylistLevelType): MediaFragment | null; diff --git a/src/controller/base-stream-controller.ts b/src/controller/base-stream-controller.ts index a5dfa6aa9da..03f6e65d9da 100644 --- a/src/controller/base-stream-controller.ts +++ b/src/controller/base-stream-controller.ts @@ -368,7 +368,12 @@ export default class BaseStreamController return; } this.loadingParts = false; - this.fragmentTracker.removeAllFragments(); + // recoverMediaError() detaches media, and dropping gaps here reloads a fragment + // already given up on. + const { fragmentTracker } = this; + const gaps = fragmentTracker.gapFragments(); + fragmentTracker.removeAllFragments(); + gaps.forEach((frag) => fragmentTracker.addAsGap(frag)); this.stopLoad(); } @@ -1758,7 +1763,9 @@ export default class BaseStreamController if (nextPart > -1 && targetBufferTime < part.start) { break; } - const loaded = part.loaded || part.gap; + // fragment-loader refuses a part whose fragment is a gap, so selecting one loads nothing. + const gap = part.gap || !!part.fragment.gap; + const loaded = part.loaded || gap; if (loaded) { nextPart = -1; } else if ( @@ -1767,7 +1774,7 @@ export default class BaseStreamController ) { nextPart = i; } - contiguous = loaded && !part.gap; + contiguous = loaded && !gap; } const part = partList[nextPart]; if (part && part.fragment !== frag) { diff --git a/src/controller/buffer-controller.ts b/src/controller/buffer-controller.ts index 8bab74ce5d3..fbbef723453 100755 --- a/src/controller/buffer-controller.ts +++ b/src/controller/buffer-controller.ts @@ -1,5 +1,8 @@ import BufferOperationQueue from './buffer-operation-queue'; -import { createDoNothingErrorAction } from './error-controller'; +import { + createDoNothingErrorAction, + isAppendStateErrorName, +} from './error-controller'; import { ErrorDetails, ErrorTypes } from '../errors'; import { Events } from '../events'; import { ElementaryStreamTypes, isMediaFragment } from '../loader/fragment'; @@ -70,6 +73,7 @@ const VIDEO_CODEC_PROFILE_REPLACE = /(avc[1234]|hvc1|hev1|dvh[1e]|vp09|av01)(?:\.[^.,]+)+/; const TRACK_REMOVED_ERROR_NAME = 'HlsJsTrackRemovedError'; +const APPEND_TIMEOUT_ERROR_NAME = 'HlsJsAppendTimeoutError'; const LOOP_FLUSH_SAFETY_MARGIN = 0.25; @@ -259,7 +263,9 @@ export default class BufferController extends Logger implements ComponentAPI { private initTracks() { const tracks = {}; - this.resetAppendProgress(); + // Only per-load cycle state belongs here. An append rejection cycle rebuilds the + // SourceBuffers, so clearing the per-fragment count here would restart the budget. + this.fragmentAppendProgress = Object.create(null); this.sourceBuffers = [ [null, null], [null, null], @@ -1012,10 +1018,42 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe } } - // Append exceptions use the existing append-error path. - if (trackProgress) { + // A track removal, an append timeout and the error names error-controller already + // reads as buffer state are not about the fragment's bytes, and keep their own paths. + const rejectedBytes = + !isQuotaError && + !isAppendStateErrorName(error.name) && + error.name !== TRACK_REMOVED_ERROR_NAME && + error.name !== APPEND_TIMEOUT_ERROR_NAME; + // A refused append adds no buffered coverage, so it spends the same per-fragment + // budget a no-progress cycle does. Parts and partial loads count here, unlike there. + const ownedByFragmentBudget = + rejectedBytes && + isMediaFragment(frag) && + !frag.gap && + !chunkMeta.iframe; + if (ownedByFragmentBudget) { + const progress = this.getFragmentAppendProgress(frag); + if (!progress.errored) { + const spent = + (this.appendsWithoutProgress[appendProgressKey(frag)] || 0) + 1 >= + this.hls.config.appendErrorMaxRetry; + if (spent) { + // Recorded before the event below, which is what marks the fragment a gap: + // fragment-loader only keeps that gap when a retry is already on the fragment. + frag.stats.retry++; + } + this.countAppendWithoutProgress(frag, chunkMeta); + } + progress.errored = true; + } else if (trackProgress) { + // Append exceptions use the existing append-error path. this.getFragmentAppendProgress(frag as MediaFragment).errored = true; } + if (!this.hls) { + // Destroyed by a synchronous BUFFER_APPEND_NO_PROGRESS listener + return; + } // in case any error occured while appending, put back segment in segments table const event: ErrorData = { type: ErrorTypes.MEDIA_ERROR, @@ -1051,7 +1089,10 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe this.warn( `Failed ${appendErrorCount}/${appendErrorMaxRetry + 1} times to append segment in "${type}" sourceBuffer with error: ${error.message}`, ); - if (appendErrorCount >= appendErrorMaxRetry) { + if ( + appendErrorCount >= appendErrorMaxRetry && + !ownedByFragmentBudget + ) { event.fatal = !isQuotaError; } const readyState = this.mediaSource?.readyState; @@ -1397,7 +1438,8 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe return progress; } - // Full-fragment cycles without growth consume the append retry budget. + // Full-fragment cycles without growth consume the append retry budget. Rejected appends + // consume it too, counted at the append-error path above. // Parts, I-Frames, gaps, and partial loads are excluded below. private checkAppendProgress( frag: Fragment, @@ -1439,6 +1481,14 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe return; } // Also count parsed fragments that produced no append operations. + this.countAppendWithoutProgress(frag, chunkMeta); + } + + private countAppendWithoutProgress( + frag: MediaFragment, + chunkMeta: ChunkMetadata, + ) { + const key = appendProgressKey(frag); const count = (this.appendsWithoutProgress[key] || 0) + 1; // Capture values before a synchronous ERROR listener can destroy Hls. const hls = this.hls; @@ -2187,7 +2237,9 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe const operation = this.currentOp(type); if (operation) { - operation.onError(new Error(`${type}-append-timeout`)); + const timeoutError = new Error(`${type}-append-timeout`); + timeoutError.name = APPEND_TIMEOUT_ERROR_NAME; + operation.onError(timeoutError); } } diff --git a/src/controller/error-controller.ts b/src/controller/error-controller.ts index bdcb0c50836..db8ba9678d6 100644 --- a/src/controller/error-controller.ts +++ b/src/controller/error-controller.ts @@ -632,12 +632,15 @@ function isCodecRelated(data: ErrorData): boolean { return false; } +export function isAppendStateErrorName(name: string): boolean { + return name === 'QuotaExceededError' || name === 'InvalidStateError'; +} + function isAppendStateRelated(data: ErrorData): boolean { const errorDetails = data.details; return ( errorDetails === ErrorDetails.BUFFER_APPEND_ERROR && - (data.error.name === 'QuotaExceededError' || - data.error.name === 'InvalidStateError') + isAppendStateErrorName(data.error.name) ); } diff --git a/src/controller/fragment-tracker.ts b/src/controller/fragment-tracker.ts index 94cc34c81ad..3e51e64df16 100644 --- a/src/controller/fragment-tracker.ts +++ b/src/controller/fragment-tracker.ts @@ -5,6 +5,8 @@ import { type MediaFragment, type Part, } from '../loader/fragment'; +import { PlaylistLevelType } from '../types/loader'; +import { getFragmentWithSN } from '../utils/level-helper'; import { userAgentChromeVersion } from '../utils/user-agent'; import type Hls from '../hls'; import type { SourceBufferName } from '../types/buffer'; @@ -19,7 +21,6 @@ import type { FragmentEntity, FragmentTimeRange, } from '../types/fragment-tracker'; -import type { PlaylistLevelType } from '../types/loader'; export const enum FragmentState { NOT_LOADED = 'NOT_LOADED', @@ -83,6 +84,7 @@ export class FragmentTracker implements ComponentAPI { this.endListFragments = this.timeRanges = null; + this.hasGaps = false; } /** @@ -294,6 +296,15 @@ export class FragmentTracker implements ComponentAPI { public addAsGap(frag: MediaFragment) { frag.gap = true; + // A live playlist refresh replaces fragment objects, so the appended copy is not always + // the one selection and the loader read. Mark the one the playlist holds at this sn. + const selectable = + frag.type === PlaylistLevelType.MAIN + ? getFragmentWithSN(this.hls?.latestLevelDetails || undefined, frag.sn) + : null; + if (selectable?.level === frag.level) { + selectable.gap = true; + } this.removeFragment(frag); this.fragBuffered(frag, true); } @@ -614,6 +625,16 @@ export class FragmentTracker implements ComponentAPI { }); } + /** Fragments determined to be gaps, for callers that clear buffer state and re-add them. */ + public gapFragments(): MediaFragment[] { + if (!this.hasGaps) { + return []; + } + return Object.keys(this.fragments) + .map((key) => this.fragments[key]?.body) + .filter((frag): frag is MediaFragment => !!frag?.gap); + } + public removeFragment(fragment: Fragment) { const fragKey = getFragmentKey(fragment); fragment.clearElementaryStreamInfo(); diff --git a/src/loader/fragment-loader.ts b/src/loader/fragment-loader.ts index 23bb650c60f..c2662b15cab 100644 --- a/src/loader/fragment-loader.ts +++ b/src/loader/fragment-loader.ts @@ -73,11 +73,15 @@ export default class FragmentLoader { this.loader.destroy(); } if (frag.gap) { - if (frag.tagList.some((tags) => tags[0] === 'GAP')) { + if ( + frag.tagList.some((tags) => tags[0] === 'GAP') || + frag.stats.retry > 0 + ) { reject(createGapLoadError(frag)); return; } else { - // Reset temporary treatment as GAP tag + // Reset temporary treatment as GAP tag, unless a retry is recorded against the + // fragment: stream-controller records a locally gapped fragment in the same field. frag.gap = false; } } diff --git a/src/utils/level-helper.ts b/src/utils/level-helper.ts index 449dd76fbaa..90a008be872 100644 --- a/src/utils/level-helper.ts +++ b/src/utils/level-helper.ts @@ -210,6 +210,9 @@ export function mergeDetails( newFrag.stats = oldFrag.stats; } + // Use locally set gap or GAP tag, as the part callback below already does + newFrag.gap = oldFrag.gap || newFrag.gap; + if (oldFrag.initSegment) { newFrag.initSegment = oldFrag.initSegment; currentInitSegment = oldFrag.initSegment; @@ -221,6 +224,16 @@ export function mergeDetails( const fragmentsToCheck = newDetails.fragmentHint ? newFragments.concat(newDetails.fragmentHint) : newFragments; + + // mapFragmentIntersection leaves out an old preload hint that has no duration yet, so a gap + // set on one is lost when the hint becomes a fragment. Carry that one field across. + const oldHint = oldDetails.fragmentHint; + if (oldHint?.gap && !oldHint.duration) { + const replacement = getFragmentWithSN(newDetails, oldHint.sn); + if (replacement) { + replacement.gap = true; + } + } if (currentInitSegment) { fragmentsToCheck.forEach((frag) => { if ( diff --git a/tests/unit/controller/base-stream-controller.ts b/tests/unit/controller/base-stream-controller.ts index db6c80fc634..1871b2ab533 100644 --- a/tests/unit/controller/base-stream-controller.ts +++ b/tests/unit/controller/base-stream-controller.ts @@ -1149,6 +1149,113 @@ describe('BaseStreamController', function () { }); }); + describe('onMediaDetaching', function () { + const detachableMedia = () => + ({ + duration: 0, + ended: false, + buffered: new TimeRangesMock(), + removeEventListener: () => undefined, + }) as unknown as HTMLMediaElement; + + it('re-adds gaps after clearing the fragment tracker', function () { + const gapped = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; + gapped.sn = 7; + gapped.gap = true; + const calls: string[] = []; + fragmentTracker.gapFragments = () => { + calls.push('gapFragments'); + return [gapped]; + }; + fragmentTracker.removeAllFragments = () => + calls.push('removeAllFragments'); + fragmentTracker.addAsGap = (frag: MediaFragment) => { + calls.push(`addAsGap:${frag.sn}`); + }; + baseStreamController.media = detachableMedia(); + + (baseStreamController as any).onMediaDetaching(null, {}); + + // recoverMediaError() detaches, so the judgement has to outlive the clear + expect(calls).to.deep.equal([ + 'gapFragments', + 'removeAllFragments', + 'addAsGap:7', + ]); + }); + + it('keeps the fragment tracker intact when the media is transferred', function () { + let cleared = false; + fragmentTracker.gapFragments = () => []; + fragmentTracker.removeAllFragments = () => { + cleared = true; + }; + baseStreamController.media = detachableMedia(); + + (baseStreamController as any).onMediaDetaching(null, { + transferMedia: {}, + }); + + expect(cleared, 'a transfer does not clear buffer state').to.equal(false); + }); + }); + + describe('getNextPart', function () { + const partList = ( + frag: MediaFragment, + count: number, + independent = true, + ): Part[] => + Array.from({ length: count }, (_, index) => ({ + index, + start: index, + duration: 1, + independent, + loaded: false, + gap: false, + fragment: frag, + })) as unknown as Part[]; + + it('selects a part of the fragment', function () { + const frag = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; + frag.sn = 1; + + expect( + (baseStreamController as any).getNextPart(partList(frag, 3), frag, 10), + ).to.not.equal(-1); + }); + + it('does not select a part of a fragment marked as a gap', function () { + const frag = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; + frag.sn = 1; + // fragment-loader rejects these with FRAG_GAP, so selecting one loads nothing + frag.gap = true; + const parts = partList(frag, 3); + + expect( + (baseStreamController as any).getNextPart(parts, frag, 10), + ).to.equal(-1); + expect( + parts.every((part) => !part.gap), + 'selection leaves the parts unmarked', + ).to.equal(true); + }); + + it('does not carry continuity across a gapped fragment', function () { + const gapped = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; + gapped.sn = 1; + gapped.gap = true; + const next = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; + next.sn = 2; + // A part that cannot be decoded on its own needs a buffered part before it + const parts = [...partList(gapped, 1), ...partList(next, 1, false)]; + + expect( + (baseStreamController as any).getNextPart(parts, next, 10), + ).to.equal(-1); + }); + }); + describe('getBufferedFrag', function () { it('should call fragmentTracker.getBufferedFrag with correct parameters', function () { const mockFrag = new Fragment(PlaylistLevelType.MAIN, 'test.ts'); diff --git a/tests/unit/controller/buffer-controller-operations.ts b/tests/unit/controller/buffer-controller-operations.ts index f91038552f9..9b3e82ab76a 100644 --- a/tests/unit/controller/buffer-controller-operations.ts +++ b/tests/unit/controller/buffer-controller-operations.ts @@ -446,6 +446,7 @@ describe('BufferController with attached media', function () { const [, errorEvent] = triggerSpy.lastCall.args; expect(errorEvent.error.message).to.equal(`${name}-append-timeout`); + expect(errorEvent.error.name).to.equal('HlsJsAppendTimeoutError'); }); }); @@ -872,6 +873,277 @@ describe('BufferController with attached media', function () { expect(errors).to.have.lengthOf(0); }); + const appendAndFail = ( + frag: Fragment, + error: Error, + part: Part | null = null, + chunkMeta: ChunkMetadata | null = null, + ) => { + const buffer = getSourceBufferTrack(bufferController, 'video') + ?.buffer as any; + buffer.appendBuffer.throws(error); + hls.trigger(Events.BUFFER_APPENDING, { + parent: PlaylistLevelType.MAIN, + type: 'video', + data: new Uint8Array(), + frag, + part, + chunkMeta: + chunkMeta || + new ChunkMetadata( + 0, + frag.sn as number, + 0, + 0, + part ? part.index : -1, + !!part, + ), + }); + }; + const noProgressErrors = (errors: ErrorData[]) => + errors.filter( + (e) => e.details === ErrorDetails.BUFFER_APPEND_NO_PROGRESS, + ); + + it('counts an append the SourceBuffer rejected', function () { + const errors: ErrorData[] = []; + hls.on(Events.ERROR, (event, data) => errors.push(data)); + const frag = newFrag(1); + + appendAndFail(frag, new Error('append failed')); + // A media source reset reloads the fragment, replacing its LoadStats + frag.stats = new LoadStats(); + appendAndFail(frag, new Error('append failed')); + + const counted = noProgressErrors(errors); + expect(counted).to.have.lengthOf(2); + expect(counted[0].appendsWithoutProgress).to.equal(1); + expect(counted[1].appendsWithoutProgress).to.equal(2); + expect(counted[1].frag?.sn).to.equal(1); + }); + + it('counts a refused part against the fragment', function () { + const errors: ErrorData[] = []; + hls.on(Events.ERROR, (event, data) => errors.push(data)); + const frag = newFrag(1); + const part = { + index: 1, + fragment: frag, + stats: new LoadStats(), + } as unknown as Part; + + // A low latency playlist delivers the fragment as parts first + appendAndFail(frag, new Error('append failed'), part); + frag.stats = new LoadStats(); + appendAndFail(frag, new Error('append failed'), part); + + const counted = noProgressErrors(errors); + expect(counted).to.have.lengthOf(2); + expect(counted[1].appendsWithoutProgress).to.equal(2); + }); + + it('counts one rejection per load, not per rejected chunk', function () { + const errors: ErrorData[] = []; + hls.on(Events.ERROR, (event, data) => errors.push(data)); + const frag = newFrag(1); + + // Every chunk of the same load is refused once the element error is set + appendAndFail(frag, new Error('append failed')); + appendAndFail(frag, new Error('append failed')); + appendAndFail(frag, new Error('append failed')); + + expect(noProgressErrors(errors)).to.have.lengthOf(1); + }); + + it('keeps the per-fragment count across the buffer reset each cycle makes', function () { + const errors: ErrorData[] = []; + hls.on(Events.ERROR, (event, data) => errors.push(data)); + const frag = newFrag(1); + + appendAndFail(frag, new Error('append failed')); + expect(noProgressErrors(errors)).to.have.lengthOf(1); + + // Every rejection cycle rebuilds the SourceBuffers, which is what the budget counts + (bufferController as any).initTracks(); + + expect( + (bufferController as any).appendsWithoutProgress.main_1_0, + 'the budget would never bind if the rebuild cleared it', + ).to.equal(1); + }); + + it('gives up on the fragment once its append budget is spent', function () { + const errors: ErrorData[] = []; + hls.on(Events.ERROR, (event, data) => errors.push(data)); + const frag = newFrag(1); + const budget = hls.config.appendErrorMaxRetry; + + for (let i = 0; i < budget; i++) { + // Each media source reset reloads the fragment, replacing its LoadStats + frag.stats = new LoadStats(); + appendAndFail(frag, new Error('append failed')); + } + + const counted = noProgressErrors(errors); + expect(counted).to.have.lengthOf(budget); + expect(counted[counted.length - 1].appendsWithoutProgress).to.equal( + budget, + ); + expect( + frag.stats.retry, + 'the fragment is recorded as having had its retries', + ).to.equal(1); + const appendErrors = errors.filter( + (e) => + e.details === ErrorDetails.BUFFER_APPEND_ERROR || + e.details === ErrorDetails.MEDIA_SOURCE_REQUIRES_RESET, + ); + const last = appendErrors[appendErrors.length - 1]; + expect(last.fatal, 'skipping a fragment does not stop loading').to.equal( + false, + ); + }); + + it('survives a listener that destroys Hls while the rejection is reported', function () { + const errors: ErrorData[] = []; + hls.on(Events.ERROR, (event, data) => { + errors.push(data); + if (data.details === ErrorDetails.BUFFER_APPEND_NO_PROGRESS) { + bufferController.destroy(); + } + }); + + appendAndFail(newFrag(1), new Error('append failed')); + + expect( + errors.filter((e) => e.details === ErrorDetails.INTERNAL_EXCEPTION), + ).to.have.lengthOf(0); + }); + + it('records the retry before announcing that the budget is spent', function () { + const frag = newFrag(1); + const budget = hls.config.appendErrorMaxRetry; + let retryWhenSpent: number | null = null; + hls.on(Events.ERROR, (event, data) => { + if ( + data.details === ErrorDetails.BUFFER_APPEND_NO_PROGRESS && + data.appendsWithoutProgress === budget + ) { + // stream-controller marks the fragment a gap from this event, and fragment-loader + // only keeps that gap when a retry is already on the fragment + retryWhenSpent = (data.frag as Fragment).stats.retry; + } + }); + + for (let i = 0; i < budget; i++) { + frag.stats = new LoadStats(); + appendAndFail(frag, new Error('append failed')); + } + + expect( + retryWhenSpent, + 'the gap is durable when it is announced', + ).to.equal(1); + }); + + it('does not count a quota, invalid state, track removal or append timeout error', function () { + const errors: ErrorData[] = []; + hls.on(Events.ERROR, (event, data) => errors.push(data)); + + const quota = new Error('quota'); + (quota as any).code = DOMException.QUOTA_EXCEEDED_ERR; + appendAndFail(newFrag(1), quota); + + const invalidState = new Error('the SourceBuffer has been removed'); + invalidState.name = 'InvalidStateError'; + appendAndFail(newFrag(4), invalidState); + + const removed = new Error('track removed'); + removed.name = 'HlsJsTrackRemovedError'; + appendAndFail(newFrag(2), removed); + + const timeout = new Error('video-append-timeout'); + timeout.name = 'HlsJsAppendTimeoutError'; + appendAndFail(newFrag(3), timeout); + + expect(noProgressErrors(errors)).to.have.lengthOf(0); + }); + + it('does not record a retry before the budget is spent', function () { + const errors: ErrorData[] = []; + hls.on(Events.ERROR, (event, data) => errors.push(data)); + const frag = newFrag(1); + + appendAndFail(frag, new Error('append failed')); + + expect(noProgressErrors(errors)).to.have.lengthOf(1); + expect( + frag.stats.retry, + 'a fragment with budget left is still fetched', + ).to.equal(0); + }); + + it('does not spend the budget on an init segment', function () { + const errors: ErrorData[] = []; + hls.on(Events.ERROR, (event, data) => errors.push(data)); + const frag = newFrag(1); + frag.sn = 'initSegment'; + + appendAndFail(frag, new Error('append failed')); + + expect( + noProgressErrors(errors), + 'an init segment keeps the existing append-error path', + ).to.have.lengthOf(0); + }); + + it('does not spend the budget on a fragment already given up on', function () { + const errors: ErrorData[] = []; + hls.on(Events.ERROR, (event, data) => errors.push(data)); + const frag = newFrag(1); + frag.gap = true; + + appendAndFail(frag, new Error('append failed')); + + expect( + noProgressErrors(errors), + 'a gap is not counted twice', + ).to.have.lengthOf(0); + }); + + it('does not spend the budget on an I-frame chunk', function () { + const errors: ErrorData[] = []; + hls.on(Events.ERROR, (event, data) => errors.push(data)); + const frag = newFrag(1); + const iframeMeta = new ChunkMetadata(0, 1, 0, 0, -1, false, 0, true); + + appendAndFail(frag, new Error('append failed'), null, iframeMeta); + + expect( + noProgressErrors(errors), + 'I-frames are excluded here as they are in checkAppendProgress', + ).to.have.lengthOf(0); + }); + + it('does not let a refused part hide the fragment cycle when the buffer was at fault', function () { + const errors: ErrorData[] = []; + hls.on(Events.ERROR, (event, data) => errors.push(data)); + const frag = newFrag(1); + const part = { + index: 0, + fragment: frag, + stats: new LoadStats(), + } as unknown as Part; + const quota = new Error('quota'); + (quota as any).code = DOMException.QUOTA_EXCEEDED_ERR; + + appendAndFail(frag, quota, part); + // The part said nothing about the fragment's bytes, so the cycle is still unjudged + runAppendCycle(frag, false); + + expect(noProgressErrors(errors)).to.have.lengthOf(1); + }); + it('emits BUFFER_APPEND_NO_PROGRESS through the append pipeline when the target buffer does not grow', function () { const errors: ErrorData[] = []; hls.on(Events.ERROR, (event, data) => { diff --git a/tests/unit/controller/fragment-tracker.ts b/tests/unit/controller/fragment-tracker.ts index 41cb027548d..1cb3d25971f 100644 --- a/tests/unit/controller/fragment-tracker.ts +++ b/tests/unit/controller/fragment-tracker.ts @@ -10,6 +10,7 @@ import { ElementaryStreamTypes, Fragment } from '../../../src/loader/fragment'; import { LoadStats } from '../../../src/loader/load-stats'; import { PlaylistLevelType } from '../../../src/types/loader'; import { ChunkMetadata } from '../../../src/types/transmuxer'; +import type { MediaFragment } from '../../../src/loader/fragment'; import type { BufferAppendedData, FragBufferedData, @@ -584,6 +585,169 @@ describe('FragmentTracker', function () { ).to.be.false; }); }); + + it('marks the fragment the playlist holds, not only the one appended', function () { + const appended = createMockFragment( + { + startPTS: 0, + endPTS: 1, + sn: 1, + level: 0, + type: PlaylistLevelType.MAIN, + }, + [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], + ); + // A live playlist refresh replaces the object while the appended copy is still in flight + const inPlaylist = createMockFragment( + { + startPTS: 0, + endPTS: 1, + sn: 1, + level: 0, + type: PlaylistLevelType.MAIN, + }, + [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], + ); + const details = { startSN: 1, fragments: [inPlaylist] }; + Object.defineProperty(hls, 'latestLevelDetails', { + configurable: true, + get: () => details, + }); + + fragmentTracker.addAsGap(appended as MediaFragment); + + expect(inPlaylist.gap, 'the selectable fragment is a gap too').to.equal( + true, + ); + }); + + it('does not mark a fragment of another level that shares the sn', function () { + const appended = createMockFragment( + { + startPTS: 0, + endPTS: 1, + sn: 1, + level: 4, + type: PlaylistLevelType.MAIN, + }, + [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], + ); + // latestLevelDetails is whichever level loaded last, which is not always this one + const otherLevel = createMockFragment( + { + startPTS: 0, + endPTS: 1, + sn: 1, + level: 3, + type: PlaylistLevelType.MAIN, + }, + [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], + ); + const details = { startSN: 1, fragments: [otherLevel] }; + Object.defineProperty(hls, 'latestLevelDetails', { + configurable: true, + get: () => details, + }); + + fragmentTracker.addAsGap(appended as MediaFragment); + + expect( + otherLevel.gap, + 'another level holds different bytes at the same sn', + ).to.not.equal(true); + }); + + it('does not mark the main fragment when another track has a gap', function () { + const mainFrag = createMockFragment( + { + startPTS: 0, + endPTS: 1, + sn: 1, + level: 0, + type: PlaylistLevelType.MAIN, + }, + [ElementaryStreamTypes.VIDEO], + ); + const audioFrag = createMockFragment( + { + startPTS: 0, + endPTS: 1, + sn: 1, + level: 0, + type: PlaylistLevelType.AUDIO, + }, + [ElementaryStreamTypes.AUDIO], + ); + // latestLevelDetails is the main playlist, so another track's sn must not reach it + const details = { startSN: 1, fragments: [mainFrag] }; + Object.defineProperty(hls, 'latestLevelDetails', { + configurable: true, + get: () => details, + }); + + fragmentTracker.addAsGap(audioFrag as MediaFragment); + + expect(mainFrag.gap, 'the main fragment is untouched').to.not.equal(true); + }); + + it('lists fragments marked as gaps so they can be re-added', function () { + const buffered = createMockFragment( + { + startPTS: 0, + endPTS: 1, + sn: 1, + level: 1, + type: PlaylistLevelType.MAIN, + }, + [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], + ); + const gapped = createMockFragment( + { + startPTS: 1, + endPTS: 2, + sn: 2, + level: 1, + type: PlaylistLevelType.MAIN, + }, + [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], + ); + expect(fragmentTracker.gapFragments(), 'no gaps tracked').to.deep.equal( + [], + ); + fragmentTracker.fragBuffered(buffered as MediaFragment, true); + fragmentTracker.addAsGap(gapped as MediaFragment); + expect(fragmentTracker.hasFragment(buffered), 'both are tracked').to.be + .true; + + const gaps = fragmentTracker.gapFragments(); + expect(gaps).to.have.lengthOf(1); + expect(gaps[0].sn, 'only the gap is listed').to.equal(2); + + fragmentTracker.removeAllFragments(); + gaps.forEach((frag) => fragmentTracker.addAsGap(frag)); + + expect(fragmentTracker.hasFragment(gapped), 'gap is re-added').to.be.true; + expect(fragmentTracker.hasFragment(buffered), 'buffered entry is gone').to + .be.false; + }); + + it('lists no gaps once the tracker is destroyed', function () { + const gapped = createMockFragment( + { + startPTS: 0, + endPTS: 1, + sn: 1, + level: 0, + type: PlaylistLevelType.MAIN, + }, + [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], + ); + fragmentTracker.addAsGap(gapped as MediaFragment); + fragmentTracker.destroy(); + + // onMediaDetaching reaches a destroyed tracker, which has no fragments left to read + expect(fragmentTracker.gapFragments()).to.deep.equal([]); + }); }); }); diff --git a/tests/unit/controller/level-helper.ts b/tests/unit/controller/level-helper.ts index dac95d6440f..770660522de 100644 --- a/tests/unit/controller/level-helper.ts +++ b/tests/unit/controller/level-helper.ts @@ -96,6 +96,37 @@ describe('LevelHelper Tests', function () { expect(actual).to.deep.equal([5]); }); + it('carries a locally set gap to the replacement fragment', function () { + const oldPlaylist = generatePlaylist([1, 2, 3]); + const newPlaylist = generatePlaylist([1, 2, 3]); + oldPlaylist.fragments[1].gap = true; + mergeDetails(oldPlaylist, newPlaylist, logger); + expect(newPlaylist.fragments[1].gap, 'gap is carried over').to.equal( + true, + ); + expect( + newPlaylist.fragments[0].gap, + 'other fragments are untouched', + ).to.not.equal(true); + }); + + it('carries a locally set gap from a preload hint that has no duration yet', function () { + const oldPlaylist = generatePlaylist([1, 2, 3]); + const newPlaylist = generatePlaylist([1, 2, 3, 4]); + const hint = new Fragment(PlaylistLevelType.MAIN, ''); + hint.sn = 4; + hint.setStart(15); + // A low latency preload hint has no duration until its parts complete + hint.duration = 0; + hint.gap = true; + oldPlaylist.fragmentHint = hint as MediaFragment; + mergeDetails(oldPlaylist, newPlaylist, logger); + expect( + newPlaylist.fragments[3].gap, + 'gap reaches the fragment the hint became', + ).to.equal(true); + }); + it('can iterate over the entire segment array', function () { const oldPlaylist = generatePlaylist([1, 2, 3]); const newPlaylist = generatePlaylist([1, 2, 3]); diff --git a/tests/unit/loader/fragment-loader.ts b/tests/unit/loader/fragment-loader.ts index 1dea228254a..115c9184c67 100644 --- a/tests/unit/loader/fragment-loader.ts +++ b/tests/unit/loader/fragment-loader.ts @@ -101,6 +101,31 @@ describe('FragmentLoader tests', function () { }); }); + it('rejects a fragment given up on locally, without a GAP tag', function () { + return new Promise((resolve, reject) => { + // The append budget marks the fragment and records the attempt it spent + frag.gap = true; + frag.stats.retry = 1; + fragmentLoader + .load(frag) + .then(() => { + reject(new Error('Fragment loader should not have resolved')); + }) + .catch((error: LoadError) => { + expect(error.data.details).to.equal(ErrorDetails.FRAG_GAP); + expect(frag.gap, 'the fragment is still a gap').to.equal(true); + resolve(); + }); + }); + }); + + it('retries a fragment marked as a gap that has not been given up on', function () { + frag.gap = true; + fragmentLoader.load(frag).catch(() => undefined); + expect(frag.gap, 'temporary treatment as a gap is reset').to.equal(false); + fragmentLoader.abort(); + }); + it('handles fragment load errors', function () { const fragmentLoaderPrivates = fragmentLoader as any; return new Promise((resolve, reject) => { From 722a9d8c181572673787ff65d3f63057137f5d5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=B0=ED=99=8D=EC=A4=80?= Date: Mon, 21 Sep 2026 13:03:50 +0900 Subject: [PATCH 2/9] Keep a locally set gap only when an append was attempted `frag.stats.retry` counts network retries as well as the attempt the append budget spends, so refusing to reset a gap on `retry > 0` alone also refused fragments that were gapped before any of their bytes reached the SourceBuffer. A fragment that hit a transient network failure and was later passed to `treatAsGap` lost the one reload the branch exists to give it, which is the "skip valid media" case raised in review. Require the append attempt as well. `stats.buffering.start` is set in `onBufferAppending` and stays zero for a media fragment gapped from a load or parse error, so it separates the two sources of a retry without a new field. Covers both terms: a gap with a retry but no append attempt is reset, and a gap with an append attempt but no retry is reset. --- src/loader/fragment-loader.ts | 10 +++++++--- tests/unit/loader/fragment-loader.ts | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/loader/fragment-loader.ts b/src/loader/fragment-loader.ts index c2662b15cab..c7b5a35d48a 100644 --- a/src/loader/fragment-loader.ts +++ b/src/loader/fragment-loader.ts @@ -73,15 +73,19 @@ export default class FragmentLoader { this.loader.destroy(); } if (frag.gap) { + const { retry, buffering } = frag.stats; if ( frag.tagList.some((tags) => tags[0] === 'GAP') || - frag.stats.retry > 0 + // A retry recorded on a fragment whose bytes reached the SourceBuffer is the + // append budget giving up on it. The same field also counts network retries, so + // require the append attempt: a fragment gapped before any append is still the + // temporary treatment this branch exists to undo. + (retry > 0 && buffering.start > 0) ) { reject(createGapLoadError(frag)); return; } else { - // Reset temporary treatment as GAP tag, unless a retry is recorded against the - // fragment: stream-controller records a locally gapped fragment in the same field. + // Reset temporary treatment as GAP tag frag.gap = false; } } diff --git a/tests/unit/loader/fragment-loader.ts b/tests/unit/loader/fragment-loader.ts index 115c9184c67..d139092a679 100644 --- a/tests/unit/loader/fragment-loader.ts +++ b/tests/unit/loader/fragment-loader.ts @@ -106,6 +106,7 @@ describe('FragmentLoader tests', function () { // The append budget marks the fragment and records the attempt it spent frag.gap = true; frag.stats.retry = 1; + frag.stats.buffering.start = 1; fragmentLoader .load(frag) .then(() => { @@ -126,6 +127,24 @@ describe('FragmentLoader tests', function () { fragmentLoader.abort(); }); + it('retries a fragment gapped with no retry recorded, after an append attempt', function () { + // treatAsGap on a fragment that appended before, with nothing spent against it + frag.gap = true; + frag.stats.buffering.start = 1; + fragmentLoader.load(frag).catch(() => undefined); + expect(frag.gap, 'temporary treatment as a gap is reset').to.equal(false); + fragmentLoader.abort(); + }); + + it('retries a fragment gapped after a network retry, with no append attempted', function () { + // A transient network failure records a retry on the same field the append budget uses + frag.gap = true; + frag.stats.retry = 1; + fragmentLoader.load(frag).catch(() => undefined); + expect(frag.gap, 'temporary treatment as a gap is reset').to.equal(false); + fragmentLoader.abort(); + }); + it('handles fragment load errors', function () { const fragmentLoaderPrivates = fragmentLoader as any; return new Promise((resolve, reject) => { From 798f04c3a0095af5d4181e94acbcae5808bf9f53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=B0=ED=99=8D=EC=A4=80?= Date: Wed, 23 Sep 2026 17:20:59 +0900 Subject: [PATCH 3/9] Mark fragments as gaps on SourceBuffer errors Replace the append budget approach. buffer-controller only names the error created in onSBUpdateError. The stream controllers mark the fragment as a gap on MEDIA_SOURCE_REQUIRES_RESET with that name, and also the previous fragment when the first part of a fragment is refused, since the parser reports incomplete bytes on the following append. Gaps are carried across detach and kept on the seek that follows re-attach. Fragment and part selection skip only fragments marked this way. The last fragment of an ended playlist is not marked. --- api-extractor/report/hls.js.api.md | 3 + src/controller/audio-stream-controller.ts | 3 + src/controller/base-stream-controller.ts | 98 ++++++- src/controller/buffer-controller.ts | 68 +---- src/controller/error-controller.ts | 7 +- src/controller/fragment-tracker.ts | 40 ++- src/controller/stream-controller.ts | 3 + src/loader/fragment-loader.ts | 10 +- src/utils/level-helper.ts | 13 - .../controller/audio-stream-controller.ts | 35 +++ .../unit/controller/base-stream-controller.ts | 257 +++++++++------- .../buffer-controller-operations.ts | 277 +----------------- tests/unit/controller/fragment-tracker.ts | 186 +++--------- tests/unit/controller/level-helper.ts | 31 -- tests/unit/controller/stream-controller.ts | 225 +++++++++++++- tests/unit/loader/fragment-loader.ts | 44 --- 16 files changed, 572 insertions(+), 728 deletions(-) diff --git a/api-extractor/report/hls.js.api.md b/api-extractor/report/hls.js.api.md index d2c6fc2edb6..55fbb61d69a 100644 --- a/api-extractor/report/hls.js.api.md +++ b/api-extractor/report/hls.js.api.md @@ -592,6 +592,8 @@ export class BaseStreamController extends TaskLoop implements NetworkComponentAP // (undocumented) protected onMediaSeeking: () => void; // (undocumented) + protected onSourceBufferError(filterType: PlaylistLevelType, data: ErrorData): void; + // (undocumented) protected onTickEnd(): void; // (undocumented) pauseBuffering(): void; @@ -2145,6 +2147,7 @@ export class FragmentTracker implements ComponentAPI { hasParts(type: PlaylistLevelType): boolean; // (undocumented) isEndListAppended(type: PlaylistLevelType): boolean; + isGap(frag: Fragment): boolean; // (undocumented) removeAllFragments(): void; // (undocumented) diff --git a/src/controller/audio-stream-controller.ts b/src/controller/audio-stream-controller.ts index 6b848d40cd8..4fbbc8387b7 100644 --- a/src/controller/audio-stream-controller.ts +++ b/src/controller/audio-stream-controller.ts @@ -850,6 +850,9 @@ class AudioStreamController this.fragmentTracker.addAsGap(data.frag as MediaFragment); } break; + case ErrorDetails.MEDIA_SOURCE_REQUIRES_RESET: + this.onSourceBufferError(PlaylistLevelType.AUDIO, data); + break; case ErrorDetails.BUFFER_ADD_CODEC_ERROR: case ErrorDetails.BUFFER_APPEND_ERROR: if (data.parent !== 'audio') { diff --git a/src/controller/base-stream-controller.ts b/src/controller/base-stream-controller.ts index 03f6e65d9da..fe75b08b3df 100644 --- a/src/controller/base-stream-controller.ts +++ b/src/controller/base-stream-controller.ts @@ -1,3 +1,4 @@ +import { SOURCE_BUFFER_ERROR_NAME } from './buffer-controller'; import { ErrorActionFlags, NetworkErrorAction } from './error-controller'; import { findFragmentByPDT, @@ -128,6 +129,8 @@ export default class BaseStreamController protected buffering: boolean = true; protected loadingParts: boolean = false; private loopSn?: string | number; + private keepGapsOnSeek: boolean = false; + private sourceBufferErrorGaps: Set = new Set(); constructor( hls: Hls, @@ -368,12 +371,16 @@ export default class BaseStreamController return; } this.loadingParts = false; - // recoverMediaError() detaches media, and dropping gaps here reloads a fragment - // already given up on. + // 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.keepGapsOnSeek = gaps.some( + (frag) => + frag.type === this.playlistType && + this.sourceBufferErrorGaps.has(gapKey(frag)), + ); this.stopLoad(); } @@ -387,6 +394,8 @@ export default class BaseStreamController this.lastCurrentTime = 0; this.startPosition = -1; this.startFragRequested = false; + this.keepGapsOnSeek = false; + this.sourceBufferErrorGaps.clear(); } protected onError(event: Events.ERROR, data: ErrorData) {} @@ -443,13 +452,18 @@ export default class BaseStreamController } if (media) { - // Remove gap fragments - this.fragmentTracker.removeFragmentsInRange( - currentTime, - Infinity, - this.playlistType, - true, - ); + if (this.keepGapsOnSeek) { + // Keep SourceBuffer error gaps on the seek that follows re-attach + this.keepGapsOnSeek = false; + } else { + // Remove gap fragments + 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; @@ -1618,6 +1632,17 @@ export default class BaseStreamController this.nextLoadPosition = programFrag.start; } } + while (programFrag && this.isSourceBufferErrorGap(programFrag)) { + const afterGap = this.filterReplacedPrimary( + getNextFrag(levelDetails, programFrag.sn), + levelDetails, + ); + if (!afterGap) { + return null; + } + this.nextLoadPosition = afterGap.start; + programFrag = afterGap; + } return programFrag; } @@ -1763,9 +1788,8 @@ export default class BaseStreamController if (nextPart > -1 && targetBufferTime < part.start) { break; } - // fragment-loader refuses a part whose fragment is a gap, so selecting one loads nothing. - const gap = part.gap || !!part.fragment.gap; - const loaded = part.loaded || gap; + const loaded = + part.loaded || part.gap || this.isSourceBufferErrorGap(part.fragment); if (loaded) { nextPart = -1; } else if ( @@ -1774,7 +1798,7 @@ export default class BaseStreamController ) { nextPart = i; } - contiguous = loaded && !gap; + contiguous = loaded && !part.gap; } const part = partList[nextPart]; if (part && part.fragment !== frag) { @@ -2217,6 +2241,50 @@ export default class BaseStreamController this.tickImmediate(); } + protected onSourceBufferError( + filterType: PlaylistLevelType, + data: ErrorData, + ) { + const { frag, part } = data; + if ( + data.error.name !== SOURCE_BUFFER_ERROR_NAME || + frag?.type !== filterType || + !isMediaFragment(frag) || + frag.endList + ) { + return; + } + // The SourceBuffer can fail on an append after the damaged one, so mark the fragment, + // and the previous one when the first part of a fragment is refused. + this.markSourceBufferErrorGap(frag); + const previous = + part?.index === 0 + ? getFragmentWithSN(this.levels?.[frag.level]?.details, frag.sn - 1) + : null; + if ( + previous && + this.fragmentTracker.getState(previous) !== FragmentState.NOT_LOADED && + !this.fragmentTracker.isGap(previous) + ) { + 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.sourceBufferErrorGaps.add(gapKey(frag)); + this.fragmentTracker.addAsGap(frag); + } + + private isSourceBufferErrorGap(frag: Fragment): boolean { + return ( + this.sourceBufferErrorGaps.has(gapKey(frag)) && + this.fragmentTracker.isGap(frag) + ); + } + protected checkRetryDate() { const now = self.performance.now(); const retryDate = this.retryDate; @@ -2797,3 +2865,7 @@ export function interstitialsEnabled(config: Readonly): boolean { config.enableInterstitialPlayback !== false ); } + +function gapKey(frag: Fragment): string { + return `${frag.level}_${frag.sn}`; +} diff --git a/src/controller/buffer-controller.ts b/src/controller/buffer-controller.ts index fbbef723453..fad67106276 100755 --- a/src/controller/buffer-controller.ts +++ b/src/controller/buffer-controller.ts @@ -1,8 +1,5 @@ import BufferOperationQueue from './buffer-operation-queue'; -import { - createDoNothingErrorAction, - isAppendStateErrorName, -} from './error-controller'; +import { createDoNothingErrorAction } from './error-controller'; import { ErrorDetails, ErrorTypes } from '../errors'; import { Events } from '../events'; import { ElementaryStreamTypes, isMediaFragment } from '../loader/fragment'; @@ -73,7 +70,7 @@ const VIDEO_CODEC_PROFILE_REPLACE = /(avc[1234]|hvc1|hev1|dvh[1e]|vp09|av01)(?:\.[^.,]+)+/; const TRACK_REMOVED_ERROR_NAME = 'HlsJsTrackRemovedError'; -const APPEND_TIMEOUT_ERROR_NAME = 'HlsJsAppendTimeoutError'; +export const SOURCE_BUFFER_ERROR_NAME = 'HlsJsSourceBufferError'; const LOOP_FLUSH_SAFETY_MARGIN = 0.25; @@ -263,9 +260,7 @@ export default class BufferController extends Logger implements ComponentAPI { private initTracks() { const tracks = {}; - // Only per-load cycle state belongs here. An append rejection cycle rebuilds the - // SourceBuffers, so clearing the per-fragment count here would restart the budget. - this.fragmentAppendProgress = Object.create(null); + this.resetAppendProgress(); this.sourceBuffers = [ [null, null], [null, null], @@ -1018,42 +1013,10 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe } } - // A track removal, an append timeout and the error names error-controller already - // reads as buffer state are not about the fragment's bytes, and keep their own paths. - const rejectedBytes = - !isQuotaError && - !isAppendStateErrorName(error.name) && - error.name !== TRACK_REMOVED_ERROR_NAME && - error.name !== APPEND_TIMEOUT_ERROR_NAME; - // A refused append adds no buffered coverage, so it spends the same per-fragment - // budget a no-progress cycle does. Parts and partial loads count here, unlike there. - const ownedByFragmentBudget = - rejectedBytes && - isMediaFragment(frag) && - !frag.gap && - !chunkMeta.iframe; - if (ownedByFragmentBudget) { - const progress = this.getFragmentAppendProgress(frag); - if (!progress.errored) { - const spent = - (this.appendsWithoutProgress[appendProgressKey(frag)] || 0) + 1 >= - this.hls.config.appendErrorMaxRetry; - if (spent) { - // Recorded before the event below, which is what marks the fragment a gap: - // fragment-loader only keeps that gap when a retry is already on the fragment. - frag.stats.retry++; - } - this.countAppendWithoutProgress(frag, chunkMeta); - } - progress.errored = true; - } else if (trackProgress) { - // Append exceptions use the existing append-error path. + // Append exceptions use the existing append-error path. + if (trackProgress) { this.getFragmentAppendProgress(frag as MediaFragment).errored = true; } - if (!this.hls) { - // Destroyed by a synchronous BUFFER_APPEND_NO_PROGRESS listener - return; - } // in case any error occured while appending, put back segment in segments table const event: ErrorData = { type: ErrorTypes.MEDIA_ERROR, @@ -1089,10 +1052,7 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe this.warn( `Failed ${appendErrorCount}/${appendErrorMaxRetry + 1} times to append segment in "${type}" sourceBuffer with error: ${error.message}`, ); - if ( - appendErrorCount >= appendErrorMaxRetry && - !ownedByFragmentBudget - ) { + if (appendErrorCount >= appendErrorMaxRetry) { event.fatal = !isQuotaError; } const readyState = this.mediaSource?.readyState; @@ -1438,8 +1398,7 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe return progress; } - // Full-fragment cycles without growth consume the append retry budget. Rejected appends - // consume it too, counted at the append-error path above. + // Full-fragment cycles without growth consume the append retry budget. // Parts, I-Frames, gaps, and partial loads are excluded below. private checkAppendProgress( frag: Fragment, @@ -1481,14 +1440,6 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe return; } // Also count parsed fragments that produced no append operations. - this.countAppendWithoutProgress(frag, chunkMeta); - } - - private countAppendWithoutProgress( - frag: MediaFragment, - chunkMeta: ChunkMetadata, - ) { - const key = appendProgressKey(frag); const count = (this.appendsWithoutProgress[key] || 0) + 1; // Capture values before a synchronous ERROR listener can destroy Hls. const hls = this.hls; @@ -2117,6 +2068,7 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe const error = new Error( `${type} SourceBuffer error. MediaSource readyState: ${readyState}`, ); + error.name = SOURCE_BUFFER_ERROR_NAME; this.error(`${error.message}`, event); // according to http://www.w3.org/TR/media-source/#sourcebuffer-append-error // SourceBuffer errors are not necessarily fatal; if so, the HTMLMediaElement will fire an error event @@ -2237,9 +2189,7 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe const operation = this.currentOp(type); if (operation) { - const timeoutError = new Error(`${type}-append-timeout`); - timeoutError.name = APPEND_TIMEOUT_ERROR_NAME; - operation.onError(timeoutError); + operation.onError(new Error(`${type}-append-timeout`)); } } diff --git a/src/controller/error-controller.ts b/src/controller/error-controller.ts index db8ba9678d6..bdcb0c50836 100644 --- a/src/controller/error-controller.ts +++ b/src/controller/error-controller.ts @@ -632,15 +632,12 @@ function isCodecRelated(data: ErrorData): boolean { return false; } -export function isAppendStateErrorName(name: string): boolean { - return name === 'QuotaExceededError' || name === 'InvalidStateError'; -} - function isAppendStateRelated(data: ErrorData): boolean { const errorDetails = data.details; return ( errorDetails === ErrorDetails.BUFFER_APPEND_ERROR && - isAppendStateErrorName(data.error.name) + (data.error.name === 'QuotaExceededError' || + data.error.name === 'InvalidStateError') ); } diff --git a/src/controller/fragment-tracker.ts b/src/controller/fragment-tracker.ts index 3e51e64df16..cad627c3ba5 100644 --- a/src/controller/fragment-tracker.ts +++ b/src/controller/fragment-tracker.ts @@ -5,8 +5,6 @@ import { type MediaFragment, type Part, } from '../loader/fragment'; -import { PlaylistLevelType } from '../types/loader'; -import { getFragmentWithSN } from '../utils/level-helper'; import { userAgentChromeVersion } from '../utils/user-agent'; import type Hls from '../hls'; import type { SourceBufferName } from '../types/buffer'; @@ -21,6 +19,7 @@ import type { FragmentEntity, FragmentTimeRange, } from '../types/fragment-tracker'; +import type { PlaylistLevelType } from '../types/loader'; export const enum FragmentState { NOT_LOADED = 'NOT_LOADED', @@ -84,7 +83,6 @@ export class FragmentTracker implements ComponentAPI { this.endListFragments = this.timeRanges = null; - this.hasGaps = false; } /** @@ -296,15 +294,6 @@ export class FragmentTracker implements ComponentAPI { public addAsGap(frag: MediaFragment) { frag.gap = true; - // A live playlist refresh replaces fragment objects, so the appended copy is not always - // the one selection and the loader read. Mark the one the playlist holds at this sn. - const selectable = - frag.type === PlaylistLevelType.MAIN - ? getFragmentWithSN(this.hls?.latestLevelDetails || undefined, frag.sn) - : null; - if (selectable?.level === frag.level) { - selectable.gap = true; - } this.removeFragment(frag); this.fragBuffered(frag, true); } @@ -625,16 +614,6 @@ export class FragmentTracker implements ComponentAPI { }); } - /** Fragments determined to be gaps, for callers that clear buffer state and re-add them. */ - public gapFragments(): MediaFragment[] { - if (!this.hasGaps) { - return []; - } - return Object.keys(this.fragments) - .map((key) => this.fragments[key]?.body) - .filter((frag): frag is MediaFragment => !!frag?.gap); - } - public removeFragment(fragment: Fragment) { const fragKey = getFragmentKey(fragment); fragment.clearElementaryStreamInfo(); @@ -652,6 +631,23 @@ export class FragmentTracker implements ComponentAPI { } } + /** Whether the fragment tracked at this key is a gap. A playlist refresh drops it from the object. */ + public isGap(frag: Fragment): boolean { + return ( + this.hasGaps && this.fragments[getFragmentKey(frag)]?.body.gap === true + ); + } + + /** Fragments determined to be gaps, for callers that clear buffer state and re-add them. */ + public gapFragments(): MediaFragment[] { + if (!this.hasGaps) { + return []; + } + return Object.keys(this.fragments) + .map((key) => this.fragments[key]?.body) + .filter((frag): frag is MediaFragment => !!frag?.gap); + } + public removeAllFragments() { this.fragments = Object.create(null); this.endListFragments = Object.create(null); diff --git a/src/controller/stream-controller.ts b/src/controller/stream-controller.ts index b023d54af18..0898a665e5b 100644 --- a/src/controller/stream-controller.ts +++ b/src/controller/stream-controller.ts @@ -1048,6 +1048,9 @@ export default class StreamController this.fragmentTracker.addAsGap(data.frag as MediaFragment); } break; + case ErrorDetails.MEDIA_SOURCE_REQUIRES_RESET: + this.onSourceBufferError(PlaylistLevelType.MAIN, data); + break; case ErrorDetails.BUFFER_ADD_CODEC_ERROR: case ErrorDetails.BUFFER_APPEND_ERROR: if (data.parent !== 'main') { diff --git a/src/loader/fragment-loader.ts b/src/loader/fragment-loader.ts index c7b5a35d48a..23bb650c60f 100644 --- a/src/loader/fragment-loader.ts +++ b/src/loader/fragment-loader.ts @@ -73,15 +73,7 @@ export default class FragmentLoader { this.loader.destroy(); } if (frag.gap) { - const { retry, buffering } = frag.stats; - if ( - frag.tagList.some((tags) => tags[0] === 'GAP') || - // A retry recorded on a fragment whose bytes reached the SourceBuffer is the - // append budget giving up on it. The same field also counts network retries, so - // require the append attempt: a fragment gapped before any append is still the - // temporary treatment this branch exists to undo. - (retry > 0 && buffering.start > 0) - ) { + if (frag.tagList.some((tags) => tags[0] === 'GAP')) { reject(createGapLoadError(frag)); return; } else { diff --git a/src/utils/level-helper.ts b/src/utils/level-helper.ts index 90a008be872..449dd76fbaa 100644 --- a/src/utils/level-helper.ts +++ b/src/utils/level-helper.ts @@ -210,9 +210,6 @@ export function mergeDetails( newFrag.stats = oldFrag.stats; } - // Use locally set gap or GAP tag, as the part callback below already does - newFrag.gap = oldFrag.gap || newFrag.gap; - if (oldFrag.initSegment) { newFrag.initSegment = oldFrag.initSegment; currentInitSegment = oldFrag.initSegment; @@ -224,16 +221,6 @@ export function mergeDetails( const fragmentsToCheck = newDetails.fragmentHint ? newFragments.concat(newDetails.fragmentHint) : newFragments; - - // mapFragmentIntersection leaves out an old preload hint that has no duration yet, so a gap - // set on one is lost when the hint becomes a fragment. Carry that one field across. - const oldHint = oldDetails.fragmentHint; - if (oldHint?.gap && !oldHint.duration) { - const replacement = getFragmentWithSN(newDetails, oldHint.sn); - if (replacement) { - replacement.gap = true; - } - } if (currentInitSegment) { fragmentsToCheck.forEach((frag) => { if ( diff --git a/tests/unit/controller/audio-stream-controller.ts b/tests/unit/controller/audio-stream-controller.ts index 323b9eb84ff..51b61f5aae7 100644 --- a/tests/unit/controller/audio-stream-controller.ts +++ b/tests/unit/controller/audio-stream-controller.ts @@ -6,6 +6,7 @@ import AudioStreamController from '../../../src/controller/audio-stream-controll import BaseStreamController, { State, } from '../../../src/controller/base-stream-controller'; +import { SOURCE_BUFFER_ERROR_NAME } from '../../../src/controller/buffer-controller'; import { FragmentTracker } from '../../../src/controller/fragment-tracker'; import { ErrorDetails, ErrorTypes } from '../../../src/errors'; import { Events } from '../../../src/events'; @@ -543,6 +544,40 @@ describe('AudioStreamController', function () { }); }); + describe('onError MEDIA_SOURCE_REQUIRES_RESET', function () { + const resetError = (frag: Fragment) => { + const error = new Error( + 'audio SourceBuffer error. MediaSource readyState: ended', + ); + error.name = SOURCE_BUFFER_ERROR_NAME; + return { + type: ErrorTypes.MEDIA_ERROR, + details: ErrorDetails.MEDIA_SOURCE_REQUIRES_RESET, + fatal: false, + error, + parent: frag.type, + frag, + part: null, + }; + }; + + it('marks the audio fragment as a gap after a SourceBuffer error', function () { + const frag = new Fragment(PlaylistLevelType.AUDIO, ''); + frag.sn = 4; + frag.level = 0; + (audioStreamController as any).onError(Events.ERROR, resetError(frag)); + expect(frag.gap).to.equal(true); + }); + + it('ignores a main fragment', function () { + const frag = new Fragment(PlaylistLevelType.MAIN, ''); + frag.sn = 4; + frag.level = 0; + (audioStreamController as any).onError(Events.ERROR, resetError(frag)); + expect(frag.gap).to.not.equal(true); + }); + }); + describe('audio track switching with flushImmediate flag', function () { beforeEach(function () { audioStreamController.levels = tracks; diff --git a/tests/unit/controller/base-stream-controller.ts b/tests/unit/controller/base-stream-controller.ts index 1871b2ab533..67048a8edb4 100644 --- a/tests/unit/controller/base-stream-controller.ts +++ b/tests/unit/controller/base-stream-controller.ts @@ -3,6 +3,7 @@ import sinon from 'sinon'; import sinonChai from 'sinon-chai'; import { hlsDefaultConfig } from '../../../src/config'; import { State } from '../../../src/controller/base-stream-controller'; +import { SOURCE_BUFFER_ERROR_NAME } from '../../../src/controller/buffer-controller'; import BaseStreamControllerImpl from '../../../src/controller/stream-controller'; import { ErrorDetails, ErrorTypes } from '../../../src/errors'; import Hls from '../../../src/hls'; @@ -16,6 +17,7 @@ import { BufferHelper } from '../../../src/utils/buffer-helper'; import { TimeRangesMock } from '../../mocks/time-ranges.mock'; import type BaseStreamController from '../../../src/controller/base-stream-controller'; import type { MediaFragment, Part } from '../../../src/loader/fragment'; +import type { ErrorData } from '../../../src/types/events'; import type { BufferInfo } from '../../../src/utils/buffer-helper'; use(sinonChai); @@ -38,6 +40,9 @@ type BaseStreamControllerTestable = Omit< | 'fragPrevious' | 'tickImmediate' | 'hls' + | 'getNextFragment' + | 'onMediaDetaching' + | 'onSourceBufferError' > & { media: HTMLMediaElement | null; _streamEnded: (bufferInfo: BufferInfo, levelDetails: LevelDetails) => boolean; @@ -60,6 +65,12 @@ type BaseStreamControllerTestable = Omit< fragPrevious: MediaFragment | null; tickImmediate: () => void; hls: Hls; + getNextFragment: ( + pos: number, + levelDetails: LevelDetails, + ) => MediaFragment | null; + onMediaDetaching: (event: any, data: any) => void; + onSourceBufferError: (filterType: PlaylistLevelType, data: ErrorData) => void; }; describe('BaseStreamController', function () { @@ -78,6 +89,16 @@ describe('BaseStreamController', function () { isEndListAppended() { return true; }, + gapSn: null, + isGap(frag) { + return this.gapSn !== null && frag.sn === this.gapSn; + }, + gapFragments() { + return this.gaps || []; + }, + gaps: [], + addAsGap: sinon.spy(), + removeAllFragments: sinon.spy(), removeFragmentsInRange: sinon.spy(), }; baseStreamController = new BaseStreamControllerImpl( @@ -116,6 +137,28 @@ describe('BaseStreamController', function () { return details; } + function refuse(frag: Fragment, part: Part | null = null, name?: string) { + const error = new Error( + 'video SourceBuffer error. MediaSource readyState: ended', + ); + error.name = name || SOURCE_BUFFER_ERROR_NAME; + baseStreamController.onSourceBufferError(PlaylistLevelType.MAIN, { + type: ErrorTypes.MEDIA_ERROR, + details: ErrorDetails.MEDIA_SOURCE_REQUIRES_RESET, + fatal: false, + error, + frag, + part, + }); + } + + function mainFrag(sn: number, level: number): MediaFragment { + const frag = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; + frag.sn = sn; + frag.level = level; + return frag; + } + describe('_streamEnded', function () { it('returns false if the stream is live', function () { const levelDetails = levelDetailsWithEndSequenceVodOrLive(3, true); @@ -161,6 +204,65 @@ describe('BaseStreamController', function () { }); }); + describe('getNextFragment after a SourceBuffer error', function () { + it('returns the fragment after the refused one', function () { + const levelDetails = levelDetailsWithEndSequenceVodOrLive(4); + // a live playlist refresh builds a new object for the same sn and level + refuse(mainFrag(1, levelDetails.fragments[1].level)); + fragmentTracker.gapSn = 1; + const frag = baseStreamController.getNextFragment(5, levelDetails); + expect(frag?.sn, 'skips to the next fragment').to.equal(2); + expect(baseStreamController.nextLoadPosition).to.equal(10); + }); + + it('does not skip a tracked gap that no SourceBuffer error marked', function () { + const levelDetails = levelDetailsWithEndSequenceVodOrLive(4); + fragmentTracker.gapSn = 1; + expect( + baseStreamController.getNextFragment(5, levelDetails)?.sn, + ).to.equal(1); + }); + + it('does not skip a refused fragment the tracker no longer holds as a gap', function () { + const levelDetails = levelDetailsWithEndSequenceVodOrLive(4); + refuse(levelDetails.fragments[1]); + fragmentTracker.gapSn = null; + expect( + baseStreamController.getNextFragment(5, levelDetails)?.sn, + ).to.equal(1); + }); + + it('returns null when the refused fragment is the last one', function () { + const levelDetails = levelDetailsWithEndSequenceVodOrLive(2); + refuse(levelDetails.fragments[1]); + fragmentTracker.gapSn = 1; + expect(baseStreamController.getNextFragment(5, levelDetails)).to.equal( + null, + ); + }); + }); + + describe('onSourceBufferError', function () { + it('marks a refused fragment as a gap', function () { + const frag = mainFrag(1, 0); + refuse(frag); + expect(fragmentTracker.addAsGap).to.have.been.calledOnceWith(frag); + }); + + it('marks the fragment of a refused part', function () { + const frag = mainFrag(1, 0); + const part = { index: 1, fragment: frag, gap: false } as unknown as Part; + refuse(frag, part); + expect(fragmentTracker.addAsGap).to.have.been.calledOnceWith(frag); + expect(part.gap).to.equal(false); + }); + + it('ignores append errors that are not SourceBuffer errors', function () { + refuse(mainFrag(1, 0), null, 'InvalidStateError'); + expect(fragmentTracker.addAsGap).to.not.have.been.called; + }); + }); + describe('Seeking Logic', function () { describe('onMediaSeeking', function () { it('should handle backward seek behavior correctly', function () { @@ -214,6 +316,54 @@ describe('BaseStreamController', function () { resetSpy.restore(); }); + it('keeps gaps on the seek that follows a detach which carried a refused fragment', function () { + media.removeEventListener = sinon.spy(); + const frag = mainFrag(1, 0); + refuse(frag); + fragmentTracker.gaps = [frag]; + baseStreamController.onMediaDetaching(null, {}); + baseStreamController.media = media; + media.currentTime = 10.0; + + baseStreamController.onMediaSeeking(); + expect( + fragmentTracker.removeFragmentsInRange, + 'the re-attach seek keeps them', + ).to.have.not.been.called; + + baseStreamController.onMediaSeeking(); + expect( + fragmentTracker.removeFragmentsInRange, + 'a later seek removes them', + ).to.have.been.calledOnce; + }); + + it('does not keep carried gaps that no SourceBuffer error marked', function () { + media.removeEventListener = sinon.spy(); + fragmentTracker.gaps = [mainFrag(1, 0)]; + baseStreamController.onMediaDetaching(null, {}); + baseStreamController.media = media; + media.currentTime = 10.0; + + baseStreamController.onMediaSeeking(); + expect(fragmentTracker.removeFragmentsInRange).to.have.been.calledOnce; + }); + + it('does not keep gaps carried for another playlist type', function () { + media.removeEventListener = sinon.spy(); + refuse(mainFrag(1, 0)); + const audioFrag = new Fragment(PlaylistLevelType.AUDIO, ''); + audioFrag.sn = 1; + audioFrag.level = 0; + fragmentTracker.gaps = [audioFrag]; + baseStreamController.onMediaDetaching(null, {}); + baseStreamController.media = media; + media.currentTime = 10.0; + + baseStreamController.onMediaSeeking(); + expect(fragmentTracker.removeFragmentsInRange).to.have.been.calledOnce; + }); + it('should call fragmentTracker.removeFragmentsInRange when media exists', function () { baseStreamController.lastCurrentTime = 5.0; media.currentTime = 10.0; @@ -1149,113 +1299,6 @@ describe('BaseStreamController', function () { }); }); - describe('onMediaDetaching', function () { - const detachableMedia = () => - ({ - duration: 0, - ended: false, - buffered: new TimeRangesMock(), - removeEventListener: () => undefined, - }) as unknown as HTMLMediaElement; - - it('re-adds gaps after clearing the fragment tracker', function () { - const gapped = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; - gapped.sn = 7; - gapped.gap = true; - const calls: string[] = []; - fragmentTracker.gapFragments = () => { - calls.push('gapFragments'); - return [gapped]; - }; - fragmentTracker.removeAllFragments = () => - calls.push('removeAllFragments'); - fragmentTracker.addAsGap = (frag: MediaFragment) => { - calls.push(`addAsGap:${frag.sn}`); - }; - baseStreamController.media = detachableMedia(); - - (baseStreamController as any).onMediaDetaching(null, {}); - - // recoverMediaError() detaches, so the judgement has to outlive the clear - expect(calls).to.deep.equal([ - 'gapFragments', - 'removeAllFragments', - 'addAsGap:7', - ]); - }); - - it('keeps the fragment tracker intact when the media is transferred', function () { - let cleared = false; - fragmentTracker.gapFragments = () => []; - fragmentTracker.removeAllFragments = () => { - cleared = true; - }; - baseStreamController.media = detachableMedia(); - - (baseStreamController as any).onMediaDetaching(null, { - transferMedia: {}, - }); - - expect(cleared, 'a transfer does not clear buffer state').to.equal(false); - }); - }); - - describe('getNextPart', function () { - const partList = ( - frag: MediaFragment, - count: number, - independent = true, - ): Part[] => - Array.from({ length: count }, (_, index) => ({ - index, - start: index, - duration: 1, - independent, - loaded: false, - gap: false, - fragment: frag, - })) as unknown as Part[]; - - it('selects a part of the fragment', function () { - const frag = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; - frag.sn = 1; - - expect( - (baseStreamController as any).getNextPart(partList(frag, 3), frag, 10), - ).to.not.equal(-1); - }); - - it('does not select a part of a fragment marked as a gap', function () { - const frag = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; - frag.sn = 1; - // fragment-loader rejects these with FRAG_GAP, so selecting one loads nothing - frag.gap = true; - const parts = partList(frag, 3); - - expect( - (baseStreamController as any).getNextPart(parts, frag, 10), - ).to.equal(-1); - expect( - parts.every((part) => !part.gap), - 'selection leaves the parts unmarked', - ).to.equal(true); - }); - - it('does not carry continuity across a gapped fragment', function () { - const gapped = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; - gapped.sn = 1; - gapped.gap = true; - const next = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; - next.sn = 2; - // A part that cannot be decoded on its own needs a buffered part before it - const parts = [...partList(gapped, 1), ...partList(next, 1, false)]; - - expect( - (baseStreamController as any).getNextPart(parts, next, 10), - ).to.equal(-1); - }); - }); - describe('getBufferedFrag', function () { it('should call fragmentTracker.getBufferedFrag with correct parameters', function () { const mockFrag = new Fragment(PlaylistLevelType.MAIN, 'test.ts'); diff --git a/tests/unit/controller/buffer-controller-operations.ts b/tests/unit/controller/buffer-controller-operations.ts index 9b3e82ab76a..e948461b861 100644 --- a/tests/unit/controller/buffer-controller-operations.ts +++ b/tests/unit/controller/buffer-controller-operations.ts @@ -1,7 +1,9 @@ import { expect, use } from 'chai'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; -import BufferController from '../../../src/controller/buffer-controller'; +import BufferController, { + SOURCE_BUFFER_ERROR_NAME, +} from '../../../src/controller/buffer-controller'; import { FragmentTracker } from '../../../src/controller/fragment-tracker'; import { ErrorDetails, ErrorTypes } from '../../../src/errors'; import { Events } from '../../../src/events'; @@ -210,6 +212,7 @@ describe('BufferController with attached media', function () { expect(sbErrorObject.message).equals( 'audio SourceBuffer error. MediaSource readyState: open', ); + expect(sbErrorObject.name).equals(SOURCE_BUFFER_ERROR_NAME); expect( triggerSpy, 'ERROR should have been triggered in response to the SourceBuffer error', @@ -446,7 +449,6 @@ describe('BufferController with attached media', function () { const [, errorEvent] = triggerSpy.lastCall.args; expect(errorEvent.error.message).to.equal(`${name}-append-timeout`); - expect(errorEvent.error.name).to.equal('HlsJsAppendTimeoutError'); }); }); @@ -873,277 +875,6 @@ describe('BufferController with attached media', function () { expect(errors).to.have.lengthOf(0); }); - const appendAndFail = ( - frag: Fragment, - error: Error, - part: Part | null = null, - chunkMeta: ChunkMetadata | null = null, - ) => { - const buffer = getSourceBufferTrack(bufferController, 'video') - ?.buffer as any; - buffer.appendBuffer.throws(error); - hls.trigger(Events.BUFFER_APPENDING, { - parent: PlaylistLevelType.MAIN, - type: 'video', - data: new Uint8Array(), - frag, - part, - chunkMeta: - chunkMeta || - new ChunkMetadata( - 0, - frag.sn as number, - 0, - 0, - part ? part.index : -1, - !!part, - ), - }); - }; - const noProgressErrors = (errors: ErrorData[]) => - errors.filter( - (e) => e.details === ErrorDetails.BUFFER_APPEND_NO_PROGRESS, - ); - - it('counts an append the SourceBuffer rejected', function () { - const errors: ErrorData[] = []; - hls.on(Events.ERROR, (event, data) => errors.push(data)); - const frag = newFrag(1); - - appendAndFail(frag, new Error('append failed')); - // A media source reset reloads the fragment, replacing its LoadStats - frag.stats = new LoadStats(); - appendAndFail(frag, new Error('append failed')); - - const counted = noProgressErrors(errors); - expect(counted).to.have.lengthOf(2); - expect(counted[0].appendsWithoutProgress).to.equal(1); - expect(counted[1].appendsWithoutProgress).to.equal(2); - expect(counted[1].frag?.sn).to.equal(1); - }); - - it('counts a refused part against the fragment', function () { - const errors: ErrorData[] = []; - hls.on(Events.ERROR, (event, data) => errors.push(data)); - const frag = newFrag(1); - const part = { - index: 1, - fragment: frag, - stats: new LoadStats(), - } as unknown as Part; - - // A low latency playlist delivers the fragment as parts first - appendAndFail(frag, new Error('append failed'), part); - frag.stats = new LoadStats(); - appendAndFail(frag, new Error('append failed'), part); - - const counted = noProgressErrors(errors); - expect(counted).to.have.lengthOf(2); - expect(counted[1].appendsWithoutProgress).to.equal(2); - }); - - it('counts one rejection per load, not per rejected chunk', function () { - const errors: ErrorData[] = []; - hls.on(Events.ERROR, (event, data) => errors.push(data)); - const frag = newFrag(1); - - // Every chunk of the same load is refused once the element error is set - appendAndFail(frag, new Error('append failed')); - appendAndFail(frag, new Error('append failed')); - appendAndFail(frag, new Error('append failed')); - - expect(noProgressErrors(errors)).to.have.lengthOf(1); - }); - - it('keeps the per-fragment count across the buffer reset each cycle makes', function () { - const errors: ErrorData[] = []; - hls.on(Events.ERROR, (event, data) => errors.push(data)); - const frag = newFrag(1); - - appendAndFail(frag, new Error('append failed')); - expect(noProgressErrors(errors)).to.have.lengthOf(1); - - // Every rejection cycle rebuilds the SourceBuffers, which is what the budget counts - (bufferController as any).initTracks(); - - expect( - (bufferController as any).appendsWithoutProgress.main_1_0, - 'the budget would never bind if the rebuild cleared it', - ).to.equal(1); - }); - - it('gives up on the fragment once its append budget is spent', function () { - const errors: ErrorData[] = []; - hls.on(Events.ERROR, (event, data) => errors.push(data)); - const frag = newFrag(1); - const budget = hls.config.appendErrorMaxRetry; - - for (let i = 0; i < budget; i++) { - // Each media source reset reloads the fragment, replacing its LoadStats - frag.stats = new LoadStats(); - appendAndFail(frag, new Error('append failed')); - } - - const counted = noProgressErrors(errors); - expect(counted).to.have.lengthOf(budget); - expect(counted[counted.length - 1].appendsWithoutProgress).to.equal( - budget, - ); - expect( - frag.stats.retry, - 'the fragment is recorded as having had its retries', - ).to.equal(1); - const appendErrors = errors.filter( - (e) => - e.details === ErrorDetails.BUFFER_APPEND_ERROR || - e.details === ErrorDetails.MEDIA_SOURCE_REQUIRES_RESET, - ); - const last = appendErrors[appendErrors.length - 1]; - expect(last.fatal, 'skipping a fragment does not stop loading').to.equal( - false, - ); - }); - - it('survives a listener that destroys Hls while the rejection is reported', function () { - const errors: ErrorData[] = []; - hls.on(Events.ERROR, (event, data) => { - errors.push(data); - if (data.details === ErrorDetails.BUFFER_APPEND_NO_PROGRESS) { - bufferController.destroy(); - } - }); - - appendAndFail(newFrag(1), new Error('append failed')); - - expect( - errors.filter((e) => e.details === ErrorDetails.INTERNAL_EXCEPTION), - ).to.have.lengthOf(0); - }); - - it('records the retry before announcing that the budget is spent', function () { - const frag = newFrag(1); - const budget = hls.config.appendErrorMaxRetry; - let retryWhenSpent: number | null = null; - hls.on(Events.ERROR, (event, data) => { - if ( - data.details === ErrorDetails.BUFFER_APPEND_NO_PROGRESS && - data.appendsWithoutProgress === budget - ) { - // stream-controller marks the fragment a gap from this event, and fragment-loader - // only keeps that gap when a retry is already on the fragment - retryWhenSpent = (data.frag as Fragment).stats.retry; - } - }); - - for (let i = 0; i < budget; i++) { - frag.stats = new LoadStats(); - appendAndFail(frag, new Error('append failed')); - } - - expect( - retryWhenSpent, - 'the gap is durable when it is announced', - ).to.equal(1); - }); - - it('does not count a quota, invalid state, track removal or append timeout error', function () { - const errors: ErrorData[] = []; - hls.on(Events.ERROR, (event, data) => errors.push(data)); - - const quota = new Error('quota'); - (quota as any).code = DOMException.QUOTA_EXCEEDED_ERR; - appendAndFail(newFrag(1), quota); - - const invalidState = new Error('the SourceBuffer has been removed'); - invalidState.name = 'InvalidStateError'; - appendAndFail(newFrag(4), invalidState); - - const removed = new Error('track removed'); - removed.name = 'HlsJsTrackRemovedError'; - appendAndFail(newFrag(2), removed); - - const timeout = new Error('video-append-timeout'); - timeout.name = 'HlsJsAppendTimeoutError'; - appendAndFail(newFrag(3), timeout); - - expect(noProgressErrors(errors)).to.have.lengthOf(0); - }); - - it('does not record a retry before the budget is spent', function () { - const errors: ErrorData[] = []; - hls.on(Events.ERROR, (event, data) => errors.push(data)); - const frag = newFrag(1); - - appendAndFail(frag, new Error('append failed')); - - expect(noProgressErrors(errors)).to.have.lengthOf(1); - expect( - frag.stats.retry, - 'a fragment with budget left is still fetched', - ).to.equal(0); - }); - - it('does not spend the budget on an init segment', function () { - const errors: ErrorData[] = []; - hls.on(Events.ERROR, (event, data) => errors.push(data)); - const frag = newFrag(1); - frag.sn = 'initSegment'; - - appendAndFail(frag, new Error('append failed')); - - expect( - noProgressErrors(errors), - 'an init segment keeps the existing append-error path', - ).to.have.lengthOf(0); - }); - - it('does not spend the budget on a fragment already given up on', function () { - const errors: ErrorData[] = []; - hls.on(Events.ERROR, (event, data) => errors.push(data)); - const frag = newFrag(1); - frag.gap = true; - - appendAndFail(frag, new Error('append failed')); - - expect( - noProgressErrors(errors), - 'a gap is not counted twice', - ).to.have.lengthOf(0); - }); - - it('does not spend the budget on an I-frame chunk', function () { - const errors: ErrorData[] = []; - hls.on(Events.ERROR, (event, data) => errors.push(data)); - const frag = newFrag(1); - const iframeMeta = new ChunkMetadata(0, 1, 0, 0, -1, false, 0, true); - - appendAndFail(frag, new Error('append failed'), null, iframeMeta); - - expect( - noProgressErrors(errors), - 'I-frames are excluded here as they are in checkAppendProgress', - ).to.have.lengthOf(0); - }); - - it('does not let a refused part hide the fragment cycle when the buffer was at fault', function () { - const errors: ErrorData[] = []; - hls.on(Events.ERROR, (event, data) => errors.push(data)); - const frag = newFrag(1); - const part = { - index: 0, - fragment: frag, - stats: new LoadStats(), - } as unknown as Part; - const quota = new Error('quota'); - (quota as any).code = DOMException.QUOTA_EXCEEDED_ERR; - - appendAndFail(frag, quota, part); - // The part said nothing about the fragment's bytes, so the cycle is still unjudged - runAppendCycle(frag, false); - - expect(noProgressErrors(errors)).to.have.lengthOf(1); - }); - it('emits BUFFER_APPEND_NO_PROGRESS through the append pipeline when the target buffer does not grow', function () { const errors: ErrorData[] = []; hls.on(Events.ERROR, (event, data) => { diff --git a/tests/unit/controller/fragment-tracker.ts b/tests/unit/controller/fragment-tracker.ts index 1cb3d25971f..305c551257b 100644 --- a/tests/unit/controller/fragment-tracker.ts +++ b/tests/unit/controller/fragment-tracker.ts @@ -585,168 +585,52 @@ describe('FragmentTracker', function () { ).to.be.false; }); }); + }); - it('marks the fragment the playlist holds, not only the one appended', function () { - const appended = createMockFragment( - { - startPTS: 0, - endPTS: 1, - sn: 1, - level: 0, - type: PlaylistLevelType.MAIN, - }, - [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], - ); - // A live playlist refresh replaces the object while the appended copy is still in flight - const inPlaylist = createMockFragment( - { - startPTS: 0, - endPTS: 1, - sn: 1, - level: 0, - type: PlaylistLevelType.MAIN, - }, - [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], - ); - const details = { startSN: 1, fragments: [inPlaylist] }; - Object.defineProperty(hls, 'latestLevelDetails', { - configurable: true, - get: () => details, - }); - - fragmentTracker.addAsGap(appended as MediaFragment); - - expect(inPlaylist.gap, 'the selectable fragment is a gap too').to.equal( - true, - ); - }); - - it('does not mark a fragment of another level that shares the sn', function () { - const appended = createMockFragment( - { - startPTS: 0, - endPTS: 1, - sn: 1, - level: 4, - type: PlaylistLevelType.MAIN, - }, - [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], - ); - // latestLevelDetails is whichever level loaded last, which is not always this one - const otherLevel = createMockFragment( + describe('gap tracking', function () { + const gapFrag = (sn: number, level: number) => + createMockFragment( { - startPTS: 0, - endPTS: 1, - sn: 1, - level: 3, - type: PlaylistLevelType.MAIN, - }, - [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], - ); - const details = { startSN: 1, fragments: [otherLevel] }; - Object.defineProperty(hls, 'latestLevelDetails', { - configurable: true, - get: () => details, - }); - - fragmentTracker.addAsGap(appended as MediaFragment); - - expect( - otherLevel.gap, - 'another level holds different bytes at the same sn', - ).to.not.equal(true); - }); - - it('does not mark the main fragment when another track has a gap', function () { - const mainFrag = createMockFragment( - { - startPTS: 0, - endPTS: 1, - sn: 1, - level: 0, + startPTS: sn, + endPTS: sn + 1, + sn, + level, type: PlaylistLevelType.MAIN, }, [ElementaryStreamTypes.VIDEO], - ); - const audioFrag = createMockFragment( - { - startPTS: 0, - endPTS: 1, - sn: 1, - level: 0, - type: PlaylistLevelType.AUDIO, - }, - [ElementaryStreamTypes.AUDIO], - ); - // latestLevelDetails is the main playlist, so another track's sn must not reach it - const details = { startSN: 1, fragments: [mainFrag] }; - Object.defineProperty(hls, 'latestLevelDetails', { - configurable: true, - get: () => details, - }); - - fragmentTracker.addAsGap(audioFrag as MediaFragment); - - expect(mainFrag.gap, 'the main fragment is untouched').to.not.equal(true); - }); - - it('lists fragments marked as gaps so they can be re-added', function () { - const buffered = createMockFragment( - { - startPTS: 0, - endPTS: 1, - sn: 1, - level: 1, - type: PlaylistLevelType.MAIN, - }, - [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], - ); - const gapped = createMockFragment( - { - startPTS: 1, - endPTS: 2, - sn: 2, - level: 1, - type: PlaylistLevelType.MAIN, - }, - [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], - ); - expect(fragmentTracker.gapFragments(), 'no gaps tracked').to.deep.equal( - [], - ); - fragmentTracker.fragBuffered(buffered as MediaFragment, true); - fragmentTracker.addAsGap(gapped as MediaFragment); - expect(fragmentTracker.hasFragment(buffered), 'both are tracked').to.be - .true; + ) as MediaFragment; - const gaps = fragmentTracker.gapFragments(); - expect(gaps).to.have.lengthOf(1); - expect(gaps[0].sn, 'only the gap is listed').to.equal(2); + it('reports a gap for a replacement object with the same key', function () { + const fragmentTracker = new FragmentTracker(new Hls({})); + const frag = gapFrag(1, 1); + expect(fragmentTracker.isGap(frag)).to.equal(false); - fragmentTracker.removeAllFragments(); - gaps.forEach((frag) => fragmentTracker.addAsGap(frag)); + fragmentTracker.addAsGap(frag); + expect(fragmentTracker.isGap(frag)).to.equal(true); - expect(fragmentTracker.hasFragment(gapped), 'gap is re-added').to.be.true; - expect(fragmentTracker.hasFragment(buffered), 'buffered entry is gone').to - .be.false; + // a live playlist refresh builds a new object for the same sn and level + const replacement = gapFrag(1, 1); + expect(replacement.gap).to.not.equal(true); + expect(fragmentTracker.isGap(replacement)).to.equal(true); }); - it('lists no gaps once the tracker is destroyed', function () { - const gapped = createMockFragment( - { - startPTS: 0, - endPTS: 1, - sn: 1, - level: 0, - type: PlaylistLevelType.MAIN, - }, - [ElementaryStreamTypes.AUDIO, ElementaryStreamTypes.VIDEO], - ); - fragmentTracker.addAsGap(gapped as MediaFragment); - fragmentTracker.destroy(); + it('does not report a gap for the same sn of another level', function () { + const fragmentTracker = new FragmentTracker(new Hls({})); + fragmentTracker.addAsGap(gapFrag(1, 1)); + expect(fragmentTracker.isGap(gapFrag(1, 2))).to.equal(false); + }); - // onMediaDetaching reaches a destroyed tracker, which has no fragments left to read - expect(fragmentTracker.gapFragments()).to.deep.equal([]); + it('stops reporting a gap once the fragment is removed', function () { + const fragmentTracker = new FragmentTracker(new Hls({})); + const frag = gapFrag(1, 1); + fragmentTracker.addAsGap(frag); + expect(fragmentTracker.gapFragments().map((f) => f.sn)).to.deep.equal([ + 1, + ]); + + fragmentTracker.removeFragment(frag); + expect(fragmentTracker.isGap(gapFrag(1, 1))).to.equal(false); + expect(fragmentTracker.gapFragments()).to.have.lengthOf(0); }); }); }); diff --git a/tests/unit/controller/level-helper.ts b/tests/unit/controller/level-helper.ts index 770660522de..dac95d6440f 100644 --- a/tests/unit/controller/level-helper.ts +++ b/tests/unit/controller/level-helper.ts @@ -96,37 +96,6 @@ describe('LevelHelper Tests', function () { expect(actual).to.deep.equal([5]); }); - it('carries a locally set gap to the replacement fragment', function () { - const oldPlaylist = generatePlaylist([1, 2, 3]); - const newPlaylist = generatePlaylist([1, 2, 3]); - oldPlaylist.fragments[1].gap = true; - mergeDetails(oldPlaylist, newPlaylist, logger); - expect(newPlaylist.fragments[1].gap, 'gap is carried over').to.equal( - true, - ); - expect( - newPlaylist.fragments[0].gap, - 'other fragments are untouched', - ).to.not.equal(true); - }); - - it('carries a locally set gap from a preload hint that has no duration yet', function () { - const oldPlaylist = generatePlaylist([1, 2, 3]); - const newPlaylist = generatePlaylist([1, 2, 3, 4]); - const hint = new Fragment(PlaylistLevelType.MAIN, ''); - hint.sn = 4; - hint.setStart(15); - // A low latency preload hint has no duration until its parts complete - hint.duration = 0; - hint.gap = true; - oldPlaylist.fragmentHint = hint as MediaFragment; - mergeDetails(oldPlaylist, newPlaylist, logger); - expect( - newPlaylist.fragments[3].gap, - 'gap reaches the fragment the hint became', - ).to.equal(true); - }); - it('can iterate over the entire segment array', function () { const oldPlaylist = generatePlaylist([1, 2, 3]); const newPlaylist = generatePlaylist([1, 2, 3]); diff --git a/tests/unit/controller/stream-controller.ts b/tests/unit/controller/stream-controller.ts index 0ee0219dbed..4656b1122fb 100644 --- a/tests/unit/controller/stream-controller.ts +++ b/tests/unit/controller/stream-controller.ts @@ -4,11 +4,16 @@ import { fakeXhr } from 'nise'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; import { State } from '../../../src/controller/base-stream-controller'; +import { SOURCE_BUFFER_ERROR_NAME } from '../../../src/controller/buffer-controller'; import { FragmentState } from '../../../src/controller/fragment-tracker'; import { ErrorDetails, ErrorTypes } from '../../../src/errors'; import { Events } from '../../../src/events'; import Hls from '../../../src/hls'; -import { ElementaryStreamTypes, Fragment } from '../../../src/loader/fragment'; +import { + ElementaryStreamTypes, + Fragment, + Part, +} from '../../../src/loader/fragment'; import { LevelDetails } from '../../../src/loader/level-details'; import { LoadStats } from '../../../src/loader/load-stats'; import M3U8Parser from '../../../src/loader/m3u8-parser'; @@ -421,6 +426,224 @@ describe('StreamController', function () { }); }); + describe('onError MEDIA_SOURCE_REQUIRES_RESET', function () { + const resetError = ( + name: string, + frag: MediaFragment, + part: Part | null = null, + ): ErrorData => { + const error = new Error( + 'video SourceBuffer error. MediaSource readyState: ended', + ); + error.name = name; + return { + type: ErrorTypes.MEDIA_ERROR, + details: ErrorDetails.MEDIA_SOURCE_REQUIRES_RESET, + fatal: false, + error, + parent: PlaylistLevelType.MAIN, + frag, + part, + }; + }; + // Live playlist with two parts per segment, on level 0 + const setLevelDetails = (count: number, live = true) => { + const details = new LevelDetails(''); + details.live = live; + details.startSN = 0; + details.endSN = count - 1; + details.partList = []; + for (let sn = 0; sn < count; sn++) { + const frag = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; + frag.sn = sn; + frag.level = 0; + frag.relurl = `${sn}.mp4`; + frag.setStart(sn * 2); + frag.duration = 2; + details.fragments.push(frag); + let previous: Part | undefined; + for (let index = 0; index < 2; index++) { + previous = new Part( + new AttrList({ + DURATION: '1', + INDEPENDENT: 'YES', + URI: `${sn}.${index}.mp4`, + }), + frag, + '', + index, + previous, + ); + details.partList.push(previous); + } + } + if (!live) { + details.fragments[count - 1].endList = true; + } + const level = new Level({ name: '', url: '', attrs, bitrate: 500000 }); + level.details = details; + streamController['levels'] = [level]; + return details; + }; + const appendParts = (details: LevelDetails, sn: number) => { + fragmentTracker.fragBuffered(details.fragments[sn], true); + details.partList!.forEach((part) => { + if (part.fragment.sn === sn) { + part.elementaryStreams.video = { + startPTS: part.start, + endPTS: part.end, + startDTS: part.start, + endDTS: part.end, + }; + } + }); + }; + const refuse = (frag: MediaFragment, part: Part | null = null) => + streamController['onError']( + Events.ERROR, + resetError(SOURCE_BUFFER_ERROR_NAME, frag, part), + ); + const newFrag = () => { + const frag = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; + frag.sn = 3; + frag.level = 0; + return frag; + }; + + it('marks the fragment as a gap after a SourceBuffer error', function () { + const frag = newFrag(); + streamController['onError']( + Events.ERROR, + resetError(SOURCE_BUFFER_ERROR_NAME, frag), + ); + expect(frag.gap).to.equal(true); + expect(fragmentTracker.getState(frag)).to.equal(FragmentState.PARTIAL); + }); + + it('ignores a reset that another append error caused', function () { + const frag = newFrag(); + streamController['onError']( + Events.ERROR, + resetError('InvalidStateError', frag), + ); + expect(frag.gap).to.not.equal(true); + expect(fragmentTracker.getState(frag)).to.equal(FragmentState.NOT_LOADED); + }); + + it('also marks the previous fragment when the first part of a fragment is refused', function () { + const details = setLevelDetails(4); + appendParts(details, 1); + refuse(details.fragments[2], details.partList![4]); + expect(fragmentTracker.isGap(details.fragments[1])).to.equal(true); + expect(fragmentTracker.isGap(details.fragments[2])).to.equal(true); + }); + + it('does not mark the previous fragment when it was not appended', function () { + const details = setLevelDetails(4); + refuse(details.fragments[2], details.partList![4]); + expect(fragmentTracker.isGap(details.fragments[1])).to.equal(false); + expect(fragmentTracker.isGap(details.fragments[2])).to.equal(true); + }); + + it('does not mark the previous fragment when a later part is refused', function () { + const details = setLevelDetails(4); + appendParts(details, 1); + refuse(details.fragments[2], details.partList![5]); + expect(fragmentTracker.isGap(details.fragments[1])).to.equal(false); + expect(fragmentTracker.isGap(details.fragments[2])).to.equal(true); + }); + + it('leaves a previous fragment gapped by another error to be retried', function () { + const details = setLevelDetails(4); + // gapped after a load error, then replaced by a playlist refresh + const loadFailed = new Fragment( + PlaylistLevelType.MAIN, + '', + ) as MediaFragment; + loadFailed.sn = 1; + loadFailed.level = 0; + fragmentTracker.addAsGap(loadFailed); + refuse(details.fragments[2], details.partList![4]); + expect( + streamController['getNextFragment'](details.fragments[1].start, details) + ?.sn, + ).to.equal(1); + }); + + it('does not mark the last fragment of an ended playlist', function () { + const details = setLevelDetails(3, false); + refuse(details.fragments[2]); + expect(fragmentTracker.getState(details.fragments[2])).to.equal( + FragmentState.NOT_LOADED, + ); + }); + + it('does not select parts of a refused fragment after a playlist refresh', function () { + const details = setLevelDetails(4); + appendParts(details, 1); + // the refresh replaced the object that was refused + const refused = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; + refused.sn = 2; + refused.level = 0; + refuse(refused, null); + const partList = details.partList!; + const next = streamController.getNextPart( + partList, + details.fragments[1], + partList[3].start + 0.1, + ); + expect(partList[next].fragment.sn).to.equal(3); + }); + + it('keeps the gap across a media detach', function () { + const details = setLevelDetails(4); + streamController['media'] = { + currentTime: 1, + addEventListener() {}, + removeEventListener() {}, + } as unknown as HTMLMediaElement; + refuse(details.fragments[2]); + streamController['onMediaDetaching'](Events.MEDIA_DETACHING, {}); + expect(fragmentTracker.isGap(details.fragments[2])).to.equal(true); + }); + + it('does not skip a gap of a new source that reuses the key of a refused fragment', function () { + refuse(setLevelDetails(4).fragments[2]); + hls.trigger(Events.MANIFEST_LOADING, { url: '' }); + const details = setLevelDetails(4); + // gapped after a load error, then replaced by a playlist refresh + const loadFailed = new Fragment( + PlaylistLevelType.MAIN, + '', + ) as MediaFragment; + loadFailed.sn = 2; + loadFailed.level = 0; + fragmentTracker.addAsGap(loadFailed); + expect( + streamController['getNextFragment'](details.fragments[2].start, details) + ?.sn, + ).to.equal(2); + }); + + it('purges gaps on the first seek after a new manifest', function () { + const details = setLevelDetails(4); + const media = { + currentTime: 1, + buffered: new TimeRangesMock(), + addEventListener() {}, + removeEventListener() {}, + } as unknown as HTMLMediaElement; + streamController['media'] = media; + refuse(details.fragments[2]); + streamController['onMediaDetaching'](Events.MEDIA_DETACHING, {}); + hls.trigger(Events.MANIFEST_LOADING, { url: '' }); + streamController['media'] = media; + fragmentTracker.addAsGap(details.fragments[3]); + streamController['onMediaSeeking'](); + expect(fragmentTracker.isGap(details.fragments[3])).to.equal(false); + }); + }); + describe('fragment loading', function () { function fragStateStub(state) { return sinon.stub(fragmentTracker, 'getState').callsFake(() => state); diff --git a/tests/unit/loader/fragment-loader.ts b/tests/unit/loader/fragment-loader.ts index d139092a679..1dea228254a 100644 --- a/tests/unit/loader/fragment-loader.ts +++ b/tests/unit/loader/fragment-loader.ts @@ -101,50 +101,6 @@ describe('FragmentLoader tests', function () { }); }); - it('rejects a fragment given up on locally, without a GAP tag', function () { - return new Promise((resolve, reject) => { - // The append budget marks the fragment and records the attempt it spent - frag.gap = true; - frag.stats.retry = 1; - frag.stats.buffering.start = 1; - fragmentLoader - .load(frag) - .then(() => { - reject(new Error('Fragment loader should not have resolved')); - }) - .catch((error: LoadError) => { - expect(error.data.details).to.equal(ErrorDetails.FRAG_GAP); - expect(frag.gap, 'the fragment is still a gap').to.equal(true); - resolve(); - }); - }); - }); - - it('retries a fragment marked as a gap that has not been given up on', function () { - frag.gap = true; - fragmentLoader.load(frag).catch(() => undefined); - expect(frag.gap, 'temporary treatment as a gap is reset').to.equal(false); - fragmentLoader.abort(); - }); - - it('retries a fragment gapped with no retry recorded, after an append attempt', function () { - // treatAsGap on a fragment that appended before, with nothing spent against it - frag.gap = true; - frag.stats.buffering.start = 1; - fragmentLoader.load(frag).catch(() => undefined); - expect(frag.gap, 'temporary treatment as a gap is reset').to.equal(false); - fragmentLoader.abort(); - }); - - it('retries a fragment gapped after a network retry, with no append attempted', function () { - // A transient network failure records a retry on the same field the append budget uses - frag.gap = true; - frag.stats.retry = 1; - fragmentLoader.load(frag).catch(() => undefined); - expect(frag.gap, 'temporary treatment as a gap is reset').to.equal(false); - fragmentLoader.abort(); - }); - it('handles fragment load errors', function () { const fragmentLoaderPrivates = fragmentLoader as any; return new Promise((resolve, reject) => { From f4ea1472026566e1ae2133037c0795731f853c3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=B0=ED=99=8D=EC=A4=80?= Date: Thu, 24 Sep 2026 13:35:39 +0900 Subject: [PATCH 4/9] Mark the previous fragment only when the buffer ends short of a refused first part The unconditional rule dropped a healthy previous fragment whenever the first part itself was damaged, as in #8035 (10 of 10 runs). Compare the parsed start of the refused part with the buffered range instead; playlist times drift from the parsed timeline by up to 0.1 s on five-part streams. --- src/controller/base-stream-controller.ts | 26 ++++++++---- tests/unit/controller/stream-controller.ts | 46 +++++++++++++++++++++- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/src/controller/base-stream-controller.ts b/src/controller/base-stream-controller.ts index fe75b08b3df..ef6ddc174da 100644 --- a/src/controller/base-stream-controller.ts +++ b/src/controller/base-stream-controller.ts @@ -2254,17 +2254,29 @@ export default class BaseStreamController ) { return; } - // The SourceBuffer can fail on an append after the damaged one, so mark the fragment, - // and the previous one when the first part of a fragment is refused. + // The SourceBuffer can fail on an append after the damaged one, so mark the fragment, and the + // previous one when the buffer ends short of a refused first part (its tail was not buffered). this.markSourceBufferErrorGap(frag); - const previous = - part?.index === 0 - ? getFragmentWithSN(this.levels?.[frag.level]?.details, frag.sn - 1) - : null; + const { sourceBufferName } = data; + const start = + part?.index === 0 && sourceBufferName + ? part.elementaryStreams[sourceBufferName]?.startPTS + : undefined; + if (start === undefined) { + return; + } + const previous = getFragmentWithSN( + this.levels?.[frag.level]?.details, + frag.sn - 1, + ); if ( previous && this.fragmentTracker.getState(previous) !== FragmentState.NOT_LOADED && - !this.fragmentTracker.isGap(previous) + !this.fragmentTracker.isGap(previous) && + !BufferHelper.isBuffered( + this.mediaBuffer || this.media, + start - this.config.maxBufferHole, + ) ) { this.markSourceBufferErrorGap(previous); } diff --git a/tests/unit/controller/stream-controller.ts b/tests/unit/controller/stream-controller.ts index 4656b1122fb..e7db41770b3 100644 --- a/tests/unit/controller/stream-controller.ts +++ b/tests/unit/controller/stream-controller.ts @@ -442,6 +442,7 @@ describe('StreamController', function () { fatal: false, error, parent: PlaylistLevelType.MAIN, + sourceBufferName: 'audiovideo', frag, part, }; @@ -498,11 +499,25 @@ describe('StreamController', function () { } }); }; - const refuse = (frag: MediaFragment, part: Part | null = null) => + // `offset` shifts the parsed media times of the part from its playlist times + const refuse = ( + frag: MediaFragment, + part: Part | null = null, + offset = 0, + ) => { + if (part) { + part.elementaryStreams.audiovideo = { + startPTS: part.start + offset, + endPTS: part.end + offset, + startDTS: part.start + offset, + endDTS: part.end + offset, + }; + } streamController['onError']( Events.ERROR, resetError(SOURCE_BUFFER_ERROR_NAME, frag, part), ); + }; const newFrag = () => { const frag = new Fragment(PlaylistLevelType.MAIN, '') as MediaFragment; frag.sn = 3; @@ -530,14 +545,41 @@ describe('StreamController', function () { expect(fragmentTracker.getState(frag)).to.equal(FragmentState.NOT_LOADED); }); - it('also marks the previous fragment when the first part of a fragment is refused', function () { + it('also marks the previous fragment when the buffer ends short of a refused first part', function () { const details = setLevelDetails(4); appendParts(details, 1); + // the tail of fragment 1 never reached the buffer: a 0.5s hole before fragment 2 at 4s + streamController['media'] = { + buffered: new TimeRangesMock([0, 3.5]), + } as unknown as HTMLMediaElement; refuse(details.fragments[2], details.partList![4]); expect(fragmentTracker.isGap(details.fragments[1])).to.equal(true); expect(fragmentTracker.isGap(details.fragments[2])).to.equal(true); }); + it('does not mark the previous fragment when the buffer reaches a refused first part', function () { + const details = setLevelDetails(4); + appendParts(details, 1); + // a hole smaller than maxBufferHole is contiguous + streamController['media'] = { + buffered: new TimeRangesMock([0, 3.98]), + } as unknown as HTMLMediaElement; + refuse(details.fragments[2], details.partList![4]); + expect(fragmentTracker.isGap(details.fragments[1])).to.equal(false); + expect(fragmentTracker.isGap(details.fragments[2])).to.equal(true); + }); + + it('measures the hole from the parsed start of the refused part', function () { + const details = setLevelDetails(4); + appendParts(details, 1); + // media runs 0.15s ahead of playlist times and fragment 1 is fully buffered + streamController['media'] = { + buffered: new TimeRangesMock([0, 3.85]), + } as unknown as HTMLMediaElement; + refuse(details.fragments[2], details.partList![4], -0.15); + expect(fragmentTracker.isGap(details.fragments[1])).to.equal(false); + }); + it('does not mark the previous fragment when it was not appended', function () { const details = setLevelDetails(4); refuse(details.fragments[2], details.partList![4]); From b77946e26be7f761544fef5a5b89e62444beb4d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=B0=ED=99=8D=EC=A4=80?= Date: Thu, 24 Sep 2026 20:23:11 +0900 Subject: [PATCH 5/9] Remove two mock tests covered by the stream-controller tests --- tests/unit/controller/base-stream-controller.ts | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/tests/unit/controller/base-stream-controller.ts b/tests/unit/controller/base-stream-controller.ts index 67048a8edb4..9f5159aa320 100644 --- a/tests/unit/controller/base-stream-controller.ts +++ b/tests/unit/controller/base-stream-controller.ts @@ -137,11 +137,11 @@ describe('BaseStreamController', function () { return details; } - function refuse(frag: Fragment, part: Part | null = null, name?: string) { + function refuse(frag: Fragment, part: Part | null = null) { const error = new Error( 'video SourceBuffer error. MediaSource readyState: ended', ); - error.name = name || SOURCE_BUFFER_ERROR_NAME; + error.name = SOURCE_BUFFER_ERROR_NAME; baseStreamController.onSourceBufferError(PlaylistLevelType.MAIN, { type: ErrorTypes.MEDIA_ERROR, details: ErrorDetails.MEDIA_SOURCE_REQUIRES_RESET, @@ -243,12 +243,6 @@ describe('BaseStreamController', function () { }); describe('onSourceBufferError', function () { - it('marks a refused fragment as a gap', function () { - const frag = mainFrag(1, 0); - refuse(frag); - expect(fragmentTracker.addAsGap).to.have.been.calledOnceWith(frag); - }); - it('marks the fragment of a refused part', function () { const frag = mainFrag(1, 0); const part = { index: 1, fragment: frag, gap: false } as unknown as Part; @@ -256,11 +250,6 @@ describe('BaseStreamController', function () { expect(fragmentTracker.addAsGap).to.have.been.calledOnceWith(frag); expect(part.gap).to.equal(false); }); - - it('ignores append errors that are not SourceBuffer errors', function () { - refuse(mainFrag(1, 0), null, 'InvalidStateError'); - expect(fragmentTracker.addAsGap).to.not.have.been.called; - }); }); describe('Seeking Logic', function () { From ae98eba064ac6866e84368b28e19f9b2d8997867 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=B0=ED=99=8D=EC=A4=80?= Date: Fri, 25 Sep 2026 17:41:00 +0900 Subject: [PATCH 6/9] Read the refused part before marking and mark live playlists only addAsGap() clears the parsed info of the fragment's parts, so with separate audio and video SourceBuffers the previous-fragment rule was silently disabled. Fragments of an ended playlist are not marked; they keep the existing retry path, as the load-error gap handling does. --- src/controller/base-stream-controller.ts | 16 +++----- .../controller/audio-stream-controller.ts | 5 +++ .../unit/controller/base-stream-controller.ts | 4 ++ tests/unit/controller/stream-controller.ts | 38 ++++++++++++++++++- 4 files changed, 51 insertions(+), 12 deletions(-) diff --git a/src/controller/base-stream-controller.ts b/src/controller/base-stream-controller.ts index ef6ddc174da..934b9c7b106 100644 --- a/src/controller/base-stream-controller.ts +++ b/src/controller/base-stream-controller.ts @@ -2245,30 +2245,26 @@ export default class BaseStreamController filterType: PlaylistLevelType, data: ErrorData, ) { - const { frag, part } = data; + 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) || - frag.endList + !details?.live ) { return; } - // The SourceBuffer can fail on an append after the damaged one, so mark the fragment, and the - // previous one when the buffer ends short of a refused first part (its tail was not buffered). - this.markSourceBufferErrorGap(frag); - const { sourceBufferName } = data; const start = part?.index === 0 && sourceBufferName ? part.elementaryStreams[sourceBufferName]?.startPTS : undefined; + this.markSourceBufferErrorGap(frag); if (start === undefined) { return; } - const previous = getFragmentWithSN( - this.levels?.[frag.level]?.details, - frag.sn - 1, - ); + // Also mark the previous fragment when its tail is missing before a refused first part + const previous = getFragmentWithSN(details, frag.sn - 1); if ( previous && this.fragmentTracker.getState(previous) !== FragmentState.NOT_LOADED && diff --git a/tests/unit/controller/audio-stream-controller.ts b/tests/unit/controller/audio-stream-controller.ts index 51b61f5aae7..48f1fa75fbd 100644 --- a/tests/unit/controller/audio-stream-controller.ts +++ b/tests/unit/controller/audio-stream-controller.ts @@ -561,6 +561,11 @@ describe('AudioStreamController', function () { }; }; + beforeEach(function () { + // only fragments of a live playlist are marked + (audioStreamController as any).levels = [{ details: { live: true } }]; + }); + it('marks the audio fragment as a gap after a SourceBuffer error', function () { const frag = new Fragment(PlaylistLevelType.AUDIO, ''); frag.sn = 4; diff --git a/tests/unit/controller/base-stream-controller.ts b/tests/unit/controller/base-stream-controller.ts index 9f5159aa320..da48a05377c 100644 --- a/tests/unit/controller/base-stream-controller.ts +++ b/tests/unit/controller/base-stream-controller.ts @@ -138,6 +138,10 @@ describe('BaseStreamController', function () { } function refuse(frag: Fragment, part: Part | null = null) { + // only fragments of a live playlist are marked + const levels: unknown[] = []; + levels[frag.level] = { details: { live: true } }; + (baseStreamController as any).levels = levels; const error = new Error( 'video SourceBuffer error. MediaSource readyState: ended', ); diff --git a/tests/unit/controller/stream-controller.ts b/tests/unit/controller/stream-controller.ts index e7db41770b3..7f130002096 100644 --- a/tests/unit/controller/stream-controller.ts +++ b/tests/unit/controller/stream-controller.ts @@ -27,7 +27,7 @@ import type { FragmentTracker } from '../../../src/controller/fragment-tracker'; import type StreamController from '../../../src/controller/stream-controller'; import type { MediaFragment } from '../../../src/loader/fragment'; import type { ParsedMultivariantPlaylist } from '../../../src/loader/m3u8-parser'; -import type { ErrorData } from '../../../src/types/events'; +import type { BufferAppendedData, ErrorData } from '../../../src/types/events'; import type { LevelAttributes } from '../../../src/types/level'; use(sinonChai); @@ -526,6 +526,7 @@ describe('StreamController', function () { }; it('marks the fragment as a gap after a SourceBuffer error', function () { + setLevelDetails(4); const frag = newFrag(); streamController['onError']( Events.ERROR, @@ -536,6 +537,7 @@ describe('StreamController', function () { }); it('ignores a reset that another append error caused', function () { + setLevelDetails(4); const frag = newFrag(); streamController['onError']( Events.ERROR, @@ -580,6 +582,34 @@ describe('StreamController', function () { expect(fragmentTracker.isGap(details.fragments[1])).to.equal(false); }); + it('reads the refused part before marking when the other SourceBuffer appended it', function () { + const details = setLevelDetails(4); + appendParts(details, 1); + streamController['media'] = { + buffered: new TimeRangesMock([0, 3.5]), + } as unknown as HTMLMediaElement; + const part = details.partList![4]; + part.elementaryStreams.video = { + startPTS: part.start, + endPTS: part.end, + startDTS: part.start, + endDTS: part.end, + }; + // the audio append of the same part went through before the video one was refused + fragmentTracker['onBufferAppended'](Events.BUFFER_APPENDED, { + type: 'audio', + frag: details.fragments[2], + part, + parent: PlaylistLevelType.MAIN, + timeRanges: {}, + } as unknown as BufferAppendedData); + streamController['onError'](Events.ERROR, { + ...resetError(SOURCE_BUFFER_ERROR_NAME, details.fragments[2], part), + sourceBufferName: 'video', + }); + expect(fragmentTracker.isGap(details.fragments[1])).to.equal(true); + }); + it('does not mark the previous fragment when it was not appended', function () { const details = setLevelDetails(4); refuse(details.fragments[2], details.partList![4]); @@ -612,9 +642,13 @@ describe('StreamController', function () { ).to.equal(1); }); - it('does not mark the last fragment of an ended playlist', function () { + it('does not mark fragments of an ended playlist', function () { const details = setLevelDetails(3, false); + refuse(details.fragments[1]); refuse(details.fragments[2]); + expect(fragmentTracker.getState(details.fragments[1])).to.equal( + FragmentState.NOT_LOADED, + ); expect(fragmentTracker.getState(details.fragments[2])).to.equal( FragmentState.NOT_LOADED, ); From 1844c9760e14651e7547cbfa6ea5c532b57dd48e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=B0=ED=99=8D=EC=A4=80?= Date: Fri, 25 Sep 2026 17:41:00 +0900 Subject: [PATCH 7/9] Test that gapFragments() returns only gaps and shorten comments --- src/controller/base-stream-controller.ts | 2 +- src/controller/fragment-tracker.ts | 4 ++-- tests/unit/controller/fragment-tracker.ts | 9 +++++++++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/controller/base-stream-controller.ts b/src/controller/base-stream-controller.ts index 934b9c7b106..c8e57dacfa0 100644 --- a/src/controller/base-stream-controller.ts +++ b/src/controller/base-stream-controller.ts @@ -453,7 +453,7 @@ export default class BaseStreamController if (media) { if (this.keepGapsOnSeek) { - // Keep SourceBuffer error gaps on the seek that follows re-attach + // Keep gap fragments on the first seek after re-attach this.keepGapsOnSeek = false; } else { // Remove gap fragments diff --git a/src/controller/fragment-tracker.ts b/src/controller/fragment-tracker.ts index cad627c3ba5..cb90c3d2b9e 100644 --- a/src/controller/fragment-tracker.ts +++ b/src/controller/fragment-tracker.ts @@ -631,14 +631,14 @@ export class FragmentTracker implements ComponentAPI { } } - /** Whether the fragment tracked at this key is a gap. A playlist refresh drops it from the object. */ + /** Whether the fragment tracked under this key is a gap */ public isGap(frag: Fragment): boolean { return ( this.hasGaps && this.fragments[getFragmentKey(frag)]?.body.gap === true ); } - /** Fragments determined to be gaps, for callers that clear buffer state and re-add them. */ + /** Tracked fragments marked as gaps */ public gapFragments(): MediaFragment[] { if (!this.hasGaps) { return []; diff --git a/tests/unit/controller/fragment-tracker.ts b/tests/unit/controller/fragment-tracker.ts index 305c551257b..7ff1acd74ad 100644 --- a/tests/unit/controller/fragment-tracker.ts +++ b/tests/unit/controller/fragment-tracker.ts @@ -620,6 +620,15 @@ describe('FragmentTracker', function () { expect(fragmentTracker.isGap(gapFrag(1, 2))).to.equal(false); }); + it('lists only the tracked fragments that are gaps', function () { + const fragmentTracker = new FragmentTracker(new Hls({})); + fragmentTracker.fragBuffered(gapFrag(1, 1), true); + fragmentTracker.addAsGap(gapFrag(2, 1)); + expect(fragmentTracker.gapFragments().map((f) => f.sn)).to.deep.equal([ + 2, + ]); + }); + it('stops reporting a gap once the fragment is removed', function () { const fragmentTracker = new FragmentTracker(new Hls({})); const frag = gapFrag(1, 1); From acb4866cac50df8efafb395d3d0cca7a86a5d3e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=B0=ED=99=8D=EC=A4=80?= Date: Fri, 25 Sep 2026 20:56:33 +0900 Subject: [PATCH 8/9] Apply the previous-fragment rule to whole-segment appends With whole-segment loading (a rewound DVR window, or a rendition without parts) a truncated tail is accepted and the next segment is refused. The rule only read the parsed start of a first part, so every reset marked one more healthy segment (7 to 35 in 60 s). Read the fragment's parsed start when there is no part; the same input now recovers with one reset. --- src/controller/base-stream-controller.ts | 7 ++++--- tests/unit/controller/stream-controller.ts | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/controller/base-stream-controller.ts b/src/controller/base-stream-controller.ts index c8e57dacfa0..3682adb5695 100644 --- a/src/controller/base-stream-controller.ts +++ b/src/controller/base-stream-controller.ts @@ -2255,15 +2255,16 @@ export default class BaseStreamController ) { return; } + const refused = part ? (part.index === 0 ? part : null) : frag; const start = - part?.index === 0 && sourceBufferName - ? part.elementaryStreams[sourceBufferName]?.startPTS + refused && sourceBufferName + ? refused.elementaryStreams[sourceBufferName]?.startPTS : undefined; this.markSourceBufferErrorGap(frag); if (start === undefined) { return; } - // Also mark the previous fragment when its tail is missing before a refused first part + // Also mark the previous fragment when the buffer ends more than maxBufferHole before the refused fragment or first part const previous = getFragmentWithSN(details, frag.sn - 1); if ( previous && diff --git a/tests/unit/controller/stream-controller.ts b/tests/unit/controller/stream-controller.ts index 7f130002096..2a39a069961 100644 --- a/tests/unit/controller/stream-controller.ts +++ b/tests/unit/controller/stream-controller.ts @@ -610,6 +610,25 @@ describe('StreamController', function () { expect(fragmentTracker.isGap(details.fragments[1])).to.equal(true); }); + it('also marks the previous fragment when the buffer ends short of a refused whole fragment', function () { + const details = setLevelDetails(4); + appendParts(details, 1); + // loaded without parts: the tail of fragment 1 never reached the buffer + streamController['media'] = { + buffered: new TimeRangesMock([0, 3.5]), + } as unknown as HTMLMediaElement; + const frag = details.fragments[2]; + frag.elementaryStreams.audiovideo = { + startPTS: frag.start, + endPTS: frag.end, + startDTS: frag.start, + endDTS: frag.end, + }; + refuse(frag); + expect(fragmentTracker.isGap(details.fragments[1])).to.equal(true); + expect(fragmentTracker.isGap(details.fragments[2])).to.equal(true); + }); + it('does not mark the previous fragment when it was not appended', function () { const details = setLevelDetails(4); refuse(details.fragments[2], details.partList![4]); From 8703bb378cde50f96710c7872be5f0251724aa99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=B0=ED=99=8D=EC=A4=80?= Date: Sat, 24 Oct 2026 09:39:13 +0900 Subject: [PATCH 9/9] Keep tracked gaps on seeks until hasEnoughToStart and into the buffer Remove the set of SourceBuffer error gaps and the keepGapsOnSeek flag. Fragment and part selection skip any fragment the tracker holds as a gap, so a gap carried across detach is skipped until a seek removes it. onMediaSeeking removes tracked gaps only once hasEnoughToStart is set and the seek lands outside the buffer. seeking is dispatched from a queued task, so the seek issued by seekToStartPos() already reads hasEnoughToStart as true, and the short seeks hls.js issues itself after re-attach (buffer-start adjustment, hole skip, nudge) all land inside the buffer. A seek out of the buffer still removes gaps so the fragments are retried. Set _hasEnoughToStart after seekToStartPos() as suggested. --- src/controller/base-stream-controller.ts | 33 +------ src/controller/stream-controller.ts | 2 +- .../unit/controller/base-stream-controller.ts | 67 +++------------ tests/unit/controller/stream-controller.ts | 86 +++++++------------ 4 files changed, 52 insertions(+), 136 deletions(-) diff --git a/src/controller/base-stream-controller.ts b/src/controller/base-stream-controller.ts index 15acd46de47..dec180c9668 100644 --- a/src/controller/base-stream-controller.ts +++ b/src/controller/base-stream-controller.ts @@ -129,8 +129,6 @@ export default class BaseStreamController protected buffering: boolean = true; protected loadingParts: boolean = false; private loopSn?: string | number; - private keepGapsOnSeek: boolean = false; - private sourceBufferErrorGaps: Set = new Set(); constructor( hls: Hls, @@ -376,11 +374,6 @@ export default class BaseStreamController const gaps = fragmentTracker.gapFragments(); fragmentTracker.removeAllFragments(); gaps.forEach((frag) => fragmentTracker.addAsGap(frag)); - this.keepGapsOnSeek = gaps.some( - (frag) => - frag.type === this.playlistType && - this.sourceBufferErrorGaps.has(gapKey(frag)), - ); this.stopLoad(); } @@ -394,8 +387,6 @@ export default class BaseStreamController this.lastCurrentTime = 0; this.startPosition = -1; this.startFragRequested = false; - this.keepGapsOnSeek = false; - this.sourceBufferErrorGaps.clear(); } protected onError(event: Events.ERROR, data: ErrorData) {} @@ -452,11 +443,8 @@ export default class BaseStreamController } if (media) { - if (this.keepGapsOnSeek) { - // Keep gap fragments on the first seek after re-attach - this.keepGapsOnSeek = false; - } else { - // Remove gap fragments + if (this.hls?.hasEnoughToStart && !fowardBuffer) { + // Remove gap fragments on a seek out of the buffer, after `seekToStartPos` this.fragmentTracker.removeFragmentsInRange( currentTime, Infinity, @@ -1632,7 +1620,7 @@ export default class BaseStreamController this.nextLoadPosition = programFrag.start; } } - while (programFrag && this.isSourceBufferErrorGap(programFrag)) { + while (programFrag && this.fragmentTracker.isGap(programFrag)) { const afterGap = this.filterReplacedPrimary( getNextFrag(levelDetails, programFrag.sn), levelDetails, @@ -1789,7 +1777,7 @@ export default class BaseStreamController break; } const loaded = - part.loaded || part.gap || this.isSourceBufferErrorGap(part.fragment); + part.loaded || part.gap || this.fragmentTracker.isGap(part.fragment); if (loaded) { nextPart = -1; } else if ( @@ -2270,7 +2258,6 @@ export default class BaseStreamController if ( previous && this.fragmentTracker.getState(previous) !== FragmentState.NOT_LOADED && - !this.fragmentTracker.isGap(previous) && !BufferHelper.isBuffered( this.mediaBuffer || this.media, start - this.config.maxBufferHole, @@ -2284,17 +2271,9 @@ export default class BaseStreamController this.warn( `Marking fragment ${frag.sn} of ${this.playlistLabel()} ${frag.level} as a gap after a SourceBuffer error`, ); - this.sourceBufferErrorGaps.add(gapKey(frag)); this.fragmentTracker.addAsGap(frag); } - private isSourceBufferErrorGap(frag: Fragment): boolean { - return ( - this.sourceBufferErrorGaps.has(gapKey(frag)) && - this.fragmentTracker.isGap(frag) - ); - } - protected checkRetryDate() { const now = self.performance.now(); const retryDate = this.retryDate; @@ -2879,7 +2858,3 @@ export function interstitialsEnabled(config: Readonly): boolean { config.enableInterstitialPlayback !== false ); } - -function gapKey(frag: Fragment): string { - return `${frag.level}_${frag.sn}`; -} diff --git a/src/controller/stream-controller.ts b/src/controller/stream-controller.ts index 6c6dace926a..8ad6648649b 100644 --- a/src/controller/stream-controller.ts +++ b/src/controller/stream-controller.ts @@ -999,8 +999,8 @@ export default class StreamController return; } if (!this._hasEnoughToStart && BufferHelper.getBuffered(media).length) { - this._hasEnoughToStart = true; this.seekToStartPos(); + this._hasEnoughToStart = true; } if (bufferedMainFragment) { this.tick(); diff --git a/tests/unit/controller/base-stream-controller.ts b/tests/unit/controller/base-stream-controller.ts index 2db2ba42337..926bd5de45d 100644 --- a/tests/unit/controller/base-stream-controller.ts +++ b/tests/unit/controller/base-stream-controller.ts @@ -209,37 +209,17 @@ describe('BaseStreamController', function () { }); }); - describe('getNextFragment after a SourceBuffer error', function () { - it('returns the fragment after the refused one', function () { + describe('getNextFragment skips tracked gaps', function () { + it('returns the fragment after the gap', function () { const levelDetails = levelDetailsWithEndSequenceVodOrLive(4); - // a live playlist refresh builds a new object for the same sn and level - refuse(mainFrag(1, levelDetails.fragments[1].level)); fragmentTracker.gapSn = 1; const frag = baseStreamController.getNextFragment(5, levelDetails); expect(frag?.sn, 'skips to the next fragment').to.equal(2); expect(baseStreamController.nextLoadPosition).to.equal(10); }); - it('does not skip a tracked gap that no SourceBuffer error marked', function () { - const levelDetails = levelDetailsWithEndSequenceVodOrLive(4); - fragmentTracker.gapSn = 1; - expect( - baseStreamController.getNextFragment(5, levelDetails)?.sn, - ).to.equal(1); - }); - - it('does not skip a refused fragment the tracker no longer holds as a gap', function () { - const levelDetails = levelDetailsWithEndSequenceVodOrLive(4); - refuse(levelDetails.fragments[1]); - fragmentTracker.gapSn = null; - expect( - baseStreamController.getNextFragment(5, levelDetails)?.sn, - ).to.equal(1); - }); - - it('returns null when the refused fragment is the last one', function () { + it('returns null when the gap is the last fragment', function () { const levelDetails = levelDetailsWithEndSequenceVodOrLive(2); - refuse(levelDetails.fragments[1]); fragmentTracker.gapSn = 1; expect(baseStreamController.getNextFragment(5, levelDetails)).to.equal( null, @@ -310,55 +290,36 @@ describe('BaseStreamController', function () { resetSpy.restore(); }); - it('keeps gaps on the seek that follows a detach which carried a refused fragment', function () { - media.removeEventListener = sinon.spy(); - const frag = mainFrag(1, 0); - refuse(frag); - fragmentTracker.gaps = [frag]; - baseStreamController.onMediaDetaching(null, {}); - baseStreamController.media = media; + it('keeps gaps on a seek before hasEnoughToStart and removes them once it is set', function () { + const hasEnoughToStart = sinon.stub(hls, 'hasEnoughToStart'); media.currentTime = 10.0; + hasEnoughToStart.get(() => false); baseStreamController.onMediaSeeking(); expect( fragmentTracker.removeFragmentsInRange, - 'the re-attach seek keeps them', + 'the seek to the start position keeps them', ).to.have.not.been.called; + hasEnoughToStart.get(() => true); baseStreamController.onMediaSeeking(); expect( fragmentTracker.removeFragmentsInRange, - 'a later seek removes them', + 'a later seek out of the buffer removes them', ).to.have.been.calledOnce; }); - it('does not keep carried gaps that no SourceBuffer error marked', function () { - media.removeEventListener = sinon.spy(); - fragmentTracker.gaps = [mainFrag(1, 0)]; - baseStreamController.onMediaDetaching(null, {}); - baseStreamController.media = media; - media.currentTime = 10.0; - - baseStreamController.onMediaSeeking(); - expect(fragmentTracker.removeFragmentsInRange).to.have.been.calledOnce; - }); - - it('does not keep gaps carried for another playlist type', function () { - media.removeEventListener = sinon.spy(); - refuse(mainFrag(1, 0)); - const audioFrag = new Fragment(PlaylistLevelType.AUDIO, ''); - audioFrag.sn = 1; - audioFrag.level = 0; - fragmentTracker.gaps = [audioFrag]; - baseStreamController.onMediaDetaching(null, {}); - baseStreamController.media = media; + it('keeps gaps on a seek into the buffer', function () { + sinon.stub(hls, 'hasEnoughToStart').get(() => true); + media.buffered = new TimeRangesMock([5, 20]); media.currentTime = 10.0; baseStreamController.onMediaSeeking(); - expect(fragmentTracker.removeFragmentsInRange).to.have.been.calledOnce; + expect(fragmentTracker.removeFragmentsInRange).to.have.not.been.called; }); it('should call fragmentTracker.removeFragmentsInRange when media exists', function () { + sinon.stub(hls, 'hasEnoughToStart').get(() => true); baseStreamController.lastCurrentTime = 5.0; media.currentTime = 10.0; diff --git a/tests/unit/controller/stream-controller.ts b/tests/unit/controller/stream-controller.ts index b60ffcd9f6c..3d664ac2bb4 100644 --- a/tests/unit/controller/stream-controller.ts +++ b/tests/unit/controller/stream-controller.ts @@ -644,23 +644,6 @@ describe('StreamController', function () { expect(fragmentTracker.isGap(details.fragments[2])).to.equal(true); }); - it('leaves a previous fragment gapped by another error to be retried', function () { - const details = setLevelDetails(4); - // gapped after a load error, then replaced by a playlist refresh - const loadFailed = new Fragment( - PlaylistLevelType.MAIN, - '', - ) as MediaFragment; - loadFailed.sn = 1; - loadFailed.level = 0; - fragmentTracker.addAsGap(loadFailed); - refuse(details.fragments[2], details.partList![4]); - expect( - streamController['getNextFragment'](details.fragments[1].start, details) - ?.sn, - ).to.equal(1); - }); - it('does not mark fragments of an ended playlist', function () { const details = setLevelDetails(3, false); refuse(details.fragments[1]); @@ -701,42 +684,6 @@ describe('StreamController', function () { streamController['onMediaDetaching'](Events.MEDIA_DETACHING, {}); expect(fragmentTracker.isGap(details.fragments[2])).to.equal(true); }); - - it('does not skip a gap of a new source that reuses the key of a refused fragment', function () { - refuse(setLevelDetails(4).fragments[2]); - hls.trigger(Events.MANIFEST_LOADING, { url: '' }); - const details = setLevelDetails(4); - // gapped after a load error, then replaced by a playlist refresh - const loadFailed = new Fragment( - PlaylistLevelType.MAIN, - '', - ) as MediaFragment; - loadFailed.sn = 2; - loadFailed.level = 0; - fragmentTracker.addAsGap(loadFailed); - expect( - streamController['getNextFragment'](details.fragments[2].start, details) - ?.sn, - ).to.equal(2); - }); - - it('purges gaps on the first seek after a new manifest', function () { - const details = setLevelDetails(4); - const media = { - currentTime: 1, - buffered: new TimeRangesMock(), - addEventListener() {}, - removeEventListener() {}, - } as unknown as HTMLMediaElement; - streamController['media'] = media; - refuse(details.fragments[2]); - streamController['onMediaDetaching'](Events.MEDIA_DETACHING, {}); - hls.trigger(Events.MANIFEST_LOADING, { url: '' }); - streamController['media'] = media; - fragmentTracker.addAsGap(details.fragments[3]); - streamController['onMediaSeeking'](); - expect(fragmentTracker.isGap(details.fragments[3])).to.equal(false); - }); }); describe('fragment loading', function () { @@ -860,6 +807,39 @@ describe('StreamController', function () { expect(seekStub).to.have.been.calledOnce; }); + it('sets hasEnoughToStart after seeking to the start position', function () { + const firstFrag = new Fragment( + PlaylistLevelType.MAIN, + '', + ) as MediaFragment; + firstFrag.duration = 5.0; + firstFrag.level = 1; + firstFrag.setStart(0); + firstFrag.sn = 1; + firstFrag.cc = 0; + firstFrag.elementaryStreams.video = { + startDTS: 0, + startPTS: 0, + endDTS: 5, + endPTS: 5, + }; + let hasEnoughToStartWhenSeeking: boolean | undefined; + // @ts-ignore + sandbox.stub(streamController, 'seekToStartPos').callsFake(() => { + hasEnoughToStartWhenSeeking = streamController.hasEnoughToStart; + }); + streamController['fragCurrent'] = streamController['fragPrevious'] = + firstFrag; + streamController['onFragBuffered'](Events.FRAG_BUFFERED, { + stats: new LoadStats(), + frag: firstFrag, + part: null, + id: firstFrag.type, + }); + expect(hasEnoughToStartWhenSeeking).to.equal(false); + expect(streamController.hasEnoughToStart).to.equal(true); + }); + it('should seek to start pos when media is buffered and seek to segment boundary', function () { streamController['config'].startOnSegmentBoundary = true; streamController['startPosition'] = 7;