use implicit descriptions for WebRTC

Daniel Gultsch created

using the parameter-free form of setLocalDescription() solves some potential race conditions
that can occur - especially once we introduce restartIce()

Change summary

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java | 24 
src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java       | 69 
2 files changed, 28 insertions(+), 65 deletions(-)

Detailed changes

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<RtpContentMap>() {
                     @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<RtpContentMap> 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<RtpContentMap>() {
             @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<RtpContentMap> encryptSessionInitiate(final RtpContentMap rtpContentMap) {

src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java 🔗

@@ -403,70 +403,36 @@ public class WebRTCWrapper {
         videoTrack.setEnabled(enabled);
     }
 
-    ListenableFuture<SessionDescription> createOffer() {
+    ListenableFuture<SessionDescription> setLocalDescription() {
         return Futures.transformAsync(getPeerConnectionFuture(), peerConnection -> {
             final SettableFuture<SessionDescription> 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<SessionDescription> createAnswer() {
-        return Futures.transformAsync(getPeerConnectionFuture(), peerConnection -> {
-            final SettableFuture<SessionDescription> future = SettableFuture.create();
-            peerConnection.createAnswer(new CreateSdpObserver() {
+            peerConnection.setLocalDescription(new SetSdpObserver() {
                 @Override
-                public void onCreateSuccess(SessionDescription sessionDescription) {
-                    future.set(sessionDescription);
+                public void onSetSuccess() {
+                    final SessionDescription description = peerConnection.getLocalDescription();
+                    Log.d(EXTENDED_LOGGING_TAG, "set local description:");
+                    logDescription(description);
+                    future.set(description);
                 }
 
                 @Override
-                public void onCreateFailure(String s) {
-                    future.setException(new IllegalStateException("Unable to create answer: " + s));
+                public void onSetFailure(final String message) {
+                    future.setException(new FailureToSetDescriptionException(message));
                 }
-            }, new MediaConstraints());
+            });
             return future;
         }, MoreExecutors.directExecutor());
     }
 
-    ListenableFuture<Void> setLocalDescription(final SessionDescription sessionDescription) {
-        Log.d(EXTENDED_LOGGING_TAG, "setting local description:");
+    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);
         }
-        return Futures.transformAsync(getPeerConnectionFuture(), peerConnection -> {
-            final SettableFuture<Void> future = SettableFuture.create();
-            peerConnection.setLocalDescription(new SetSdpObserver() {
-                @Override
-                public void onSetSuccess() {
-                    future.set(null);
-                }
-
-                @Override
-                public void onSetFailure(final String s) {
-                    future.setException(new IllegalArgumentException("unable to set local session description: " + s));
-
-                }
-            }, sessionDescription);
-            return future;
-        }, MoreExecutors.directExecutor());
     }
 
     ListenableFuture<Void> 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<Void> 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;