From 03de63175e7526f6336bdcc75dcc318d0952cdb1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= <danxuliu@gmail.com>
Date: Tue, 3 Mar 2020 10:28:29 +0100
Subject: [PATCH 1/2] Move audio element from view to model
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If a Video view is removed its audio element no longer plays the audio.
This can happen, for example, when closing the sidebar in the Files app,
which removes the call view and with it its Video views. But in that
case, even if there is no call view, the call is still active. To ensure
that the audio for another participant is played even if there is no
view for it the audio element must be part of the model instead.

Note that when closing the sidebar the audio was still heard, but that
is caused by a duplicated audio element created in the SimpleWebRTC
code (which will be fixed in a following commit).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 src/components/CallView/Video.vue               | 6 +++---
 src/utils/webrtc/models/CallParticipantModel.js | 8 ++++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/components/CallView/Video.vue b/src/components/CallView/Video.vue
index 174332660..36868dff5 100644
--- a/src/components/CallView/Video.vue
+++ b/src/components/CallView/Video.vue
@@ -23,7 +23,6 @@
 		:id="(placeholderForPromoted ? 'placeholder-' : '') + 'container_' + model.attributes.peerId + '_video_incoming'"
 		class="videoContainer"
 		:class="containerClass">
-		<audio v-if="!placeholderForPromoted" ref="audio" class="hidden" />
 		<video v-if="!placeholderForPromoted" v-show="model.attributes.videoAvailable && sharedData.videoEnabled" ref="video" />
 		<div v-if="!placeholderForPromoted" v-show="!model.attributes.videoAvailable || !sharedData.videoEnabled" class="avatar-container">
 			<Avatar v-if="model.attributes.userId"
@@ -213,11 +212,12 @@ export default {
 				return
 			}
 
-			// If there is a video track Chromium does not play audio in a video
+			// The audio is played using an audio element in the model to be
+			// able to hear it even if there is no view for it. Moreover, if
+			// there is a video track Chromium does not play audio in a video
 			// element until the video track starts to play; an audio element is
 			// thus needed to play audio when the remote peer starts with the
 			// camera available but disabled.
-			attachMediaStream(stream, this.$refs.audio, { audio: true })
 			attachMediaStream(stream, this.$refs.video)
 
 			this.$refs.video.muted = true
diff --git a/src/utils/webrtc/models/CallParticipantModel.js b/src/utils/webrtc/models/CallParticipantModel.js
index 03fd40385..e1d20daa2 100644
--- a/src/utils/webrtc/models/CallParticipantModel.js
+++ b/src/utils/webrtc/models/CallParticipantModel.js
@@ -19,6 +19,8 @@
  *
  */
 
+import attachMediaStream from 'attachmediastream'
+
 export const ConnectionState = {
 	NEW: 'new',
 	CHECKING: 'checking',
@@ -41,6 +43,9 @@ export default function CallParticipantModel(options) {
 		name: undefined,
 		connectionState: ConnectionState.NEW,
 		stream: null,
+		// The audio element is part of the model to ensure that it can be
+		// played if needed even if there is no view for it.
+		audioElement: null,
 		audioAvailable: undefined,
 		speaking: undefined,
 		videoAvailable: undefined,
@@ -81,6 +86,7 @@ CallParticipantModel.prototype = {
 	_handlePeerStreamAdded: function(peer) {
 		if (this._peer === peer) {
 			this.set('stream', this._peer.stream || null)
+			this.set('audioElement', attachMediaStream(this.get('stream'), null, { audio: true }))
 
 			// "peer.nick" is set only for users and when the MCU is not used.
 			if (this._peer.nick !== undefined) {
@@ -93,6 +99,8 @@ CallParticipantModel.prototype = {
 
 	_handlePeerStreamRemoved: function(peer) {
 		if (this._peer === peer) {
+			this.get('audioElement').srcObject = null
+			this.set('audioElement', null)
 			this.set('stream', null)
 			this.set('audioAvailable', undefined)
 			this.set('speaking', undefined)

From 57eb82f7754126d7f27c24ed2df51e0651bd5d37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= <danxuliu@gmail.com>
Date: Tue, 3 Mar 2020 10:39:23 +0100
Subject: [PATCH 2/2] Do not create audio and video elements without container
 in SimpleWebRTC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

SimpleWebRTC automatically created an audio and video elements when a
peer stream was added. Those elements were ignored by the Talk code
(which creates its own elements as needed), so they were not needed but
active nevertheless.

In the case of the audio element this caused its audio to always be
played, which would keep the audio from the other participant active
even if the audio element handled by Talk was removed or muted.

For consistency with the rest of its code now SimpleWebRTC creates the
audio and video elements for a peer only if a container for them is
provided.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
---
 src/utils/webrtc/simplewebrtc/simplewebrtc.js | 50 +++++++++----------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/src/utils/webrtc/simplewebrtc/simplewebrtc.js b/src/utils/webrtc/simplewebrtc/simplewebrtc.js
index dffcb7acc..27fee3519 100644
--- a/src/utils/webrtc/simplewebrtc/simplewebrtc.js
+++ b/src/utils/webrtc/simplewebrtc/simplewebrtc.js
@@ -263,35 +263,35 @@ SimpleWebRTC.prototype.disconnect = function() {
 SimpleWebRTC.prototype.handlePeerStreamAdded = function(peer) {
 	const self = this
 	const container = this.getRemoteVideoContainer()
-	// If there is a video track Chromium does not play audio in a video element
-	// until the video track starts to play; an audio element is thus needed to
-	// play audio when the remote peer starts with the camera available but
-	// disabled.
-	const audio = attachMediaStream(peer.stream, null, { audio: true })
-	const video = attachMediaStream(peer.stream)
-
-	video.muted = true
-
-	// At least Firefox, Opera and Edge move the video to a wrong position
-	// instead of keeping it unchanged when "transform: scaleX(1)" is used
-	// ("transform: scaleX(-1)" is fine); as it should have no effect the
-	// transform is removed.
-	if (video.style.transform === 'scaleX(1)') {
-		video.style.transform = ''
-	}
-
-	// store video element as part of peer for easy removal
-	peer.audioEl = audio
-	peer.videoEl = video
-	audio.id = this.getDomId(peer) + '-audio'
-	video.id = this.getDomId(peer)
-
 	if (container) {
+		// If there is a video track Chromium does not play audio in a video element
+		// until the video track starts to play; an audio element is thus needed to
+		// play audio when the remote peer starts with the camera available but
+		// disabled.
+		const audio = attachMediaStream(peer.stream, null, { audio: true })
+		const video = attachMediaStream(peer.stream)
+
+		video.muted = true
+
+		// At least Firefox, Opera and Edge move the video to a wrong position
+		// instead of keeping it unchanged when "transform: scaleX(1)" is used
+		// ("transform: scaleX(-1)" is fine); as it should have no effect the
+		// transform is removed.
+		if (video.style.transform === 'scaleX(1)') {
+			video.style.transform = ''
+		}
+
+		// store video element as part of peer for easy removal
+		peer.audioEl = audio
+		peer.videoEl = video
+		audio.id = this.getDomId(peer) + '-audio'
+		video.id = this.getDomId(peer)
+
 		container.appendChild(audio)
 		container.appendChild(video)
-	}
 
-	this.emit('videoAdded', video, audio, peer)
+		this.emit('videoAdded', video, audio, peer)
+	}
 
 	// send our mute status to new peer if we're muted
 	// currently called with a small delay because it arrives before