From fda45a7c86004dfee6e05dd748819fe195fd147f Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 10 Nov 2021 16:40:16 +0100 Subject: [PATCH] use implicit descriptions for WebRTC using the parameter-free form of setLocalDescription() solves some potential race conditions that can occur - especially once we introduce restartIce() --- .../xmpp/jingle/JingleRtpConnection.java | 24 ++---- .../xmpp/jingle/WebRTCWrapper.java | 75 ++++++------------- 2 files changed, 31 insertions(+), 68 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java index 8c4d14843..72d2a4837 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -537,9 +537,10 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web try { this.webRTCWrapper.setRemoteDescription(sdp).get(); addIceCandidatesFromBlackLog(); - org.webrtc.SessionDescription webRTCSessionDescription = this.webRTCWrapper.createAnswer().get(); + org.webrtc.SessionDescription webRTCSessionDescription = this.webRTCWrapper.setLocalDescription().get(); prepareSessionAccept(webRTCSessionDescription); } catch (final Exception e) { + //TODO sending the error text is worthwhile as well. Especially for FailureToSet exceptions failureToAcceptSession(e); } } @@ -569,7 +570,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web new FutureCallback() { @Override public void onSuccess(final RtpContentMap outgoingContentMap) { - sendSessionAccept(outgoingContentMap, webRTCSessionDescription); + sendSessionAccept(outgoingContentMap); } @Override @@ -581,7 +582,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web ); } - private void sendSessionAccept(final RtpContentMap rtpContentMap, final org.webrtc.SessionDescription webRTCSessionDescription) { + private void sendSessionAccept(final RtpContentMap rtpContentMap) { if (isTerminated()) { Log.w(Config.LOGTAG, id.account.getJid().asBareJid() + ": preparing session accept was too slow. already terminated. nothing to do."); return; @@ -589,11 +590,6 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web transitionOrThrow(State.SESSION_ACCEPTED); final JinglePacket sessionAccept = rtpContentMap.toJinglePacket(JinglePacket.Action.SESSION_ACCEPT, id.sessionId); send(sessionAccept); - try { - webRTCWrapper.setLocalDescription(webRTCSessionDescription).get(); - } catch (Exception e) { - failureToAcceptSession(e); - } } private ListenableFuture prepareOutgoingContentMap(final RtpContentMap rtpContentMap) { @@ -841,9 +837,10 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web return; } try { - org.webrtc.SessionDescription webRTCSessionDescription = this.webRTCWrapper.createOffer().get(); + org.webrtc.SessionDescription webRTCSessionDescription = this.webRTCWrapper.setLocalDescription().get(); prepareSessionInitiate(webRTCSessionDescription, targetState); } catch (final Exception e) { + //TODO sending the error text is worthwhile as well. Especially for FailureToSet exceptions failureToInitiateSession(e, targetState); } } @@ -877,7 +874,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web Futures.addCallback(outgoingContentMapFuture, new FutureCallback() { @Override public void onSuccess(final RtpContentMap outgoingContentMap) { - sendSessionInitiate(outgoingContentMap, webRTCSessionDescription, targetState); + sendSessionInitiate(outgoingContentMap, targetState); } @Override @@ -887,7 +884,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web }, MoreExecutors.directExecutor()); } - private void sendSessionInitiate(final RtpContentMap rtpContentMap, final org.webrtc.SessionDescription webRTCSessionDescription, final State targetState) { + private void sendSessionInitiate(final RtpContentMap rtpContentMap, final State targetState) { if (isTerminated()) { Log.w(Config.LOGTAG, id.account.getJid().asBareJid() + ": preparing session was too slow. already terminated. nothing to do."); return; @@ -895,11 +892,6 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web this.transitionOrThrow(targetState); final JinglePacket sessionInitiate = rtpContentMap.toJinglePacket(JinglePacket.Action.SESSION_INITIATE, id.sessionId); send(sessionInitiate); - try { - this.webRTCWrapper.setLocalDescription(webRTCSessionDescription).get(); - } catch (Exception e) { - failureToInitiateSession(e, targetState); - } } private ListenableFuture encryptSessionInitiate(final RtpContentMap rtpContentMap) { diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java index 751fa66f4..368b13913 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java @@ -403,70 +403,36 @@ public class WebRTCWrapper { videoTrack.setEnabled(enabled); } - ListenableFuture createOffer() { + ListenableFuture setLocalDescription() { return Futures.transformAsync(getPeerConnectionFuture(), peerConnection -> { final SettableFuture future = SettableFuture.create(); - peerConnection.createOffer(new CreateSdpObserver() { - @Override - public void onCreateSuccess(SessionDescription sessionDescription) { - future.set(sessionDescription); - } - - @Override - public void onCreateFailure(String s) { - future.setException(new IllegalStateException("Unable to create offer: " + s)); - } - }, new MediaConstraints()); - return future; - }, MoreExecutors.directExecutor()); - } - - ListenableFuture createAnswer() { - return Futures.transformAsync(getPeerConnectionFuture(), peerConnection -> { - final SettableFuture future = SettableFuture.create(); - peerConnection.createAnswer(new CreateSdpObserver() { - @Override - public void onCreateSuccess(SessionDescription sessionDescription) { - future.set(sessionDescription); - } - - @Override - public void onCreateFailure(String s) { - future.setException(new IllegalStateException("Unable to create answer: " + s)); - } - }, new MediaConstraints()); - return future; - }, MoreExecutors.directExecutor()); - } - - ListenableFuture setLocalDescription(final SessionDescription sessionDescription) { - Log.d(EXTENDED_LOGGING_TAG, "setting local description:"); - for (final String line : sessionDescription.description.split(eu.siacs.conversations.xmpp.jingle.SessionDescription.LINE_DIVIDER)) { - Log.d(EXTENDED_LOGGING_TAG, line); - } - return Futures.transformAsync(getPeerConnectionFuture(), peerConnection -> { - final SettableFuture future = SettableFuture.create(); peerConnection.setLocalDescription(new SetSdpObserver() { @Override public void onSetSuccess() { - future.set(null); + final SessionDescription description = peerConnection.getLocalDescription(); + Log.d(EXTENDED_LOGGING_TAG, "set local description:"); + logDescription(description); + future.set(description); } @Override - public void onSetFailure(final String s) { - future.setException(new IllegalArgumentException("unable to set local session description: " + s)); - + public void onSetFailure(final String message) { + future.setException(new FailureToSetDescriptionException(message)); } - }, sessionDescription); + }); return future; }, MoreExecutors.directExecutor()); } + private static void logDescription(final SessionDescription sessionDescription) { + for (final String line : sessionDescription.description.split(eu.siacs.conversations.xmpp.jingle.SessionDescription.LINE_DIVIDER)) { + Log.d(EXTENDED_LOGGING_TAG, line); + } + } + ListenableFuture setRemoteDescription(final SessionDescription sessionDescription) { Log.d(EXTENDED_LOGGING_TAG, "setting remote description:"); - for (final String line : sessionDescription.description.split(eu.siacs.conversations.xmpp.jingle.SessionDescription.LINE_DIVIDER)) { - Log.d(EXTENDED_LOGGING_TAG, line); - } + logDescription(sessionDescription); return Futures.transformAsync(getPeerConnectionFuture(), peerConnection -> { final SettableFuture future = SettableFuture.create(); peerConnection.setRemoteDescription(new SetSdpObserver() { @@ -476,9 +442,8 @@ public class WebRTCWrapper { } @Override - public void onSetFailure(String s) { - future.setException(new IllegalArgumentException("unable to set remote session description: " + s)); - + public void onSetFailure(final String message) { + future.setException(new FailureToSetDescriptionException(message)); } }, sessionDescription); return future; @@ -619,6 +584,12 @@ public class WebRTCWrapper { } + private static class FailureToSetDescriptionException extends IllegalArgumentException { + public FailureToSetDescriptionException(String message) { + super(message); + } + } + private static class CapturerChoice { private final CameraVideoCapturer cameraVideoCapturer; private final CameraEnumerationAndroid.CaptureFormat captureFormat;