Skip to content

Commit

Permalink
feat(ABR): Abr improvement config (shaka-project#5400)
Browse files Browse the repository at this point in the history
I would like to propose an improvement of the AbrConfiguration with this
2 new parameter (as optional) and a new version of the SimpleAbrManager
using that parameter for the switch this.switch_(chosenVariant /* ,
clearbuffer, safe margin*/);

Also providing information about the parameter and updated the demo app
with the capabilities to modify it.
Corrected the unit test and added specific unit test for it

---------

Co-authored-by: Álvaro Velad Galván <ladvan91@hotmail.com>
  • Loading branch information
Iragne and avelad committed Aug 15, 2023
1 parent 29aba8e commit b51ee6e
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 9 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Expand Up @@ -44,6 +44,7 @@ Google Inc. <*@google.com>
Itay Kinnrot <Itay.Kinnrot@Kaltura.com>
Jaeseok Lee <devsunb@gmail.com>
Jason Palmer <jason@jason-palmer.com>
Jean-alexandre Iragne <jairagne@gmail.com>
Jesper Haug Karsrud <jesper.karsrud@gmail.com>
Johan Sundström <oyasumi@gmail.com>
Jonas Birmé <jonas.birme@eyevinn.se>
Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTORS
Expand Up @@ -64,6 +64,7 @@ Isaac Ramirez <isaac.ramirez.herrera@gmail.com>
Jacob Trimble <modmaker@google.com>
Jaeseok Lee <devsunb@gmail.com>
Jason Palmer <jason@jason-palmer.com>
Jean-alexandre Iragne <jairagne@gmail.com>
Jeffrey Swan <jeffdswan@gmail.com>
Jesper Haug Karsrud <jesper.karsrud@gmail.com>
Jesse Gunsch <gunsch@google.com>
Expand Down
2 changes: 2 additions & 0 deletions demo/common/message_ids.js
Expand Up @@ -174,6 +174,7 @@ shakaDemo.MessageIds = {
BANDWIDTH_UPGRADE: 'DEMO_BANDWIDTH_UPGRADE',
BUFFER_BEHIND: 'DEMO_BUFFER_BEHIND',
BUFFERING_GOAL: 'DEMO_BUFFERING_GOAL',
CLEAR_BUFFER_SWITCH: 'DEMO_CLEAR_BUFFER_SWITCH',
CLOCK_SYNC_URI: 'DEMO_CLOCK_SYNC_URI',
CMCD_SECTION_HEADER: 'DEMO_CMCD_SECTION_HEADER',
CONNECTION_TIMEOUT: 'DEMO_CONNECTION_TIMEOUT',
Expand Down Expand Up @@ -269,6 +270,7 @@ shakaDemo.MessageIds = {
RESTRICT_TO_ELEMENT_SIZE: 'DEMO_RESTRICT_TO_ELEMENT_SIZE',
RESTRICT_TO_SCREEN_SIZE: 'DEMO_RESTRICT_TO_SCREEN_SIZE',
RESTRICTIONS_SECTION_HEADER: 'DEMO_RESTRICTIONS_SECTION_HEADER',
SAFE_MARGIN_SWITCH: 'DEMO_SAFE_MARGIN_SWITCH',
SAFE_SEEK_OFFSET: 'DEMO_SAFE_SEEK_OFFSET',
SAFE_SKIP_DISTANCE: 'DEMO_SAFE_SKIP_DISTANCE',
SEGMENT_RELATIVE_VTT_TIMING: 'DEMO_SEGMENT_RELATIVE_VTT_TIMING',
Expand Down
7 changes: 6 additions & 1 deletion demo/config.js
Expand Up @@ -295,7 +295,12 @@ shakaDemo.Config = class {
.addBoolInput_(MessageIds.RESTRICT_TO_SCREEN_SIZE,
'abr.restrictToScreenSize')
.addBoolInput_(MessageIds.IGNORE_DEVICE_PIXEL_RATIO,
'abr.ignoreDevicePixelRatio');
'abr.ignoreDevicePixelRatio')
.addBoolInput_(MessageIds.CLEAR_BUFFER_SWITCH,
'abr.clearBufferSwitch')
.addNumberInput_(MessageIds.SAFE_MARGIN_SWITCH,
'abr.safeMarginSwitch',
/* canBeDecimal= */ true);
this.addRetrictionsSection_('abr',
MessageIds.ADAPTATION_RESTRICTIONS_SECTION_HEADER);
}
Expand Down
2 changes: 2 additions & 0 deletions demo/locales/en.json
Expand Up @@ -39,6 +39,7 @@
"DEMO_CLEAR": "No DRM protection",
"DEMO_CLEAR_KEY": "Clear Key DRM",
"DEMO_CLOCK_SYNC_URI": "Clock Sync URI",
"DEMO_CLEAR_BUFFER_SWITCH": "Clear video buffer on abr rendition switch",
"DEMO_CMCD_SECTION_HEADER": "CMCD",
"DEMO_COMPILED_DEBUG": "Compiled (Debug)",
"DEMO_COMPILED_RELEASE": "Compiled (Release)",
Expand Down Expand Up @@ -212,6 +213,7 @@
"DEMO_RESTRICT_TO_ELEMENT_SIZE": "Restrict to element size",
"DEMO_RESTRICT_TO_SCREEN_SIZE": "Restrict to screen size",
"DEMO_RESTRICTIONS_SECTION_HEADER": "Restrictions",
"DEMO_SAFE_MARGIN_SWITCH": "Safe margin on abr switch rendition",
"DEMO_SAFE_SEEK_OFFSET": "Safe Seek Offset",
"DEMO_SAFE_SKIP_DISTANCE": "Safe Skip Distance",
"DEMO_SEGMENT_RELATIVE_VTT_TIMING": "Enable segment-relative VTT Timing",
Expand Down
8 changes: 8 additions & 0 deletions demo/locales/source.json
Expand Up @@ -159,6 +159,10 @@
"description": "The name of a configuration value.",
"message": "Clock Sync URI"
},
"DEMO_CLEAR_BUFFER_SWITCH": {
"description": "Clear video buffer on abr rendition switch.",
"message": "Clear buffer"
},
"DEMO_CMCD_SECTION_HEADER": {
"description": "The header for a section of configuration values.",
"message": "[JARGON:CMCD]"
Expand Down Expand Up @@ -847,6 +851,10 @@
"description": "The header for a section of configuration values.",
"message": "Restrictions"
},
"DEMO_SAFE_MARGIN_SWITCH": {
"description": "Safe margin on abr switch rendition",
"message": "safe margin"
},
"DEMO_SAFE_SEEK_OFFSET": {
"description": "The name of a configuration value.",
"message": "Safe Seek Offset"
Expand Down
17 changes: 16 additions & 1 deletion externs/shaka/player.js
Expand Up @@ -1265,7 +1265,9 @@ shaka.extern.AdsConfiguration;
* advanced: shaka.extern.AdvancedAbrConfiguration,
* restrictToElementSize: boolean,
* restrictToScreenSize: boolean,
* ignoreDevicePixelRatio: boolean
* ignoreDevicePixelRatio: boolean,
* clearBufferSwitch: boolean,
* safeMarginSwitch: number
* }}
*
* @property {boolean} enabled
Expand Down Expand Up @@ -1307,6 +1309,19 @@ shaka.extern.AdsConfiguration;
* If true,device pixel ratio is ignored when restricting the quality to
* media element size or screen size.
* Defaults false.
* @property {boolean} clearBufferSwitch
* If true, the buffer will be cleared during the switch.
* The default automatic behavior is false to have a smoother transition.
* On some device it's better to clear buffer.
* Defaults false.
* @property {number} safeMarginSwitch
* Optional amount of buffer (in seconds) to
* retain when clearing the buffer during the automatic switch.
* Useful for switching variant quickly without causing a buffering event.
* Defaults to 0 if not provided. Ignored if clearBuffer is false.
* Can cause hiccups on some browsers if chosen too small, e.g.
* The amount of two segments is a fair minimum to consider as safeMargin
* value.
* @exportDoc
*/
shaka.extern.AbrConfiguration;
Expand Down
9 changes: 6 additions & 3 deletions lib/abr/simple_abr_manager.js
Expand Up @@ -64,7 +64,8 @@ shaka.abr.SimpleAbrManager = class {
}
const chosenVariant = this.chooseVariant();
if (chosenVariant) {
this.switch_(chosenVariant);
this.switch_(chosenVariant, this.config_.clearBufferSwitch,
this.config_.safeMarginSwitch);
}
}
};
Expand Down Expand Up @@ -106,7 +107,8 @@ shaka.abr.SimpleAbrManager = class {
if (this.config_.restrictToElementSize) {
const chosenVariant = this.chooseVariant();
if (chosenVariant) {
this.switch_(chosenVariant);
this.switch_(chosenVariant, this.config_.clearBufferSwitch,
this.config_.safeMarginSwitch);
}
}
});
Expand Down Expand Up @@ -382,7 +384,8 @@ shaka.abr.SimpleAbrManager = class {
'Calling switch_(), bandwidth=' + currentBandwidthKbps + ' kbps');
// If any of these chosen streams are already chosen, Player will filter
// them out before passing the choices on to StreamingEngine.
this.switch_(chosenVariant);
this.switch_(chosenVariant, this.config_.clearBufferSwitch,
this.config_.safeMarginSwitch);
}
}

