Conversation
millerx
requested review from
hjon,
maxwellmooney13,
patrickfeeney03 and
zservies
as code owners
There was a problem hiding this comment.
Jon is going to help me with the package-lock.
There was a problem hiding this comment.
I went ahead and just fixed it myself because I think that might be why the GitHub Action is failing. Basically it pointed a bunch of things to nexus/jfrog, probably because your npm is setup that way. For our open source stuff, your npm registry has to be setup for the default https://registry.npmjs.org/.
tl;dr - I fixed the lockfile, it was pointing to nexus/jfrog for some dependencies probably because you have a .npmrc under your user configured that way for Bitbucket stuff.
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
|
|
||
| ### Added | ||
| * [STREAM-1056](https://inindca.atlassian.net/browse/STREAM-1056) Added documentation for live screen monitoring functionality | ||
| * [STREAM-151](https://inindca.atlassian.net/browse/STREAM-151) - Send media statistics to the server. Aggregates stats, calculates a MOS score and pushes at regular intervals. |
There was a problem hiding this comment.
Can you move this up to under Unreleased? Looks like it got added to v13.0.0 when you rebased.
| return; | ||
| } | ||
|
|
||
| if (stats.tracks.length === 0 || stats.remoteTracks.length === 0) { |
There was a problem hiding this comment.
More of a question to the team - is there any value in adding some logging before these early returns?
| } | ||
|
|
||
| private onSessionStarted(session: IExtendedMediaSession) { | ||
| if (session == this.session) { |
|
|
||
| const roundTripTimeSum = totalRtt - this.baselineRtt; | ||
| const roundTripTimeMeasurements = totalRttMeasurements - this.baselineRttMeasurements; | ||
| const averageRoundTripTime = roundTripTimeSum / roundTripTimeMeasurements; |
There was a problem hiding this comment.
Should there be a guard around roundTripeTimeMeasurements so we don't end up dividing by 0? I'm not sure if that's a valid scenario.
| private stopGatheringStats() { | ||
| if (this.statsGatherer) { | ||
| this.statsGatherer.off('stats', this.boundStatsHandler); | ||
| this.statsGatherer = null; |
There was a problem hiding this comment.
Because statsGatherer?: StatsGatherer is optional, it can only be of type StatsGatherer or undefined. This probably doesn't throw an error for us right now because we have strictNullChecks: false in our typescript config, but this should technically be undefined instead of null.
| } | ||
|
|
||
| private startGatheringStats() { | ||
| this.statsGatherer = new StatsGatherer(this.session.peerConnection) |
There was a problem hiding this comment.
We should check for an existing stats gatherer before creating a new one so we don't end up with any leaks.
if (this.statsGatherer) {
this.logger.warn('Cannot create stats gatherer, one already exists');
return;
}
Or we should call this.stopStatsGatherer() to cleanup any potential existing gatherers before creating another.
2 participants
MERGE CHECKLIST
[STREAM-<ticket number>] - <description of changes>or[<issue number>] - <description of changes>.nexton npm.