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

fix: more accurate segment choices and logging #1127

Merged
merged 14 commits into from
May 26, 2021
Merged

fix: more accurate segment choices and logging #1127

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a4a45df
fix: more accurate segment choices and logging
brandonocasey May 5, 2021
d2d4aab
Merge branch 'main' into fix/better-segment-choice
brandonocasey May 24, 2021
127e075
ie 11 fix
brandonocasey May 24, 2021
6ddcf4c
pass segments to getSyncSegmentCandidate
brandonocasey May 25, 2021
fa97a95
Update src/segment-loader.js
brandonocasey May 25, 2021
eae4f41
Update src/segment-loader.js
brandonocasey May 25, 2021
17dda63
inccorrect assertion text
brandonocasey May 25, 2021
ad088c5
fix trailing space
brandonocasey May 25, 2021
8ebbc50
time fudge factor comment
brandonocasey May 26, 2021
c6f36e4
call updateTimingInfoEnd and add some comments
brandonocasey May 26, 2021
2bd3f53
better comment
brandonocasey May 26, 2021
dd29591
Merge remote-tracking branch 'origin/main' into fix/better-segment-ch…
brandonocasey May 26, 2021
a971ff0
add PART-TARGET
brandonocasey May 26, 2021
504f434
totalParts -> zeroBasedPartCount
brandonocasey May 26, 2021
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
8 changes: 2 additions & 6 deletions src/segment-loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,10 @@ const segmentInfoString = (segmentInfo) => {

const hasPartIndex = typeof partIndex === 'number';
const name = segmentInfo.segment.uri ? 'segment' : 'pre-segment';
const partCount = segment.parts ? segment.parts.length : 0;
const preloadPartCount = segment.preloadHints ?
segment.preloadHints.filter((h) => h.type === 'PART').length :
0;
const zeroBasedPartCount = partCount + preloadPartCount - 1 - (preloadPartCount > 0 ? 1 : 0);
const totalParts = hasPartIndex ? getKnownPartCount({preloadSegment: segment}) - 1 : 0;

return `${name} [${seq + index}/${seq + segmentLen}]` +
(hasPartIndex ? ` part [${partIndex}/${zeroBasedPartCount}]` : '') +
(hasPartIndex ? ` part [${partIndex}/${totalParts}]` : '') +
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this take into account the 0 based indexing vs total. Maybe we can just say `[index 0, 3 total]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should keep the name zeroBasedPartCount here I guess. I do subtract 1 to make it zero based.

` segment start/end [${segment.start} => ${segment.end}]` +
(hasPartIndex ? ` part start/end [${part.start} => ${part.end}]` : '') +
` startOfSegment [${startOfSegment}]` +
Expand Down
10 changes: 10 additions & 0 deletions test/playlist-loader.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2143,6 +2143,7 @@ QUnit.module('Playlist Loader', function(hooks) {
this.requests.shift().respond(
200, null,
'#EXTM3U\n' +
'#EXT-X-PART-INF:PART-TARGET=1\n' +
'#EXT-X-MEDIA-SEQUENCE:0\n' +
'#EXTINF:2\n' +
'low-1.ts\n' +
Expand All @@ -2163,6 +2164,7 @@ QUnit.module('Playlist Loader', function(hooks) {
this.requests.shift().respond(
200, null,
'#EXTM3U\n' +
'#EXT-X-PART-INF:PART-TARGET=1\n' +
'#EXT-X-MEDIA-SEQUENCE:0\n' +
'#EXTINF:2\n' +
'low-1.ts\n' +
Expand All @@ -2182,6 +2184,7 @@ QUnit.module('Playlist Loader', function(hooks) {
this.requests.shift().respond(
200, null,
'#EXTM3U\n' +
'#EXT-X-PART-INF:PART-TARGET=1\n' +
'#EXT-X-MEDIA-SEQUENCE:0\n' +
'#EXTINF:2\n' +
'segment0.ts\n' +
Expand All @@ -2208,6 +2211,7 @@ QUnit.module('Playlist Loader', function(hooks) {
this.requests.shift().respond(
200, null,
'#EXTM3U\n' +
'#EXT-X-PART-INF:PART-TARGET=1\n' +
'#EXT-X-MEDIA-SEQUENCE:0\n' +
'#EXT-X-SERVER-CONTROL:CAN-SKIP-UNTIL=3\n' +
'#EXTINF:2\n' +
Expand Down Expand Up @@ -2242,6 +2246,7 @@ QUnit.module('Playlist Loader', function(hooks) {
this.requests.shift().respond(
200, null,
'#EXTM3U\n' +
'#EXT-X-PART-INF:PART-TARGET=1\n' +
'#EXT-X-MEDIA-SEQUENCE:0\n' +
'#EXT-X-SERVER-CONTROL:CAN-SKIP-UNTIL=3,CAN-SKIP-DATERANGES=YES\n' +
'#EXTINF:2\n' +
Expand Down Expand Up @@ -2276,6 +2281,7 @@ QUnit.module('Playlist Loader', function(hooks) {
this.requests.shift().respond(
200, null,
'#EXTM3U\n' +
'#EXT-X-PART-INF:PART-TARGET=1\n' +
'#EXT-X-MEDIA-SEQUENCE:0\n' +
'#EXT-X-SERVER-CONTROL:CAN-BLOCK-RELOAD=YES\n' +
'#EXTINF:2\n' +
Expand Down Expand Up @@ -2309,6 +2315,7 @@ QUnit.module('Playlist Loader', function(hooks) {
this.requests.shift().respond(
200, null,
'#EXTM3U\n' +
'#EXT-X-PART-INF:PART-TARGET=1\n' +
'#EXT-X-MEDIA-SEQUENCE:0\n' +
'#EXT-X-SERVER-CONTROL:CAN-BLOCK-RELOAD=YES\n' +
'#EXTINF:2\n' +
Expand Down Expand Up @@ -2341,6 +2348,7 @@ QUnit.module('Playlist Loader', function(hooks) {
this.requests.shift().respond(
200, null,
'#EXTM3U\n' +
'#EXT-X-PART-INF:PART-TARGET=1\n' +
'#EXT-X-MEDIA-SEQUENCE:0\n' +
'#EXT-X-SERVER-CONTROL:CAN-BLOCK-RELOAD=YES\n' +
'#EXTINF:2\n' +
Expand Down Expand Up @@ -2373,6 +2381,7 @@ QUnit.module('Playlist Loader', function(hooks) {
this.requests.shift().respond(
200, null,
'#EXTM3U\n' +
'#EXT-X-PART-INF:PART-TARGET=1\n' +
'#EXT-X-MEDIA-SEQUENCE:0\n' +
'#EXT-X-SERVER-CONTROL:CAN-BLOCK-RELOAD=YES\n' +
'#EXTINF:2\n' +
Expand Down Expand Up @@ -2407,6 +2416,7 @@ QUnit.module('Playlist Loader', function(hooks) {
this.requests.shift().respond(
200, null,
'#EXTM3U\n' +
'#EXT-X-PART-INF:PART-TARGET=1\n' +
'#EXT-X-MEDIA-SEQUENCE:0\n' +
'#EXT-X-SERVER-CONTROL:CAN-BLOCK-RELOAD=YES,CAN-SKIP-UNTIL=3\n' +
'#EXTINF:2\n' +
Expand Down