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.

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

Bugfix/ll hls endofstream #4516

Merged
merged 4 commits into from
Jan 20, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
21 changes: 20 additions & 1 deletion src/controller/base-stream-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,28 @@ export default class BaseStreamController
if (
!levelDetails.live &&
fragCurrent &&
fragCurrent.sn === levelDetails.endSN &&
// NOTE: Because of the way parts are currently parsed/represented in the playlist, we can end up
// in situations where the current fragment is actually greater than levelDetails.endSN. While
// this feels like the "wrong place" to account for that, this is a narrower/safer change than
// updating e.g. M3U8Parser::parseLevelPlaylist().
fragCurrent.sn >= levelDetails.endSN &&
Comment on lines +145 to +149
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

!bufferInfo.nextStart
) {
const partList = (levelDetails as LevelDetails).partList;
// Since the last part isn't guaranteed to correspond to fragCurrent for ll-hls, check instead if the last part is buffered.
if (partList && partList.length) {
const lastPart = partList[partList.length - 1];

// Checking the midpoint of the part for potential margin of error and related issues.
// NOTE: Technically I believe parts could yield content that is < the computed duration (including potential a duration of 0)
// and still be spec-compliant, so there may still be edge cases here. Likewise, there could be issues in end of stream
// part mismatches for independent audio and video playlists/segments.
const lastPartBuffered = BufferHelper.isBuffered(
this.media,
lastPart.start + lastPart.duration / 2
);
return lastPartBuffered;
}
const fragState = fragmentTracker.getState(fragCurrent);
return (
fragState === FragmentState.PARTIAL || fragState === FragmentState.OK
Expand Down
36 changes: 36 additions & 0 deletions tests/mocks/time-ranges.mock.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
const assertValidRange = (name, length, index) => {
if (index >= length || index < 0) {
throw new DOMException(
`Failed to execute '${name}' on 'TimeRanges': The index provided (${index}) is greater than the maximum bound (${length}).`
);
}
return true;
};

export class TimeRangesMock {
_ranges = [];

// Accepts an argument list of [start, end] tuples or { start: number, end: number } objects
constructor(...ranges) {
this._ranges = ranges.map((range) =>
Array.isArray(range) ? range : [range.start, range.end]
);
}

get length() {
const { _ranges: ranges } = this;
return ranges.length;
}

start(i) {
const { _ranges: ranges, length } = this;
assertValidRange('start', length, i);
return ranges[i] && ranges[i][0];
}

end(i) {
const { _ranges: ranges, length } = this;
assertValidRange('end', length, i);
return ranges[i] && ranges[i][1];
}
}
21 changes: 21 additions & 0 deletions tests/unit/controller/base-stream-controller.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import BaseStreamController from '../../../src/controller/stream-controller';
import Hls from '../../../src/hls';
import { FragmentState } from '../../../src/controller/fragment-tracker';
import { TimeRangesMock } from '../../mocks/time-ranges.mock';

describe('BaseStreamController', function () {
let baseStreamController;
Expand Down Expand Up @@ -75,5 +76,25 @@ describe('BaseStreamController', function () {
`fragState is ${fragmentTracker.getState()}, expecting OK`
).to.be.true;
});

it('returns true if parts are buffered for low latency content', function () {
media.buffered = new TimeRangesMock([0, 1]);
baseStreamController.fragCurrent = { sn: 10 };
levelDetails.endSN = 10;
levelDetails.partList = [{ start: 0, duration: 1 }];

expect(baseStreamController._streamEnded(bufferInfo, levelDetails)).to.be
.true;
});

it('returns true even if fragCurrent is after the last fragment due to low latency content modeling', function () {
media.buffered = new TimeRangesMock([0, 1]);
baseStreamController.fragCurrent = { sn: 11 };
levelDetails.endSN = 10;
levelDetails.partList = [{ start: 0, duration: 1 }];

expect(baseStreamController._streamEnded(bufferInfo, levelDetails)).to.be
.true;
});
});
});