Merge pull request #1738 from virtuacoplenny/lenny/connection-stats-pub-sub

ref(stats): process stats through one pub/sub

conference.js

@@ -48,6 +48,7 @@ import {
     trackAdded,
     trackRemoved
 } from './react/features/base/tracks';
+import { statsEmitter } from './react/features/connection-indicator';
 import { showDesktopPicker } from  './react/features/desktop-picker';
 import {
     mediaPermissionPromptVisibilityChanged,
@@ -64,8 +65,6 @@ const ConferenceErrors = JitsiMeetJS.errors.conference;
 const TrackEvents = JitsiMeetJS.events.track;
 const TrackErrors = JitsiMeetJS.errors.track;
 
-const ConnectionQualityEvents = JitsiMeetJS.events.connectionQuality;
-
 const eventEmitter = new EventEmitter();
 
 let room;
@@ -1726,16 +1725,7 @@ export default {
             }
         });
 
-        room.on(ConnectionQualityEvents.LOCAL_STATS_UPDATED,
-            (stats) => {
-                APP.UI.updateLocalStats(stats.connectionQuality, stats);
-
-        });
-
-        room.on(ConnectionQualityEvents.REMOTE_STATS_UPDATED,
-            (id, stats) => {
-                APP.UI.updateRemoteStats(id, stats.connectionQuality, stats);
-        });
+        statsEmitter.startListeningForStats(room);
 
         room.addCommandListener(this.commands.defaults.ETHERPAD, ({value}) => {
             APP.UI.initEtherpad(value);

modules/UI/UI.js

@@ -972,25 +972,6 @@ UI.hideStats = function () {
     VideoLayout.hideStats();
 };
 
-/**
- * Update local connection quality statistics.
- * @param {number} percent
- * @param {object} stats
- */
-UI.updateLocalStats = function (percent, stats) {
-    VideoLayout.updateLocalConnectionStats(percent, stats);
-};
-
-/**
- * Update connection quality statistics for remote user.
- * @param {string} id user id
- * @param {number} percent
- * @param {object} stats
- */
-UI.updateRemoteStats = function (id, percent, stats) {
-    VideoLayout.updateConnectionStats(id, percent, stats);
-};
-
 /**
  * Mark video as interrupted or not.
  * @param {boolean} interrupted if video is interrupted

modules/UI/videolayout/LocalVideo.js

@@ -37,7 +37,7 @@ function LocalVideo(VideoLayout, emitter) {
     this.setDisplayName();
 
     this.addAudioLevelIndicator();
-    this.updateConnectionIndicator();
+    this.updateIndicators();
 }
 
 LocalVideo.prototype = Object.create(SmallVideo.prototype);

modules/UI/videolayout/RemoteVideo.js

@@ -48,7 +48,7 @@ function RemoteVideo(user, VideoLayout, emitter) {
     this.hasRemoteVideoMenu = false;
     this._supportsRemoteControl = false;
     this.addRemoteVideoContainer();
-    this.updateConnectionIndicator();
+    this.updateIndicators();
     this.setDisplayName();
     this.bindHoverHandler();
     this.flipX = false;
@@ -632,18 +632,6 @@ RemoteVideo.prototype.addRemoteStreamElement = function (stream) {
     }
 };
 
-RemoteVideo.prototype.updateResolution = function (resolution) {
-    this.updateConnectionIndicator({ resolution });
-};
-
-/**
- * Updates this video framerate indication.
- * @param framerate the value to update
- */
-RemoteVideo.prototype.updateFramerate = function (framerate) {
-    this.updateConnectionIndicator({ framerate });
-};
-
 /**
  * Sets the display name for the given video span id.
  *

modules/UI/videolayout/SmallVideo.js

@@ -84,13 +84,14 @@ function SmallVideo(VideoLayout) {
     this.disableUpdateView = false;
 
     /**
-     * Statistics to display within the connection indicator. With new updates,
-     * only changed values are updated through assignment to a new reference.
+     * The current state of the user's bridge connection. The value should be
+     * a string as enumerated in the library's participantConnectionStatus
+     * constants.
      *
      * @private
-     * @type {object}
+     * @type {string|null}
      */
-    this._cachedConnectionStats = {};
+    this._connectionStatus = null;
 
     /**
      * Whether or not the ConnectionIndicator's popover is hovered. Modifies
@@ -260,18 +261,6 @@ SmallVideo.prototype.bindHoverHandler = function () {
     );
 };
 
-/**
- * Updates the data for the indicator
- * @param id the id of the indicator
- * @param percent the percent for connection quality
- * @param object the data
- */
-SmallVideo.prototype.updateConnectionStats = function (percent, object) {
-    const newStats = Object.assign({}, object, { percent });
-
-    this.updateConnectionIndicator(newStats);
-};
-
 /**
  * Unmounts the ConnectionIndicator component.
 
@@ -289,7 +278,8 @@ SmallVideo.prototype.removeConnectionIndicator = function () {
  * @returns {void}
  */
 SmallVideo.prototype.updateConnectionStatus = function (connectionStatus) {
-    this.updateConnectionIndicator({ connectionStatus });
+    this._connectionStatus = connectionStatus;
+    this.updateIndicators();
 };
 
 /**
@@ -741,21 +731,6 @@ SmallVideo.prototype.initBrowserSpecificProperties = function() {
     }
 };
 
-/**
- * Creates or updates the connection indicator. Updates the previously known
- * statistics about the participant's connection.
- *
- * @param {Object} newStats - New statistics to merge with previously known
- * statistics about the participant's connection.
- * @returns {void}
- */
-SmallVideo.prototype.updateConnectionIndicator = function (newStats = {}) {
-    this._cachedConnectionStats
-        = Object.assign({}, this._cachedConnectionStats, newStats);
-
-    this.updateIndicators();
-};
-
 /**
  * Updates the React element responsible for showing connection status, dominant
  * speaker, and raised hand icons. Uses instance variables to get the necessary
@@ -775,11 +750,12 @@ SmallVideo.prototype.updateIndicators = function () {
         <div>
             { this._showConnectionIndicator
                 ? <ConnectionIndicator
+                    connectionStatus = { this._connectionStatus }
                     iconSize = { iconSize }
                     isLocalVideo = { this.isLocal }
                     onHover = { this._onPopoverHover }
                     showMoreLink = { this.isLocal }
-                    stats = { this._cachedConnectionStats } />
+                    userID = { this.id } />
                 : null }
             { this._showRaisedHand
                 ? <RaisedHandIndicator iconSize = { iconSize } /> : null }

modules/UI/videolayout/VideoLayout.js

@@ -211,6 +211,11 @@ var VideoLayout = {
         if (largeVideo && !largeVideo.id) {
             this.updateLargeVideo(APP.conference.getMyUserId(), true);
         }
+
+        // FIXME: replace this call with a generic update call once SmallVideo
+        // only contains a ReactElement. Then remove this call once the
+        // Filmstrip is fully in React.
+        localVideoThumbnail.updateIndicators();
     },
 
     /**
@@ -766,60 +771,6 @@ var VideoLayout = {
         }
     },
 
-    /**
-     * Updates local stats
-     * @param percent
-     * @param object
-     */
-    updateLocalConnectionStats (percent, object) {
-        const { framerate, resolution } = object;
-
-        // FIXME overwrites 'lib-jitsi-meet' internal object
-        // Why library internal objects are passed as event's args ?
-        object.resolution = resolution[APP.conference.getMyUserId()];
-        object.framerate = framerate[APP.conference.getMyUserId()];
-        localVideoThumbnail.updateConnectionStats(percent, object);
-
-        Object.keys(resolution).forEach(function (id) {
-            if (APP.conference.isLocalId(id)) {
-                return;
-            }
-
-            let resolutionValue = resolution[id];
-            let remoteVideo = remoteVideos[id];
-
-            if (resolutionValue && remoteVideo) {
-                remoteVideo.updateResolution(resolutionValue);
-            }
-        });
-
-        Object.keys(framerate).forEach(function (id) {
-            if (APP.conference.isLocalId(id)) {
-                return;
-            }
-
-            const framerateValue = framerate[id];
-            const remoteVideo = remoteVideos[id];
-
-            if (framerateValue && remoteVideo) {
-                remoteVideo.updateFramerate(framerateValue);
-            }
-        });
-    },
-
-    /**
-     * Updates remote stats.
-     * @param id the id associated with the stats
-     * @param percent the connection quality percent
-     * @param object the stats data
-     */
-    updateConnectionStats (id, percent, object) {
-        let remoteVideo = remoteVideos[id];
-        if (remoteVideo) {
-            remoteVideo.updateConnectionStats(percent, object);
-        }
-    },
-
     /**
      * Hides the connection indicator
      * @param id

react/features/connection-indicator/components/ConnectionIndicator.js

@@ -5,6 +5,8 @@ import JitsiPopover from '../../../../modules/UI/util/JitsiPopover';
 import { JitsiParticipantConnectionStatus } from '../../base/lib-jitsi-meet';
 import { ConnectionStatsTable } from '../../connection-stats';
 
+import statsEmitter from '../statsEmitter';
+
 declare var $: Object;
 declare var interfaceConfig: Object;
 
@@ -57,6 +59,14 @@ class ConnectionIndicator extends Component {
      * @static
      */
     static propTypes = {
+        /**
+         * The current condition of the user's connection, matching one of the
+         * enumerated values in the library.
+         *
+         * @type {JitsiParticipantConnectionStatus}
+         */
+        connectionStatus: React.PropTypes.string,
+
         /**
          * Whether or not the displays stats are for local video.
          */
@@ -74,25 +84,15 @@ class ConnectionIndicator extends Component {
         showMoreLink: React.PropTypes.bool,
 
         /**
-         * An object that contains statistics related to connection quality.
-         *
-         * {
-         *     bandwidth: Object,
-         *     bitrate: Object,
-         *     connectionStatus: String,
-         *     framerate: Object,
-         *     packetLoss: Object,
-         *     percent: Number,
-         *     resolution: Object,
-         *     transport:  Array
-         * }
+         * Invoked to obtain translated strings.
          */
-        stats: React.PropTypes.object,
+        t: React.PropTypes.func,
 
         /**
-         * Invoked to obtain translated strings.
+         * The user ID associated with the displayed connection indication and
+         * stats.
          */
-        t: React.PropTypes.func
+        userID: React.PropTypes.string
     };
 
     /**
@@ -121,10 +121,19 @@ class ConnectionIndicator extends Component {
              *
              * @type {boolean}
              */
-            showMoreStats: false
+            showMoreStats: false,
+
+            /**
+             * Cache of the stats received from subscribing to stats emitting.
+             * The keys should be the name of the stat. With each stat update,
+             * updates stats are mixed in with cached stats and a new stats
+             * object is set in state.
+             */
+            stats: {}
         };
 
         // Bind event handlers so they are only bound once for every instance.
+        this._onStatsUpdated = this._onStatsUpdated.bind(this);
         this._onToggleShowMore = this._onToggleShowMore.bind(this);
         this._setRootElement = this._setRootElement.bind(this);
     }
@@ -136,6 +145,9 @@ class ConnectionIndicator extends Component {
      * returns {void}
      */
     componentDidMount() {
+        statsEmitter.subscribeToClientStats(
+            this.props.userID, this._onStatsUpdated);
+
         this.popover = new JitsiPopover($(this._rootElement), {
             content: this._renderStatisticsTable(),
             skin: 'black',
@@ -153,7 +165,14 @@ class ConnectionIndicator extends Component {
      * @inheritdoc
      * returns {void}
      */
-    componentDidUpdate() {
+    componentDidUpdate(prevProps) {
+        if (prevProps.userID !== this.props.userID) {
+            statsEmitter.unsubscribeToClientStats(
+                prevProps.userID, this._onStatsUpdated);
+            statsEmitter.subscribeToClientStats(
+                this.props.userID, this._onStatsUpdated);
+        }
+
         this.popover.updateContent(this._renderStatisticsTable());
     }
 
@@ -164,6 +183,9 @@ class ConnectionIndicator extends Component {
      * returns {void}
      */
     componentWillUnmount() {
+        statsEmitter.unsubscribeToClientStats(
+            this.props.userID, this._onStatsUpdated);
+
         this.popover.forceHide();
         this.popover.remove();
     }
@@ -186,6 +208,30 @@ class ConnectionIndicator extends Component {
         );
     }
 
+    /**
+     * Callback invoked when new connection stats associated with the passed in
+     * user ID are available. Will update the component's display of current
+     * statistics.
+     *
+     * @param {Object} stats - Connection stats from the library.
+     * @private
+     * @returns {void}
+     */
+    _onStatsUpdated(stats = {}) {
+        const { connectionQuality } = stats;
+        const newPercentageState = typeof connectionQuality === 'undefined'
+            ? {} : { percent: connectionQuality };
+        const newStats = Object.assign(
+            {},
+            this.state.stats,
+            stats,
+            newPercentageState);
+
+        this.setState({
+            stats: newStats
+        });
+    }
+
     /**
      * Callback to invoke when the show more link in the popover content is
      * clicked. Sets the state which will determine if the popover should show
@@ -204,7 +250,7 @@ class ConnectionIndicator extends Component {
      * @returns {ReactElement}
      */
     _renderIcon() {
-        switch (this.props.stats.connectionStatus) {
+        switch (this.props.connectionStatus) {
         case JitsiParticipantConnectionStatus.INTERRUPTED:
             return (
                 <span className = 'connection_lost'>
@@ -218,7 +264,7 @@ class ConnectionIndicator extends Component {
                 </span>
             );
         default: {
-            const { percent } = this.props.stats;
+            const { percent } = this.state.stats;
             const width = QUALITY_TO_WIDTH.find(x => percent >= x.percent);
             const iconWidth = width && width.width
                 ? { width: width && width.width } : {};
@@ -253,7 +299,7 @@ class ConnectionIndicator extends Component {
             packetLoss,
             resolution,
             transport
-        } = this.props.stats;
+        } = this.state.stats;
 
         return (
             <ConnectionStatsTable

react/features/connection-indicator/index.js

@@ -1 +1,3 @@
 export * from './components';
+
+export { default as statsEmitter } from './statsEmitter';

react/features/connection-indicator/statsEmitter.js

@@ -0,0 +1,153 @@
+import _ from 'lodash';
+
+import JitsiMeetJS from '../base/lib-jitsi-meet';
+
+declare var APP: Object;
+
+/**
+ * Contains all the callbacks to be notified when stats are updated.
+ *
+ * {
+ *     userId: Function[]
+ * }
+ */
+const subscribers = {};
+
+/**
+ * A singleton that acts as a pub/sub service for connection stat updates.
+ */
+const statsEmitter = {
+    /**
+     * Have {@code statsEmitter} subscribe to stat updates from a given
+     * conference.
+     *
+     * @param {JitsiConference} conference - The conference for which
+     * {@code statsEmitter} should subscribe for stat updates.
+     * @returns {void}
+     */
+    startListeningForStats(conference) {
+        const { connectionQuality } = JitsiMeetJS.events;
+
+        conference.on(connectionQuality.LOCAL_STATS_UPDATED,
+            stats => this._onStatsUpdated(conference.myUserId(), stats));
+
+        conference.on(connectionQuality.REMOTE_STATS_UPDATED,
+            (id, stats) => this._emitStatsUpdate(id, stats));
+    },
+
+    /**
+     * Add a subscriber to be notified when stats are updated for a specified
+     * user id.
+     *
+     * @param {string} id - The user id whose stats updates are of interest.
+     * @param {Function} callback - The function to invoke when stats for the
+     * user have been updated.
+     * @returns {void}
+     */
+    subscribeToClientStats(id, callback) {
+        if (!id) {
+            return;
+        }
+
+        if (!subscribers[id]) {
+            subscribers[id] = [];
+        }
+
+        subscribers[id].push(callback);
+    },
+
+    /**
+     * Remove a subscriber that is listening for stats updates for a specified
+     * user id.
+     *
+     * @param {string} id - The user id whose stats updates are no longer of
+     * interest.
+     * @param {Function} callback - The function that is currently subscribed to
+     * stat updates for the specified user id.
+     * @returns {void}
+     */
+    unsubscribeToClientStats(id, callback) {
+        if (!subscribers[id]) {
+            return;
+        }
+
+        const filteredSubscribers = subscribers[id].filter(
+            subscriber => subscriber !== callback);
+
+        if (filteredSubscribers.length) {
+            subscribers[id] = filteredSubscribers;
+        } else {
+            delete subscribers[id];
+        }
+    },
+
+    /**
+     * Emit a stat update to all those listening for a specific user's
+     * connection stats.
+     *
+     * @param {string} id - The user id the stats are associated with.
+     * @param {Object} stats - New connection stats for the user.
+     * @returns {void}
+     */
+    _emitStatsUpdate(id, stats = {}) {
+        const callbacks = subscribers[id] || [];
+
+        callbacks.forEach(callback => {
+            callback(stats);
+        });
+    },
+
+    /**
+     * Emit a stat update to all those listening for local stat updates. Will
+     * also update listeners of remote user stats of changes related to their
+     * stats.
+     *
+     * @param {string} currentUserId - The user id for the local user.
+     * @param {Object} stats - Connection stats for the local user as provided
+     * by the library.
+     * @returns {void}
+     */
+    _onStatsUpdated(currentUserId, stats) {
+        const allUserFramerates = stats.framerate;
+        const allUserResolutions = stats.resolution;
+
+        const currentUserFramerate = allUserFramerates[currentUserId];
+        const currentUserResolution = allUserResolutions[currentUserId];
+
+        // FIXME resolution and framerate are hashes keyed off of user ids with
+        // stat values. Receivers of stats expect resolution and framerate to
+        // be primatives, not hashes, so overwrites the 'lib-jitsi-meet' stats
+        // objects.
+        stats.framerate = currentUserFramerate;
+        stats.resolution = currentUserResolution;
+
+        this._emitStatsUpdate(currentUserId, stats);
+
+        // Get all the unique user ids from the framerate and resolution stats
+        // and update remote user stats as needed.
+        const framerateUserIds = Object.keys(allUserFramerates);
+        const resolutionUserIds = Object.keys(allUserResolutions);
+
+        _.union(framerateUserIds, resolutionUserIds)
+            .filter(id => id !== currentUserId)
+            .forEach(id => {
+                const remoteUserStats = {};
+
+                const framerate = allUserFramerates[id];
+
+                if (framerate) {
+                    remoteUserStats.framerate = framerate;
+                }
+
+                const resolution = allUserResolutions[id];
+
+                if (resolution) {
+                    remoteUserStats.resolution = resolution;
+                }
+
+                this._emitStatsUpdate(id, remoteUserStats);
+            });
+    }
+};
+
+export default statsEmitter;