feat: refactor youbora implementation #6
Conversation
* Ignore vscode config
…t-js-youbora into youbora-analytics # Conflicts: # dist/playkit-js-youbora.js # dist/playkit-js-youbora.js.map # src/youbora.js # test/src/youbora.spec.js
src/youbora.js
Outdated
| super(name, player, config); | ||
| this._youbora = new YouboraPlugin(this.player, this.config); | ||
| this.config.extraParams = this._getCustomParams(); |
There was a problem hiding this comment.
This transformation is redundant, the customer should configure us with extraParams
There was a problem hiding this comment.
this was only relevant for V2
src/youbora-adapter.js
Outdated
|
|
||
| /** Version of the plugin. ie: 5.1.0-name */ | ||
| this.pluginVersion = '5.3.0-' + pkg.version + '-kalturaplaykit-js'; | ||
| this.pluginVersion = '5.3.0-' + pkg.version + '-' + PLAYER_NAME; |
There was a problem hiding this comment.
Take the version from the plugin itself: http://developer.nicepeopleatwork.com/apidocs/js/$YB.html
And remove the comment above with "5.1.0-name"
There was a problem hiding this comment.
@yairans 5.3.0 is still the version string of Youbora plugin(and the actual plugin is 5.3.10 by the way), need to get it from the Youbora plugin YB.version.
This way we don't have to update it manually each time we upgrade.
Also, update the test for it that uses the hard coded string as well.
src/youbora-adapter.js
Outdated
| $YB.plugins.KalturaV3.prototype.getBitrate = function () { | ||
| let activeVideo = this.player.getActiveTracks().video; | ||
| if (activeVideo && activeVideo.bandwidth) { | ||
| return activeVideo.bandwidth / 1024; |
There was a problem hiding this comment.
Need to verify what units are expected by Youbora, e.g. Kbps or bps
src/youbora-adapter.js
Outdated
| }); | ||
| }; | ||
|
|
||
| $YB.plugins.KalturaV3.prototype.setMetadata = function () { | ||
| this.setOptions({ |
There was a problem hiding this comment.
Need to check if this should be merged with possible user supplied data
src/youbora.js
Outdated
|
|
||
| /** | ||
| * The default configuration of the plugin. | ||
| * @type {Object} | ||
| * @static | ||
| */ | ||
| static defaultConfig: Object = { | ||
| accountCode: 'powerdev' | ||
| haltOnError: false, | ||
| transactionCode: 'Free' |
There was a problem hiding this comment.
Is this field required by default?
There was a problem hiding this comment.
no. actually i don't remember where i take it from :)
deleted.
|
@yairans upgrade to latest 5.4.6 version of the plugin as part of this PR. |
src/youbora-adapter.js
Outdated
| $YB.plugins.KalturaV3 = function (player, options) { | ||
| try { | ||
| /** Name and platform of the plugin.*/ | ||
| this.pluginName = PLAYER_NAME; |
There was a problem hiding this comment.
@yairans what is this used for?
There was a problem hiding this comment.
is part of the payload. player property
src/youbora.js
Outdated
| */ | ||
| constructor(name: string, player: Player, config: Object) { | ||
| super(name, player, config); | ||
| this.config.extraParams = this._getCustomParams(); |
There was a problem hiding this comment.
@yairans I think we don't need this transform, we should pass the config object for Youbora as a passthrough
…into youbora-analytics # Conflicts: # .eslintignore # .gitignore # .travis.yml # README.md # karma.conf.js # package.json # src/youbora.js # test/src/youbora.spec.js # webpack.config.js # yarn.lock
…t-js-youbora into youbora-analytics # Conflicts: # .gitignore
No description provided.