Expand Down
2 changes: 2 additions & 0 deletions lib/util/player_configuration.js
Expand Up @@ -288,6 +288,8 @@ shaka.util.PlayerConfiguration = class {
restrictToElementSize: false,
restrictToScreenSize: false,
ignoreDevicePixelRatio: false,
clearBufferSwitch: false,
safeMarginSwitch: 0,
};

const cmcd = {
Expand Down
36 changes: 32 additions & 4 deletions test/abr/simple_abr_manager_unit.js
Expand Up @@ -158,7 +158,7 @@ describe('SimpleAbrManager', () => {
// and variant 5 - for bandwidth = 6e5
const expectedVariant = (bandwidth == 6e5) ? variants[5] : variants[2];

expect(switchCallback).toHaveBeenCalledWith(expectedVariant);
expect(switchCallback).toHaveBeenCalledWith(expectedVariant, false, 0);
});
}

Expand Down Expand Up @@ -205,7 +205,7 @@ describe('SimpleAbrManager', () => {
// Expect variants 4 to be chosen
const expectedVariant = variants[3];

expect(switchCallback).toHaveBeenCalledWith(expectedVariant);
expect(switchCallback).toHaveBeenCalledWith(expectedVariant, false, 0);
});

it('does not call switchCallback() if not enabled', () => {
Expand Down Expand Up @@ -283,7 +283,35 @@ describe('SimpleAbrManager', () => {

// The second parameter is missing to indicate that the buffer should not be
// cleared.
expect(switchCallback).toHaveBeenCalledWith(jasmine.any(Object));
expect(switchCallback).toHaveBeenCalledWith(jasmine.any(Object), false, 0);
});

it('does clear the buffer on upgrade with safemargin to 4', () => {
// Simulate some segments being downloaded at a high rate, to trigger an
// upgrade.
const bandwidth = 5e5;
const bytesPerSecond = sufficientBWMultiplier * bandwidth / 8.0;

// Set the clear buffer to true and the safe margin to 4.
config.clearBufferSwitch = true;
config.safeMarginSwitch = 4;
abrManager.configure(config);

abrManager.setVariants(variants);
abrManager.chooseVariant();

abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

abrManager.enable();

// Make another call to segmentDownloaded(). switchCallback() will be
// called to upgrade.
abrManager.segmentDownloaded(1000, bytesPerSecond);

// The second parameter is missing to indicate that the buffer should not be
// cleared.
expect(switchCallback).toHaveBeenCalledWith(jasmine.any(Object), true, 4);
});

it('does not clear the buffer on downgrade', () => {
Expand All @@ -310,7 +338,7 @@ describe('SimpleAbrManager', () => {

// The second parameter is missing to indicate that the buffer should not be
// cleared.
expect(switchCallback).toHaveBeenCalledWith(jasmine.any(Object));
expect(switchCallback).toHaveBeenCalledWith(jasmine.any(Object), false, 0);
});

it('will respect restrictions', () => {
Expand Down

0 comments on commit b51ee6e

Please sign in to comment.