address code review

modules/detection/ActiveDeviceDetector.js

@@ -1,4 +1,5 @@
 import { getLogger } from 'jitsi-meet-logger';
+
 import * as JitsiTrackEvents from '../../JitsiTrackEvents';
 import RTC from '../RTC/RTC';
 import Statistics from '../statistics/statistics';
@@ -74,8 +75,10 @@ export default function getActiveAudioDevice() {
                 // Cancel the detection in case no devices was found with audioLevel > 0 in the set timeout.
                 setTimeout(() => {
                     stopActiveDevices(availableDevices);
-                    resolve({ deviceId: '',
-                        deviceLabel: '' });
+                    resolve({
+                        deviceId: '',
+                        deviceLabel: '' }
+                    );
                 }, DETECTION_TIMEOUT);
 
             });

modules/detection/NoAudioSignalDetection.js

@@ -1,5 +1,7 @@
 import EventEmitter from 'events';
+
 import * as JitsiConferenceEvents from '../../JitsiConferenceEvents';
+
 import * as DetectionEvents from './DetectionEvents';
 
 // We wait a certain time interval for constant silence input from the current device to account for
@@ -16,6 +18,8 @@ const SILENCE_PERIOD_MS = 4000;
  */
 export default class NoAudioSignalDetection extends EventEmitter {
     /**
+     * Creates new NoAudioSignalDetection.
+     *
      * @param conference the JitsiConference instance that created us.
      * @constructor
      */
@@ -42,32 +46,19 @@ export default class NoAudioSignalDetection extends EventEmitter {
     /**
      * Generated event triggered by a change in the current conference audio input state.
      *
-     * @param {*} audioLevel
+     * @param {*} audioLevel - The audio level of the ssrc.
      * @fires DetectionEvents.AUDIO_INPUT_STATE_CHANGE
      */
     _handleAudioInputStateChange(audioLevel) {
         // Current audio input state of the active local track in the conference, true for audio input false for no
         // audio input.
         const status = audioLevel !== 0;
-        let shouldTrigger;
 
-        // If we this is the first audio event picked up or the current status is different from the previous trigger
+        // If this is the first audio event picked up or the current status is different from the previous trigger
         // the event.
-        if (this._hasAudioInput === null) {
-            shouldTrigger = true;
-        } else if (this._hasAudioInput !== status) {
-            shouldTrigger = true;
-        }
-
-        if (shouldTrigger) {
+        if (this._hasAudioInput === null || this._hasAudioInput !== status) {
             this._hasAudioInput = status;
 
-            /**
-             * Event fired when the audio input state of the conference changes, true for audio input false otherwise.
-             *
-             * @event DetectionEvents.AUDIO_INPUT_STATE_CHANGE
-             * @type {boolean}
-             */
             this.emit(DetectionEvents.AUDIO_INPUT_STATE_CHANGE, this._hasAudioInput);
         }
     }
@@ -87,12 +78,6 @@ export default class NoAudioSignalDetection extends EventEmitter {
             this._timeoutTrigger = setTimeout(() => {
                 this._eventFired = true;
 
-                /**
-                 * Event fired when there is no audio input for a predefined period of time.
-                 *
-                 * @event DetectionEvents.AUDIO_INPUT_STATE_CHANGE
-                 * @type {void}
-                 */
                 this.emit(DetectionEvents.NO_AUDIO_INPUT);
             }, SILENCE_PERIOD_MS);
         } else if (audioLevel !== 0 && this._timeoutTrigger) {
@@ -118,16 +103,10 @@ export default class NoAudioSignalDetection extends EventEmitter {
         // Get currently active local tracks from the TraceablePeerConnection
         const localSSRCs = tpc.localSSRCs.get(this._audioTrack.rtcId);
 
-        // Check that currently selected audio stream has ssrc in the TraceablePeerConnection
-        if (!localSSRCs) {
-            return;
-        }
 
         // Only target the current active track in the tpc. For some reason audio levels for previous
         // devices are also picked up from the PeerConnection so we filter them out.
-        const isCurrentTrack = localSSRCs.ssrcs.includes(ssrc);
-
-        if (!isCurrentTrack) {
+        if (!localSSRCs || !localSSRCs.ssrcs.includes(ssrc)) {
             return;
         }
 

modules/detection/TrackVADEmitter.js

@@ -1,7 +1,8 @@
 import EventEmitter from 'events';
-import { VAD_SCORE_PUBLISHED } from './DetectionEvents';
+
 import RTC from '../RTC/RTC';
 
+import { VAD_SCORE_PUBLISHED } from './DetectionEvents';
 
 /**
  * Connects an audio JitsiLocalTrack to a vadProcessor using WebAudio ScriptProcessorNode.
@@ -101,7 +102,11 @@ export default class TrackVADEmitter extends EventEmitter {
     _initializeAudioContext() {
         this._audioSource = this._audioContext.createMediaStreamSource(this._localTrack.stream);
 
-        // TODO AudioProcessingNode is deprecated check and replace with alternative.
+        // TODO AudioProcessingNode is deprecated in the web audio specifications and the recommended replacement
+        // is audio worklet, however at the point of implementation AudioProcessingNode was still de de facto way
+        // of achieving this functionality and supported in all major browsers as opposed to audio worklet which
+        // was only available in Chrome. This todo is just a reminder that we should replace AudioProcessingNode
+        // with audio worklet when it's mature enough and has more browser support.
         // We don't need stereo for determining the VAD score so we create a single channel processing node.
         this._audioProcessingNode = this._audioContext.createScriptProcessor(this._procNodeSampleRate, 1, 1);
     }
@@ -129,15 +134,6 @@ export default class TrackVADEmitter extends EventEmitter {
             const pcmSample = completeInData.slice(i, i + this._vadSampleSize);
             const vadScore = this._vadProcessor.calculateAudioFrameVAD(pcmSample);
 
-            /**
-             * VAD score publish event
-             *
-             * @event VAD_SCORE_PUBLISHED
-             * @type {Object}
-             * @property {Date}   timestamp - Exact time at which processed PCM sample was generated.
-             * @property {number} score - VAD score on a scale from 0 to 1 (i.e. 0.7)
-             * @property {string} deviceId - Device id of the associated track.
-             */
             this.emit(VAD_SCORE_PUBLISHED, {
                 timestamp: sampleTimestamp,
                 score: vadScore,

modules/detection/VADReportingService.js

@@ -1,5 +1,6 @@
 import EventEmitter from 'events';
 import { getLogger } from 'jitsi-meet-logger';
+
 import * as DetectionEvents from './DetectionEvents';
 import TrackVADEmitter from './TrackVADEmitter';
 

modules/detection/VADTalkMutedDetection.js

@@ -1,6 +1,8 @@
 import { EventEmitter } from 'events';
-import * as JitsiConferenceEvents from '../../JitsiConferenceEvents';
 import { getLogger } from 'jitsi-meet-logger';
+
+import * as JitsiConferenceEvents from '../../JitsiConferenceEvents';
+
 import { VAD_SCORE_PUBLISHED, VAD_TALK_WHILE_MUTED } from './DetectionEvents';
 import TrackVADEmitter from './TrackVADEmitter';
 
@@ -79,7 +81,9 @@ export default class VADTalkMutedDetection extends EventEmitter {
          */
         this._vadInitTracker = null;
 
-
+        /**
+         * Listens for {@link TrackVADEmitter} events and processes them.
+         */
         this._processVADScore = this._processVADScore.bind(this);
 
         /**
@@ -88,30 +92,6 @@ export default class VADTalkMutedDetection extends EventEmitter {
         conference.on(JitsiConferenceEvents.TRACK_ADDED, this._trackAdded.bind(this));
         conference.on(JitsiConferenceEvents.TRACK_REMOVED, this._trackRemoved.bind(this));
         conference.on(JitsiConferenceEvents.TRACK_MUTE_CHANGED, this._trackMuteChanged.bind(this));
-
-        // TODO do we need to handle the case where tracks are removed, make sure this cleans up properly so
-        // we don't have any leeks i.e. stale JitsiLocalTracks
-    }
-
-    /**
-     * Determine if the current score is high enough that we should start the final score processing, and make sure
-     * there isn't already a process operation ongoing.
-     *
-     * @param {number} score - PCM sample VAD score.
-     * @return {boolean}
-     */
-    _shouldStartVADCompute(vadScore) {
-        return vadScore > VAD_VOICE_LEVEL && !this._processing;
-    }
-
-    /**
-     * Determine if the computed score over the configured timestamp should trigger an event.
-     *
-     * @param {number} computedScore - Computed VAD score.
-     * @returns {boolean} - Should or shouldn't trigger.
-     */
-    _shouldTriggerNotification(computedScore) {
-        return computedScore > VAD_AVG_THRESHOLD;
     }
 
     /**
@@ -139,15 +119,7 @@ export default class VADTalkMutedDetection extends EventEmitter {
      * @returns {number} - Score average.
      */
     _calculateAverage(scoreArray) {
-        let avg = 0;
-
-        if (scoreArray.length) {
-            const sum = scoreArray.reduce((a, b) => a + b);
-
-            avg = sum / scoreArray.length;
-        }
-
-        return avg;
+        return scoreArray.length > 0 ? scoreArray.reduce((a, b) => a + b) / scoreArray.length : 0;
     }
 
     /**
@@ -158,13 +130,7 @@ export default class VADTalkMutedDetection extends EventEmitter {
     _calculateVADScore() {
         const score = this._calculateAverage(this._scoreArray);
 
-        if (this._shouldTriggerNotification(score)) {
-            /**
-             * User is talking while the mic is muted, generate event.
-             *
-             * @event VAD_TALK_WHILE_MUTED.
-             * @type {Object}
-             */
+        if (score > VAD_AVG_THRESHOLD) {
             this.emit(VAD_TALK_WHILE_MUTED, {});
 
             // Event was fired. Stop event emitter and remove listeners so no residue events kick off after this point
@@ -188,7 +154,7 @@ export default class VADTalkMutedDetection extends EventEmitter {
     _processVADScore(vadScore) {
         // Because we remove all listeners on the vadEmitter once the main event is triggered,
         // there is no need to check for rogue events.
-        if (this._shouldStartVADCompute(vadScore.score)) {
+        if (vadScore.score > VAD_VOICE_LEVEL && !this._processing) {
             this._processing = true;
 
             // Start gathering VAD scores for the configured period of time.