refactor / address code review

This commit is contained in:
Andrei Gavrilescu 2019-11-22 16:02:59 +02:00 committed by Hristo Terezov
parent 55f35933e8
commit 191da551e3
8 changed files with 66 additions and 221 deletions

View File

@ -1,143 +0,0 @@
// @flow
import EventEmitter from 'events';
import { ACTIVE_DEVICE_DETECTED } from './Events';
import logger from '../../logger';
import JitsiMeetJS from '../../../lib-jitsi-meet';
const JitsiTrackEvents = JitsiMeetJS.events.track;
// If after 3000 ms the detector did not find any active devices consider that there aren't any usable ones available
// i.e. audioLevel > 0.008
const DETECTION_TIMEOUT = 3000;
/**
* Detect active input devices based on their audio levels, currently this is very simplistic. It works by simply
* checking all monitored devices for TRACK_AUDIO_LEVEL_CHANGED if a device has a audio level > 0.008 ( 0.008 is
* no input from the perspective of a JitsiLocalTrack ), at which point it triggers a ACTIVE_DEVICE_DETECTED event.
* If there are no devices that meet that criteria for DETECTION_TIMEOUT an event with empty deviceLabel parameter
* will be triggered,
* signaling that no active device was detected.
* TODO Potentially improve the active device detection using rnnoise VAD scoring.
*/
export class ActiveDeviceDetector extends EventEmitter {
/**
* Currently monitored devices.
*/
_availableDevices: Array<Object>;
/**
* State flag, check if the instance was destroyed.
*/
_destroyed: boolean = false;
/**
* Create active device detector.
*
* @param {Array<MediaDeviceInfo>} micDeviceList - Device list that is monitored inside the service.
*
* @returns {ActiveDeviceDetector}
*/
static async create(micDeviceList: Array<MediaDeviceInfo>) {
const availableDevices = [];
try {
for (const micDevice of micDeviceList) {
const localTrack = await JitsiMeetJS.createLocalTracks({
devices: [ 'audio' ],
micDeviceId: micDevice.deviceId
});
// We provide a specific deviceId thus we expect a single JitsiLocalTrack to be returned.
availableDevices.push(localTrack[0]);
}
return new ActiveDeviceDetector(availableDevices);
} catch (error) {
logger.error('Cleaning up remaining JitsiLocalTrack, due to ActiveDeviceDetector create fail!');
for (const device of availableDevices) {
device.stopStream();
}
throw error;
}
}
/**
* Constructor.
*
* @param {Array<Object>} availableDevices - Device list that is monitored inside the service.
*/
constructor(availableDevices: Array<Object>) {
super();
this._availableDevices = availableDevices;
// Setup event handlers for monitored devices.
for (const device of this._availableDevices) {
device.on(JitsiTrackEvents.TRACK_AUDIO_LEVEL_CHANGED, audioLevel => {
this._handleAudioLevelEvent(device, audioLevel);
});
}
// Cancel the detection in case no devices was found with audioLevel > 0 in te set timeout.
setTimeout(this._handleDetectionTimeout.bind(this), DETECTION_TIMEOUT);
}
/**
* Handle what happens if no device publishes a score in the defined time frame, i.e. Emit an event with empty
* deviceLabel.
*
* @returns {void}
*/
_handleDetectionTimeout() {
if (!this._destroyed) {
this.emit(ACTIVE_DEVICE_DETECTED, { deviceLabel: '',
audioLevel: 0 });
this.destroy();
}
}
/**
* Handles audio level event generated by JitsiLocalTracks.
*
* @param {Object} device - Label of the emitting track.
* @param {number} audioLevel - Audio level generated by device.
*
* @returns {void}
*/
_handleAudioLevelEvent(device, audioLevel) {
if (!this._destroyed) {
// This is a very naive approach but works is most, a more accurate approach would ne to use rnnoise
// in order to limit the number of false positives.
// The 0.008 constant is due to how LocalStatsCollector from lib-jitsi-meet publishes audio-levels, in this
// case 0.008 denotes no input.
// TODO potentially refactor lib-jitsi-meet to expose this constant as a function. i.e. getSilenceLevel.
if (audioLevel > 0.008) {
this.emit(ACTIVE_DEVICE_DETECTED, { deviceId: device.deviceId,
deviceLabel: device.track.label,
audioLevel });
this.destroy();
}
}
}
/**
* Destroy the ActiveDeviceDetector, clean up the currently monitored devices associated JitsiLocalTracks.
*
* @returns {void}.
*/
destroy() {
if (this._destroyed) {
return;
}
for (const device of this._availableDevices) {
device.removeAllListeners();
device.stopStream();
}
this._destroyed = true;
}
}

View File

