From 6493b09565eb2ed0237351e0778a6f269ed49b9f Mon Sep 17 00:00:00 2001 From: paweldomas Date: Tue, 11 Jul 2017 15:06:58 +0200 Subject: [PATCH 1/5] feat: add config.startAudioOnly When the 'startAudioOnly' config option is set to true the conference will start in the audio only mode. --- conference.js | 34 +++++++++++++++++--- config.js | 1 + react/features/base/conference/middleware.js | 34 +++++++++++++++++++- 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/conference.js b/conference.js index 25252db96..ef1099acc 100644 --- a/conference.js +++ b/conference.js @@ -26,6 +26,7 @@ import { conferenceFailed, conferenceJoined, conferenceLeft, + toggleAudioOnly, EMAIL_COMMAND, lockStateChanged } from './react/features/base/conference'; @@ -459,11 +460,14 @@ export default { * Creates local media tracks and connects to a room. Will show error * dialogs in case accessing the local microphone and/or camera failed. Will * show guidance overlay for users on how to give access to camera and/or - * microphone, + * microphone. * @param {string} roomName * @param {object} options - * @param {boolean} options.startScreenSharing - if true should - * start with screensharing instead of camera video. + * @param {boolean} options.startAudioOnly=false - if true then + * only audio track will be created and the audio only mode will be turned + * on. + * @param {boolean} options.startScreenSharing=false - if true + * should start with screensharing instead of camera video. * @returns {Promise.} */ createInitialLocalTracksAndConnect(roomName, options = {}) { @@ -482,8 +486,18 @@ export default { // First try to retrieve both audio and video. let tryCreateLocalTracks; + // FIXME there is no video muted indication visible on the remote side, + // after starting in audio only (there's no video track) // FIXME the logic about trying to go audio only on error is duplicated - if (options.startScreenSharing) { + if (options.startAudioOnly) { + tryCreateLocalTracks + = createLocalTracks({ devices: ['audio'] }, true) + .catch(err => { + audioOnlyError = err; + + return []; + }); + } else if (options.startScreenSharing) { tryCreateLocalTracks = this._createDesktopTrack() .then(desktopStream => { return createLocalTracks({ devices: ['audio'] }, true) @@ -591,6 +605,7 @@ export default { analytics.init(); return this.createInitialLocalTracksAndConnect( options.roomName, { + startAudioOnly: config.startAudioOnly, startScreenSharing: config.startScreenSharing }); }).then(([tracks, con]) => { @@ -649,6 +664,15 @@ export default { this.updateVideoIconEnabled(); } + // Enable audio only mode + if (config.startAudioOnly) { + // It is important to have that toggled after video muted + // state is adjusted by the code about lack of video tracks + // above. That's because audio only will store muted state + // on toggle action. + APP.store.dispatch(toggleAudioOnly()); + } + this._initDeviceList(); if (config.iAmRecorder) @@ -2035,7 +2059,7 @@ export default { JitsiMeetJS.mediaDevices.enumerateDevices(devices => { // Ugly way to synchronize real device IDs with local // storage and settings menu. This is a workaround until - // getConstraints() method will be implemented + // getConstraints() method will be implemented // in browsers. if (localAudio) { APP.settings.setMicDeviceId( diff --git a/config.js b/config.js index 38a7942d5..011e2a2b2 100644 --- a/config.js +++ b/config.js @@ -72,6 +72,7 @@ var config = { // eslint-disable-line no-unused-vars // page redirection when call is hangup disableSimulcast: false, // requireDisplayName: true, // Forces the participants that doesn't have display name to enter it when they enter the room. + startAudioOnly: false, // Will start the conference in the audio only mode (no video is being received nor sent) startScreenSharing: false, // Will try to start with screensharing instead of camera // startAudioMuted: 10, // every participant after the Nth will start audio muted // startVideoMuted: 10, // every participant after the Nth will start video muted diff --git a/react/features/base/conference/middleware.js b/react/features/base/conference/middleware.js index bd61aaae4..fbec4c46a 100644 --- a/react/features/base/conference/middleware.js +++ b/react/features/base/conference/middleware.js @@ -15,7 +15,7 @@ import { _setAudioOnlyVideoMuted, setLastN } from './actions'; -import { SET_AUDIO_ONLY, SET_LASTN } from './actionTypes'; +import { CONFERENCE_JOINED, SET_AUDIO_ONLY, SET_LASTN } from './actionTypes'; import { _addLocalTracksToConference, _handleParticipantError, @@ -33,6 +33,9 @@ MiddlewareRegistry.register(store => next => action => { case CONNECTION_ESTABLISHED: return _connectionEstablished(store, next, action); + case CONFERENCE_JOINED: + return _conferenceJoined(store, next, action); + case PIN_PARTICIPANT: return _pinParticipant(store, next, action); @@ -76,6 +79,35 @@ function _connectionEstablished(store, next, action) { return result; } +/** + * Does extra sync up on properties that may need to be updated, after + * the conference was joined. + * + * @param {Store} store - The Redux store in which the specified action is being + * dispatched. + * @param {Dispatch} next - The Redux dispatch function to dispatch the + * specified action to the specified store. + * @param {Action} action - The Redux action CONFERENCE_JOINED which is being + * dispatched in the specified store. + * @private + * @returns {Object} The new state that is the result of the reduction of the + * specified action. + */ +function _conferenceJoined(store, next, action) { + const result = next(action); + const { audioOnly, conference } + = store.getState()['features/base/conference']; + + // FIXME On Web the audio only mode for "start audio only" is toggled before + // conference is added to the redux store ("on conference joined" action) + // and the LastN value needs to be synchronized here. + if (audioOnly && conference.getLastN() !== 0) { + store.dispatch(setLastN(0)); + } + + return result; +} + /** * Notifies the feature base/conference that the action PIN_PARTICIPANT is being * dispatched within a specific Redux store. Pins the specified remote From 00d3d3c09adb12bfb1ff5a320511b8483ca1cd95 Mon Sep 17 00:00:00 2001 From: paweldomas Date: Wed, 19 Jul 2017 12:08:51 +0200 Subject: [PATCH 2/5] fix(VideoLayout): muted for no tracks Will make the UI display audio/video muted icon for remote participants with no audio/video track. --- conference.js | 2 -- modules/UI/videolayout/VideoLayout.js | 39 ++++++++++++++++++++++----- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/conference.js b/conference.js index ef1099acc..06bcf4287 100644 --- a/conference.js +++ b/conference.js @@ -486,8 +486,6 @@ export default { // First try to retrieve both audio and video. let tryCreateLocalTracks; - // FIXME there is no video muted indication visible on the remote side, - // after starting in audio only (there's no video track) // FIXME the logic about trying to go audio only on error is duplicated if (options.startAudioOnly) { tryCreateLocalTracks diff --git a/modules/UI/videolayout/VideoLayout.js b/modules/UI/videolayout/VideoLayout.js index 03872a3fe..de3232634 100644 --- a/modules/UI/videolayout/VideoLayout.js +++ b/modules/UI/videolayout/VideoLayout.js @@ -331,13 +331,11 @@ var VideoLayout = { remoteVideo.addRemoteStreamElement(stream); - // if track is muted make sure we reflect that - if(stream.isMuted()) - { - if(stream.getType() === "audio") - this.onAudioMute(stream.getParticipantId(), true); - else - this.onVideoMute(stream.getParticipantId(), true); + // Make sure track's muted state is reflected + if (stream.getType() === "audio") { + this.onAudioMute(stream.getParticipantId(), stream.isMuted()); + } else { + this.onVideoMute(stream.getParticipantId(), stream.isMuted()); } }, @@ -348,6 +346,30 @@ var VideoLayout = { if (remoteVideo) { remoteVideo.removeRemoteStreamElement(stream); } + this.updateMutedForNoTracks(id, stream.getType()); + }, + + /** + * FIXME get rid of this method once muted indicator are reactified (by + * making sure that user with no tracks is displayed as muted ) + * + * If participant has no tracks will make the UI display muted status. + * @param {string} participantId + * @param {string} mediaType 'audio' or 'video' + */ + updateMutedForNoTracks(participantId, mediaType) { + const participant = APP.conference.getParticipantById(participantId); + + if (participant + && !participant.getTracksByMediaType(mediaType).length) { + if (mediaType === 'audio') { + APP.UI.setAudioMuted(participantId, true); + } else if (mediaType === 'video') { + APP.UI.setVideoMuted(participantId, true); + } else { + logger.error(`Unsupported media type: ${mediaType}`); + } + } }, /** @@ -441,6 +463,9 @@ var VideoLayout = { this._setRemoteControlProperties(user, remoteVideo); this.addRemoteVideoContainer(id, remoteVideo); + this.updateMutedForNoTracks(id, 'audio'); + this.updateMutedForNoTracks(id, 'video'); + const remoteVideosCount = Object.keys(remoteVideos).length; if (remoteVideosCount === 1) { From a5f61714bdbca070f8332da6413758b589ea36d7 Mon Sep 17 00:00:00 2001 From: paweldomas Date: Thu, 20 Jul 2017 14:29:15 +0200 Subject: [PATCH 3/5] fix: unmute video on audio only switch off Will unmute local video (and ask for permissions if needed) in case user started in audio only mode and is turing it off. --- conference.js | 78 ++++++++++++++++++++++++---------------------- modules/API/API.js | 4 ++- 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/conference.js b/conference.js index 06bcf4287..ab45c5b85 100644 --- a/conference.js +++ b/conference.js @@ -72,7 +72,7 @@ const eventEmitter = new EventEmitter(); let room; let connection; let localAudio, localVideo; -let initialAudioMutedState = false, initialVideoMutedState = false; +let initialAudioMutedState = false; import {VIDEO_CONTAINER_TYPE} from "./modules/UI/videolayout/VideoContainer"; @@ -495,6 +495,11 @@ export default { return []; }); + + // Enable audio only mode + if (config.startAudioOnly) { + APP.store.dispatch(toggleAudioOnly()); + } } else if (options.startScreenSharing) { tryCreateLocalTracks = this._createDesktopTrack() .then(desktopStream => { @@ -608,8 +613,9 @@ export default { }); }).then(([tracks, con]) => { tracks.forEach(track => { - if((track.isAudioTrack() && initialAudioMutedState) - || (track.isVideoTrack() && initialVideoMutedState)) { + if (track.isAudioTrack() && initialAudioMutedState) { + track.mute(); + } else if (track.isVideoTrack() && this.videoMuted) { track.mute(); } }); @@ -662,15 +668,6 @@ export default { this.updateVideoIconEnabled(); } - // Enable audio only mode - if (config.startAudioOnly) { - // It is important to have that toggled after video muted - // state is adjusted by the code about lack of video tracks - // above. That's because audio only will store muted state - // on toggle action. - APP.store.dispatch(toggleAudioOnly()); - } - this._initDeviceList(); if (config.iAmRecorder) @@ -722,22 +719,41 @@ export default { /** * Simulates toolbar button click for video mute. Used by shortcuts and API. * @param mute true for mute and false for unmute. + * @param {boolean} [showUI] when set to false will not display any error + * dialogs in case of media permissions error. */ - muteVideo(mute) { - muteLocalVideo(mute); + muteVideo(mute, showUI = true) { + if (!localVideo && this.videoMuted && !mute) { + // Try to create local video if there wasn't any. + // This handles the case when user joined with no video + // (dismissed screen sharing screen or in audio only mode), but + // decided to add it later on by clicking on muted video icon or + // turning off the audio only mode. + // + // FIXME when local track creation is moved to react/redux + // it should take care of the use case described above + createLocalTracks({ devices: ['video'] }, false) + .then(([videoTrack]) => videoTrack) + .catch(error => { + // FIXME should send some feedback to the API on error ? + if (showUI) { + APP.UI.showDeviceErrorDialog(null, error); + } + // Rollback the video muted status by using null track + return null; + }) + .then(videoTrack => this.useVideoStream(videoTrack)); + } else { + muteLocalVideo(mute); + } }, /** * Simulates toolbar button click for video mute. Used by shortcuts and API. - * @param {boolean} force - If the track is not created, the operation - * will be executed after the track is created. Otherwise the operation - * will be ignored. + * @param {boolean} [showUI] when set to false will not display any error + * dialogs in case of media permissions error. */ - toggleVideoMuted(force = false) { - if(!localVideo && force) { - initialVideoMutedState = !initialVideoMutedState; - return; - } - this.muteVideo(!this.videoMuted); + toggleVideoMuted(showUI = true) { + this.muteVideo(!this.videoMuted, showUI); }, /** * Retrieve list of conference participants (without local user). @@ -1731,20 +1747,8 @@ export default { APP.UI.addListener(UIEvents.VIDEO_MUTED, muted => { if (this.isAudioOnly() && !muted) { this._displayAudioOnlyTooltip('videoMute'); - } else if (!localVideo && this.videoMuted && !muted) { - // Maybe try to create local video if there wasn't any ? - // This handles the case when user joined with no video - // (dismissed screen sharing screen), but decided to add it - // later on by clicking on muted video icon. - createLocalTracks({ devices: ['video'] }, false) - .then(([videoTrack]) => { - APP.conference.useVideoStream(videoTrack); - }) - .catch(error => { - APP.UI.showDeviceErrorDialog(null, error); - }); } else { - muteLocalVideo(muted); + this.muteVideo(muted); } }); @@ -1946,7 +1950,7 @@ export default { ); APP.UI.addListener(UIEvents.TOGGLE_AUDIO_ONLY, audioOnly => { - muteLocalVideo(audioOnly); + this.muteVideo(audioOnly); // Immediately update the UI by having remote videos and the large // video update themselves instead of waiting for some other event diff --git a/modules/API/API.js b/modules/API/API.js index 1a884a35c..7a21ca1c5 100644 --- a/modules/API/API.js +++ b/modules/API/API.js @@ -36,7 +36,9 @@ function initCommands() { 'display-name': APP.conference.changeLocalDisplayName.bind(APP.conference), 'toggle-audio': () => APP.conference.toggleAudioMuted(true), - 'toggle-video': () => APP.conference.toggleVideoMuted(true), + 'toggle-video': () => { + APP.conference.toggleVideoMuted(false /* no UI */); + }, 'toggle-film-strip': APP.UI.toggleFilmstrip, 'toggle-chat': APP.UI.toggleChat, 'toggle-contact-list': APP.UI.toggleContactList, From 6ac23c80865e654cee09f3f18b0cd947fce7cb40 Mon Sep 17 00:00:00 2001 From: paweldomas Date: Fri, 21 Jul 2017 11:12:33 +0200 Subject: [PATCH 4/5] fix(conference): early video muted state If muteVideo is called, before local tracks have been initialized it will be synced up once the tracks are created (or not). --- conference.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/conference.js b/conference.js index ab45c5b85..da40f89d2 100644 --- a/conference.js +++ b/conference.js @@ -422,6 +422,12 @@ function _connectionFailedHandler(error) { } export default { + /** + * Flag used to delay modification of the muted status of local media tracks + * until those are created (or not, but at that point it's certain that + * the tracks won't exist). + */ + _localTracksInitialized: false, isModerator: false, audioMuted: false, videoMuted: false, @@ -620,6 +626,7 @@ export default { } }); logger.log('initialized with %s local tracks', tracks.length); + this._localTracksInitialized = true; con.addEventListener( ConnectionEvents.CONNECTION_FAILED, _connectionFailedHandler); @@ -711,6 +718,8 @@ export default { */ toggleAudioMuted(force = false) { if(!localAudio && force) { + // NOTE this logic will be adjusted to the same one as for the video + // once 'startWithAudioMuted' option is added. initialAudioMutedState = !initialAudioMutedState; return; } @@ -723,6 +732,13 @@ export default { * dialogs in case of media permissions error. */ muteVideo(mute, showUI = true) { + // Not ready to modify track's state yet + if (!this._localTracksInitialized) { + this.videoMuted = mute; + + return; + } + if (!localVideo && this.videoMuted && !mute) { // Try to create local video if there wasn't any. // This handles the case when user joined with no video @@ -744,6 +760,13 @@ export default { }) .then(videoTrack => this.useVideoStream(videoTrack)); } else { + // FIXME if localVideo exists and the permissions are blocked + // while video muted it will fail to unmute and UI will get out of + // sync (the toolbar will show unmuted even though unmute failed). + // But for some reason that only happens when toggling off from + // the audio only mode - the same scenario works fine from toolbar. + // This is very rare corner case and supposedly this will get fixed + // once everything goes to react/redux. muteLocalVideo(mute); } }, From e08171f602330d9f44390f469c2cbcaf33567f7f Mon Sep 17 00:00:00 2001 From: paweldomas Date: Mon, 24 Jul 2017 11:20:32 +0200 Subject: [PATCH 5/5] fix: video muted out of sync When video is unmuted when toggling off the audio only mode it dispatches video muted status, but does not roll it back in case it fails. That was causing toolbar button on Web to display incorrect video muted status. --- conference.js | 56 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/conference.js b/conference.js index da40f89d2..c3321b731 100644 --- a/conference.js +++ b/conference.js @@ -175,27 +175,40 @@ function getDisplayName(id) { * result of user interaction */ function muteLocalAudio(muted) { - muteLocalMedia(localAudio, muted, 'Audio'); + muteLocalMedia(localAudio, muted); } -function muteLocalMedia(localMedia, muted, localMediaTypeString) { - if (!localMedia) { - return; +/** + * Mute or unmute local media stream if it exists. + * @param {JitsiLocalTrack} localTrack + * @param {boolean} muted + * + * @returns {Promise} resolved in case mute/unmute operations succeeds or + * rejected with an error if something goes wrong. It is expected that often + * the error will be of the {@link JitsiTrackError} type, but it's not + * guaranteed. + */ +function muteLocalMedia(localTrack, muted) { + if (!localTrack) { + return Promise.resolve(); } const method = muted ? 'mute' : 'unmute'; - localMedia[method]().catch(reason => { - logger.warn(`${localMediaTypeString} ${method} was rejected:`, reason); - }); + return localTrack[method](); } /** * Mute or unmute local video stream if it exists. * @param {boolean} muted if video stream should be muted or unmuted. + * + * @returns {Promise} resolved in case mute/unmute operations succeeds or + * rejected with an error if something goes wrong. It is expected that often + * the error will be of the {@link JitsiTrackError} type, but it's not + * guaranteed. */ function muteLocalVideo(muted) { - muteLocalMedia(localVideo, muted, 'Video'); + return muteLocalMedia(localVideo, muted); } /** @@ -739,6 +752,12 @@ export default { return; } + const maybeShowErrorDialog = (error) => { + if (showUI) { + APP.UI.showDeviceErrorDialog(null, error); + } + }; + if (!localVideo && this.videoMuted && !mute) { // Try to create local video if there wasn't any. // This handles the case when user joined with no video @@ -752,22 +771,21 @@ export default { .then(([videoTrack]) => videoTrack) .catch(error => { // FIXME should send some feedback to the API on error ? - if (showUI) { - APP.UI.showDeviceErrorDialog(null, error); - } + maybeShowErrorDialog(error); + // Rollback the video muted status by using null track return null; }) .then(videoTrack => this.useVideoStream(videoTrack)); } else { - // FIXME if localVideo exists and the permissions are blocked - // while video muted it will fail to unmute and UI will get out of - // sync (the toolbar will show unmuted even though unmute failed). - // But for some reason that only happens when toggling off from - // the audio only mode - the same scenario works fine from toolbar. - // This is very rare corner case and supposedly this will get fixed - // once everything goes to react/redux. - muteLocalVideo(mute); + const oldMutedStatus = this.videoMuted; + + muteLocalVideo(mute) + .catch(error => { + maybeShowErrorDialog(error); + this.videoMuted = oldMutedStatus; + APP.UI.setVideoMuted(this.getMyUserId(), this.videoMuted); + }); } }, /**