From 215f79f383186206153704a38eb9019389026616 Mon Sep 17 00:00:00 2001 From: prlanzarin Date: Thu, 14 Nov 2019 16:35:56 +0000 Subject: [PATCH 1/2] Account for mDNS candidates on gUM fallback for recvonly peers Also added some client logging for those cases Moved video-provider gUM fallback detection to be used only on recvonly streams --- .../ui/components/screenshare/service.js | 23 +++++++++-- .../components/video-provider/component.jsx | 28 +++++++------ .../ui/services/audio-manager/index.js | 20 ++++++---- .../imports/utils/safari-webrtc.js | 39 +++++++++++++------ 4 files changed, 75 insertions(+), 35 deletions(-) diff --git a/bigbluebutton-html5/imports/ui/components/screenshare/service.js b/bigbluebutton-html5/imports/ui/components/screenshare/service.js index a3d0638d7b..3d915a1543 100644 --- a/bigbluebutton-html5/imports/ui/components/screenshare/service.js +++ b/bigbluebutton-html5/imports/ui/components/screenshare/service.js @@ -1,6 +1,8 @@ import Screenshare from '/imports/api/screenshare'; import KurentoBridge from '/imports/api/screenshare/client/bridge'; import Settings from '/imports/ui/services/settings'; +import logger from '/imports/startup/client/logger'; +import { tryGenerateIceCandidates } from '../../../utils/safari-webrtc'; // when the meeting information has been updated check to see if it was // screensharing. If it has changed either trigger a call to receive video @@ -24,9 +26,24 @@ const presenterScreenshareHasEnded = () => { // if remote screenshare has been started connect and display the video stream const presenterScreenshareHasStarted = () => { - // references a function in the global namespace inside kurento-extension.js - // that we load dynamically - KurentoBridge.kurentoWatchVideo(); + // KurentoBridge.kurentoWatchVideo: references a function in the global + // namespace inside kurento-extension.js that we load dynamically + + // WebRTC restrictions may need a capture device permission to release + // useful ICE candidates on recvonly/no-gUM peers + tryGenerateIceCandidates().then(() => { + KurentoBridge.kurentoWatchVideo(); + }).catch(error => { + logger.error({ + logCode: 'screenshare_no_valid_candidate_gum_failure', + extraInfo: { + errorName: error.name, + errorMessage: error.message, + } + }, `Forced gUM to release additional ICE candidates failed due to ${error.name}.`); + // The fallback gUM failed. Try it anyways and hope for the best. + KurentoBridge.kurentoWatchVideo(); + }); }; const shareScreen = (onFail) => { diff --git a/bigbluebutton-html5/imports/ui/components/video-provider/component.jsx b/bigbluebutton-html5/imports/ui/components/video-provider/component.jsx index 5b4701b366..d1aee8fb7c 100755 --- a/bigbluebutton-html5/imports/ui/components/video-provider/component.jsx +++ b/bigbluebutton-html5/imports/ui/components/video-provider/component.jsx @@ -209,7 +209,6 @@ class VideoProvider extends Component { } componentDidMount() { - this.checkIceConnectivity(); document.addEventListener('joinVideo', this.shareWebcam); // TODO find a better way to do this document.addEventListener('exitVideo', this.unshareWebcam); this.ws.onmessage = this.onWsMessage; @@ -374,17 +373,6 @@ class VideoProvider extends Component { } } - checkIceConnectivity() { - // Webkit ICE restrictions demand a capture device permission to release - // host candidates - if (browser().name === 'safari') { - const { intl } = this.props; - tryGenerateIceCandidates().catch(() => { - VideoProvider.notifyError(intl.formatMessage(intlSFUErrors[2021])); - }); - } - } - _sendPauseStream(id, role, state) { this.sendMessage({ cameraId: id, @@ -584,6 +572,22 @@ class VideoProvider extends Component { this.webRtcPeers[id] = {}; + // WebRTC restrictions may need a capture device permission to release + // useful ICE candidates on recvonly/no-gUM peers + if (!shareWebcam) { + try { + await tryGenerateIceCandidates(); + } catch (error) { + logger.error({ + logCode: 'video_provider_no_valid_candidate_gum_failure', + extraInfo: { + errorName: error.name, + errorMessage: error.message, + } + }, `Forced gUM to release additional ICE candidates failed due to ${error.name}.`); + }; + } + try { iceServers = await fetchWebRTCMappedStunTurnServers(sessionToken); } catch (error) { diff --git a/bigbluebutton-html5/imports/ui/services/audio-manager/index.js b/bigbluebutton-html5/imports/ui/services/audio-manager/index.js index 442699f6c9..e7ea715e31 100755 --- a/bigbluebutton-html5/imports/ui/services/audio-manager/index.js +++ b/bigbluebutton-html5/imports/ui/services/audio-manager/index.js @@ -163,14 +163,18 @@ class AudioManager { inputStream: this.createListenOnlyStream(), }; - // Webkit ICE restrictions demand a capture device permission to release - // host candidates - if (name === 'safari') { - try { - await tryGenerateIceCandidates(); - } catch (e) { - this.notify(this.intl.formatMessage(this.messages.error.ICE_NEGOTIATION_FAILED)); - } + // WebRTC restrictions may need a capture device permission to release + // useful ICE candidates on recvonly/no-gUM peers + try { + await tryGenerateIceCandidates(); + } catch (error) { + logger.error({ + logCode: 'listenonly_no_valid_candidate_gum_failure', + extraInfo: { + errorName: error.name, + errorMessage: error.message, + } + }, `Forced gUM to release additional ICE candidates failed due to ${error.name}.`); } // Call polyfills for webrtc client if navigator is "iOS Webview" diff --git a/bigbluebutton-html5/imports/utils/safari-webrtc.js b/bigbluebutton-html5/imports/utils/safari-webrtc.js index e7dcb72558..f1101cc146 100644 --- a/bigbluebutton-html5/imports/utils/safari-webrtc.js +++ b/bigbluebutton-html5/imports/utils/safari-webrtc.js @@ -1,6 +1,7 @@ import { fetchWebRTCMappedStunTurnServers } from '/imports/utils/fetchStunTurnServers'; import Auth from '/imports/ui/services/auth'; import { Session } from 'meteor/session'; +import logger from '/imports/startup/client/logger'; const defaultIceServersList = [ { urls: 'stun:stun.l.google.com:19302' }, @@ -39,16 +40,15 @@ export function canGenerateIceCandidates() { } getIceServersList().catch((e) => { - reject(); + reject(e); }).then((iceServersReceived) => { const pc = new RTCPeerConnection({ iceServers: iceServersReceived }); let countIceCandidates = 0; try { pc.addTransceiver('audio'); } catch (e) { } - pc.onicecandidate = function (e) { if (countIceCandidates) return; - if (e.candidate) { + if (e.candidate && e.candidate.candidate.indexOf('.local') === -1) { countIceCandidates++; Session.set('canGenerateIceCandidates', true); resolve(); @@ -56,7 +56,10 @@ export function canGenerateIceCandidates() { }; pc.onicegatheringstatechange = function (e) { - if (e.currentTarget.iceGatheringState == 'complete' && countIceCandidates == 0) reject(); + if (e.currentTarget.iceGatheringState === 'complete' && countIceCandidates === 0) { + logger.warn({ logCode: 'no_valid_candidate' }, 'No useful ICE candidate found. Will request gUM permission.'); + reject(); + } }; setTimeout(() => { @@ -70,19 +73,31 @@ export function canGenerateIceCandidates() { }); } +/* + * Try to generate candidates for a recvonly RTCPeerConnection without + * a gUM permission and check if there are any candidates generated other than + * a mDNS host candidate. If there aren't, forcefully request gUM permission + * for mic (best chance of a gUM working is mic) to try and make the browser + * generate at least srflx candidates. + * This is a workaround due to a behaviour some browsers display (mainly Safari) + * where they won't generate srflx or relay candidates if no gUM permission is + * given. Since our media servers aren't able to make it work by prflx + * candidates, we need to do this. + */ export function tryGenerateIceCandidates() { return new Promise((resolve, reject) => { - canGenerateIceCandidates().then((ok) => { + canGenerateIceCandidates().then(() => { resolve(); - }).catch((e) => { - navigator.mediaDevices.getUserMedia({ audio: true, video: false }).then((stream) => { - canGenerateIceCandidates().then((ok) => { + }).catch(() => { + navigator.mediaDevices.getUserMedia({ audio: true, video: false }).then(() => { + logger.info({ logCode: 'no_valid_candidate_gum_success' }, 'Forced gUM to release additional ICE candidates succeeded.'); + canGenerateIceCandidates().then(() => { resolve(); - }).catch((e) => { - reject(); + }).catch((error) => { + reject(error); }); - }).catch((e) => { - reject(); + }).catch((error) => { + reject(error); }); }); }); From b52214f0b5f199f5a17dec4191555dbd8a10ffdb Mon Sep 17 00:00:00 2001 From: prlanzarin Date: Thu, 14 Nov 2019 19:07:35 +0000 Subject: [PATCH 2/2] Make some imports use absolute paths Some more linting as well --- .../imports/ui/components/screenshare/service.js | 6 +++--- .../imports/ui/components/video-provider/component.jsx | 6 +++--- .../imports/ui/services/audio-manager/index.js | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/bigbluebutton-html5/imports/ui/components/screenshare/service.js b/bigbluebutton-html5/imports/ui/components/screenshare/service.js index 3d915a1543..48575d89a8 100644 --- a/bigbluebutton-html5/imports/ui/components/screenshare/service.js +++ b/bigbluebutton-html5/imports/ui/components/screenshare/service.js @@ -2,7 +2,7 @@ import Screenshare from '/imports/api/screenshare'; import KurentoBridge from '/imports/api/screenshare/client/bridge'; import Settings from '/imports/ui/services/settings'; import logger from '/imports/startup/client/logger'; -import { tryGenerateIceCandidates } from '../../../utils/safari-webrtc'; +import { tryGenerateIceCandidates } from '/imports/utils/safari-webrtc'; // when the meeting information has been updated check to see if it was // screensharing. If it has changed either trigger a call to receive video @@ -33,13 +33,13 @@ const presenterScreenshareHasStarted = () => { // useful ICE candidates on recvonly/no-gUM peers tryGenerateIceCandidates().then(() => { KurentoBridge.kurentoWatchVideo(); - }).catch(error => { + }).catch((error) => { logger.error({ logCode: 'screenshare_no_valid_candidate_gum_failure', extraInfo: { errorName: error.name, errorMessage: error.message, - } + }, }, `Forced gUM to release additional ICE candidates failed due to ${error.name}.`); // The fallback gUM failed. Try it anyways and hope for the best. KurentoBridge.kurentoWatchVideo(); diff --git a/bigbluebutton-html5/imports/ui/components/video-provider/component.jsx b/bigbluebutton-html5/imports/ui/components/video-provider/component.jsx index d1aee8fb7c..fa2493c59a 100755 --- a/bigbluebutton-html5/imports/ui/components/video-provider/component.jsx +++ b/bigbluebutton-html5/imports/ui/components/video-provider/component.jsx @@ -16,7 +16,7 @@ import { updateWebcamStats, } from '/imports/ui/services/network-information/index'; -import { tryGenerateIceCandidates } from '../../../utils/safari-webrtc'; +import { tryGenerateIceCandidates } from '/imports/utils/safari-webrtc'; import Auth from '/imports/ui/services/auth'; import VideoService from './service'; @@ -583,9 +583,9 @@ class VideoProvider extends Component { extraInfo: { errorName: error.name, errorMessage: error.message, - } + }, }, `Forced gUM to release additional ICE candidates failed due to ${error.name}.`); - }; + } } try { diff --git a/bigbluebutton-html5/imports/ui/services/audio-manager/index.js b/bigbluebutton-html5/imports/ui/services/audio-manager/index.js index e7ea715e31..4436f2e7dc 100755 --- a/bigbluebutton-html5/imports/ui/services/audio-manager/index.js +++ b/bigbluebutton-html5/imports/ui/services/audio-manager/index.js @@ -7,8 +7,8 @@ import logger from '/imports/startup/client/logger'; import { notify } from '/imports/ui/services/notification'; import browser from 'browser-detect'; import playAndRetry from '/imports/utils/mediaElementPlayRetry'; -import iosWebviewAudioPolyfills from '../../../utils/ios-webview-audio-polyfills'; -import { tryGenerateIceCandidates } from '../../../utils/safari-webrtc'; +import iosWebviewAudioPolyfills from '/imports/utils/ios-webview-audio-polyfills'; +import { tryGenerateIceCandidates } from '/imports/utils/safari-webrtc'; import AudioErrors from './error-codes'; const MEDIA = Meteor.settings.public.media; @@ -173,7 +173,7 @@ class AudioManager { extraInfo: { errorName: error.name, errorMessage: error.message, - } + }, }, `Forced gUM to release additional ICE candidates failed due to ${error.name}.`); }