From bb13dc49a6d89a1f1a50c762b3c241fc5d4b9757 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sun, 7 Mar 2021 08:13:35 +0100 Subject: [PATCH] Make CallView use CallFeed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- res/css/views/voip/_CallView.scss | 1 + res/css/views/voip/_VideoFeed.scss | 18 +++- src/CallHandler.tsx | 12 +-- src/components/views/voip/CallView.tsx | 122 ++++++++++++++++++------ src/components/views/voip/VideoFeed.tsx | 100 ++++++++++++++----- 5 files changed, 185 insertions(+), 68 deletions(-) diff --git a/res/css/views/voip/_CallView.scss b/res/css/views/voip/_CallView.scss index 7e2d12e539..ed3ff2afa9 100644 --- a/res/css/views/voip/_CallView.scss +++ b/res/css/views/voip/_CallView.scss @@ -112,6 +112,7 @@ limitations under the License. z-index: 30; border-radius: 8px; overflow: hidden; + display: flex; } .mx_CallView_video_hold { diff --git a/res/css/views/voip/_VideoFeed.scss b/res/css/views/voip/_VideoFeed.scss index 8ead8bba3e..46cdf4f52c 100644 --- a/res/css/views/voip/_VideoFeed.scss +++ b/res/css/views/voip/_VideoFeed.scss @@ -14,11 +14,21 @@ See the License for the specific language governing permissions and limitations under the License. */ -.mx_VideoFeed_remote { - width: 100%; - height: 100%; +.mx_VideoFeed_voice { + // We don't want to collide with the call controls that have 52px of height + padding-bottom: 52px; + background-color: $inverted-bg-color; +} + +.mx_VideoFeed_video { background-color: #000; - z-index: 50; +} + +.mx_VideoFeed_remote { + flex: 1; + display: flex; + justify-content: center; + align-items: center; } .mx_VideoFeed_local { diff --git a/src/CallHandler.tsx b/src/CallHandler.tsx index 42a38c7a54..e090270c95 100644 --- a/src/CallHandler.tsx +++ b/src/CallHandler.tsx @@ -621,7 +621,6 @@ export default class CallHandler { private async placeCall( roomId: string, type: PlaceCallType, - localElement: HTMLVideoElement, remoteElement: HTMLVideoElement, ) { Analytics.trackEvent('voip', 'placeCall', 'type', type); CountlyAnalytics.instance.trackStartCall(roomId, type === PlaceCallType.Video, false); @@ -643,10 +642,7 @@ export default class CallHandler { if (type === PlaceCallType.Voice) { call.placeVoiceCall(); } else if (type === 'video') { - call.placeVideoCall( - remoteElement, - localElement, - ); + call.placeVideoCall(); } else if (type === PlaceCallType.ScreenSharing) { const screenCapErrorString = PlatformPeg.get().screenCaptureErrorString(); if (screenCapErrorString) { @@ -660,8 +656,6 @@ export default class CallHandler { } call.placeScreenSharingCall( - remoteElement, - localElement, async () : Promise => { const {finished} = Modal.createDialog(DesktopCapturerSourcePicker); const [source] = await finished; @@ -715,14 +709,12 @@ export default class CallHandler { } else if (members.length === 2) { console.info(`Place ${payload.type} call in ${payload.room_id}`); - this.placeCall(payload.room_id, payload.type, payload.local_element, payload.remote_element); + this.placeCall(payload.room_id, payload.type); } else { // > 2 dis.dispatch({ action: "place_conference_call", room_id: payload.room_id, type: payload.type, - remote_element: payload.remote_element, - local_element: payload.local_element, }); } } diff --git a/src/components/views/voip/CallView.tsx b/src/components/views/voip/CallView.tsx index fbd30cbc9b..86b17dcab2 100644 --- a/src/components/views/voip/CallView.tsx +++ b/src/components/views/voip/CallView.tsx @@ -20,7 +20,7 @@ import dis from '../../../dispatcher/dispatcher'; import CallHandler from '../../../CallHandler'; import {MatrixClientPeg} from '../../../MatrixClientPeg'; import { _t, _td } from '../../../languageHandler'; -import VideoFeed, { VideoFeedType } from "./VideoFeed"; +import VideoFeed from './VideoFeed'; import RoomAvatar from "../avatars/RoomAvatar"; import { CallState, CallType, MatrixCall, CallEvent } from 'matrix-js-sdk/src/webrtc/call'; import classNames from 'classnames'; @@ -30,6 +30,7 @@ import {alwaysAboveLeftOf, alwaysAboveRightOf, ChevronFace, ContextMenuButton} f import CallContextMenu from '../context_menus/CallContextMenu'; import { avatarUrlForMember } from '../../../Avatar'; import DialpadContextMenu from '../context_menus/DialpadContextMenu'; +import { CallFeed } from 'matrix-js-sdk/src/webrtc/callFeed'; interface IProps { // The call for us to display @@ -58,6 +59,7 @@ interface IState { controlsVisible: boolean, showMoreMenu: boolean, showDialpad: boolean, + feeds: CallFeed[], } function getFullScreenElement() { @@ -112,6 +114,7 @@ export default class CallView extends React.Component { controlsVisible: true, showMoreMenu: false, showDialpad: false, + feeds: this.props.call.getFeeds(), } this.updateCallListeners(null, this.props.call); @@ -169,11 +172,13 @@ export default class CallView extends React.Component { oldCall.removeListener(CallEvent.State, this.onCallState); oldCall.removeListener(CallEvent.LocalHoldUnhold, this.onCallLocalHoldUnhold); oldCall.removeListener(CallEvent.RemoteHoldUnhold, this.onCallRemoteHoldUnhold); + oldCall.removeListener(CallEvent.FeedsChanged, this.onFeedsChanged); } if (newCall) { newCall.on(CallEvent.State, this.onCallState); newCall.on(CallEvent.LocalHoldUnhold, this.onCallLocalHoldUnhold); newCall.on(CallEvent.RemoteHoldUnhold, this.onCallRemoteHoldUnhold); + newCall.on(CallEvent.FeedsChanged, this.onFeedsChanged); } } @@ -183,6 +188,10 @@ export default class CallView extends React.Component { }); }; + private onFeedsChanged = (newFeeds: Array) => { + this.setState({feeds: newFeeds}); + } + private onCallLocalHoldUnhold = () => { this.setState({ isLocalOnHold: this.props.call.isLocalOnHold(), @@ -486,44 +495,71 @@ export default class CallView extends React.Component { }); } - if (this.props.call.type === CallType.Video) { - let localVideoFeed = null; - let onHoldContent = null; - let onHoldBackground = null; - const backgroundStyle: CSSProperties = {}; - const containerClasses = classNames({ - mx_CallView_video: true, - mx_CallView_video_hold: isOnHold, - }); - if (isOnHold) { - onHoldContent =
- {onHoldText} -
; + const avatarSize = this.props.pipMode ? 76 : 160; + if (isOnHold) { + if (this.props.call.type === CallType.Video) { + const containerClasses = classNames({ + mx_CallView_video: true, + mx_CallView_video_hold: isOnHold, + }); + let onHoldContent = null; + let onHoldBackground = null; + const backgroundStyle: CSSProperties = {}; + onHoldContent = ( +
+ {onHoldText} +
+ ); const backgroundAvatarUrl = avatarUrlForMember( - // is it worth getting the size of the div to pass here? + // is it worth getting the size of the div to pass here? this.props.call.getOpponentMember(), 1024, 1024, 'crop', ); backgroundStyle.backgroundImage = 'url(' + backgroundAvatarUrl + ')'; onHoldBackground =
; - } - if (!this.state.vidMuted) { - localVideoFeed = ; - } - contentView =
- {onHoldBackground} - - {localVideoFeed} - {onHoldContent} - {callControls} -
; - } else { - const avatarSize = this.props.pipMode ? 76 : 160; + contentView = ( +
+ {onHoldBackground} + {onHoldContent} + {callControls} +
+ ); + } else { + const classes = classNames({ + mx_CallView_voice: true, + mx_CallView_voice_hold: isOnHold, + }); + + contentView =( +
+
+
+ +
+
+
{onHoldText}
+ {callControls} +
+ ); + } + } else if (this.props.call.noIncomingFeeds()) { + // Here we're reusing the css classes from voice on hold, because + // I am lazy. If this gets merged, the CallView might be subject + // to change anyway - I might take an axe to this file in order to + // try to get other things working const classes = classNames({ mx_CallView_voice: true, - mx_CallView_voice_hold: isOnHold, }); + // Saying "Connecting" here isn't really true, but the best thing + // I can come up with, but this might be subject to change as well contentView =
@@ -534,7 +570,33 @@ export default class CallView extends React.Component { />
-
{onHoldText}
+
{_t("Connecting")}
+ {callControls} +
; + } else { + const containerClasses = classNames({ + mx_CallView_video: true, + }); + + // TODO: Later the CallView should probably be reworked to support any + // number of feeds but now we can always expect there to be two feeds + const feeds = this.state.feeds.map((feed, i) => { + // Here we check to hide local audio feeds to achieve the same UI/UX + // as before. But once again this might be subject to change + if (feed.isAudioOnly() && feed.isLocal()) return; + return ( + + ); + }); + + contentView =
+ {feeds} {callControls}
; } diff --git a/src/components/views/voip/VideoFeed.tsx b/src/components/views/voip/VideoFeed.tsx index 1e950f3a2a..be674630b3 100644 --- a/src/components/views/voip/VideoFeed.tsx +++ b/src/components/views/voip/VideoFeed.tsx @@ -18,50 +18,81 @@ import classnames from 'classnames'; import { MatrixCall } from 'matrix-js-sdk/src/webrtc/call'; import React, {createRef} from 'react'; import SettingsStore from "../../../settings/SettingsStore"; - -export enum VideoFeedType { - Local, - Remote, -} +import { CallFeed, CallFeedEvent } from 'matrix-js-sdk/src/webrtc/callFeed'; +import { MatrixClientPeg } from '../../../MatrixClientPeg'; +import { logger } from 'matrix-js-sdk/src/logger'; +import MemberAvatar from "../avatars/MemberAvatar" +import CallHandler from '../../../CallHandler'; interface IProps { call: MatrixCall, - type: VideoFeedType, + feed: CallFeed, + + // Whether this call view is for picture-in-pictue mode + // otherwise, it's the larger call view when viewing the room the call is in. + // This is sort of a proxy for a number of things but we currently have no + // need to control those things separately, so this is simpler. + pipMode?: boolean; // a callback which is called when the video element is resized // due to a change in video metadata onResize?: (e: Event) => void, } -export default class VideoFeed extends React.Component { +interface IState { + audioOnly: boolean; +} + +export default class VideoFeed extends React.Component { private vid = createRef(); - componentDidMount() { - this.vid.current.addEventListener('resize', this.onResize); - this.setVideoElement(); + constructor(props: IProps) { + super(props); + + this.state = { + audioOnly: this.props.feed.isAudioOnly(), + }; } - componentDidUpdate(prevProps) { - if (this.props.call !== prevProps.call) { - this.setVideoElement(); + componentDidMount() { + this.props.feed.addListener(CallFeedEvent.NewStream, this.onNewStream); + if (!this.vid.current) return; + // A note on calling methods on media elements: + // We used to have queues per media element to serialise all calls on those elements. + // The reason given for this was that load() and play() were racing. However, we now + // never call load() explicitly so this seems unnecessary. However, serialising every + // operation was causing bugs where video would not resume because some play command + // had got stuck and all media operations were queued up behind it. If necessary, we + // should serialise the ones that need to be serialised but then be able to interrupt + // them with another load() which will cancel the pending one, but since we don't call + // load() explicitly, it shouldn't be a problem. - Dave + this.vid.current.srcObject = this.props.feed.stream; + this.vid.current.autoplay = true; + this.vid.current.muted = true; + try { + this.vid.current.play(); + } catch (e) { + logger.info("Failed to play video element with feed", this.props.feed, e); } } componentWillUnmount() { + this.props.feed.removeListener(CallFeedEvent.NewStream, this.onNewStream); + if (!this.vid.current) return; this.vid.current.removeEventListener('resize', this.onResize); + this.vid.current.pause(); + this.vid.current.srcObject = null; } - private setVideoElement() { - if (this.props.type === VideoFeedType.Local) { - this.props.call.setLocalVideoElement(this.vid.current); - } else { - this.props.call.setRemoteVideoElement(this.vid.current); - } + onNewStream = (newStream: MediaStream) => { + this.setState({ audioOnly: this.props.feed.isAudioOnly()}); + if (!this.vid.current) return; + this.vid.current.srcObject = newStream; } onResize = (e) => { - if (this.props.onResize) { + if (this.props.onResize && !this.props.feed.isLocal()) { this.props.onResize(e); } }; @@ -69,14 +100,35 @@ export default class VideoFeed extends React.Component { render() { const videoClasses = { mx_VideoFeed: true, - mx_VideoFeed_local: this.props.type === VideoFeedType.Local, - mx_VideoFeed_remote: this.props.type === VideoFeedType.Remote, + mx_VideoFeed_local: this.props.feed.isLocal(), + mx_VideoFeed_remote: !this.props.feed.isLocal(), + mx_VideoFeed_voice: this.state.audioOnly, + mx_VideoFeed_video: !this.state.audioOnly, mx_VideoFeed_mirror: ( - this.props.type === VideoFeedType.Local && + this.props.feed.isLocal() && SettingsStore.getValue('VideoView.flipVideoHorizontally') ), }; - return