@ -1,8 +0,0 @@
// Event triggered when the ActiveDeviceDetector finds an audio device that has audio input.
// Note it does not check if the input is valid or not it simply checks for intensity > 0.008.
// Event structure:
// { deviceId: string,
// deviceLabel: string,
// audioLevel: number }
// TO DO. Potentially use rnnoise service to get a more accurate reading.
export const ACTIVE_DEVICE_DETECTED = 'active_device_detected';

View File

@ -1,4 +1,2 @@
export * from './device-detect/ActiveDeviceDetector';
export * from './device-detect/Events';
export * from './vad-reporter/Events';
export * from './vad-reporter/VADReportingService';

View File

@ -141,7 +141,7 @@ ReducerRegistry.register('features/base/tracks', (state = [], action) => {
ReducerRegistry.register('features/base/no-src-data', (state = {}, action) => {
switch (action.type) {
case SET_NO_SRC_DATA_NOTI_UID:
return set(state, 'noSrcDataNotiUid', action.uid);
return set(state, 'noSrcDataNotificationUid', action.uid);
default:
return state;

View File

@ -8,4 +8,4 @@
* uid: ?number
* }
*/
export const SET_NO_AUDIO_SIGNAL_NOTI_UID = 'SET_NO_AUDIO_SIGNAL_NOTI_UID';
export const SET_NO_AUDIO_SIGNAL_NOTIFICATION_UID = 'SET_NO_AUDIO_SIGNAL_NOTIFICATION_UID';

View File

@ -1,6 +1,6 @@
// @flow
import { SET_NO_AUDIO_SIGNAL_NOTI_UID } from './actionTypes';
import { SET_NO_AUDIO_SIGNAL_NOTIFICATION_UID } from './actionTypes';
/**
* Sets UID of the the pending notification to use it when hiding
@ -9,13 +9,13 @@ import { SET_NO_AUDIO_SIGNAL_NOTI_UID } from './actionTypes';
*
* @param {?number} uid - The UID of the notification.
* @returns {{
* type: SET_NO_AUDIO_SIGNAL_NOTI_UID,
* type: SET_NO_AUDIO_SIGNAL_NOTIFICATION_UID,
* uid: number
* }}
*/
export function setNoAudioSignalNotificationUid(uid: ?number) {
return {
type: SET_NO_AUDIO_SIGNAL_NOTI_UID,
type: SET_NO_AUDIO_SIGNAL_NOTIFICATION_UID,
uid
};
}

View File

@ -3,14 +3,10 @@ import { setNoAudioSignalNotificationUid } from './actions';
import { APP_WILL_MOUNT, APP_WILL_UNMOUNT } from '../base/app';
import { CONFERENCE_JOINED } from '../base/conference';
import {
ACTIVE_DEVICE_DETECTED,
ActiveDeviceDetector,
filterAudioDevices,
formatDeviceLabel,
getAvailableDevices,
setAudioInputDevice
} from '../base/devices';
import { JitsiConferenceEvents } from '../base/lib-jitsi-meet';
import JitsiMeetJS, { JitsiConferenceEvents } from '../base/lib-jitsi-meet';
import { MiddlewareRegistry } from '../base/redux';
import { updateSettings } from '../base/settings';
import { playSound, registerSound, unregisterSound } from '../base/sounds';
@ -22,7 +18,7 @@ MiddlewareRegistry.register(store => next => async action => {
const result = next(action);
const { dispatch, getState } = store;
const { conference } = action;
let audioDetectService = null;
let confAudioInputState;
switch (action.type) {
case APP_WILL_MOUNT:
@ -33,84 +29,86 @@ MiddlewareRegistry.register(store => next => async action => {
break;
case CONFERENCE_JOINED: {
conference.on(JitsiConferenceEvents.TRACK_ADDED, track => {
conference.on(JitsiConferenceEvents.AUDIO_INPUT_STATE_CHANGE, hasAudioInput => {
const { noAudioSignalNotificationUid } = getState()['features/no-audio-signal'];
if (track.isAudioTrack() && track.isLocal()) {
// In case the device is switched attempt to destroy, this should prevent the notification firing
// when the device was switched, however it is possible that a user switches the device and the
// notification from the previous devices pops up, but this will probably happen very rarely and even
// if it does it's not that disruptive to the ux.
if (audioDetectService) {
audioDetectService.destroy();
audioDetectService = null;
}
confAudioInputState = hasAudioInput;
// When a new track is added hide the current notification is one is displayed, and reset the redux
// state so that we begin monitoring on the new device as well.
if (noAudioSignalNotificationUid) {
dispatch(hideNotification(noAudioSignalNotificationUid));
dispatch(setNoAudioSignalNotificationUid());
}
if (noAudioSignalNotificationUid && hasAudioInput) {
dispatch(hideNotification(noAudioSignalNotificationUid));
dispatch(setNoAudioSignalNotificationUid());
}
});
conference.on(JitsiConferenceEvents.NO_AUDIO_INPUT, async () => {
const { noSrcDataNotiUid } = getState()['features/base/no-src-data'];
const { noSrcDataNotificationUid } = getState()['features/base/no-src-data'];
// In case the 'no data detected from source' notification was already shown, we prevent the
// no audio signal notification as it's redundant i.e. it's clear that the users microphone is
// muted from system settings.
if (noSrcDataNotiUid) {
if (noSrcDataNotificationUid) {
return;
}
const devices = await dispatch(getAvailableDevices());
const audioDevices = filterAudioDevices(devices);
// Force the flag to false in case AUDIO_INPUT_STATE_CHANGE is received after the notification is displayed,
// thus making sure we check properly if the notification should display.
confAudioInputState = false;
audioDetectService = await ActiveDeviceDetector.create(audioDevices);
audioDetectService.on(ACTIVE_DEVICE_DETECTED, detectEvent => {
let descriptionKey = 'toolbar.noAudioSignalDesc';
let customActionNameKey = null;
let customActionHandler = null;
const activeDevice = await JitsiMeetJS.getActiveAudioDevice();
// In case the detector picked up a device show a notification with a device suggestion
if (detectEvent.deviceLabel !== '') {
descriptionKey = 'toolbar.noAudioSignalDescSuggestion';
if (confAudioInputState) {
return;
}
// Preferably the label should be passed as an argument paired with a i18next string, however
// at the point of the implementation the showNotification function only supports doing that for
// the description.
// TODO Add support for arguments to showNotification title and customAction strings.
customActionNameKey = `Use ${formatDeviceLabel(detectEvent.deviceLabel)}`;
customActionHandler = () => {
// Select device callback
dispatch(
updateSettings({
userSelectedMicDeviceId: detectEvent.deviceId,
userSelectedMicDeviceLabel: detectEvent.deviceLabel
})
);
// In case there is a previous notification displayed just hide it.
const { noAudioSignalNotificationUid } = getState()['features/no-audio-signal'];
dispatch(setAudioInputDevice(detectEvent.deviceId));
};
}
if (noAudioSignalNotificationUid) {
dispatch(hideNotification(noAudioSignalNotificationUid));
dispatch(setNoAudioSignalNotificationUid());
}
const notification = showNotification({
titleKey: 'toolbar.noAudioSignalTitle',
descriptionKey,
customActionNameKey,
customActionHandler
});
dispatch(notification);
let descriptionKey = 'toolbar.noAudioSignalDesc';
let customActionNameKey;
let customActionHandler;
dispatch(playSound(NO_AUDIO_SIGNAL_SOUND_ID));
// In case the detector picked up a device show a notification with a device suggestion
if (activeDevice.deviceLabel !== '') {
descriptionKey = 'toolbar.noAudioSignalDescSuggestion';
// Store the current notification uid so we can check for this state and hide it in case
// a new track was added, thus changing the context of the notification
dispatch(setNoAudioSignalNotificationUid(notification.uid));
// Preferably the label should be passed as an argument paired with a i18next string, however
// at the point of the implementation the showNotification function only supports doing that for
// the description.
// TODO Add support for arguments to showNotification title and customAction strings.
customActionNameKey = `Use ${formatDeviceLabel(activeDevice.deviceLabel)}`;
customActionHandler = () => {
// Select device callback
dispatch(
updateSettings({
userSelectedMicDeviceId: activeDevice.deviceId,
userSelectedMicDeviceLabel: activeDevice.deviceLabel
})
);
dispatch(setAudioInputDevice(activeDevice.deviceId));
};
}
const notification = showNotification({
titleKey: 'toolbar.noAudioSignalTitle',
descriptionKey,
customActionNameKey,
customActionHandler
});
dispatch(notification);
dispatch(playSound(NO_AUDIO_SIGNAL_SOUND_ID));
// Store the current notification uid so we can check for this state and hide it in case
// a new track was added, thus changing the context of the notification
dispatch(setNoAudioSignalNotificationUid(notification.uid));
});
break;
}

View File

@ -2,14 +2,14 @@
import { ReducerRegistry, set } from '../base/redux';
import { SET_NO_AUDIO_SIGNAL_NOTI_UID } from './actionTypes';
import { SET_NO_AUDIO_SIGNAL_NOTIFICATION_UID } from './actionTypes';
/**
* Reduces the redux actions of the feature no audio signal
*/
ReducerRegistry.register('features/no-audio-signal', (state = {}, action) => {
switch (action.type) {
case SET_NO_AUDIO_SIGNAL_NOTI_UID:
case SET_NO_AUDIO_SIGNAL_NOTIFICATION_UID:
return set(state, 'noAudioSignalNotificationUid', action.uid);
}