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

fix(Tizen): Adding gapPadding to gap manager to solve Tizen issue #7331

Merged
merged 8 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions demo/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,9 @@ shakaDemo.Config = class {
.addNumberInput_('Gap detection threshold',
'streaming.gapDetectionThreshold',
/* canBeDecimal= */ true)
.addNumberInput_('Gap padding',
'streaming.gapPadding',
/* canBeDecimal= */ true)
.addNumberInput_('Gap Jump Timer Time', 'streaming.gapJumpTimerTime',
/* canBeDecimal= */ true)
.addNumberInput_('Buffering Goal', 'streaming.bufferingGoal',
Expand Down
8 changes: 8 additions & 0 deletions externs/shaka/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -1537,6 +1537,7 @@ shaka.extern.LiveSyncConfiguration;
* alwaysStreamText: boolean,
* startAtSegmentBoundary: boolean,
* gapDetectionThreshold: number,
* gapPadding: number,
* gapJumpTimerTime: number,
* durationBackoff: number,
* safeSeekOffset: number,
Expand Down Expand Up @@ -1630,6 +1631,13 @@ shaka.extern.LiveSyncConfiguration;
* jump.
* <br>
* Defaults to <code>0.5</code>.
* @property {number} gapPadding
* Padding added only for Xbox, Legacy Edge and Tizen.
* Based on our research (specific to Tizen), the gapPadding value must be
* greater than your GOP length.
* It’s crucial to verify this value according to your actual stream.
* <br>
* Defaults to <code>0.01</code> for Xbox and Legacy Edge, Tizen at 2.
* @property {number} gapJumpTimerTime
* The polling time in seconds to check for gaps in the media.
* <br>
Expand Down
6 changes: 4 additions & 2 deletions lib/media/gap_jumping_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,10 @@ shaka.media.GapJumpingController = class {
// often rounds value we want to set as currentTime and we are not able
// to jump over the gap.
if (shaka.util.Platform.isLegacyEdge() ||
shaka.util.Platform.isXboxOne()) {
jumpTo = Math.ceil((jumpTo + 0.01) * 100) / 100;
shaka.util.Platform.isXboxOne() ||
shaka.util.Platform.isTizen()) {
const gapPadding = this.config_.gapPadding;
jumpTo = Math.ceil((jumpTo + gapPadding) * 100) / 100;
}
const seekEnd = this.timeline_.getSeekRangeEnd();
if (jumpTo >= seekEnd) {
Expand Down
5 changes: 5 additions & 0 deletions lib/util/player_configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ shaka.util.PlayerConfiguration = class {
alwaysStreamText: false,
startAtSegmentBoundary: false,
gapDetectionThreshold: 0.5,
gapPadding: 0.01,
gapJumpTimerTime: 0.25 /* seconds */,
durationBackoff: 1,
// Offset by 5 seconds since Chromecast takes a few seconds to start
Expand Down Expand Up @@ -284,6 +285,10 @@ shaka.util.PlayerConfiguration = class {
streaming.stallSkip = 0;
}

if (shaka.util.Platform.isTizen()) {
streaming.gapPadding = 2;
}

const offline = {
// We need to set this to a throw-away implementation for now as our
// default implementation will need to reference other fields in the
Expand Down
50 changes: 31 additions & 19 deletions test/media/playhead_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,18 @@ describe('Playhead', () => {
playhead.release();
});

function calculateGap(time) {
let jumpTo = time;
if (shaka.util.Platform.isLegacyEdge() ||
shaka.util.Platform.isXboxOne() ||
shaka.util.Platform.isTizen()) {
const gapPadding = shaka.util.PlayerConfiguration.createDefault()
.streaming.gapPadding;
jumpTo = Math.ceil((jumpTo + gapPadding) * 100) / 100;
}
return jumpTo;
}

function setMockDate(seconds) {
const minutes = Math.floor(seconds / 60);
seconds %= 60;
Expand Down Expand Up @@ -900,7 +912,7 @@ describe('Playhead', () => {
start: 5,
waitingAt: 10,
expectEvent: true,
expectedEndTime: 11,
expectedEndTime: calculateGap(11),
});

playingTest('won\'t skip a buffered range', {
Expand All @@ -909,7 +921,7 @@ describe('Playhead', () => {
start: 5,
waitingAt: 10,
expectEvent: true,
expectedEndTime: 11,
expectedEndTime: calculateGap(11),
});

playingTest('will jump gap into last buffer', {
Expand All @@ -918,7 +930,7 @@ describe('Playhead', () => {
start: 15,
waitingAt: 20,
expectEvent: true,
expectedEndTime: 21,
expectedEndTime: calculateGap(21),
});
}); // with small gaps

Expand All @@ -928,7 +940,7 @@ describe('Playhead', () => {
start: 5,
waitingAt: 10,
expectEvent: true,
expectedEndTime: 30,
expectedEndTime: calculateGap(30),
});

playingTest('will only jump one buffer', {
Expand All @@ -937,7 +949,7 @@ describe('Playhead', () => {
start: 5,
waitingAt: 10,
expectEvent: true,
expectedEndTime: 30,
expectedEndTime: calculateGap(30),
});

playingTest('will jump into last buffer', {
Expand All @@ -946,7 +958,7 @@ describe('Playhead', () => {
start: 24,
waitingAt: 30,
expectEvent: true,
expectedEndTime: 50,
expectedEndTime: calculateGap(50),
});
}); // with large gaps

Expand Down Expand Up @@ -1019,7 +1031,7 @@ describe('Playhead', () => {
start: 3,
seekTo: 10.4,
expectEvent: true,
expectedEndTime: 11,
expectedEndTime: calculateGap(11),
});

seekTest('won\'t jump multiple buffers', {
Expand All @@ -1028,7 +1040,7 @@ describe('Playhead', () => {
start: 3,
seekTo: 10.4,
expectEvent: true,
expectedEndTime: 11,
expectedEndTime: calculateGap(11),
});

seekTest('will jump into last range with seeking', {
Expand All @@ -1037,15 +1049,15 @@ describe('Playhead', () => {
start: 3,
seekTo: 20.5,
expectEvent: true,
expectedEndTime: 21,
expectedEndTime: calculateGap(21),
});

seekTest('treats large gaps as small if playhead near end', {
buffered: [{start: 0, end: 10}, {start: 30, end: 40}],
start: 3,
seekTo: 29.2,
expectEvent: true,
expectedEndTime: 30,
expectedEndTime: calculateGap(30),
});
}); // with small gaps

Expand All @@ -1055,7 +1067,7 @@ describe('Playhead', () => {
start: 5,
seekTo: 12,
expectEvent: true,
expectedEndTime: 30,
expectedEndTime: calculateGap(30),
});
}); // with large gaps
}); // with buffered seeks
Expand All @@ -1080,7 +1092,7 @@ describe('Playhead', () => {
start: 4,
seekTo: 0,
expectEvent: true,
expectedEndTime: 0.2,
expectedEndTime: calculateGap(0.2),
});

seekTest('will jump when seeking into gap', {
Expand All @@ -1090,7 +1102,7 @@ describe('Playhead', () => {
start: 3,
seekTo: 30.2,
expectEvent: true,
expectedEndTime: 31,
expectedEndTime: calculateGap(31),
});

seekTest('will jump when seeking to the end of a range', {
Expand All @@ -1100,7 +1112,7 @@ describe('Playhead', () => {
start: 3,
seekTo: 30,
expectEvent: true,
expectedEndTime: 31,
expectedEndTime: calculateGap(31),
});

seekTest('won\'t jump when past end', {
Expand Down Expand Up @@ -1141,7 +1153,7 @@ describe('Playhead', () => {
start: 24,
seekTo: 1.6,
expectEvent: true,
expectedEndTime: 2,
expectedEndTime: calculateGap(2),
});
}); // with small gaps

Expand All @@ -1152,7 +1164,7 @@ describe('Playhead', () => {
start: 25,
seekTo: 0,
expectEvent: true,
expectedEndTime: 20,
expectedEndTime: calculateGap(20),
});

seekTest('will jump large gaps', {
Expand All @@ -1162,7 +1174,7 @@ describe('Playhead', () => {
start: 3,
seekTo: 32,
expectEvent: true,
expectedEndTime: 40,
expectedEndTime: calculateGap(40),
});
}); // with large gaps
}); // with unbuffered seeks
Expand Down Expand Up @@ -1237,7 +1249,7 @@ describe('Playhead', () => {
jasmine.clock().tick(500);

expect(seekCount).toBe(1);
expect(currentTime).toBe(10);
expect(currentTime).toBe(calculateGap(10));
});

it('doesn\'t gap jump if paused', () => {
Expand Down Expand Up @@ -1287,7 +1299,7 @@ describe('Playhead', () => {
jasmine.clock().tick(500);

// There SHOULD have been a gap jump.
expect(video.currentTime).toBe(10);
expect(video.currentTime).toBe(calculateGap(10));
});

// Regression test for https://github.com/shaka-project/shaka-player/issues/3451
Expand Down
Loading