From 841041123642c7274d285529873d3b92f52d2cc5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=A0imon=20Brandner?= <simon.bra.ag@gmail.com>
Date: Wed, 10 Mar 2021 08:31:01 +0100
Subject: [PATCH] Handle audio
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
---
 src/CallHandler.tsx                     | 22 --------
 src/CallMediaHandler.js                 |  3 -
 src/components/views/voip/VideoFeed.tsx | 75 +++++++++++++++++--------
 3 files changed, 52 insertions(+), 48 deletions(-)

diff --git a/src/CallHandler.tsx b/src/CallHandler.tsx
index e090270c95..60c440d145 100644
--- a/src/CallHandler.tsx
+++ b/src/CallHandler.tsx
@@ -137,21 +137,6 @@ export enum PlaceCallType {
     ScreenSharing = 'screensharing',
 }
 
-function getRemoteAudioElement(): HTMLAudioElement {
-    // this needs to be somewhere at the top of the DOM which
-    // always exists to avoid audio interruptions.
-    // Might as well just use DOM.
-    const remoteAudioElement = document.getElementById("remoteAudio") as HTMLAudioElement;
-    if (!remoteAudioElement) {
-        console.error(
-            "Failed to find remoteAudio element - cannot play audio!" +
-            "You need to add an <audio/> to the DOM.",
-        );
-        return null;
-    }
-    return remoteAudioElement;
-}
-
 export default class CallHandler {
     private calls = new Map<string, MatrixCall>(); // roomId -> call
     private audioPromises = new Map<AudioID, Promise<void>>();
@@ -538,11 +523,6 @@ export default class CallHandler {
         }
     }
 
-    private setCallAudioElement(call: MatrixCall) {
-        const audioElement = getRemoteAudioElement();
-        if (audioElement) call.setRemoteAudioElement(audioElement);
-    }
-
     private setCallState(call: MatrixCall, status: CallState) {
         const mappedRoomId = CallHandler.roomIdForCall(call);
 
@@ -635,7 +615,6 @@ export default class CallHandler {
         this.calls.set(roomId, call);
 
         this.setCallListeners(call);
-        this.setCallAudioElement(call);
 
         this.setActiveCallRoomId(roomId);
 
@@ -787,7 +766,6 @@ export default class CallHandler {
 
                 const call = this.calls.get(payload.room_id);
                 call.answer();
-                this.setCallAudioElement(call);
                 this.setActiveCallRoomId(payload.room_id);
                 CountlyAnalytics.instance.trackJoinCall(payload.room_id, call.type === CallType.Video, false);
                 dis.dispatch({
diff --git a/src/CallMediaHandler.js b/src/CallMediaHandler.js
index 8d56467c57..2a619fe7aa 100644
--- a/src/CallMediaHandler.js
+++ b/src/CallMediaHandler.js
@@ -50,18 +50,15 @@ export default {
     },
 
     loadDevices: function() {
-        const audioOutDeviceId = SettingsStore.getValue("webrtc_audiooutput");
         const audioDeviceId = SettingsStore.getValue("webrtc_audioinput");
         const videoDeviceId = SettingsStore.getValue("webrtc_videoinput");
 
-        Matrix.setMatrixCallAudioOutput(audioOutDeviceId);
         Matrix.setMatrixCallAudioInput(audioDeviceId);
         Matrix.setMatrixCallVideoInput(videoDeviceId);
     },
 
     setAudioOutput: function(deviceId) {
         SettingsStore.setValue("webrtc_audiooutput", null, SettingLevel.DEVICE, deviceId);
-        Matrix.setMatrixCallAudioOutput(deviceId);
     },
 
     setAudioInput: function(deviceId) {
diff --git a/src/components/views/voip/VideoFeed.tsx b/src/components/views/voip/VideoFeed.tsx
index be674630b3..4ad41b322e 100644
--- a/src/components/views/voip/VideoFeed.tsx
+++ b/src/components/views/voip/VideoFeed.tsx
@@ -23,6 +23,7 @@ import { MatrixClientPeg } from '../../../MatrixClientPeg';
 import { logger } from 'matrix-js-sdk/src/logger';
 import MemberAvatar from "../avatars/MemberAvatar"
 import CallHandler from '../../../CallHandler';
+import CallMediaHandler from "../../../CallMediaHandler";
 
 interface IProps {
     call: MatrixCall,
@@ -45,7 +46,8 @@ interface IState {
 }
 
 export default class VideoFeed extends React.Component<IProps, IState> {
-    private vid = createRef<HTMLVideoElement>();
+    private video = createRef<HTMLVideoElement>();
+    private audio = createRef<HTMLAudioElement>();
 
     constructor(props: IProps) {
         super(props);
@@ -57,38 +59,64 @@ export default class VideoFeed extends React.Component<IProps, IState> {
 
     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;
+
+        const audioOutput = CallMediaHandler.getAudioOutput();
+        const currentMedia = this.getCurrentMedia();
+
+        currentMedia.srcObject = this.props.feed.stream;
+        currentMedia.autoplay = true;
+        currentMedia.muted = false;
+
         try {
-            this.vid.current.play();
+            if (audioOutput) {
+                // This seems quite unreliable in Chrome, although I haven't yet managed to make a jsfiddle where
+                // it fails.
+                // It seems reliable if you set the sink ID after setting the srcObject and then set the sink ID
+                // back to the default after the call is over - Dave
+                currentMedia.setSinkId(audioOutput);
+            }
         } catch (e) {
-            logger.info("Failed to play video element with feed", this.props.feed, e);
+            console.error("Couldn't set requested audio output device: using default", e);
+            logger.warn("Couldn't set requested audio output device: using default", e);
+        }
+
+        try {
+            // 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
+            currentMedia.play()
+        } catch (e) {
+            logger.info("Failed to play media 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;
+        this.video.current?.removeEventListener('resize', this.onResize);
+
+        const currentMedia = this.getCurrentMedia();
+        currentMedia.pause();
+        currentMedia.srcObject = null;
+        // As per comment in componentDidMount, setting the sink ID back to the
+        // default once the call is over makes setSinkId work reliably. - Dave
+        // Since we are not using the same element anymore, the above doesn't
+        // seem to be necessary - Šimon
+    }
+
+    getCurrentMedia() {
+        return this.audio.current || this.video.current;
     }
 
     onNewStream = (newStream: MediaStream) => {
         this.setState({ audioOnly: this.props.feed.isAudioOnly()});
-        if (!this.vid.current) return;
-        this.vid.current.srcObject = newStream;
+        const currentMedia = this.getCurrentMedia();
+        currentMedia.srcObject = newStream;
     }
 
     onResize = (e) => {
@@ -123,11 +151,12 @@ export default class VideoFeed extends React.Component<IProps, IState> {
                         height={avatarSize}
                         width={avatarSize}
                     />
+                    <audio ref={this.audio}></audio>
                 </div>
             );
         } else {
             return (
-                <video className={classnames(videoClasses)} ref={this.vid} />
+                <video className={classnames(videoClasses)} ref={this.video} />
             );
         }
     }