haskal
/
jiti-meet
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(device-selection): do not reuse tracks in previews 

Device selection has live previews that reuse the current local
audio and video tracks for the sake of internet explorer. This
means when the local video was muted, device selection would
show a muted message. It is preferred to show a live preview
even when muted.

The changes include:
- Passing device ids into DeviceSelectionDialog, not tracks.
- Setting default selected devices to use for live previews.
- Removing all checks in DeviceSelectionDialog involving local tracks.
- Catching and displaying errors when creating a live video preview.
This commit is contained in:
Leonard Kim 2017-04-25 19:15:19 -07:00
parent d24d5d95dd
commit 929bc8b8b9
5 changed files with 258 additions and 282 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
css/modals/device-selection/_device-selection.scss
View File

@ -79,7 +79,7 @@
border-radius: 3px;
}
.video-input-preview-muted {
.video-input-preview-error {
color: $participantNameColor;
display: none;
left: 0;
@ -89,12 +89,10 @@
top: 50%;
}
&.video-muted {
/* TOFIX: to be removed when we move out from muted preview */
&.video-preview-has-error {
background: black;
/* TOFIX-END */
.video-input-preview-muted {
.video-input-preview-error {
display: block;
}
}

2
lang/main.json
View File

@ -429,9 +429,9 @@
"speakerTime": "Speaker Time"
},
"deviceSelection": {
"currentlyVideoMuted": "Video is currently muted",
"deviceSettings": "Device settings",
"noPermission": "Permission not granted",
"previewUnavailable": "Preview unavailable",
"selectADevice": "Select a device",
"testAudio": "Test sound"
},

9
react/features/device-selection/actions.js
View File

@ -12,16 +12,13 @@ import { DeviceSelectionDialog } from './components';
* @returns {Function}
*/
export function openDeviceSelectionDialog() {
return (dispatch, getState) => {
return dispatch => {
JitsiMeetJS.mediaDevices.isDeviceListAvailable()
.then(isDeviceListAvailable => {
const state = getState();
const conference = state['features/base/conference'].conference;
dispatch(openDialog(DeviceSelectionDialog, {
currentAudioInputId: APP.settings.getMicDeviceId(),
currentAudioOutputId: APP.settings.getAudioOutputDeviceId(),
currentAudioTrack: conference.getLocalAudioTrack(),
currentVideoTrack: conference.getLocalVideoTrack(),
currentVideoInputId: APP.settings.getCameraDeviceId(),
disableAudioInputChange:
!JitsiMeetJS.isMultipleAudioInputSupported(),
disableDeviceChange: !isDeviceListAvailable

419
react/features/device-selection/components/DeviceSelectionDialog.js
View File

@ -34,29 +34,25 @@ class DeviceSelectionDialog extends Component {
* All known audio and video devices split by type. This prop comes from
* the app state.
*/
_devices: React.PropTypes.object,
_availableDevices: React.PropTypes.object,
/**
* Device id for the current audio output device.
* Device id for the current audio input device. This device will be set
* as the default audio input device to preview.
*/
currentAudioInputId: React.PropTypes.string,
/**
* Device id for the current audio output device. This device will be
* set as the default audio output device to preview.
*/
currentAudioOutputId: React.PropTypes.string,
/**
* JitsiLocalTrack for the current local audio.
*
* JitsiLocalTracks for the current audio and video, if any, should be
* passed in for re-use in the previews. This is needed for Internet
* Explorer, which cannot get multiple tracks from the same device, even
* across tabs.
* Device id for the current video input device. This device will be set
* as the default video input device to preview.
*/
currentAudioTrack: React.PropTypes.object,
/**
* JitsiLocalTrack for the current local video.
*
* Needed for reuse. See comment for propTypes.currentAudioTrack.
*/
currentVideoTrack: React.PropTypes.object,
currentVideoInputId: React.PropTypes.string,
/**
* Whether or not the audio selector can be interacted with. If true,
@ -78,12 +74,12 @@ class DeviceSelectionDialog extends Component {
dispatch: React.PropTypes.func,
/**
* Whether or not new audio input source can be selected.
* Whether or not a new audio input source can be selected.
*/
hasAudioPermission: React.PropTypes.bool,
/**
* Whether or not new video input sources can be selected.
* Whether or not a new video input sources can be selected.
*/
hasVideoPermission: React.PropTypes.bool,
@ -117,15 +113,40 @@ class DeviceSelectionDialog extends Component {
constructor(props) {
super(props);
const { _availableDevices } = this.props;
this.state = {
// JitsiLocalTracks to use for live previewing.
// JitsiLocalTrack to use for live previewing of audio input.
previewAudioTrack: null,
// JitsiLocalTrack to use for live previewing of video input.
previewVideoTrack: null,
// Device ids to keep track of new selections.
videInput: null,
audioInput: null,
audioOutput: null
// An message describing a problem with obtaining a video preview.
previewVideoTrackError: null,
// The audio input device id to show as selected by default.
selectedAudioInputId: this.props.currentAudioInputId || '',
// The audio output device id to show as selected by default.
selectedAudioOutputId: this.props.currentAudioOutputId || '',
// The video input device id to show as selected by default.
// FIXME: On temasys, without a device selected and put into local
// storage as the default device to use, the current video device id
// is a blank string. This is because the library gets a local video
// track and then maps the track's device id by matching the track's
// label to the MediaDeviceInfos returned from enumerateDevices. In
// WebRTC, the track label is expected to return the camera device
// label. However, temasys video track labels refer to track id, not
// device label, so the library cannot match the track to a device.
// The workaround of defaulting to the first videoInput available
// is re-used from the previous device settings implementation.
selectedVideoInputId: this.props.currentVideoInputId
|| (_availableDevices.videoInput
&& _availableDevices.videoInput[0]
&& _availableDevices.videoInput[0].deviceId)
|| ''
};
// Preventing closing while cleaning up previews is important for
@ -134,16 +155,29 @@ class DeviceSelectionDialog extends Component {
// closure until cleanup is complete ensures no errors in the process.
this._isClosing = false;
// Bind event handlers so they are only bound once for every instance.
this._closeModal = this._closeModal.bind(this);
this._getAndSetAudioOutput = this._getAndSetAudioOutput.bind(this);
this._getAndSetAudioTrack = this._getAndSetAudioTrack.bind(this);
this._getAndSetVideoTrack = this._getAndSetVideoTrack.bind(this);
this._onCancel = this._onCancel.bind(this);
this._onSubmit = this._onSubmit.bind(this);
this._updateAudioOutput = this._updateAudioOutput.bind(this);
this._updateAudioInput = this._updateAudioInput.bind(this);
this._updateVideoInput = this._updateVideoInput.bind(this);
}
/**
* Clean up any preview tracks that might not have been cleaned up already.
* Sets default device choices so a choice is pre-selected in the dropdowns
* and live previews are created.
*
* @inheritdoc
*/
componentDidMount() {
this._updateAudioOutput(this.state.selectedAudioOutputId);
this._updateAudioInput(this.state.selectedAudioInputId);
this._updateVideoInput(this.state.selectedVideoInputId);
}
/**
* Disposes preview tracks that might not already be disposed.
*
* @inheritdoc
*/
@ -173,8 +207,8 @@ class DeviceSelectionDialog extends Component {
<div className = 'device-selection-column column-video'>
<div className = 'device-selection-video-container'>
<VideoInputPreview
track = { this.state.previewVideoTrack
|| this.props.currentVideoTrack } />
error = { this.state.previewVideoTrackError }
track = { this.state.previewVideoTrack } />
</div>
{ this._renderAudioInputPreview() }
</div>
@ -197,17 +231,10 @@ class DeviceSelectionDialog extends Component {
* promise can be for video cleanup and another for audio cleanup.
*/
_attemptPreviewTrackCleanup() {
const cleanupPromises = [];
if (!this._isPreviewingCurrentVideoTrack()) {
cleanupPromises.push(this._disposeVideoPreview());
}
if (!this._isPreviewingCurrentAudioTrack()) {
cleanupPromises.push(this._disposeAudioPreview());
}
return cleanupPromises;
return Promise.all([
this._disposeVideoPreview(),
this._disposeAudioPreview()
]);
}
/**
@ -243,147 +270,7 @@ class DeviceSelectionDialog extends Component {
}
/**
* Callback invoked when a new audio output device has been selected.
* Updates the internal state of the user's selection.
*
* @param {string} deviceId - The id of the chosen audio output device.
* @private
* @returns {void}
*/
_getAndSetAudioOutput(deviceId) {
this.setState({
audioOutput: deviceId
});
}
/**
* Callback invoked when a new audio input device has been selected.
* Updates the internal state of the user's selection as well as the audio
* track that should display in the preview. Will reuse the current local
* audio track if it has been selected.
*
* @param {string} deviceId - The id of the chosen audio input device.
* @private
* @returns {void}
*/
_getAndSetAudioTrack(deviceId) {
this.setState({
audioInput: deviceId
}, () => {
const cleanupPromise = this._isPreviewingCurrentAudioTrack()
? Promise.resolve() : this._disposeAudioPreview();
if (this._isCurrentAudioTrack(deviceId)) {
cleanupPromise
.then(() => {
this.setState({
previewAudioTrack: this.props.currentAudioTrack
});
});
} else {
cleanupPromise
.then(() => createLocalTrack('audio', deviceId))
.then(jitsiLocalTrack => {
this.setState({
previewAudioTrack: jitsiLocalTrack
});
});
}
});
}
/**
* Callback invoked when a new video input device has been selected. Updates
* the internal state of the user's selection as well as the video track
* that should display in the preview. Will reuse the current local video
* track if it has been selected.
*
* @param {string} deviceId - The id of the chosen video input device.
* @private
* @returns {void}
*/
_getAndSetVideoTrack(deviceId) {
this.setState({
videoInput: deviceId
}, () => {
const cleanupPromise = this._isPreviewingCurrentVideoTrack()
? Promise.resolve() : this._disposeVideoPreview();
if (this._isCurrentVideoTrack(deviceId)) {
cleanupPromise
.then(() => {
this.setState({
previewVideoTrack: this.props.currentVideoTrack
});
});
} else {
cleanupPromise
.then(() => createLocalTrack('video', deviceId))
.then(jitsiLocalTrack => {
this.setState({
previewVideoTrack: jitsiLocalTrack
});
});
}
});
}
/**
* Utility function for determining if the current local audio track has the
* passed in device id.
*
* @param {string} deviceId - The device id to match against.
* @private
* @returns {boolean} True if the device id is being used by the local audio
* track.
*/
_isCurrentAudioTrack(deviceId) {
return this.props.currentAudioTrack
&& this.props.currentAudioTrack.getDeviceId() === deviceId;
}
/**
* Utility function for determining if the current local video track has the
* passed in device id.
*
* @param {string} deviceId - The device id to match against.
* @private
* @returns {boolean} True if the device id is being used by the local
* video track.
*/
_isCurrentVideoTrack(deviceId) {
return this.props.currentVideoTrack
&& this.props.currentVideoTrack.getDeviceId() === deviceId;
}
/**
* Utility function for detecting if the current audio preview track is not
* the currently used audio track.
*
* @private
* @returns {boolean} True if the current audio track is being used for
* the preview.
*/
_isPreviewingCurrentAudioTrack() {
return !this.state.previewAudioTrack
|| this.state.previewAudioTrack === this.props.currentAudioTrack;
}
/**
* Utility function for detecting if the current video preview track is not
* the currently used video track.
*
* @private
* @returns {boolean} True if the current video track is being used as the
* preview.
*/
_isPreviewingCurrentVideoTrack() {
return !this.state.previewVideoTrack
|| this.state.previewVideoTrack === this.props.currentVideoTrack;
}
/**
* Cleans existing preview tracks and signal to closeDeviceSelectionDialog.
* Disposes preview tracks and signals to close DeviceSelectionDialog.
*
* @private
* @returns {boolean} Returns false to prevent closure until cleanup is
@ -406,7 +293,7 @@ class DeviceSelectionDialog extends Component {
}
/**
* Identify changes to the preferred input/output devices and perform
* Identifies changes to the preferred input/output devices and perform
* necessary cleanup and requests to use those devices. Closes the modal
* after cleanup and device change requests complete.
*
@ -421,32 +308,26 @@ class DeviceSelectionDialog extends Component {
this._isClosing = true;
const deviceChangePromises = [];
const deviceChangePromises = this._attemptPreviewTrackCleanup()
.then(() => {
if (this.state.selectedVideoInputId
!== this.props.currentVideoInputId) {
this.props.dispatch(
setVideoInputDevice(this.state.selectedVideoInputId));
}
if (this.state.videoInput && !this._isPreviewingCurrentVideoTrack()) {
const changeVideoPromise = this._disposeVideoPreview()
.then(() => {
this.props.dispatch(setVideoInputDevice(
this.state.videoInput));
});
if (this.state.selectedAudioInputId
!== this.props.currentAudioInputId) {
this.props.dispatch(
setAudioInputDevice(this.state.selectedAudioInputId));
}
deviceChangePromises.push(changeVideoPromise);
}
if (this.state.audioInput && !this._isPreviewingCurrentAudioTrack()) {
const changeAudioPromise = this._disposeAudioPreview()
.then(() => {
this.props.dispatch(setAudioInputDevice(
this.state.audioInput));
});
deviceChangePromises.push(changeAudioPromise);
}
if (this.state.audioOutput
&& this.state.audioOutput !== this.props.currentAudioOutputId) {
this.props.dispatch(setAudioOutputDevice(this.state.audioOutput));
}
if (this.state.selectedAudioOutputId
!== this.props.currentAudioOutputId) {
this.props.dispatch(
setAudioOutputDevice(this.state.selectedAudioOutputId));
}
});
Promise.all(deviceChangePromises)
.then(this._closeModal)
@ -470,8 +351,7 @@ class DeviceSelectionDialog extends Component {
return (
<AudioInputPreview
track = { this.state.previewAudioTrack
|| this.props.currentAudioTrack } />
track = { this.state.previewAudioTrack } />
);
}
@ -489,8 +369,7 @@ class DeviceSelectionDialog extends Component {
return (
<AudioOutputPreview
deviceId = { this.state.audioOutput
|| this.props.currentAudioOutputId } />
deviceId = { this.state.selectedAudioOutputId } />
);
}
@ -515,70 +394,120 @@ class DeviceSelectionDialog extends Component {
* @returns {Array<ReactElement>} DeviceSelector instances.
*/
_renderSelectors() {
const availableDevices = this.props._devices;
const currentAudioId = this.state.audioInput
|| (this.props.currentAudioTrack
&& this.props.currentAudioTrack.getDeviceId());
const currentAudioOutId = this.state.audioOutput
|| this.props.currentAudioOutputId;
// FIXME: On temasys, without a device selected and put into local
// storage as the default device to use, the current video device id is
// a blank string. This is because the library gets a local video track
// and then maps the track's device id by matching the track's label to
// the MediaDeviceInfos returned from enumerateDevices. In WebRTC, the
// track label is expected to return the camera device label. However,
// temasys video track labels refer to track id, not device label, so
// the library cannot match the track to a device. The workaround of
// defaulting to the first videoInput available has been re-used from
// the previous device settings implementation.
const currentVideoId = this.state.videoInput
|| (this.props.currentVideoTrack
&& this.props.currentVideoTrack.getDeviceId())
|| (availableDevices.videoInput[0]
&& availableDevices.videoInput[0].deviceId)
|| ''; // DeviceSelector expects a string for prop selectedDeviceId.
const { _availableDevices } = this.props;
const configurations = [
{
devices: availableDevices.videoInput,
devices: _availableDevices.videoInput,
hasPermission: this.props.hasVideoPermission,
icon: 'icon-camera',
isDisabled: this.props.disableDeviceChange,
key: 'videoInput',
label: 'settings.selectCamera',
onSelect: this._getAndSetVideoTrack,
selectedDeviceId: currentVideoId
onSelect: this._updateVideoInput,
selectedDeviceId: this.state.selectedVideoInputId
},
{
devices: availableDevices.audioInput,
devices: _availableDevices.audioInput,
hasPermission: this.props.hasAudioPermission,
icon: 'icon-microphone',
isDisabled: this.props.disableAudioInputChange
|| this.props.disableDeviceChange,
key: 'audioInput',
label: 'settings.selectMic',
onSelect: this._getAndSetAudioTrack,
selectedDeviceId: currentAudioId
onSelect: this._updateAudioInput,
selectedDeviceId: this.state.selectedAudioInputId
}
];
if (!this.props.hideAudioOutputSelect) {
configurations.push({
devices: availableDevices.audioOutput,
devices: _availableDevices.audioOutput,
hasPermission: this.props.hasAudioPermission
|| this.props.hasVideoPermission,
icon: 'icon-volume',
isDisabled: this.props.disableDeviceChange,
key: 'audioOutput',
label: 'settings.selectAudioOutput',
onSelect: this._getAndSetAudioOutput,
selectedDeviceId: currentAudioOutId
onSelect: this._updateAudioOutput,
selectedDeviceId: this.state.selectedAudioOutputId
});
}
return configurations.map(this._renderSelector);
}
/**
* Callback invoked when a new audio input device has been selected. Updates
* the internal state of the user's selection as well as the audio track
* that should display in the preview.
*
* @param {string} deviceId - The id of the chosen audio input device.
* @private
* @returns {void}
*/
_updateAudioInput(deviceId) {
this.setState({
selectedAudioInputId: deviceId
}, () => {
this._disposeAudioPreview()
.then(() => createLocalTrack('audio', deviceId))
.then(jitsiLocalTrack => {
this.setState({
previewAudioTrack: jitsiLocalTrack
});
})
.catch(() => {
this.setState({
previewAudioTrack: null
});
});
});
}
/**
* Callback invoked when a new audio output device has been selected.
* Updates the internal state of the user's selection.
*
* @param {string} deviceId - The id of the chosen audio output device.
* @private
* @returns {void}
*/
_updateAudioOutput(deviceId) {
this.setState({
selectedAudioOutputId: deviceId
});
}
/**
* Callback invoked when a new video input device has been selected. Updates
* the internal state of the user's selection as well as the video track
* that should display in the preview.
*
* @param {string} deviceId - The id of the chosen video input device.
* @private
* @returns {void}
*/
_updateVideoInput(deviceId) {
this.setState({
selectedVideoInputId: deviceId
}, () => {
this._disposeVideoPreview()
.then(() => createLocalTrack('video', deviceId))
.then(jitsiLocalTrack => {
this.setState({
previewVideoTrack: jitsiLocalTrack,
previewVideoTrackError: null
});
})
.catch(() => {
this.setState({
previewVideoTrack: null,
previewVideoTrackError:
this.props.t('deviceSelection.previewUnavailable')
});
});
});
}
}
/**
@ -588,12 +517,12 @@ class DeviceSelectionDialog extends Component {
* @param {Object} state - The Redux state.
* @private
* @returns {{
* _devices: Object
* _availableDevices: Object
* }}
*/
function _mapStateToProps(state) {
return {
_devices: state['features/base/devices']
_availableDevices: state['features/base/devices']
};
}

102
react/features/device-selection/components/VideoInputPreview.js
View File

@ -2,7 +2,7 @@ import React, { Component } from 'react';
import { translate } from '../../base/i18n';
const VIDEO_MUTE_CLASS = 'video-muted';
const VIDEO_ERROR_CLASS = 'video-preview-has-error';
/**
* React component for displaying video. This component defers to lib-jitsi-meet
@ -17,12 +17,18 @@ class VideoInputPreview extends Component {
* @static
*/
static propTypes = {
/**
* An error message to display instead of a preview. Displaying an error
* will take priority over displaying a video preview.
*/
error: React.PropTypes.string,
/**
* Invoked to obtain translated strings.
*/
t: React.PropTypes.func,
/*
/**
* The JitsiLocalTrack to display.
*/
track: React.PropTypes.object
@ -37,9 +43,37 @@ class VideoInputPreview extends Component {
constructor(props) {
super(props);
/**
* The internal reference to the DOM/HTML element intended for showing
* error messages.
*
* @private
* @type {HTMLDivElement}
*/
this._errorElement = null;
/**
* The internal reference to topmost DOM/HTML element backing the React
* {@code Component}. Accessed directly for toggling a classname to
* indicate an error is present so styling can be changed to display it.
*
* @private
* @type {HTMLDivElement}
*/
this._rootElement = null;
/**
* The internal reference to the DOM/HTML element intended for
* displaying a video. This element may be an HTML video element or a
* temasys video object.
*
* @private
* @type {HTMLVideoElement|Object}
*/
this._videoElement = null;
// Bind event handlers so they are only bound once for every instance.
this._setErrorElement = this._setErrorElement.bind(this);
this._setRootElement = this._setRootElement.bind(this);
this._setVideoElement = this._setVideoElement.bind(this);
}
@ -51,7 +85,11 @@ class VideoInputPreview extends Component {
* @returns {void}
*/
componentDidMount() {
this._attachTrack(this.props.track);
if (this.props.error) {
this._updateErrorView(this.props.error);
} else {
this._attachTrack(this.props.track);
}
}
/**
@ -80,9 +118,9 @@ class VideoInputPreview extends Component {
autoPlay = { true }
className = 'video-input-preview-display flipVideoX'
ref = { this._setVideoElement } />
<div className = 'video-input-preview-muted'>
{ this.props.t('deviceSelection.currentlyVideoMuted') }
</div>
<div
className = 'video-input-preview-error'
ref = { this._setErrorElement } />
</div>
);
}
@ -99,8 +137,15 @@ class VideoInputPreview extends Component {
* @returns {void}
*/
shouldComponentUpdate(nextProps) {
if (nextProps.track !== this.props.track) {
const hasNewTrack = nextProps.track !== this.props.track;
if (hasNewTrack || nextProps.error) {
this._detachTrack(this.props.track);
this._updateErrorView(nextProps.error);
}
// Never attempt to show the new track if there is an error present.
if (hasNewTrack && !nextProps.error) {
this._attachTrack(nextProps.track);
}
@ -123,17 +168,9 @@ class VideoInputPreview extends Component {
return;
}
// Do not attempt to display a preview if the track is muted, as the
// library will simply return a falsy value for the element anyway.
if (track.isMuted()) {
this._showMuteOverlay(true);
} else {
this._showMuteOverlay(false);
const updatedVideoElement = track.attach(this._videoElement);
const updatedVideoElement = track.attach(this._videoElement);
this._setVideoElement(updatedVideoElement);
}
this._setVideoElement(updatedVideoElement);
}
/**
@ -159,6 +196,19 @@ class VideoInputPreview extends Component {
}
}
/**
* Sets an instance variable for the component's element intended for
* displaying error messages. The element will be accessed directly to
* display an error message.
*
* @param {Object} element - DOM element intended for displaying errors.
* @private
* @returns {void}
*/
_setErrorElement(element) {
this._errorElement = element;
}
/**
* Sets the component's root element.
*
@ -183,20 +233,22 @@ class VideoInputPreview extends Component {
}
/**
* Adds or removes a class to the component's parent node to indicate mute
* status.
* Adds or removes a class to the component's parent node to indicate an
* error has occurred. Also sets the error text.
*
* @param {boolean} shouldShow - True if the mute class should be added and
* false if the class should be removed.
* @param {string} error - The error message to display. If falsy, error
* message display will be hidden.
* @private
* @returns {void}
*/
_showMuteOverlay(shouldShow) {
if (shouldShow) {
this._rootElement.classList.add(VIDEO_MUTE_CLASS);
_updateErrorView(error) {
if (error) {
this._rootElement.classList.add(VIDEO_ERROR_CLASS);
} else {
this._rootElement.classList.remove(VIDEO_MUTE_CLASS);
this._rootElement.classList.remove(VIDEO_ERROR_CLASS);
}
this._errorElement.innerText = error || '';
}
}