From 244de8096fe9631efdac32546687be1aab3bc553 Mon Sep 17 00:00:00 2001 From: virtuacoplenny Date: Fri, 14 Jul 2017 12:22:27 -0700 Subject: [PATCH] feat(local-video): convert to react (#1705) * feat(local-video): convert to react - Create a VideoTrack component for displaying a video element. This mirrors native also having a VideoTrack component. - The VideoTrack component does not let React update it to prevent the video element from re-rendering, which could cause flickers and would not work with temasys's overriding of the video element. - VideoTrack extends AbstractVideoTrack to mirror native implementation and to get the dispatch of the onplaying event. - Remove the onclick handler on the video element. Honestly, I didn't get it to work, and did not try, but it is also unnecessary because another handler already exists on the video wrapper. * ref(device-selection): VideoInputPreview uses VideoTrack to show video * squash into conversion: change css selectors * squash into conversion: mix in abstract props * squash into conversion: change shouldComponentUpdate check * squash: update comment about why triggerOnPlayingUpdate is used --- css/_videolayout_default.scss | 8 +- modules/UI/videolayout/LocalVideo.js | 46 ++-- .../media/components/AbstractVideoTrack.js | 27 ++- .../base/media/components/web/VideoTrack.js | 202 ++++++++++++++++ .../base/media/components/web/index.js | 1 + .../components/VideoInputPreview.js | 224 +----------------- 6 files changed, 274 insertions(+), 234 deletions(-) create mode 100644 react/features/base/media/components/web/VideoTrack.js diff --git a/css/_videolayout_default.scss b/css/_videolayout_default.scss index f8e35391f..ba6dd7e76 100644 --- a/css/_videolayout_default.scss +++ b/css/_videolayout_default.scss @@ -157,8 +157,8 @@ -o-transform: scale(-1, 1); } -#localVideoWrapper>video, -#localVideoWrapper>object { +#localVideoWrapper video, +#localVideoWrapper object { border-radius: $borderRadius !important; cursor: hand; object-fit: cover; @@ -183,8 +183,8 @@ #sharedVideo, #etherpad, -#localVideoWrapper>video, -#localVideoWrapper>object, +#localVideoWrapper video, +#localVideoWrapper object, #localVideoWrapper, #largeVideoWrapper, #largeVideoWrapper>video, diff --git a/modules/UI/videolayout/LocalVideo.js b/modules/UI/videolayout/LocalVideo.js index 08c8c464f..ef639285e 100644 --- a/modules/UI/videolayout/LocalVideo.js +++ b/modules/UI/videolayout/LocalVideo.js @@ -1,11 +1,18 @@ /* global $, config, interfaceConfig, APP, JitsiMeetJS */ + +/* eslint-disable no-unused-vars */ +import React, { Component } from 'react'; +import ReactDOM from 'react-dom'; +import { Provider } from 'react-redux'; + +import { VideoTrack } from '../../../react/features/base/media'; +/* eslint-enable no-unused-vars */ + const logger = require("jitsi-meet-logger").getLogger(__filename); -import UIUtil from "../util/UIUtil"; import UIEvents from "../../../service/UI/UIEvents"; import SmallVideo from "./SmallVideo"; -const RTCUIUtils = JitsiMeetJS.util.RTCUIHelper; const TrackEvents = JitsiMeetJS.events.track; function LocalVideo(VideoLayout, emitter) { @@ -85,31 +92,34 @@ LocalVideo.prototype.changeVideo = function (stream) { localVideoContainerSelector.off('click'); localVideoContainerSelector.on('click', localVideoClick); - let localVideo = document.createElement('video'); - localVideo.id = this.localVideoId = 'localVideo_' + stream.getId(); - - RTCUIUtils.setAutoPlay(localVideo, true); - RTCUIUtils.setVolume(localVideo, 0); + this.localVideoId = 'localVideo_' + stream.getId(); var localVideoContainer = document.getElementById('localVideoWrapper'); - // Put the new video always in front - UIUtil.prependChild(localVideoContainer, localVideo); - // Add click handler to both video and video wrapper elements in case - // there's no video. - - // onclick has to be used with Temasys plugin - localVideo.onclick = localVideoClick; + /* jshint ignore:start */ + ReactDOM.render( + + + , + localVideoContainer + ); + /* jshint ignore:end */ let isVideo = stream.videoType != "desktop"; this._enableDisableContextMenu(isVideo); this.setFlipX(isVideo? APP.settings.getLocalFlipX() : false); - // Attach WebRTC stream - localVideo = stream.attach(localVideo); - let endedHandler = () => { - localVideoContainer.removeChild(localVideo); + // Only remove if there is no video and not a transition state. + // Previous non-react logic created a new video element with each track + // removal whereas react reuses the video component so it could be the + // stream ended but a new one is being used. + if (this.videoStream.isEnded()) { + ReactDOM.unmountComponentAtNode(localVideoContainer); + } + // when removing only the video element and we are on stage // update the stage if (this.isCurrentlyOnLargeVideo()) { diff --git a/react/features/base/media/components/AbstractVideoTrack.js b/react/features/base/media/components/AbstractVideoTrack.js index 4c7bbe25b..472e3d9e0 100644 --- a/react/features/base/media/components/AbstractVideoTrack.js +++ b/react/features/base/media/components/AbstractVideoTrack.js @@ -12,6 +12,18 @@ import { Video } from './_'; * @abstract */ export default class AbstractVideoTrack extends Component { + /** + * Default values for AbstractVideoTrack component's properties. + * + * @static + */ + static defaultProps = { + /** + * Dispatch an action when the video starts playing. + */ + triggerOnPlayingUpdate: true + }; + /** * AbstractVideoTrack component's property types. * @@ -19,7 +31,18 @@ export default class AbstractVideoTrack extends Component { */ static propTypes = { dispatch: React.PropTypes.func, + + /** + * Whether or not the store should be updated about the playing status + * of the video. Defaults to true. One use case for setting this prop + * to false is using multiple locals streams from the same video source, + * such as when previewing video. In those cases, the store may have no + * need to be updated about the existence or state of the stream. + */ + triggerOnPlayingUpdate: React.PropTypes.bool, + videoTrack: React.PropTypes.object, + waitForVideoStarted: React.PropTypes.bool, /** @@ -117,7 +140,9 @@ export default class AbstractVideoTrack extends Component { _onVideoPlaying() { const videoTrack = this.props.videoTrack; - if (videoTrack && !videoTrack.videoStarted) { + if (this.props.triggerOnPlayingUpdate + && videoTrack + && !videoTrack.videoStarted) { this.props.dispatch(trackVideoStarted(videoTrack.jitsiTrack)); } } diff --git a/react/features/base/media/components/web/VideoTrack.js b/react/features/base/media/components/web/VideoTrack.js new file mode 100644 index 000000000..7e891728d --- /dev/null +++ b/react/features/base/media/components/web/VideoTrack.js @@ -0,0 +1,202 @@ +import React from 'react'; +import { connect } from 'react-redux'; + +import AbstractVideoTrack from '../AbstractVideoTrack'; + +/** + * Component that renders a video element for a passed in video track. + * + * @extends AbstractVideoTrack + */ +class VideoTrack extends AbstractVideoTrack { + /** + * Default values for {@code VideoTrack} component's properties. + * + * @static + */ + static defaultProps = { + ...AbstractVideoTrack.defaultProps, + + className: '', + + id: '' + }; + + /** + * {@code VideoTrack} component's property types. + * + * @static + */ + static propTypes = { + ...AbstractVideoTrack.propTypes, + + /** + * CSS classes to add to the video element. + */ + className: React.PropTypes.string, + + /** + * The value of the id attribute of the video. Used by the torture tests + * to locate video elements. + */ + id: React.PropTypes.string + }; + + /** + * Initializes a new VideoTrack instance. + * + * @param {Object} props - The read-only properties with which the new + * instance is to be initialized. + */ + constructor(props) { + super(props); + + /** + * 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._setVideoElement = this._setVideoElement.bind(this); + } + + /** + * Invokes the library for rendering the video on initial display. Sets the + * volume level to zero to ensure no sound plays. + * + * @inheritdoc + * @returns {void} + */ + componentDidMount() { + // Add these attributes directly onto the video element so temasys can + // use them when converting the video to an object. + this._videoElement.volume = 0; + this._videoElement.onplaying = this._onVideoPlaying; + + this._attachTrack(this.props.videoTrack); + } + + /** + * Remove any existing associations between the current video track and the + * component's video element. + * + * @inheritdoc + * @returns {void} + */ + componentWillUnmount() { + this._detachTrack(this.props.videoTrack); + } + + /** + * Updates the video display only if a new track is added. This component's + * updating is blackboxed from React to prevent re-rendering of video + * element, as the lib uses track.attach(videoElement) instead. Also, + * re-rendering cannot be used with temasys, which replaces video elements + * with an object. + * + * @inheritdoc + * @returns {boolean} - False is always returned to blackbox this component. + * from React. + */ + shouldComponentUpdate(nextProps) { + const currentJitsiTrack = this.props.videoTrack + && this.props.videoTrack.jitsiTrack; + const nextJitsiTrack = nextProps.videoTrack + && nextProps.videoTrack.jitsiTrack; + + if (currentJitsiTrack !== nextJitsiTrack) { + this._detachTrack(this.props.videoTrack); + this._attachTrack(nextProps.videoTrack); + } + + return false; + } + + /** + * Renders the video element. + * + * @override + * @returns {ReactElement} + */ + render() { + // The wrapping div is necessary because temasys will replace the video + // with an object but react will keep expecting the video element. The + // div gives a constant element for react to keep track of. + return ( +
+
+ ); + } + + /** + * Calls into the passed in track to associate the track with the + * component's video element and render video. + * + * @param {Object} videoTrack - The redux representation of the + * {@code JitsiLocalTrack}. + * @private + * @returns {void} + */ + _attachTrack(videoTrack) { + if (!videoTrack || !videoTrack.jitsiTrack) { + return; + } + + const updatedVideoElement + = videoTrack.jitsiTrack.attach(this._videoElement); + + // Sets the instance variable for the video element again as the element + // maybe have been replaced with a new object by temasys. + this._setVideoElement(updatedVideoElement); + } + + /** + * Removes the association to the component's video element from the passed + * in redux representation of jitsi video track to stop the track from + * rendering. With temasys, the video element must still be visible for + * detaching to complete. + * + * @param {Object} videoTrack - The redux representation of the + * {@code JitsiLocalTrack}. + * @private + * @returns {void} + */ + _detachTrack(videoTrack) { + // Detach the video element from the track only if it has already + // been attached. This accounts for a special case with temasys + // where if detach is being called before attach, the video + // element is converted to Object without updating this + // component's reference to the video element. + if (this._videoElement + && videoTrack + && videoTrack.jitsiTrack + && videoTrack.jitsiTrack.containers.includes(this._videoElement)) { + videoTrack.jitsiTrack.detach(this._videoElement); + } + } + + /** + * Sets an instance variable for the component's video element so it can be + * referenced later for attaching and detaching a JitsiLocalTrack. + * + * @param {Object} element - DOM element for the component's video display. + * @private + * @returns {void} + */ + _setVideoElement(element) { + this._videoElement = element; + } +} + +export default connect()(VideoTrack); diff --git a/react/features/base/media/components/web/index.js b/react/features/base/media/components/web/index.js index 528c1e495..d7d5e1dc0 100644 --- a/react/features/base/media/components/web/index.js +++ b/react/features/base/media/components/web/index.js @@ -1 +1,2 @@ export { default as Audio } from './Audio'; +export { default as VideoTrack } from './VideoTrack'; diff --git a/react/features/device-selection/components/VideoInputPreview.js b/react/features/device-selection/components/VideoInputPreview.js index ef536f595..70921b933 100644 --- a/react/features/device-selection/components/VideoInputPreview.js +++ b/react/features/device-selection/components/VideoInputPreview.js @@ -1,6 +1,6 @@ import React, { Component } from 'react'; -import { translate } from '../../base/i18n'; +import { VideoTrack } from '../../base/media'; const VIDEO_ERROR_CLASS = 'video-preview-has-error'; @@ -23,86 +23,12 @@ class VideoInputPreview extends Component { */ error: React.PropTypes.string, - /** - * Invoked to obtain translated strings. - */ - t: React.PropTypes.func, - /** * The JitsiLocalTrack to display. */ track: React.PropTypes.object }; - /** - * Initializes a new VideoInputPreview instance. - * - * @param {Object} props - The read-only React Component props with which - * the new instance is to be initialized. - */ - 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); - } - - /** - * Invokes the library for rendering the video on initial display. - * - * @inheritdoc - * @returns {void} - */ - componentDidMount() { - if (this.props.error) { - this._updateErrorView(this.props.error); - } else { - this._attachTrack(this.props.track); - } - } - - /** - * Remove any existing associations between the current previewed track and - * the component's video element. - * - * @inheritdoc - * @returns {void} - */ - componentWillUnmount() { - this._detachTrack(this.props.track); - } - /** * Implements React's {@link Component#render()}. * @@ -110,146 +36,22 @@ class VideoInputPreview extends Component { * @returns {ReactElement} */ render() { + const { error } = this.props; + const errorClass = error ? VIDEO_ERROR_CLASS : ''; + const className = `video-input-preview ${errorClass}`; + return ( -
-