Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Guard against exceptions caused by destroying player in buffer stall error callbacks #4786

Merged
merged 1 commit into from
Jul 19, 2022
Merged

Guard against exceptions caused by destroying player in buffer stall error callbacks #4786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 26 additions & 8 deletions src/controller/gap-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const SKIP_BUFFER_RANGE_START = 0.05;

export default class GapController {
private config: HlsConfig;
private media: HTMLMediaElement;
private media: HTMLMediaElement | null = null;
private fragmentTracker: FragmentTracker;
private hls: Hls;
private nudgeRetry: number = 0;
Expand All @@ -32,8 +32,9 @@ export default class GapController {
}

public destroy() {
this.media = null;
// @ts-ignore
this.hls = this.fragmentTracker = this.media = null;
this.hls = this.fragmentTracker = null;
}

/**
Expand All @@ -44,6 +45,9 @@ export default class GapController {
*/
public poll(lastCurrentTime: number) {
const { config, media, stalled } = this;
if (media === null) {
return;
}
const { currentTime, seeking } = media;
const seeked = this.seeking && !seeking;
const beginSeek = !this.seeking && seeking;
Expand Down Expand Up @@ -142,7 +146,10 @@ export default class GapController {
const stalledDuration = tnow - stalled;
if (!seeking && stalledDuration >= STALL_MINIMUM_DURATION_MS) {
// Report stalling after trying to fix
this._reportStall(bufferInfo.len);
this._reportStall(bufferInfo);
if (!this.media) {
return;
}
}

const bufferedWithHoles = BufferHelper.bufferInfo(
Expand All @@ -164,6 +171,9 @@ export default class GapController {
stalledDurationMs: number
) {
const { config, fragmentTracker, media } = this;
if (media === null) {
return;
}
const currentTime = media.currentTime;

const partial = fragmentTracker.getPartialFragment(currentTime);
Expand All @@ -173,7 +183,7 @@ export default class GapController {
const targetTime = this._trySkipBufferHole(partial);
// we return here in this case, meaning
// the branch below only executes when we don't handle a partial fragment
if (targetTime) {
if (targetTime || !this.media) {
return;
}
}
Expand All @@ -200,19 +210,21 @@ export default class GapController {
* @param bufferLen - The playhead distance from the end of the current buffer segment.
* @private
*/
private _reportStall(bufferLen) {
private _reportStall(bufferInfo: BufferInfo) {
const { hls, media, stallReported } = this;
if (!stallReported) {
if (!stallReported && media) {
// Report stalled error once
this.stallReported = true;
logger.warn(
`Playback stalling at @${media.currentTime} due to low buffer (buffer=${bufferLen})`
`Playback stalling at @${
media.currentTime
} due to low buffer (${JSON.stringify(bufferInfo)})`
);
hls.trigger(Events.ERROR, {
type: ErrorTypes.MEDIA_ERROR,
details: ErrorDetails.BUFFER_STALLED_ERROR,
fatal: false,
buffer: bufferLen,
buffer: bufferInfo.len,
});
}
}
Expand All @@ -224,6 +236,9 @@ export default class GapController {
*/
private _trySkipBufferHole(partial: Fragment | null): number {
const { config, hls, media } = this;
if (media === null) {
return 0;
}
const currentTime = media.currentTime;
let lastEndTime = 0;
// Check if currentTime is between unbuffered regions of partial fragments
Expand Down Expand Up @@ -266,6 +281,9 @@ export default class GapController {
*/
private _tryNudgeBuffer() {
const { config, hls, media, nudgeRetry } = this;
if (media === null) {
return;
}
const currentTime = media.currentTime;
this.nudgeRetry++;

Expand Down
4 changes: 2 additions & 2 deletions tests/unit/controller/gap-controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe('GapController', function () {

describe('_reportStall', function () {
it('should report a stall with the current buffer length if it has not already been reported', function () {
gapController._reportStall(42);
gapController._reportStall({ len: 42 });
expect(triggerSpy).to.have.been.calledWith(Events.ERROR, {
type: ErrorTypes.MEDIA_ERROR,
details: ErrorDetails.BUFFER_STALLED_ERROR,
Expand All @@ -83,7 +83,7 @@ describe('GapController', function () {

it('should not report a stall if it was already reported', function () {
gapController.stallReported = true;
gapController._reportStall(42);
gapController._reportStall({ len: 42 });
expect(triggerSpy).to.not.have.been.called;
});
});
Expand Down