From 7d7e158fd75dd447e16ec8c5e9b96e5594a82fee Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 3 Nov 2021 16:00:26 +0100 Subject: [PATCH 01/24] code clean up for LocationProvider --- .../conversations/utils/LocationProvider.java | 42 +++++++++++-------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/utils/LocationProvider.java b/src/main/java/eu/siacs/conversations/utils/LocationProvider.java index afb39a008..3eb786e39 100644 --- a/src/main/java/eu/siacs/conversations/utils/LocationProvider.java +++ b/src/main/java/eu/siacs/conversations/utils/LocationProvider.java @@ -4,6 +4,8 @@ import android.content.Context; import android.telephony.TelephonyManager; import android.util.Log; +import androidx.core.content.ContextCompat; + import org.osmdroid.util.GeoPoint; import java.io.BufferedReader; @@ -16,11 +18,14 @@ import eu.siacs.conversations.R; public class LocationProvider { - public static final GeoPoint FALLBACK = new GeoPoint(0.0,0.0); + public static final GeoPoint FALLBACK = new GeoPoint(0.0, 0.0); - public static String getUserCountry(Context context) { + public static String getUserCountry(final Context context) { try { - final TelephonyManager tm = (TelephonyManager) context.getSystemService(Context.TELEPHONY_SERVICE); + final TelephonyManager tm = ContextCompat.getSystemService(context, TelephonyManager.class); + if (tm == null) { + return getUserCountryFallback(); + } final String simCountry = tm.getSimCountryIso(); if (simCountry != null && simCountry.length() == 2) { // SIM country code is available return simCountry.toUpperCase(Locale.US); @@ -30,40 +35,41 @@ public class LocationProvider { return networkCountry.toUpperCase(Locale.US); } } - } catch (Exception e) { - // fallthrough + return getUserCountryFallback(); + } catch (final Exception e) { + return getUserCountryFallback(); } - Locale locale = Locale.getDefault(); + } + + private static String getUserCountryFallback() { + final Locale locale = Locale.getDefault(); return locale.getCountry(); } - public static GeoPoint getGeoPoint(Context context) { + public static GeoPoint getGeoPoint(final Context context) { return getGeoPoint(context, getUserCountry(context)); } - public static synchronized GeoPoint getGeoPoint(Context context, String country) { - try { - BufferedReader reader = new BufferedReader(new InputStreamReader(context.getResources().openRawResource(R.raw.countries))); + public static synchronized GeoPoint getGeoPoint(final Context context, final String country) { + try (BufferedReader reader = new BufferedReader(new InputStreamReader(context.getResources().openRawResource(R.raw.countries)))) { String line; - while((line = reader.readLine()) != null) { - String[] parts = line.split("\\s+",4); + while ((line = reader.readLine()) != null) { + final String[] parts = line.split("\\s+", 4); if (parts.length == 4) { if (country.equalsIgnoreCase(parts[0])) { try { return new GeoPoint(Double.parseDouble(parts[1]), Double.parseDouble(parts[2])); - } catch (NumberFormatException e) { + } catch (final NumberFormatException e) { return FALLBACK; } } - } else { - Log.d(Config.LOGTAG,"unable to parse line="+line); } } - } catch (IOException e) { - Log.d(Config.LOGTAG,e.getMessage()); + } catch (final IOException e) { + Log.d(Config.LOGTAG, "unable to parse country->geo map", e); } return FALLBACK; } -} +} \ No newline at end of file From d4cbf2e11e6a2883c4583a0ab598be0433f7fcad Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sun, 7 Nov 2021 11:35:00 +0100 Subject: [PATCH 02/24] take intent type into account when sharing with conversations --- .../conversations/services/XmppConnectionService.java | 4 ++-- .../eu/siacs/conversations/ui/ConversationFragment.java | 9 +++++---- .../eu/siacs/conversations/ui/ConversationsActivity.java | 1 + .../eu/siacs/conversations/ui/ShareWithActivity.java | 8 +++++++- .../java/eu/siacs/conversations/ui/util/Attachment.java | 8 ++++---- 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java index 4222b9faa..815182680 100644 --- a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java +++ b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java @@ -572,8 +572,8 @@ public class XmppConnectionService extends Service { } } - public void attachImageToConversation(final Conversation conversation, final Uri uri, final UiCallback callback) { - final String mimeType = MimeUtils.guessMimeTypeFromUri(this, uri); + public void attachImageToConversation(final Conversation conversation, final Uri uri, final String type, final UiCallback callback) { + final String mimeType = MimeUtils.guessMimeTypeFromUriAndMime(this, uri, type); final String compressPictures = getCompressPicturesPreference(); if ("never".equals(compressPictures) diff --git a/src/main/java/eu/siacs/conversations/ui/ConversationFragment.java b/src/main/java/eu/siacs/conversations/ui/ConversationFragment.java index a019f282a..0077684a5 100644 --- a/src/main/java/eu/siacs/conversations/ui/ConversationFragment.java +++ b/src/main/java/eu/siacs/conversations/ui/ConversationFragment.java @@ -688,14 +688,14 @@ public class ConversationFragment extends XmppFragment implements EditMessage.Ke toggleInputMethod(); } - private void attachImageToConversation(Conversation conversation, Uri uri) { + private void attachImageToConversation(Conversation conversation, Uri uri, String type) { if (conversation == null) { return; } final Toast prepareFileToast = Toast.makeText(getActivity(), getText(R.string.preparing_image), Toast.LENGTH_LONG); prepareFileToast.show(); activity.delegateUriPermissionsToService(uri); - activity.xmppConnectionService.attachImageToConversation(conversation, uri, + activity.xmppConnectionService.attachImageToConversation(conversation, uri, type, new UiCallback() { @Override @@ -889,7 +889,7 @@ public class ConversationFragment extends XmppFragment implements EditMessage.Ke attachLocationToConversation(conversation, attachment.getUri()); } else if (attachment.getType() == Attachment.Type.IMAGE) { Log.d(Config.LOGTAG, "ConversationsActivity.commitAttachments() - attaching image to conversations. CHOOSE_IMAGE"); - attachImageToConversation(conversation, attachment.getUri()); + attachImageToConversation(conversation, attachment.getUri(), attachment.getMime()); } else { Log.d(Config.LOGTAG, "ConversationsActivity.commitAttachments() - attaching file to conversations. CHOOSE_FILE/RECORD_VOICE/RECORD_VIDEO"); attachFileToConversation(conversation, attachment.getUri(), attachment.getMime()); @@ -2186,13 +2186,14 @@ public class ConversationFragment extends XmppFragment implements EditMessage.Ke final boolean asQuote = extras.getBoolean(ConversationsActivity.EXTRA_AS_QUOTE); final boolean pm = extras.getBoolean(ConversationsActivity.EXTRA_IS_PRIVATE_MESSAGE, false); final boolean doNotAppend = extras.getBoolean(ConversationsActivity.EXTRA_DO_NOT_APPEND, false); + final String type = extras.getString(ConversationsActivity.EXTRA_TYPE); final List uris = extractUris(extras); if (uris != null && uris.size() > 0) { if (uris.size() == 1 && "geo".equals(uris.get(0).getScheme())) { mediaPreviewAdapter.addMediaPreviews(Attachment.of(getActivity(), uris.get(0), Attachment.Type.LOCATION)); } else { final List cleanedUris = cleanUris(new ArrayList<>(uris)); - mediaPreviewAdapter.addMediaPreviews(Attachment.of(getActivity(), cleanedUris)); + mediaPreviewAdapter.addMediaPreviews(Attachment.of(getActivity(), cleanedUris, type)); } toggleInputMethod(); return; diff --git a/src/main/java/eu/siacs/conversations/ui/ConversationsActivity.java b/src/main/java/eu/siacs/conversations/ui/ConversationsActivity.java index fbdba5724..cc46ed33f 100644 --- a/src/main/java/eu/siacs/conversations/ui/ConversationsActivity.java +++ b/src/main/java/eu/siacs/conversations/ui/ConversationsActivity.java @@ -99,6 +99,7 @@ public class ConversationsActivity extends XmppActivity implements OnConversatio public static final String EXTRA_DO_NOT_APPEND = "do_not_append"; public static final String EXTRA_POST_INIT_ACTION = "post_init_action"; public static final String POST_ACTION_RECORD_VOICE = "record_voice"; + public static final String EXTRA_TYPE = "type"; private static final List VIEW_AND_SHARE_ACTIONS = Arrays.asList( ACTION_VIEW_CONVERSATION, diff --git a/src/main/java/eu/siacs/conversations/ui/ShareWithActivity.java b/src/main/java/eu/siacs/conversations/ui/ShareWithActivity.java index cb698691e..d03928c8c 100644 --- a/src/main/java/eu/siacs/conversations/ui/ShareWithActivity.java +++ b/src/main/java/eu/siacs/conversations/ui/ShareWithActivity.java @@ -33,7 +33,8 @@ public class ShareWithActivity extends XmppActivity implements XmppConnectionSer refreshUi(); } - private class Share { + private static class Share { + public String type; ArrayList uris = new ArrayList<>(); public String account; public String contact; @@ -65,6 +66,7 @@ public class ShareWithActivity extends XmppActivity implements XmppConnectionSer @Override public void onRequestPermissionsResult(int requestCode, String[] permissions, int[] grantResults) { + super.onRequestPermissionsResult(requestCode, permissions, grantResults); if (grantResults.length > 0) if (grantResults[0] == PackageManager.PERMISSION_GRANTED) { if (requestCode == REQUEST_STORAGE_PERMISSION) { @@ -139,6 +141,7 @@ public class ShareWithActivity extends XmppActivity implements XmppConnectionSer } else if (type != null && uri != null) { this.share.uris.clear(); this.share.uris.add(uri); + this.share.type = type; } else { this.share.text = text; this.share.asQuote = asQuote; @@ -193,6 +196,9 @@ public class ShareWithActivity extends XmppActivity implements XmppConnectionSer intent.setAction(Intent.ACTION_SEND_MULTIPLE); intent.putParcelableArrayListExtra(Intent.EXTRA_STREAM, share.uris); intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION); + if (share.type != null) { + intent.putExtra(ConversationsActivity.EXTRA_TYPE, share.type); + } } else if (share.text != null) { intent.setAction(ConversationsActivity.ACTION_VIEW_CONVERSATION); intent.putExtra(Intent.EXTRA_TEXT, share.text); diff --git a/src/main/java/eu/siacs/conversations/ui/util/Attachment.java b/src/main/java/eu/siacs/conversations/ui/util/Attachment.java index 4083d5b04..b539c70ef 100644 --- a/src/main/java/eu/siacs/conversations/ui/util/Attachment.java +++ b/src/main/java/eu/siacs/conversations/ui/util/Attachment.java @@ -136,10 +136,10 @@ public class Attachment implements Parcelable { return Collections.singletonList(new Attachment(uri, type, mime)); } - public static List of(final Context context, List uris) { - List attachments = new ArrayList<>(); - for (Uri uri : uris) { - final String mime = MimeUtils.guessMimeTypeFromUri(context, uri); + public static List of(final Context context, List uris, final String type) { + final List attachments = new ArrayList<>(); + for (final Uri uri : uris) { + final String mime = MimeUtils.guessMimeTypeFromUriAndMime(context, uri, type); attachments.add(new Attachment(uri, mime != null && isImage(mime) ? Type.IMAGE : Type.FILE, mime)); } return attachments; From b5786787f011607b2aacc869c2a9d1c83ffc871f Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Tue, 9 Nov 2021 14:27:26 +0100 Subject: [PATCH 03/24] bump libphone number library --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 8edf10065..a95b2ff8e 100644 --- a/build.gradle +++ b/build.gradle @@ -76,7 +76,7 @@ dependencies { implementation "com.squareup.okhttp3:okhttp:4.9.2" implementation 'com.google.guava:guava:30.1.1-android' - quicksyImplementation 'io.michaelrocks:libphonenumber-android:8.12.18' + quicksyImplementation 'io.michaelrocks:libphonenumber-android:8.12.36' implementation fileTree(include: ['libwebrtc-m92.aar'], dir: 'libs') } From fda45a7c86004dfee6e05dd748819fe195fd147f Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 10 Nov 2021 16:40:16 +0100 Subject: [PATCH 04/24] 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; From 4ec0996dffa2381dc7399a7dc8bfa67e2cdf24ed Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 11 Nov 2021 11:19:37 +0100 Subject: [PATCH 05/24] webrtc: include oldState in onConnectionChange --- .../xmpp/jingle/JingleRtpConnection.java | 4 ++-- .../conversations/xmpp/jingle/WebRTCWrapper.java | 12 ++++++++---- 2 files changed, 10 insertions(+), 6 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 72d2a4837..af4e05ba0 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -1330,8 +1330,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } @Override - public void onConnectionChange(final PeerConnection.PeerConnectionState newState) { - Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnectionState changed to " + newState); + public void onConnectionChange(final PeerConnection.PeerConnectionState oldState, final PeerConnection.PeerConnectionState newState) { + Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnectionState changed: "+oldState+"->" + newState); if (newState == PeerConnection.PeerConnectionState.CONNECTED && this.rtpConnectionStarted == 0) { this.rtpConnectionStarted = SystemClock.elapsedRealtime(); } 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 368b13913..f5b2c0b87 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java @@ -88,6 +88,7 @@ public class WebRTCWrapper { private final Handler mainHandler = new Handler(Looper.getMainLooper()); private VideoTrack localVideoTrack = null; private VideoTrack remoteVideoTrack = null; + private PeerConnection.PeerConnectionState currentState; private final PeerConnection.Observer peerConnectionObserver = new PeerConnection.Observer() { @Override public void onSignalingChange(PeerConnection.SignalingState signalingState) { @@ -98,8 +99,9 @@ public class WebRTCWrapper { } @Override - public void onConnectionChange(PeerConnection.PeerConnectionState newState) { - eventCallback.onConnectionChange(newState); + public void onConnectionChange(final PeerConnection.PeerConnectionState newState) { + eventCallback.onConnectionChange(currentState, newState); + currentState = newState; } @Override @@ -150,7 +152,7 @@ public class WebRTCWrapper { @Override public void onRenegotiationNeeded() { - + Log.d(EXTENDED_LOGGING_TAG,"onRenegotiationNeeded - current state: "+currentState); } @Override @@ -261,6 +263,8 @@ public class WebRTCWrapper { throw new InitializationException("Unable to create PeerConnection"); } + this.currentState = peerConnection.connectionState(); + final Optional optionalCapturerChoice = media.contains(Media.VIDEO) ? getVideoCapturer() : Optional.absent(); if (optionalCapturerChoice.isPresent()) { @@ -531,7 +535,7 @@ public class WebRTCWrapper { public interface EventCallback { void onIceCandidate(IceCandidate iceCandidate); - void onConnectionChange(PeerConnection.PeerConnectionState newState); + void onConnectionChange(PeerConnection.PeerConnectionState oldState, PeerConnection.PeerConnectionState newState); void onAudioDeviceChanged(AppRTCAudioManager.AudioDevice selectedAudioDevice, Set availableAudioDevices); } From 61851e5f843307f2f90de892fd4fefbff5b19d46 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 11 Nov 2021 14:40:15 +0100 Subject: [PATCH 06/24] do not automacially hang up failed webrtc sessions --- .../conversations/ui/RtpSessionActivity.java | 16 +++-- .../xmpp/jingle/JingleRtpConnection.java | 66 +++++++++++-------- .../xmpp/jingle/RtpEndUserState.java | 1 + .../xmpp/jingle/WebRTCWrapper.java | 16 ++++- src/main/res/values/strings.xml | 1 + 5 files changed, 64 insertions(+), 36 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java b/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java index 96aa00db0..b2cf583b5 100644 --- a/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java +++ b/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java @@ -96,7 +96,12 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe ); private static final List STATES_SHOWING_SWITCH_TO_CHAT = Arrays.asList( RtpEndUserState.CONNECTING, - RtpEndUserState.CONNECTED + RtpEndUserState.CONNECTED, + RtpEndUserState.RECONNECTING + ); + private static final List STATES_CONSIDERED_CONNECTED = Arrays.asList( + RtpEndUserState.CONNECTED, + RtpEndUserState.RECONNECTING ); private static final String PROXIMITY_WAKE_LOCK_TAG = "conversations:in-rtp-session"; private static final int REQUEST_ACCEPT_CALL = 0x1111; @@ -502,7 +507,7 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe private boolean isConnected() { final JingleRtpConnection connection = this.rtpConnectionReference != null ? this.rtpConnectionReference.get() : null; - return connection != null && connection.getEndUserState() == RtpEndUserState.CONNECTED; + return connection != null && STATES_CONSIDERED_CONNECTED.contains(connection.getEndUserState()); } private boolean switchToPictureInPicture() { @@ -661,6 +666,9 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe case CONNECTED: setTitle(R.string.rtp_state_connected); break; + case RECONNECTING: + setTitle(R.string.rtp_state_reconnecting); + break; case ACCEPTING_CALL: setTitle(R.string.rtp_state_accepting_call); break; @@ -803,7 +811,7 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe @SuppressLint("RestrictedApi") private void updateInCallButtonConfiguration(final RtpEndUserState state, final Set media) { - if (state == RtpEndUserState.CONNECTED && !isPictureInPicture()) { + if (STATES_CONSIDERED_CONNECTED.contains(state) && !isPictureInPicture()) { Preconditions.checkArgument(media.size() > 0, "Media must not be empty"); if (media.contains(Media.VIDEO)) { final JingleRtpConnection rtpConnection = requireRtpConnection(); @@ -998,7 +1006,7 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe RendererCommon.ScalingType.SCALE_ASPECT_FILL, RendererCommon.ScalingType.SCALE_ASPECT_FIT ); - if (state == RtpEndUserState.CONNECTED) { + if (STATES_CONSIDERED_CONNECTED.contains(state)) { binding.appBarLayout.setVisibility(View.GONE); getWindow().addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN); binding.remoteVideoWrapper.setVisibility(View.VISIBLE); 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 af4e05ba0..2d322ead9 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -1035,24 +1035,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web return RtpEndUserState.CONNECTING; } case SESSION_ACCEPTED: - //TODO refactor this out into separate method (that uses switch for better readability) - final PeerConnection.PeerConnectionState state; - try { - state = webRTCWrapper.getState(); - } catch (final WebRTCWrapper.PeerConnectionNotInitialized e) { - //We usually close the WebRTCWrapper *before* transitioning so we might still - //be in SESSION_ACCEPTED even though the peerConnection has been torn down - return RtpEndUserState.ENDING_CALL; - } - if (state == PeerConnection.PeerConnectionState.CONNECTED) { - return RtpEndUserState.CONNECTED; - } else if (state == PeerConnection.PeerConnectionState.NEW || state == PeerConnection.PeerConnectionState.CONNECTING) { - return RtpEndUserState.CONNECTING; - } else if (state == PeerConnection.PeerConnectionState.CLOSED) { - return RtpEndUserState.ENDING_CALL; - } else { - return rtpConnectionStarted == 0 ? RtpEndUserState.CONNECTIVITY_ERROR : RtpEndUserState.CONNECTIVITY_LOST_ERROR; - } + return getPeerConnectionStateAsEndUserState(); case REJECTED: case REJECTED_RACED: case TERMINATED_DECLINED_OR_BUSY: @@ -1082,6 +1065,29 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web throw new IllegalStateException(String.format("%s has no equivalent EndUserState", this.state)); } + + private RtpEndUserState getPeerConnectionStateAsEndUserState() { + final PeerConnection.PeerConnectionState state; + try { + state = webRTCWrapper.getState(); + } catch (final WebRTCWrapper.PeerConnectionNotInitialized e) { + //We usually close the WebRTCWrapper *before* transitioning so we might still + //be in SESSION_ACCEPTED even though the peerConnection has been torn down + return RtpEndUserState.ENDING_CALL; + } + switch (state) { + case CONNECTED: + return RtpEndUserState.CONNECTED; + case NEW: + case CONNECTING: + return RtpEndUserState.CONNECTING; + case CLOSED: + return RtpEndUserState.ENDING_CALL; + default: + return rtpConnectionStarted == 0 ? RtpEndUserState.CONNECTIVITY_ERROR : RtpEndUserState.RECONNECTING; + } + } + public Set getMedia() { final State current = getState(); if (current == State.NULL) { @@ -1331,29 +1337,31 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web @Override public void onConnectionChange(final PeerConnection.PeerConnectionState oldState, final PeerConnection.PeerConnectionState newState) { - Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnectionState changed: "+oldState+"->" + newState); + Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnectionState changed: " + oldState + "->" + newState); + final boolean neverConnected = this.rtpConnectionStarted == 0; if (newState == PeerConnection.PeerConnectionState.CONNECTED && this.rtpConnectionStarted == 0) { this.rtpConnectionStarted = SystemClock.elapsedRealtime(); } if (newState == PeerConnection.PeerConnectionState.CLOSED && this.rtpConnectionEnded == 0) { this.rtpConnectionEnded = SystemClock.elapsedRealtime(); } - //TODO 'failed' means we need to restart ICE - // - //TODO 'disconnected' can probably be ignored as "This is a less stringent test than failed - // and may trigger intermittently and resolve just as spontaneously on less reliable networks, - // or during temporary disconnections. When the problem resolves, the connection may return - // to the connected state." - // Obviously the UI needs to reflect this new state with a 'reconnecting' display or something - if (Arrays.asList(PeerConnection.PeerConnectionState.FAILED, PeerConnection.PeerConnectionState.DISCONNECTED).contains(newState)) { + + if (neverConnected && Arrays.asList(PeerConnection.PeerConnectionState.FAILED, PeerConnection.PeerConnectionState.DISCONNECTED).contains(newState)) { if (isTerminated()) { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": not sending session-terminate after connectivity error because session is already in state " + this.state); return; } new Thread(this::closeWebRTCSessionAfterFailedConnection).start(); - } else { - updateEndUserState(); + } else if (newState == PeerConnection.PeerConnectionState.FAILED) { + Log.d(Config.LOGTAG, "attempting to restart ICE"); + webRTCWrapper.restartIce(); } + updateEndUserState(); + } + + @Override + public void onRenegotiationNeeded() { + Log.d(Config.LOGTAG, "onRenegotiationNeeded()"); } private void closeWebRTCSessionAfterFailedConnection() { diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpEndUserState.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpEndUserState.java index 61536bb7c..9a431bc01 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpEndUserState.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpEndUserState.java @@ -4,6 +4,7 @@ public enum RtpEndUserState { INCOMING_CALL, //received a 'propose' message CONNECTING, //session-initiate or session-accepted but no webrtc peer connection yet CONNECTED, //session-accepted and webrtc peer connection is connected + RECONNECTING, //session-accepted and webrtc peer connection was connected once but is currently disconnected or failed FINDING_DEVICE, //'propose' has been sent out; no 184 ack yet RINGING, //'propose' has been sent out and it has been 184 acked ACCEPTING_CALL, //'proceed' message has been sent; but no session-initiate has been received 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 f5b2c0b87..6e4a94539 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java @@ -100,13 +100,14 @@ public class WebRTCWrapper { @Override public void onConnectionChange(final PeerConnection.PeerConnectionState newState) { - eventCallback.onConnectionChange(currentState, newState); + final PeerConnection.PeerConnectionState oldState = currentState; currentState = newState; + eventCallback.onConnectionChange(oldState, newState); } @Override public void onIceConnectionChange(PeerConnection.IceConnectionState iceConnectionState) { - + Log.d(EXTENDED_LOGGING_TAG, "onIceConnectionChange(" + iceConnectionState + ")"); } @Override @@ -152,7 +153,10 @@ public class WebRTCWrapper { @Override public void onRenegotiationNeeded() { - Log.d(EXTENDED_LOGGING_TAG,"onRenegotiationNeeded - current state: "+currentState); + Log.d(EXTENDED_LOGGING_TAG, "onRenegotiationNeeded()"); + if (currentState != null && currentState != PeerConnection.PeerConnectionState.NEW) { + eventCallback.onRenegotiationNeeded(); + } } @Override @@ -293,6 +297,10 @@ public class WebRTCWrapper { this.peerConnection = peerConnection; } + void restartIce() { + requirePeerConnection().restartIce(); + } + synchronized void close() { final PeerConnection peerConnection = this.peerConnection; final CapturerChoice capturerChoice = this.capturerChoice; @@ -538,6 +546,8 @@ public class WebRTCWrapper { void onConnectionChange(PeerConnection.PeerConnectionState oldState, PeerConnection.PeerConnectionState newState); void onAudioDeviceChanged(AppRTCAudioManager.AudioDevice selectedAudioDevice, Set availableAudioDevices); + + void onRenegotiationNeeded(); } private static abstract class SetSdpObserver implements SdpObserver { diff --git a/src/main/res/values/strings.xml b/src/main/res/values/strings.xml index 4e520e856..dae88c606 100644 --- a/src/main/res/values/strings.xml +++ b/src/main/res/values/strings.xml @@ -904,6 +904,7 @@ Incoming video call Connecting Connected + Reconnecting Accepting call Ending call Answer From 9843b72f6fc3993313c404c2ab0e5aa70b4c6a77 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 11 Nov 2021 15:23:45 +0100 Subject: [PATCH 07/24] always use Camera2Enumerator --- .../siacs/conversations/xmpp/jingle/WebRTCWrapper.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) 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 6e4a94539..c1201206a 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java @@ -476,16 +476,8 @@ public class WebRTCWrapper { requirePeerConnection().addIceCandidate(iceCandidate); } - private CameraEnumerator getCameraEnumerator() { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { - return new Camera2Enumerator(requireContext()); - } else { - return new Camera1Enumerator(); - } - } - private Optional getVideoCapturer() { - final CameraEnumerator enumerator = getCameraEnumerator(); + final CameraEnumerator enumerator = new Camera2Enumerator(requireContext()); final Set deviceNames = ImmutableSet.copyOf(enumerator.getDeviceNames()); for (final String deviceName : deviceNames) { if (isFrontFacing(enumerator, deviceName)) { From 9c3f55bef220351e5eff5790b38ccec93aa6c573 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 11 Nov 2021 16:52:18 +0100 Subject: [PATCH 08/24] use stopwatch to keep track of jingle rtp session duration --- .../conversations/ui/RtpSessionActivity.java | 11 ++--- .../conversations/utils/TimeFrameUtils.java | 12 ++++-- .../xmpp/jingle/JingleRtpConnection.java | 40 ++++++++++--------- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java b/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java index b2cf583b5..d0bdbb788 100644 --- a/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java +++ b/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java @@ -939,14 +939,11 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe this.binding.duration.setVisibility(View.GONE); return; } - final long rtpConnectionStarted = connection.getRtpConnectionStarted(); - final long rtpConnectionEnded = connection.getRtpConnectionEnded(); - if (rtpConnectionStarted != 0) { - final long ended = rtpConnectionEnded == 0 ? SystemClock.elapsedRealtime() : rtpConnectionEnded; - this.binding.duration.setText(TimeFrameUtils.formatTimePassed(rtpConnectionStarted, ended, false)); - this.binding.duration.setVisibility(View.VISIBLE); - } else { + if (connection.zeroDuration()) { this.binding.duration.setVisibility(View.GONE); + } else { + this.binding.duration.setText(TimeFrameUtils.formatElapsedTime(connection.getCallDuration(), false)); + this.binding.duration.setVisibility(View.VISIBLE); } } diff --git a/src/main/java/eu/siacs/conversations/utils/TimeFrameUtils.java b/src/main/java/eu/siacs/conversations/utils/TimeFrameUtils.java index 1cb78db0c..9e7946d57 100644 --- a/src/main/java/eu/siacs/conversations/utils/TimeFrameUtils.java +++ b/src/main/java/eu/siacs/conversations/utils/TimeFrameUtils.java @@ -71,10 +71,14 @@ public class TimeFrameUtils { public static String formatTimePassed(final long since, final long to, final boolean withMilliseconds) { final long passed = (since < 0) ? 0 : (to - since); - final int hours = (int) (passed / 3600000); - final int minutes = (int) (passed / 60000) % 60; - final int seconds = (int) (passed / 1000) % 60; - final int milliseconds = (int) (passed / 100) % 10; + return formatElapsedTime(passed, withMilliseconds); + } + + public static String formatElapsedTime(final long elapsed, final boolean withMilliseconds) { + final int hours = (int) (elapsed / 3600000); + final int minutes = (int) (elapsed / 60000) % 60; + final int seconds = (int) (elapsed / 1000) % 60; + final int milliseconds = (int) (elapsed / 100) % 10; if (hours > 0) { return String.format(Locale.ENGLISH, "%d:%02d:%02d", hours, minutes, seconds); } else if (withMilliseconds) { 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 2d322ead9..ed1f35dd2 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -1,6 +1,5 @@ package eu.siacs.conversations.xmpp.jingle; -import android.os.SystemClock; import android.util.Log; import androidx.annotation.NonNull; @@ -8,6 +7,7 @@ import androidx.annotation.Nullable; import com.google.common.base.Optional; import com.google.common.base.Preconditions; +import com.google.common.base.Stopwatch; import com.google.common.base.Strings; import com.google.common.base.Throwables; import com.google.common.collect.Collections2; @@ -29,8 +29,10 @@ import java.util.ArrayDeque; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Queue; import java.util.Set; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -147,8 +149,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web private Set proposedMedia; private RtpContentMap initiatorRtpContentMap; private RtpContentMap responderRtpContentMap; - private long rtpConnectionStarted = 0; //time of 'connected' - private long rtpConnectionEnded = 0; + private final Stopwatch sessionDuration = Stopwatch.createUnstarted(); + private final Queue stateHistory = new LinkedList<>(); private ScheduledFuture ringingTimeoutFuture; JingleRtpConnection(JingleConnectionManager jingleConnectionManager, Id id, Jid initiator) { @@ -1056,7 +1058,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web return RtpEndUserState.RETRACTED; } case TERMINATED_CONNECTIVITY_ERROR: - return rtpConnectionStarted == 0 ? RtpEndUserState.CONNECTIVITY_ERROR : RtpEndUserState.CONNECTIVITY_LOST_ERROR; + return zeroDuration() ? RtpEndUserState.CONNECTIVITY_ERROR : RtpEndUserState.CONNECTIVITY_LOST_ERROR; case TERMINATED_APPLICATION_FAILURE: return RtpEndUserState.APPLICATION_ERROR; case TERMINATED_SECURITY_ERROR: @@ -1084,7 +1086,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web case CLOSED: return RtpEndUserState.ENDING_CALL; default: - return rtpConnectionStarted == 0 ? RtpEndUserState.CONNECTIVITY_ERROR : RtpEndUserState.RECONNECTING; + return zeroDuration() ? RtpEndUserState.CONNECTIVITY_ERROR : RtpEndUserState.RECONNECTING; } } @@ -1338,15 +1340,18 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web @Override public void onConnectionChange(final PeerConnection.PeerConnectionState oldState, final PeerConnection.PeerConnectionState newState) { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnectionState changed: " + oldState + "->" + newState); - final boolean neverConnected = this.rtpConnectionStarted == 0; - if (newState == PeerConnection.PeerConnectionState.CONNECTED && this.rtpConnectionStarted == 0) { - this.rtpConnectionStarted = SystemClock.elapsedRealtime(); - } - if (newState == PeerConnection.PeerConnectionState.CLOSED && this.rtpConnectionEnded == 0) { - this.rtpConnectionEnded = SystemClock.elapsedRealtime(); + this.stateHistory.add(newState); + if (newState == PeerConnection.PeerConnectionState.CONNECTED) { + this.sessionDuration.start(); + } else if (this.sessionDuration.isRunning()) { + this.sessionDuration.stop(); } - if (neverConnected && Arrays.asList(PeerConnection.PeerConnectionState.FAILED, PeerConnection.PeerConnectionState.DISCONNECTED).contains(newState)) { + final boolean neverConnected = !this.stateHistory.contains(PeerConnection.PeerConnectionState.CONNECTED); + final boolean failedOrDisconnected = Arrays.asList(PeerConnection.PeerConnectionState.FAILED, PeerConnection.PeerConnectionState.DISCONNECTED).contains(newState); + + + if (neverConnected && failedOrDisconnected) { if (isTerminated()) { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": not sending session-terminate after connectivity error because session is already in state " + this.state); return; @@ -1375,12 +1380,12 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } } - public long getRtpConnectionStarted() { - return this.rtpConnectionStarted; + public boolean zeroDuration() { + return this.sessionDuration.elapsed(TimeUnit.NANOSECONDS) <= 0; } - public long getRtpConnectionEnded() { - return this.rtpConnectionEnded; + public long getCallDuration() { + return this.sessionDuration.elapsed(TimeUnit.MILLISECONDS); } public AppRTCAudioManager getAudioManager() { @@ -1507,8 +1512,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } private void writeLogMessage(final State state) { - final long started = this.rtpConnectionStarted; - long duration = started <= 0 ? 0 : SystemClock.elapsedRealtime() - started; + final long duration = getCallDuration(); if (state == State.TERMINATED_SUCCESS || (state == State.TERMINATED_CONNECTIVITY_ERROR && duration > 0)) { writeLogMessageSuccess(duration); } else { From b6dee6da6a16a383ca1cac097719e5aebafd77b5 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 11 Nov 2021 17:05:32 +0100 Subject: [PATCH 09/24] reverse: webrtc: include oldState in onConnectionChange MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit turns out we don’t need it and a better way is for RtpConnection to keep track of *all* states in the current generation --- .../conversations/xmpp/jingle/JingleRtpConnection.java | 4 ++-- .../siacs/conversations/xmpp/jingle/WebRTCWrapper.java | 10 +++------- 2 files changed, 5 insertions(+), 9 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 ed1f35dd2..37c51bfdc 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -1338,8 +1338,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } @Override - public void onConnectionChange(final PeerConnection.PeerConnectionState oldState, final PeerConnection.PeerConnectionState newState) { - Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnectionState changed: " + oldState + "->" + newState); + public void onConnectionChange(final PeerConnection.PeerConnectionState newState) { + Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnectionState changed to" + newState); this.stateHistory.add(newState); if (newState == PeerConnection.PeerConnectionState.CONNECTED) { this.sessionDuration.start(); 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 c1201206a..5ee6c5d5e 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java @@ -88,7 +88,6 @@ public class WebRTCWrapper { private final Handler mainHandler = new Handler(Looper.getMainLooper()); private VideoTrack localVideoTrack = null; private VideoTrack remoteVideoTrack = null; - private PeerConnection.PeerConnectionState currentState; private final PeerConnection.Observer peerConnectionObserver = new PeerConnection.Observer() { @Override public void onSignalingChange(PeerConnection.SignalingState signalingState) { @@ -100,9 +99,7 @@ public class WebRTCWrapper { @Override public void onConnectionChange(final PeerConnection.PeerConnectionState newState) { - final PeerConnection.PeerConnectionState oldState = currentState; - currentState = newState; - eventCallback.onConnectionChange(oldState, newState); + eventCallback.onConnectionChange(newState); } @Override @@ -154,6 +151,7 @@ public class WebRTCWrapper { @Override public void onRenegotiationNeeded() { Log.d(EXTENDED_LOGGING_TAG, "onRenegotiationNeeded()"); + final PeerConnection.PeerConnectionState currentState = peerConnection == null ? null : peerConnection.connectionState(); if (currentState != null && currentState != PeerConnection.PeerConnectionState.NEW) { eventCallback.onRenegotiationNeeded(); } @@ -267,8 +265,6 @@ public class WebRTCWrapper { throw new InitializationException("Unable to create PeerConnection"); } - this.currentState = peerConnection.connectionState(); - final Optional optionalCapturerChoice = media.contains(Media.VIDEO) ? getVideoCapturer() : Optional.absent(); if (optionalCapturerChoice.isPresent()) { @@ -535,7 +531,7 @@ public class WebRTCWrapper { public interface EventCallback { void onIceCandidate(IceCandidate iceCandidate); - void onConnectionChange(PeerConnection.PeerConnectionState oldState, PeerConnection.PeerConnectionState newState); + void onConnectionChange(PeerConnection.PeerConnectionState newState); void onAudioDeviceChanged(AppRTCAudioManager.AudioDevice selectedAudioDevice, Set availableAudioDevices); From 717c83753f24ffbccabe17c23100268f9146040a Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 11 Nov 2021 21:02:15 +0100 Subject: [PATCH 10/24] delay discovered ice candidates until JingleRtpConnection is ready to receive otherwise setLocalDescription and the arrival of first candidates might race the rtpContentDescription being set --- .../xmpp/jingle/JingleRtpConnection.java | 39 +++++++++++++++++-- .../xmpp/jingle/WebRTCWrapper.java | 28 ++++++++++++- 2 files changed, 62 insertions(+), 5 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 37c51bfdc..4796d0c58 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -34,6 +34,7 @@ import java.util.List; import java.util.Map; import java.util.Queue; import java.util.Set; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -567,6 +568,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web final SessionDescription sessionDescription = SessionDescription.parse(webRTCSessionDescription.description); final RtpContentMap respondingRtpContentMap = RtpContentMap.of(sessionDescription); this.responderRtpContentMap = respondingRtpContentMap; + webRTCWrapper.setIsReadyToReceiveIceCandidates(true); final ListenableFuture outgoingContentMapFuture = prepareOutgoingContentMap(respondingRtpContentMap); Futures.addCallback(outgoingContentMapFuture, new FutureCallback() { @@ -872,6 +874,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web final SessionDescription sessionDescription = SessionDescription.parse(webRTCSessionDescription.description); final RtpContentMap rtpContentMap = RtpContentMap.of(sessionDescription); this.initiatorRtpContentMap = rtpContentMap; + this.webRTCWrapper.setIsReadyToReceiveIceCandidates(true); final ListenableFuture outgoingContentMapFuture = encryptSessionInitiate(rtpContentMap); Futures.addCallback(outgoingContentMapFuture, new FutureCallback() { @Override @@ -1339,7 +1342,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web @Override public void onConnectionChange(final PeerConnection.PeerConnectionState newState) { - Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnectionState changed to" + newState); + Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnectionState changed to " + newState); this.stateHistory.add(newState); if (newState == PeerConnection.PeerConnectionState.CONNECTED) { this.sessionDuration.start(); @@ -1348,7 +1351,10 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } final boolean neverConnected = !this.stateHistory.contains(PeerConnection.PeerConnectionState.CONNECTED); - final boolean failedOrDisconnected = Arrays.asList(PeerConnection.PeerConnectionState.FAILED, PeerConnection.PeerConnectionState.DISCONNECTED).contains(newState); + final boolean failedOrDisconnected = Arrays.asList( + PeerConnection.PeerConnectionState.FAILED, + PeerConnection.PeerConnectionState.DISCONNECTED + ).contains(newState); if (neverConnected && failedOrDisconnected) { @@ -1356,7 +1362,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": not sending session-terminate after connectivity error because session is already in state " + this.state); return; } - new Thread(this::closeWebRTCSessionAfterFailedConnection).start(); + webRTCWrapper.execute(this::closeWebRTCSessionAfterFailedConnection); } else if (newState == PeerConnection.PeerConnectionState.FAILED) { Log.d(Config.LOGTAG, "attempting to restart ICE"); webRTCWrapper.restartIce(); @@ -1367,6 +1373,33 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web @Override public void onRenegotiationNeeded() { Log.d(Config.LOGTAG, "onRenegotiationNeeded()"); + this.webRTCWrapper.execute(this::renegotiate); + } + + private void renegotiate() { + this.webRTCWrapper.setIsReadyToReceiveIceCandidates(false); + try { + final SessionDescription sessionDescription = setLocalSessionDescription(); + final RtpContentMap rtpContentMap = RtpContentMap.of(sessionDescription); + setRenegotiatedContentMap(rtpContentMap); + this.webRTCWrapper.setIsReadyToReceiveIceCandidates(true); + } catch (final Exception e) { + Log.d(Config.LOGTAG, "failed to renegotiate", e); + //TODO send some sort of failure (comparable to when initiating) + } + } + + private void setRenegotiatedContentMap(final RtpContentMap rtpContentMap) { + if (isInitiator()) { + this.initiatorRtpContentMap = rtpContentMap; + } else { + this.responderRtpContentMap = rtpContentMap; + } + } + + private SessionDescription setLocalSessionDescription() throws ExecutionException, InterruptedException { + final org.webrtc.SessionDescription sessionDescription = this.webRTCWrapper.setLocalDescription().get(); + return SessionDescription.parse(sessionDescription.description); } private void closeWebRTCSessionAfterFailedConnection() { 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 5ee6c5d5e..9ea4cd389 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java @@ -45,8 +45,13 @@ import org.webrtc.voiceengine.WebRtcAudioEffects; import java.util.ArrayList; import java.util.Collections; +import java.util.LinkedList; import java.util.List; +import java.util.Queue; import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -59,6 +64,8 @@ public class WebRTCWrapper { private static final String EXTENDED_LOGGING_TAG = WebRTCWrapper.class.getSimpleName(); + private final ExecutorService executorService = Executors.newSingleThreadExecutor(); + //we should probably keep this in sync with: https://github.com/signalapp/Signal-Android/blob/master/app/src/main/java/org/thoughtcrime/securesms/ApplicationContext.java#L296 private static final Set HARDWARE_AEC_BLACKLIST = new ImmutableSet.Builder() .add("Pixel") @@ -79,6 +86,8 @@ public class WebRTCWrapper { private static final int CAPTURING_MAX_FRAME_RATE = 30; private final EventCallback eventCallback; + private final AtomicBoolean readyToReceivedIceCandidates = new AtomicBoolean(false); + private final Queue iceCandidates = new LinkedList<>(); private final AppRTCAudioManager.AudioManagerEvents audioManagerEvents = new AppRTCAudioManager.AudioManagerEvents() { @Override public void onAudioDeviceChanged(AppRTCAudioManager.AudioDevice selectedAudioDevice, Set availableAudioDevices) { @@ -125,7 +134,11 @@ public class WebRTCWrapper { @Override public void onIceCandidate(IceCandidate iceCandidate) { - eventCallback.onIceCandidate(iceCandidate); + if (readyToReceivedIceCandidates.get()) { + eventCallback.onIceCandidate(iceCandidate); + } else { + iceCandidates.add(iceCandidate); + } } @Override @@ -294,7 +307,14 @@ public class WebRTCWrapper { } void restartIce() { - requirePeerConnection().restartIce(); + executorService.execute(()-> requirePeerConnection().restartIce()); + } + + public void setIsReadyToReceiveIceCandidates(final boolean ready) { + readyToReceivedIceCandidates.set(ready); + while(ready && iceCandidates.peek() != null) { + eventCallback.onIceCandidate(iceCandidates.poll()); + } } synchronized void close() { @@ -528,6 +548,10 @@ public class WebRTCWrapper { return appRTCAudioManager; } + void execute(final Runnable command) { + executorService.execute(command); + } + public interface EventCallback { void onIceCandidate(IceCandidate iceCandidate); From 5b80c62a637fabea89ceafb39dc9353bc50773d6 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sun, 14 Nov 2021 18:22:18 +0100 Subject: [PATCH 11/24] treat transport-info w/o candidates and changed credentials as offer --- .../eu/siacs/conversations/xml/Element.java | 5 +- .../xmpp/jingle/JingleRtpConnection.java | 193 +++++++++++++----- .../xmpp/jingle/RtpContentMap.java | 34 ++- .../xmpp/jingle/WebRTCWrapper.java | 34 ++- .../jingle/stanzas/IceUdpTransportInfo.java | 45 +++- 5 files changed, 254 insertions(+), 57 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/xml/Element.java b/src/main/java/eu/siacs/conversations/xml/Element.java index c0ece7f4c..4d53a17b7 100644 --- a/src/main/java/eu/siacs/conversations/xml/Element.java +++ b/src/main/java/eu/siacs/conversations/xml/Element.java @@ -1,5 +1,7 @@ package eu.siacs.conversations.xml; +import org.jetbrains.annotations.NotNull; + import java.util.ArrayList; import java.util.Hashtable; import java.util.List; @@ -165,8 +167,9 @@ public class Element { return this.attributes; } + @NotNull public String toString() { - StringBuilder elementOutput = new StringBuilder(); + final StringBuilder elementOutput = new StringBuilder(); if ((content == null) && (children.size() == 0)) { Tag emptyTag = Tag.empty(name); emptyTag.setAtttributes(this.attributes); 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 4796d0c58..71cdb02c4 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -25,7 +25,6 @@ import org.webrtc.IceCandidate; import org.webrtc.PeerConnection; import org.webrtc.VideoTrack; -import java.util.ArrayDeque; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -142,7 +141,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } private final WebRTCWrapper webRTCWrapper = new WebRTCWrapper(this); - private final ArrayDeque>> pendingIceCandidates = new ArrayDeque<>(); + //TODO convert to Queue>? + private final Queue> pendingIceCandidates = new LinkedList<>(); private final OmemoVerification omemoVerification = new OmemoVerification(); private final Message message; private State state = State.NULL; @@ -193,7 +193,6 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web @Override synchronized void deliverPacket(final JinglePacket jinglePacket) { - Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": packet delivered to JingleRtpConnection"); switch (jinglePacket.getAction()) { case SESSION_INITIATE: receiveSessionInitiate(jinglePacket); @@ -254,23 +253,29 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web private void receiveTransportInfo(final JinglePacket jinglePacket) { //Due to the asynchronicity of processing session-init we might move from NULL|PROCEED to INITIALIZED only after transport-info has been received if (isInState(State.NULL, State.PROCEED, State.SESSION_INITIALIZED, State.SESSION_INITIALIZED_PRE_APPROVED, State.SESSION_ACCEPTED)) { - respondOk(jinglePacket); final RtpContentMap contentMap; try { contentMap = RtpContentMap.of(jinglePacket); - } catch (IllegalArgumentException | NullPointerException e) { + } catch (final IllegalArgumentException | NullPointerException e) { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": improperly formatted contents; ignoring", e); + respondOk(jinglePacket); return; } final Set> candidates = contentMap.contents.entrySet(); if (this.state == State.SESSION_ACCEPTED) { + //zero candidates + modified credentials are an ICE restart offer + if (checkForIceRestart(contentMap, jinglePacket)) { + return; + } + respondOk(jinglePacket); try { processCandidates(candidates); } catch (final WebRTCWrapper.PeerConnectionNotInitialized e) { Log.w(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnection was not initialized when processing transport info. this usually indicates a race condition that can be ignored"); } } else { - pendingIceCandidates.push(candidates); + respondOk(jinglePacket); + pendingIceCandidates.addAll(candidates); } } else { if (isTerminated()) { @@ -283,37 +288,106 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } } + private boolean checkForIceRestart(final RtpContentMap rtpContentMap, final JinglePacket jinglePacket) { + final RtpContentMap existing = getRemoteContentMap(); + final Map existingCredentials = existing.getCredentials(); + final Map newCredentials = rtpContentMap.getCredentials(); + if (!existingCredentials.keySet().equals(newCredentials.keySet())) { + return false; + } + if (existingCredentials.equals(newCredentials)) { + return false; + } + final boolean isOffer = rtpContentMap.emptyCandidates(); + Log.d(Config.LOGTAG, "detected ICE restart. offer=" + isOffer + " " + jinglePacket); + //TODO reset to 'actpass'? + final RtpContentMap restartContentMap = existing.modifiedCredentials(newCredentials); + try { + if (applyIceRestart(isOffer, restartContentMap)) { + return false; + } else { + Log.d(Config.LOGTAG, "responding with tie break"); + //TODO respond with conflict + return true; + } + } catch (Exception e) { + Log.d(Config.LOGTAG, "failure to apply ICE restart. sending error", e); + //TODO send some kind of error + return true; + } + } + + private boolean applyIceRestart(final boolean isOffer, final RtpContentMap restartContentMap) throws ExecutionException, InterruptedException { + final SessionDescription sessionDescription = SessionDescription.of(restartContentMap); + final org.webrtc.SessionDescription.Type type = isOffer ? org.webrtc.SessionDescription.Type.OFFER : org.webrtc.SessionDescription.Type.ANSWER; + org.webrtc.SessionDescription sdp = new org.webrtc.SessionDescription(type, sessionDescription.toString()); + if (isOffer && webRTCWrapper.getSignalingState() != PeerConnection.SignalingState.STABLE) { + if (isInitiator()) { + //We ignore the offer and respond with tie-break. This will clause the responder not to apply the content map + return false; + } + //rollback our own local description. should happen automatically but doesn't + webRTCWrapper.rollbackLocalDescription().get(); + } + webRTCWrapper.setRemoteDescription(sdp).get(); + if (isInitiator()) { + this.responderRtpContentMap = restartContentMap; + } else { + this.initiatorRtpContentMap = restartContentMap; + } + if (isOffer) { + webRTCWrapper.setIsReadyToReceiveIceCandidates(false); + final SessionDescription localSessionDescription = setLocalSessionDescription(); + setLocalContentMap(RtpContentMap.of(localSessionDescription)); + webRTCWrapper.setIsReadyToReceiveIceCandidates(true); + } + return true; + } + private void processCandidates(final Set> contents) { - final RtpContentMap rtpContentMap = isInitiator() ? this.responderRtpContentMap : this.initiatorRtpContentMap; + for (final Map.Entry content : contents) { + processCandidate(content); + } + } + + private void processCandidate(final Map.Entry content) { + final RtpContentMap rtpContentMap = getRemoteContentMap(); + final List indices = toIdentificationTags(rtpContentMap); + final String sdpMid = content.getKey(); //aka content name + final IceUdpTransportInfo transport = content.getValue().transport; + final IceUdpTransportInfo.Credentials credentials = transport.getCredentials(); + + //TODO check that credentials remained the same + + for (final IceUdpTransportInfo.Candidate candidate : transport.getCandidates()) { + final String sdp; + try { + sdp = candidate.toSdpAttribute(credentials.ufrag); + } catch (final IllegalArgumentException e) { + Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": ignoring invalid ICE candidate " + e.getMessage()); + continue; + } + final int mLineIndex = indices.indexOf(sdpMid); + if (mLineIndex < 0) { + Log.w(Config.LOGTAG, "mLineIndex not found for " + sdpMid + ". available indices " + indices); + } + final IceCandidate iceCandidate = new IceCandidate(sdpMid, mLineIndex, sdp); + Log.d(Config.LOGTAG, "received candidate: " + iceCandidate); + this.webRTCWrapper.addIceCandidate(iceCandidate); + } + } + + private RtpContentMap getRemoteContentMap() { + return isInitiator() ? this.responderRtpContentMap : this.initiatorRtpContentMap; + } + + private List toIdentificationTags(final RtpContentMap rtpContentMap) { final Group originalGroup = rtpContentMap.group; final List identificationTags = originalGroup == null ? rtpContentMap.getNames() : originalGroup.getIdentificationTags(); if (identificationTags.size() == 0) { Log.w(Config.LOGTAG, id.account.getJid().asBareJid() + ": no identification tags found in initial offer. we won't be able to calculate mLineIndices"); } - processCandidates(identificationTags, contents); - } - - private void processCandidates(final List indices, final Set> contents) { - for (final Map.Entry content : contents) { - final String ufrag = content.getValue().transport.getAttribute("ufrag"); - for (final IceUdpTransportInfo.Candidate candidate : content.getValue().transport.getCandidates()) { - final String sdp; - try { - sdp = candidate.toSdpAttribute(ufrag); - } catch (IllegalArgumentException e) { - Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": ignoring invalid ICE candidate " + e.getMessage()); - continue; - } - final String sdpMid = content.getKey(); - final int mLineIndex = indices.indexOf(sdpMid); - if (mLineIndex < 0) { - Log.w(Config.LOGTAG, "mLineIndex not found for " + sdpMid + ". available indices " + indices); - } - final IceCandidate iceCandidate = new IceCandidate(sdpMid, mLineIndex, sdp); - Log.d(Config.LOGTAG, "received candidate: " + iceCandidate); - this.webRTCWrapper.addIceCandidate(iceCandidate); - } - } + return identificationTags; } private ListenableFuture receiveRtpContentMap(final JinglePacket jinglePacket, final boolean expectVerification) { @@ -401,11 +475,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } if (transition(target, () -> this.initiatorRtpContentMap = contentMap)) { respondOk(jinglePacket); - - final Set> candidates = contentMap.contents.entrySet(); - if (candidates.size() > 0) { - pendingIceCandidates.push(candidates); - } + pendingIceCandidates.addAll(contentMap.contents.entrySet()); if (target == State.SESSION_INITIALIZED_PRE_APPROVED) { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": automatically accepting session-initiate"); sendSessionAccept(); @@ -495,8 +565,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web sendSessionTerminate(Reason.FAILED_APPLICATION); return; } - final List identificationTags = contentMap.group == null ? contentMap.getNames() : contentMap.group.getIdentificationTags(); - processCandidates(identificationTags, contentMap.contents.entrySet()); + processCandidates(contentMap.contents.entrySet()); } private void sendSessionAccept() { @@ -558,9 +627,10 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } private void addIceCandidatesFromBlackLog() { - while (!this.pendingIceCandidates.isEmpty()) { - processCandidates(this.pendingIceCandidates.poll()); - Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": added candidates from back log"); + Map.Entry foo; + while ((foo = this.pendingIceCandidates.poll()) != null) { + processCandidate(foo); + Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": added candidate from back log"); } } @@ -1335,7 +1405,13 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web @Override public void onIceCandidate(final IceCandidate iceCandidate) { - final IceUdpTransportInfo.Candidate candidate = IceUdpTransportInfo.Candidate.fromSdpAttribute(iceCandidate.sdp); + final RtpContentMap rtpContentMap = isInitiator() ? this.initiatorRtpContentMap : this.responderRtpContentMap; + final Collection currentUfrags = Collections2.transform(rtpContentMap.getCredentials().values(), c -> c.ufrag); + final IceUdpTransportInfo.Candidate candidate = IceUdpTransportInfo.Candidate.fromSdpAttribute(iceCandidate.sdp, currentUfrags); + if (candidate == null) { + Log.d(Config.LOGTAG,"ignoring (not sending) candidate: "+iceCandidate.toString()); + return; + } Log.d(Config.LOGTAG, "sending candidate: " + iceCandidate.toString()); sendTransportInfo(iceCandidate.sdpMid, candidate); } @@ -1373,23 +1449,42 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web @Override public void onRenegotiationNeeded() { Log.d(Config.LOGTAG, "onRenegotiationNeeded()"); - this.webRTCWrapper.execute(this::renegotiate); + this.webRTCWrapper.execute(this::initiateIceRestart); } - private void renegotiate() { + private void initiateIceRestart() { + PeerConnection.SignalingState signalingState = webRTCWrapper.getSignalingState(); + Log.d(Config.LOGTAG, "initiateIceRestart() - " + signalingState); + if (signalingState != PeerConnection.SignalingState.STABLE) { + return; + } this.webRTCWrapper.setIsReadyToReceiveIceCandidates(false); + final SessionDescription sessionDescription; try { - final SessionDescription sessionDescription = setLocalSessionDescription(); - final RtpContentMap rtpContentMap = RtpContentMap.of(sessionDescription); - setRenegotiatedContentMap(rtpContentMap); - this.webRTCWrapper.setIsReadyToReceiveIceCandidates(true); + sessionDescription = setLocalSessionDescription(); } catch (final Exception e) { Log.d(Config.LOGTAG, "failed to renegotiate", e); //TODO send some sort of failure (comparable to when initiating) + return; } + final RtpContentMap rtpContentMap = RtpContentMap.of(sessionDescription); + final RtpContentMap transportInfo = rtpContentMap.transportInfo(); + final JinglePacket jinglePacket = transportInfo.toJinglePacket(JinglePacket.Action.TRANSPORT_INFO, id.sessionId); + Log.d(Config.LOGTAG, "initiating ice restart: " + jinglePacket); + jinglePacket.setTo(id.with); + xmppConnectionService.sendIqPacket(id.account, jinglePacket, (account, response) -> { + if (response.getType() == IqPacket.TYPE.RESULT) { + Log.d(Config.LOGTAG, "received success to our ice restart"); + setLocalContentMap(rtpContentMap); + webRTCWrapper.setIsReadyToReceiveIceCandidates(true); + } else { + Log.d(Config.LOGTAG, "received failure to our ice restart"); + //TODO handle tie-break. Rollback? + } + }); } - private void setRenegotiatedContentMap(final RtpContentMap rtpContentMap) { + private void setLocalContentMap(final RtpContentMap rtpContentMap) { if (isInitiator()) { this.initiatorRtpContentMap = rtpContentMap; } else { diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java index 9baffcf81..99db8bd34 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java @@ -1,7 +1,5 @@ package eu.siacs.conversations.xmpp.jingle; -import android.util.Log; - import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.base.Strings; @@ -17,9 +15,9 @@ import org.checkerframework.checker.nullness.compatqual.NullableDecl; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; -import eu.siacs.conversations.Config; import eu.siacs.conversations.xmpp.jingle.stanzas.Content; import eu.siacs.conversations.xmpp.jingle.stanzas.GenericDescription; import eu.siacs.conversations.xmpp.jingle.stanzas.GenericTransportInfo; @@ -137,7 +135,37 @@ public class RtpContentMap { final IceUdpTransportInfo newTransportInfo = transportInfo.cloneWrapper(); newTransportInfo.addChild(candidate); return new RtpContentMap(null, ImmutableMap.of(contentName, new DescriptionTransport(null, newTransportInfo))); + } + RtpContentMap transportInfo() { + return new RtpContentMap( + null, + Maps.transformValues(contents, dt -> new DescriptionTransport(null, dt.transport.cloneWrapper())) + ); + } + + public Map getCredentials() { + return Maps.transformValues(contents, dt -> dt.transport.getCredentials()); + } + + public boolean emptyCandidates() { + int count = 0; + for (DescriptionTransport descriptionTransport : contents.values()) { + count += descriptionTransport.transport.getCandidates().size(); + } + return count == 0; + } + + public RtpContentMap modifiedCredentials(Map credentialsMap) { + final ImmutableMap.Builder contentMapBuilder = new ImmutableMap.Builder<>(); + for (final Map.Entry content : contents.entrySet()) { + final RtpDescription rtpDescription = content.getValue().description; + IceUdpTransportInfo transportInfo = content.getValue().transport; + final IceUdpTransportInfo.Credentials credentials = Objects.requireNonNull(credentialsMap.get(content.getKey())); + final IceUdpTransportInfo modifiedTransportInfo = transportInfo.modifyCredentials(credentials); + contentMapBuilder.put(content.getKey(), new DescriptionTransport(rtpDescription, modifiedTransportInfo)); + } + return new RtpContentMap(this.group, contentMapBuilder.build()); } public static class DescriptionTransport { 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 9ea4cd389..401121b86 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java @@ -17,7 +17,6 @@ import com.google.common.util.concurrent.SettableFuture; import org.webrtc.AudioSource; import org.webrtc.AudioTrack; -import org.webrtc.Camera1Enumerator; import org.webrtc.Camera2Enumerator; import org.webrtc.CameraEnumerationAndroid; import org.webrtc.CameraEnumerator; @@ -87,6 +86,7 @@ public class WebRTCWrapper { private final EventCallback eventCallback; private final AtomicBoolean readyToReceivedIceCandidates = new AtomicBoolean(false); + private final AtomicBoolean ignoreOnRenegotiationNeeded = new AtomicBoolean(false); private final Queue iceCandidates = new LinkedList<>(); private final AppRTCAudioManager.AudioManagerEvents audioManagerEvents = new AppRTCAudioManager.AudioManagerEvents() { @Override @@ -163,6 +163,10 @@ public class WebRTCWrapper { @Override public void onRenegotiationNeeded() { + if (ignoreOnRenegotiationNeeded.get()) { + Log.d(EXTENDED_LOGGING_TAG, "ignoring onRenegotiationNeeded()"); + return; + } Log.d(EXTENDED_LOGGING_TAG, "onRenegotiationNeeded()"); final PeerConnection.PeerConnectionState currentState = peerConnection == null ? null : peerConnection.connectionState(); if (currentState != null && currentState != PeerConnection.PeerConnectionState.NEW) { @@ -307,12 +311,12 @@ public class WebRTCWrapper { } void restartIce() { - executorService.execute(()-> requirePeerConnection().restartIce()); + executorService.execute(() -> requirePeerConnection().restartIce()); } public void setIsReadyToReceiveIceCandidates(final boolean ready) { readyToReceivedIceCandidates.set(ready); - while(ready && iceCandidates.peek() != null) { + while (ready && iceCandidates.peek() != null) { eventCallback.onIceCandidate(iceCandidates.poll()); } } @@ -452,6 +456,26 @@ public class WebRTCWrapper { }, MoreExecutors.directExecutor()); } + public ListenableFuture rollbackLocalDescription() { + final SettableFuture future = SettableFuture.create(); + final SessionDescription rollback = new SessionDescription(SessionDescription.Type.ROLLBACK, ""); + ignoreOnRenegotiationNeeded.set(true); + requirePeerConnection().setLocalDescription(new SetSdpObserver() { + @Override + public void onSetSuccess() { + future.set(null); + ignoreOnRenegotiationNeeded.set(false); + } + + @Override + public void onSetFailure(final String message) { + future.setException(new FailureToSetDescriptionException(message)); + } + }, rollback); + return future; + } + + 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); @@ -552,6 +576,10 @@ public class WebRTCWrapper { executorService.execute(command); } + public PeerConnection.SignalingState getSignalingState() { + return requirePeerConnection().signalingState(); + } + public interface EventCallback { void onIceCandidate(IceCandidate iceCandidate); diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java index 022c4d2dd..2b8770578 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java @@ -1,6 +1,7 @@ package eu.siacs.conversations.xmpp.jingle.stanzas; import com.google.common.base.Joiner; +import com.google.common.base.Objects; import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.collect.ArrayListMultimap; @@ -8,6 +9,7 @@ import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; +import java.util.Collection; import java.util.HashMap; import java.util.Hashtable; import java.util.LinkedHashMap; @@ -58,6 +60,12 @@ public class IceUdpTransportInfo extends GenericTransportInfo { return fingerprint == null ? null : Fingerprint.upgrade(fingerprint); } + public Credentials getCredentials() { + final String ufrag = this.getAttribute("ufrag"); + final String password = this.getAttribute("pwd"); + return new Credentials(ufrag, password); + } + public List getCandidates() { final ImmutableList.Builder builder = new ImmutableList.Builder<>(); for (final Element child : getChildren()) { @@ -74,6 +82,37 @@ public class IceUdpTransportInfo extends GenericTransportInfo { return transportInfo; } + public IceUdpTransportInfo modifyCredentials(Credentials credentials) { + final IceUdpTransportInfo transportInfo = new IceUdpTransportInfo(); + transportInfo.setAttribute("ufrag", credentials.ufrag); + transportInfo.setAttribute("pwd", credentials.password); + transportInfo.setChildren(getChildren()); + return transportInfo; + } + + public static class Credentials { + public final String ufrag; + public final String password; + + public Credentials(String ufrag, String password) { + this.ufrag = ufrag; + this.password = password; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Credentials that = (Credentials) o; + return Objects.equal(ufrag, that.ufrag) && Objects.equal(password, that.password); + } + + @Override + public int hashCode() { + return Objects.hashCode(ufrag, password); + } + } + public static class Candidate extends Element { private Candidate() { @@ -89,7 +128,7 @@ public class IceUdpTransportInfo extends GenericTransportInfo { } // https://tools.ietf.org/html/draft-ietf-mmusic-ice-sip-sdp-39#section-5.1 - public static Candidate fromSdpAttribute(final String attribute) { + public static Candidate fromSdpAttribute(final String attribute, Collection currentUfrags) { final String[] pair = attribute.split(":", 2); if (pair.length == 2 && "candidate".equals(pair[0])) { final String[] segments = pair[1].split(" "); @@ -105,6 +144,10 @@ public class IceUdpTransportInfo extends GenericTransportInfo { for (int i = 6; i < segments.length - 1; i = i + 2) { additional.put(segments[i], segments[i + 1]); } + final String ufrag = additional.get("ufrag"); + if (ufrag != null && !currentUfrags.contains(ufrag)) { + return null; + } final Candidate candidate = new Candidate(); candidate.setAttribute("component", component); candidate.setAttribute("foundation", foundation); From 3f402b132b122f0018a9b27afe846d558ae7b40c Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Mon, 15 Nov 2021 13:03:04 +0100 Subject: [PATCH 12/24] respond with tie-break to prevent ICE restart race --- .../xmpp/jingle/JingleRtpConnection.java | 80 +++++++++++-------- .../jingle/stanzas/IceUdpTransportInfo.java | 9 +++ 2 files changed, 56 insertions(+), 33 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 71cdb02c4..e6a34cd8e 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -261,22 +261,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web respondOk(jinglePacket); return; } - final Set> candidates = contentMap.contents.entrySet(); - if (this.state == State.SESSION_ACCEPTED) { - //zero candidates + modified credentials are an ICE restart offer - if (checkForIceRestart(contentMap, jinglePacket)) { - return; - } - respondOk(jinglePacket); - try { - processCandidates(candidates); - } catch (final WebRTCWrapper.PeerConnectionNotInitialized e) { - Log.w(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnection was not initialized when processing transport info. this usually indicates a race condition that can be ignored"); - } - } else { - respondOk(jinglePacket); - pendingIceCandidates.addAll(candidates); - } + receiveTransportInfo(jinglePacket, contentMap); } else { if (isTerminated()) { respondOk(jinglePacket); @@ -288,7 +273,26 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } } - private boolean checkForIceRestart(final RtpContentMap rtpContentMap, final JinglePacket jinglePacket) { + private void receiveTransportInfo(final JinglePacket jinglePacket, final RtpContentMap contentMap) { + final Set> candidates = contentMap.contents.entrySet(); + if (this.state == State.SESSION_ACCEPTED) { + //zero candidates + modified credentials are an ICE restart offer + if (checkForIceRestart(jinglePacket, contentMap)) { + return; + } + respondOk(jinglePacket); + try { + processCandidates(candidates); + } catch (final WebRTCWrapper.PeerConnectionNotInitialized e) { + Log.w(Config.LOGTAG, id.account.getJid().asBareJid() + ": PeerConnection was not initialized when processing transport info. this usually indicates a race condition that can be ignored"); + } + } else { + respondOk(jinglePacket); + pendingIceCandidates.addAll(candidates); + } + } + + private boolean checkForIceRestart(final JinglePacket jinglePacket, final RtpContentMap rtpContentMap) { final RtpContentMap existing = getRemoteContentMap(); final Map existingCredentials = existing.getCredentials(); final Map newCredentials = rtpContentMap.getCredentials(); @@ -299,25 +303,30 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web return false; } final boolean isOffer = rtpContentMap.emptyCandidates(); - Log.d(Config.LOGTAG, "detected ICE restart. offer=" + isOffer + " " + jinglePacket); - //TODO reset to 'actpass'? + if (isOffer) { + Log.d(Config.LOGTAG, "received offer to restart ICE " + newCredentials); + } else { + Log.d(Config.LOGTAG, "received confirmation of ICE restart" + newCredentials); + } + //TODO rewrite setup attribute + //https://groups.google.com/g/discuss-webrtc/c/DfpIMwvUfeM + //https://datatracker.ietf.org/doc/html/draft-ietf-mmusic-dtls-sdp-15#section-5.5 final RtpContentMap restartContentMap = existing.modifiedCredentials(newCredentials); try { - if (applyIceRestart(isOffer, restartContentMap)) { - return false; + if (applyIceRestart(jinglePacket, restartContentMap, isOffer)) { + return isOffer; } else { - Log.d(Config.LOGTAG, "responding with tie break"); - //TODO respond with conflict + respondWithTieBreak(jinglePacket); return true; } } catch (Exception e) { Log.d(Config.LOGTAG, "failure to apply ICE restart. sending error", e); - //TODO send some kind of error + //TODO respond OK and then terminate session return true; } } - private boolean applyIceRestart(final boolean isOffer, final RtpContentMap restartContentMap) throws ExecutionException, InterruptedException { + private boolean applyIceRestart(final JinglePacket jinglePacket, final RtpContentMap restartContentMap, final boolean isOffer) throws ExecutionException, InterruptedException { final SessionDescription sessionDescription = SessionDescription.of(restartContentMap); final org.webrtc.SessionDescription.Type type = isOffer ? org.webrtc.SessionDescription.Type.OFFER : org.webrtc.SessionDescription.Type.ANSWER; org.webrtc.SessionDescription sdp = new org.webrtc.SessionDescription(type, sessionDescription.toString()); @@ -339,6 +348,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web webRTCWrapper.setIsReadyToReceiveIceCandidates(false); final SessionDescription localSessionDescription = setLocalSessionDescription(); setLocalContentMap(RtpContentMap.of(localSessionDescription)); + //We need to respond OK before sending any candidates + respondOk(jinglePacket); webRTCWrapper.setIsReadyToReceiveIceCandidates(true); } return true; @@ -447,6 +458,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web private void receiveSessionInitiate(final JinglePacket jinglePacket, final RtpContentMap contentMap) { try { contentMap.requireContentDescriptions(); + //TODO require actpass contentMap.requireDTLSFingerprint(); } catch (final RuntimeException e) { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": improperly formatted contents", Throwables.getRootCause(e)); @@ -1072,8 +1084,16 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web this.finish(); } + private void respondWithTieBreak(final JinglePacket jinglePacket) { + respondWithJingleError(jinglePacket, "tie-break", "conflict", "cancel"); + } + private void respondWithOutOfOrder(final JinglePacket jinglePacket) { - jingleConnectionManager.respondWithJingleError(id.account, jinglePacket, "out-of-order", "unexpected-request", "wait"); + respondWithJingleError(jinglePacket, "out-of-order", "unexpected-request", "wait"); + } + + void respondWithJingleError(final IqPacket original, String jingleCondition, String condition, String conditionType) { + jingleConnectionManager.respondWithJingleError(id.account, original, jingleCondition, condition, conditionType); } private void respondOk(final JinglePacket jinglePacket) { @@ -1409,7 +1429,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web final Collection currentUfrags = Collections2.transform(rtpContentMap.getCredentials().values(), c -> c.ufrag); final IceUdpTransportInfo.Candidate candidate = IceUdpTransportInfo.Candidate.fromSdpAttribute(iceCandidate.sdp, currentUfrags); if (candidate == null) { - Log.d(Config.LOGTAG,"ignoring (not sending) candidate: "+iceCandidate.toString()); + Log.d(Config.LOGTAG, "ignoring (not sending) candidate: " + iceCandidate.toString()); return; } Log.d(Config.LOGTAG, "sending candidate: " + iceCandidate.toString()); @@ -1448,16 +1468,10 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web @Override public void onRenegotiationNeeded() { - Log.d(Config.LOGTAG, "onRenegotiationNeeded()"); this.webRTCWrapper.execute(this::initiateIceRestart); } private void initiateIceRestart() { - PeerConnection.SignalingState signalingState = webRTCWrapper.getSignalingState(); - Log.d(Config.LOGTAG, "initiateIceRestart() - " + signalingState); - if (signalingState != PeerConnection.SignalingState.STABLE) { - return; - } this.webRTCWrapper.setIsReadyToReceiveIceCandidates(false); final SessionDescription sessionDescription; try { diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java index 2b8770578..1586557b7 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java @@ -1,6 +1,7 @@ package eu.siacs.conversations.xmpp.jingle.stanzas; import com.google.common.base.Joiner; +import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.common.base.Preconditions; import com.google.common.base.Strings; @@ -111,6 +112,14 @@ public class IceUdpTransportInfo extends GenericTransportInfo { public int hashCode() { return Objects.hashCode(ufrag, password); } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("ufrag", ufrag) + .add("password", password) + .toString(); + } } public static class Candidate extends Element { From 0a3947b8e308fbeb04492542dba3e726d81a3104 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Mon, 15 Nov 2021 17:18:43 +0100 Subject: [PATCH 13/24] terminate with application failure when failing to apply ICE restart This is fairly unlikely to happen in practice --- .../xmpp/jingle/JingleRtpConnection.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 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 e6a34cd8e..04c43ec1c 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -319,14 +319,17 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web respondWithTieBreak(jinglePacket); return true; } - } catch (Exception e) { - Log.d(Config.LOGTAG, "failure to apply ICE restart. sending error", e); - //TODO respond OK and then terminate session + } catch (final Exception exception) { + respondOk(jinglePacket); + final Throwable rootCause = Throwables.getRootCause(exception); + Log.d(Config.LOGTAG, "failure to apply ICE restart", rootCause); + webRTCWrapper.close(); + sendSessionTerminate(Reason.ofThrowable(rootCause), rootCause.getMessage()); return true; } } - private boolean applyIceRestart(final JinglePacket jinglePacket, final RtpContentMap restartContentMap, final boolean isOffer) throws ExecutionException, InterruptedException { + private boolean applyIceRestart(final JinglePacket jinglePacket, final RtpContentMap restartContentMap, final boolean isOffer) throws ExecutionException, InterruptedException { final SessionDescription sessionDescription = SessionDescription.of(restartContentMap); final org.webrtc.SessionDescription.Type type = isOffer ? org.webrtc.SessionDescription.Type.OFFER : org.webrtc.SessionDescription.Type.ANSWER; org.webrtc.SessionDescription sdp = new org.webrtc.SessionDescription(type, sessionDescription.toString()); @@ -574,7 +577,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } catch (final Exception e) { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": unable to set remote description after receiving session-accept", Throwables.getRootCause(e)); webRTCWrapper.close(); - sendSessionTerminate(Reason.FAILED_APPLICATION); + sendSessionTerminate(Reason.FAILED_APPLICATION, Throwables.getRootCause(e).getMessage()); return; } processCandidates(contentMap.contents.entrySet()); @@ -624,7 +627,6 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web 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); } } @@ -633,9 +635,10 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web if (isTerminated()) { return; } - Log.d(Config.LOGTAG, "unable to send session accept", Throwables.getRootCause(throwable)); + final Throwable rootCause = Throwables.getRootCause(throwable); + Log.d(Config.LOGTAG, "unable to send session accept", rootCause); webRTCWrapper.close(); - sendSessionTerminate(Reason.ofThrowable(throwable)); + sendSessionTerminate(Reason.ofThrowable(rootCause), rootCause.getMessage()); } private void addIceCandidatesFromBlackLog() { From 70b5d8d81aa3059623570972a58b7c595c1e1f26 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Mon, 15 Nov 2021 21:49:31 +0100 Subject: [PATCH 14/24] set proper peer dtls setup on ice restart received --- .../xmpp/jingle/JingleRtpConnection.java | 31 +++++++++----- .../xmpp/jingle/RtpContentMap.java | 16 ++++++-- .../xmpp/jingle/SessionDescription.java | 5 ++- .../jingle/stanzas/IceUdpTransportInfo.java | 40 +++++++++++++++++-- 4 files changed, 74 insertions(+), 18 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 04c43ec1c..c0ddfd96a 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -303,16 +303,18 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web return false; } final boolean isOffer = rtpContentMap.emptyCandidates(); - if (isOffer) { - Log.d(Config.LOGTAG, "received offer to restart ICE " + newCredentials); - } else { - Log.d(Config.LOGTAG, "received confirmation of ICE restart" + newCredentials); - } - //TODO rewrite setup attribute - //https://groups.google.com/g/discuss-webrtc/c/DfpIMwvUfeM - //https://datatracker.ietf.org/doc/html/draft-ietf-mmusic-dtls-sdp-15#section-5.5 - final RtpContentMap restartContentMap = existing.modifiedCredentials(newCredentials); + final RtpContentMap restartContentMap; try { + if (isOffer) { + Log.d(Config.LOGTAG, "received offer to restart ICE " + newCredentials.values()); + restartContentMap = existing.modifiedCredentials(newCredentials, IceUdpTransportInfo.Setup.ACTPASS); + } else { + final IceUdpTransportInfo.Setup setup = getPeerDtlsSetup(); + Log.d(Config.LOGTAG, "received confirmation of ICE restart" + newCredentials.values()+" peer_setup="+setup); + // DTLS setup attribute needs to be rewritten to reflect current peer state + // https://groups.google.com/g/discuss-webrtc/c/DfpIMwvUfeM + restartContentMap = existing.modifiedCredentials(newCredentials, setup); + } if (applyIceRestart(jinglePacket, restartContentMap, isOffer)) { return isOffer; } else { @@ -329,6 +331,14 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } } + private IceUdpTransportInfo.Setup getPeerDtlsSetup() { + final IceUdpTransportInfo.Setup responderSetup = this.responderRtpContentMap.getDtlsSetup(); + if (responderSetup == null || responderSetup == IceUdpTransportInfo.Setup.ACTPASS) { + throw new IllegalStateException("Invalid DTLS setup value in responder content map"); + } + return isInitiator() ? responderSetup : responderSetup.flip(); + } + private boolean applyIceRestart(final JinglePacket jinglePacket, final RtpContentMap restartContentMap, final boolean isOffer) throws ExecutionException, InterruptedException { final SessionDescription sessionDescription = SessionDescription.of(restartContentMap); final org.webrtc.SessionDescription.Type type = isOffer ? org.webrtc.SessionDescription.Type.OFFER : org.webrtc.SessionDescription.Type.ANSWER; @@ -1496,7 +1506,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web webRTCWrapper.setIsReadyToReceiveIceCandidates(true); } else { Log.d(Config.LOGTAG, "received failure to our ice restart"); - //TODO handle tie-break. Rollback? + //TODO ignore tie break (maybe rollback?) + //TODO handle other errors } }); } diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java index 99db8bd34..5366460ec 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java @@ -6,6 +6,7 @@ import com.google.common.base.Strings; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -104,7 +105,8 @@ public class RtpContentMap { if (fingerprint == null || Strings.isNullOrEmpty(fingerprint.getContent()) || Strings.isNullOrEmpty(fingerprint.getHash())) { throw new SecurityException(String.format("Use of DTLS-SRTP (XEP-0320) is required for content %s", entry.getKey())); } - if (Strings.isNullOrEmpty(fingerprint.getSetup())) { + final IceUdpTransportInfo.Setup setup = fingerprint.getSetup(); + if (setup == null) { throw new SecurityException(String.format("Use of DTLS-SRTP (XEP-0320) is required for content %s but missing setup attribute", entry.getKey())); } } @@ -148,6 +150,14 @@ public class RtpContentMap { return Maps.transformValues(contents, dt -> dt.transport.getCredentials()); } + public IceUdpTransportInfo.Setup getDtlsSetup() { + final Set setups = ImmutableSet.copyOf(Collections2.transform( + contents.values(), + dt->dt.transport.getFingerprint().getSetup() + )); + return setups.size() == 1 ? Iterables.getFirst(setups, null) : null; + } + public boolean emptyCandidates() { int count = 0; for (DescriptionTransport descriptionTransport : contents.values()) { @@ -156,13 +166,13 @@ public class RtpContentMap { return count == 0; } - public RtpContentMap modifiedCredentials(Map credentialsMap) { + public RtpContentMap modifiedCredentials(Map credentialsMap, final IceUdpTransportInfo.Setup setup) { final ImmutableMap.Builder contentMapBuilder = new ImmutableMap.Builder<>(); for (final Map.Entry content : contents.entrySet()) { final RtpDescription rtpDescription = content.getValue().description; IceUdpTransportInfo transportInfo = content.getValue().transport; final IceUdpTransportInfo.Credentials credentials = Objects.requireNonNull(credentialsMap.get(content.getKey())); - final IceUdpTransportInfo modifiedTransportInfo = transportInfo.modifyCredentials(credentials); + final IceUdpTransportInfo modifiedTransportInfo = transportInfo.modifyCredentials(credentials, setup); contentMapBuilder.put(content.getKey(), new DescriptionTransport(rtpDescription, modifiedTransportInfo)); } return new RtpContentMap(this.group, contentMapBuilder.build()); diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java index 39031c4a9..e113146b1 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java @@ -156,7 +156,10 @@ public class SessionDescription { final IceUdpTransportInfo.Fingerprint fingerprint = transport.getFingerprint(); if (fingerprint != null) { mediaAttributes.put("fingerprint", fingerprint.getHash() + " " + fingerprint.getContent()); - mediaAttributes.put("setup", fingerprint.getSetup()); + final IceUdpTransportInfo.Setup setup = fingerprint.getSetup(); + if (setup != null) { + mediaAttributes.put("setup", setup.toString().toLowerCase(Locale.ROOT)); + } } final ImmutableList.Builder formatBuilder = new ImmutableList.Builder<>(); for (RtpDescription.PayloadType payloadType : description.getPayloadTypes()) { diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java index 1586557b7..9af1186fc 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java @@ -10,6 +10,7 @@ import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; +import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.Hashtable; @@ -83,11 +84,19 @@ public class IceUdpTransportInfo extends GenericTransportInfo { return transportInfo; } - public IceUdpTransportInfo modifyCredentials(Credentials credentials) { + public IceUdpTransportInfo modifyCredentials(final Credentials credentials, final Setup setup) { final IceUdpTransportInfo transportInfo = new IceUdpTransportInfo(); transportInfo.setAttribute("ufrag", credentials.ufrag); transportInfo.setAttribute("pwd", credentials.password); - transportInfo.setChildren(getChildren()); + for (final Element child : getChildren()) { + if (child.getName().equals("fingerprint") && Namespace.JINGLE_APPS_DTLS.equals(child.getNamespace())) { + final Fingerprint fingerprint = new Fingerprint(); + fingerprint.setAttributes(new Hashtable<>(child.getAttributes())); + fingerprint.setContent(child.getContent()); + fingerprint.setAttribute("setup", setup.toString().toLowerCase(Locale.ROOT)); + transportInfo.addChild(fingerprint); + } + } return transportInfo; } @@ -337,8 +346,31 @@ public class IceUdpTransportInfo extends GenericTransportInfo { return this.getAttribute("hash"); } - public String getSetup() { - return this.getAttribute("setup"); + public Setup getSetup() { + final String setup = this.getAttribute("setup"); + return setup == null ? null : Setup.of(setup); + } + } + + public enum Setup { + ACTPASS, PASSIVE, ACTIVE; + + public static Setup of(String setup) { + try { + return valueOf(setup.toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException e) { + return null; + } + } + + public Setup flip() { + if (this == PASSIVE) { + return ACTIVE; + } + if (this == ACTIVE) { + return PASSIVE; + } + throw new IllegalStateException(this.name()+" can not be flipped"); } } } From 0698fa0d8c55507d0f52051d47603dd8ca3bef9e Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Tue, 16 Nov 2021 11:21:11 +0100 Subject: [PATCH 15/24] store peer dtls setup for later use in ice restart --- .../xmpp/jingle/JingleRtpConnection.java | 34 ++++++++++++++----- .../xmpp/jingle/RtpContentMap.java | 6 +++- 2 files changed, 30 insertions(+), 10 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 c0ddfd96a..ce726a2f0 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -150,6 +150,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web private Set proposedMedia; private RtpContentMap initiatorRtpContentMap; private RtpContentMap responderRtpContentMap; + private IceUdpTransportInfo.Setup peerDtlsSetup; private final Stopwatch sessionDuration = Stopwatch.createUnstarted(); private final Queue stateHistory = new LinkedList<>(); private ScheduledFuture ringingTimeoutFuture; @@ -332,11 +333,18 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } private IceUdpTransportInfo.Setup getPeerDtlsSetup() { - final IceUdpTransportInfo.Setup responderSetup = this.responderRtpContentMap.getDtlsSetup(); - if (responderSetup == null || responderSetup == IceUdpTransportInfo.Setup.ACTPASS) { - throw new IllegalStateException("Invalid DTLS setup value in responder content map"); + final IceUdpTransportInfo.Setup peerSetup = this.peerDtlsSetup; + if (peerSetup == null || peerSetup == IceUdpTransportInfo.Setup.ACTPASS) { + throw new IllegalStateException("Invalid peer setup"); } - return isInitiator() ? responderSetup : responderSetup.flip(); + return peerSetup; + } + + private void storePeerDtlsSetup(final IceUdpTransportInfo.Setup setup) { + if (setup == null || setup == IceUdpTransportInfo.Setup.ACTPASS) { + throw new IllegalArgumentException("Trying to store invalid peer dtls setup"); + } + this.peerDtlsSetup = setup; } private boolean applyIceRestart(final JinglePacket jinglePacket, final RtpContentMap restartContentMap, final boolean isOffer) throws ExecutionException, InterruptedException { @@ -352,11 +360,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web webRTCWrapper.rollbackLocalDescription().get(); } webRTCWrapper.setRemoteDescription(sdp).get(); - if (isInitiator()) { - this.responderRtpContentMap = restartContentMap; - } else { - this.initiatorRtpContentMap = restartContentMap; - } + setRemoteContentMap(restartContentMap); if (isOffer) { webRTCWrapper.setIsReadyToReceiveIceCandidates(false); final SessionDescription localSessionDescription = setLocalSessionDescription(); @@ -364,6 +368,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web //We need to respond OK before sending any candidates respondOk(jinglePacket); webRTCWrapper.setIsReadyToReceiveIceCandidates(true); + } else { + storePeerDtlsSetup(restartContentMap.getDtlsSetup()); } return true; } @@ -569,6 +575,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web private void receiveSessionAccept(final RtpContentMap contentMap) { this.responderRtpContentMap = contentMap; + this.storePeerDtlsSetup(contentMap.getDtlsSetup()); final SessionDescription sessionDescription; try { sessionDescription = SessionDescription.of(contentMap); @@ -663,6 +670,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web final SessionDescription sessionDescription = SessionDescription.parse(webRTCSessionDescription.description); final RtpContentMap respondingRtpContentMap = RtpContentMap.of(sessionDescription); this.responderRtpContentMap = respondingRtpContentMap; + storePeerDtlsSetup(respondingRtpContentMap.getDtlsSetup().flip()); webRTCWrapper.setIsReadyToReceiveIceCandidates(true); final ListenableFuture outgoingContentMapFuture = prepareOutgoingContentMap(respondingRtpContentMap); Futures.addCallback(outgoingContentMapFuture, @@ -1520,6 +1528,14 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } } + private void setRemoteContentMap(final RtpContentMap rtpContentMap) { + if (isInitiator()) { + this.responderRtpContentMap = rtpContentMap; + } else { + this.initiatorRtpContentMap = rtpContentMap; + } + } + private SessionDescription setLocalSessionDescription() throws ExecutionException, InterruptedException { final org.webrtc.SessionDescription sessionDescription = this.webRTCWrapper.setLocalDescription().get(); return SessionDescription.parse(sessionDescription.description); diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java index 5366460ec..091bf7c10 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java @@ -155,7 +155,11 @@ public class RtpContentMap { contents.values(), dt->dt.transport.getFingerprint().getSetup() )); - return setups.size() == 1 ? Iterables.getFirst(setups, null) : null; + final IceUdpTransportInfo.Setup setup = Iterables.getFirst(setups, null); + if (setups.size() == 1 && setup != null) { + return setup; + } + throw new IllegalStateException("Content map doesn't have distinct DTLS setup"); } public boolean emptyCandidates() { From 297a843b9c6b245f4e3bb07a23e487a4f6d5d9f8 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Tue, 16 Nov 2021 13:17:10 +0100 Subject: [PATCH 16/24] use implicit rollback (needed to be enabled on libwebrtc) --- .../xmpp/jingle/JingleRtpConnection.java | 2 -- .../xmpp/jingle/WebRTCWrapper.java | 26 +------------------ 2 files changed, 1 insertion(+), 27 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 ce726a2f0..5c474a754 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -356,8 +356,6 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web //We ignore the offer and respond with tie-break. This will clause the responder not to apply the content map return false; } - //rollback our own local description. should happen automatically but doesn't - webRTCWrapper.rollbackLocalDescription().get(); } webRTCWrapper.setRemoteDescription(sdp).get(); setRemoteContentMap(restartContentMap); 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 401121b86..13b695b0e 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java @@ -86,7 +86,6 @@ public class WebRTCWrapper { private final EventCallback eventCallback; private final AtomicBoolean readyToReceivedIceCandidates = new AtomicBoolean(false); - private final AtomicBoolean ignoreOnRenegotiationNeeded = new AtomicBoolean(false); private final Queue iceCandidates = new LinkedList<>(); private final AppRTCAudioManager.AudioManagerEvents audioManagerEvents = new AppRTCAudioManager.AudioManagerEvents() { @Override @@ -163,10 +162,6 @@ public class WebRTCWrapper { @Override public void onRenegotiationNeeded() { - if (ignoreOnRenegotiationNeeded.get()) { - Log.d(EXTENDED_LOGGING_TAG, "ignoring onRenegotiationNeeded()"); - return; - } Log.d(EXTENDED_LOGGING_TAG, "onRenegotiationNeeded()"); final PeerConnection.PeerConnectionState currentState = peerConnection == null ? null : peerConnection.connectionState(); if (currentState != null && currentState != PeerConnection.PeerConnectionState.NEW) { @@ -277,6 +272,7 @@ public class WebRTCWrapper { rtcConfig.continualGatheringPolicy = PeerConnection.ContinualGatheringPolicy.GATHER_CONTINUALLY; rtcConfig.sdpSemantics = PeerConnection.SdpSemantics.UNIFIED_PLAN; rtcConfig.rtcpMuxPolicy = PeerConnection.RtcpMuxPolicy.NEGOTIATE; + rtcConfig.enableImplicitRollback = true; final PeerConnection peerConnection = peerConnectionFactory.createPeerConnection(rtcConfig, peerConnectionObserver); if (peerConnection == null) { throw new InitializationException("Unable to create PeerConnection"); @@ -456,26 +452,6 @@ public class WebRTCWrapper { }, MoreExecutors.directExecutor()); } - public ListenableFuture rollbackLocalDescription() { - final SettableFuture future = SettableFuture.create(); - final SessionDescription rollback = new SessionDescription(SessionDescription.Type.ROLLBACK, ""); - ignoreOnRenegotiationNeeded.set(true); - requirePeerConnection().setLocalDescription(new SetSdpObserver() { - @Override - public void onSetSuccess() { - future.set(null); - ignoreOnRenegotiationNeeded.set(false); - } - - @Override - public void onSetFailure(final String message) { - future.setException(new FailureToSetDescriptionException(message)); - } - }, rollback); - return future; - } - - 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); From abb671616cf0b085d9dbe2fc3b1d3eb446decab2 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Tue, 16 Nov 2021 15:17:12 +0100 Subject: [PATCH 17/24] synchronize setDescription calls --- .../conversations/xmpp/XmppConnection.java | 1 - .../xmpp/jingle/JingleRtpConnection.java | 30 +++++++++-------- .../xmpp/jingle/WebRTCWrapper.java | 33 ++++++++++--------- 3 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index abe5d161f..c3a3b1532 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -54,7 +54,6 @@ import javax.net.ssl.X509TrustManager; import eu.siacs.conversations.Config; import eu.siacs.conversations.R; -import eu.siacs.conversations.crypto.DomainHostnameVerifier; import eu.siacs.conversations.crypto.XmppDomainVerifier; import eu.siacs.conversations.crypto.axolotl.AxolotlService; import eu.siacs.conversations.crypto.sasl.Anonymous; 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 5c474a754..66cf5c23b 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -311,7 +311,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web restartContentMap = existing.modifiedCredentials(newCredentials, IceUdpTransportInfo.Setup.ACTPASS); } else { final IceUdpTransportInfo.Setup setup = getPeerDtlsSetup(); - Log.d(Config.LOGTAG, "received confirmation of ICE restart" + newCredentials.values()+" peer_setup="+setup); + Log.d(Config.LOGTAG, "received confirmation of ICE restart" + newCredentials.values() + " peer_setup=" + setup); // DTLS setup attribute needs to be rewritten to reflect current peer state // https://groups.google.com/g/discuss-webrtc/c/DfpIMwvUfeM restartContentMap = existing.modifiedCredentials(newCredentials, setup); @@ -319,12 +319,18 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web if (applyIceRestart(jinglePacket, restartContentMap, isOffer)) { return isOffer; } else { + Log.d(Config.LOGTAG,"ignored ice restart. offer="+isOffer); respondWithTieBreak(jinglePacket); return true; } } catch (final Exception exception) { respondOk(jinglePacket); final Throwable rootCause = Throwables.getRootCause(exception); + if (rootCause instanceof WebRTCWrapper.PeerConnectionNotInitialized) { + Log.d(Config.LOGTAG,"ignoring PeerConnectionNotInitialized"); + //TODO don’t respond OK but respond with out-of-order + return true; + } Log.d(Config.LOGTAG, "failure to apply ICE restart", rootCause); webRTCWrapper.close(); sendSessionTerminate(Reason.ofThrowable(rootCause), rootCause.getMessage()); @@ -1466,21 +1472,18 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } final boolean neverConnected = !this.stateHistory.contains(PeerConnection.PeerConnectionState.CONNECTED); - final boolean failedOrDisconnected = Arrays.asList( - PeerConnection.PeerConnectionState.FAILED, - PeerConnection.PeerConnectionState.DISCONNECTED - ).contains(newState); - - if (neverConnected && failedOrDisconnected) { - if (isTerminated()) { - Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": not sending session-terminate after connectivity error because session is already in state " + this.state); + if (newState == PeerConnection.PeerConnectionState.FAILED) { + if (neverConnected) { + if (isTerminated()) { + Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": not sending session-terminate after connectivity error because session is already in state " + this.state); + return; + } + webRTCWrapper.execute(this::closeWebRTCSessionAfterFailedConnection); return; + } else { + webRTCWrapper.restartIce(); } - webRTCWrapper.execute(this::closeWebRTCSessionAfterFailedConnection); - } else if (newState == PeerConnection.PeerConnectionState.FAILED) { - Log.d(Config.LOGTAG, "attempting to restart ICE"); - webRTCWrapper.restartIce(); } updateEndUserState(); } @@ -1491,6 +1494,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } private void initiateIceRestart() { + this.stateHistory.clear(); this.webRTCWrapper.setIsReadyToReceiveIceCandidates(false); final SessionDescription sessionDescription; try { 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 13b695b0e..0712fa900 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java @@ -431,7 +431,7 @@ public class WebRTCWrapper { videoTrack.setEnabled(enabled); } - ListenableFuture setLocalDescription() { + synchronized ListenableFuture setLocalDescription() { return Futures.transformAsync(getPeerConnectionFuture(), peerConnection -> { final SettableFuture future = SettableFuture.create(); peerConnection.setLocalDescription(new SetSdpObserver() { @@ -458,7 +458,7 @@ public class WebRTCWrapper { } } - ListenableFuture setRemoteDescription(final SessionDescription sessionDescription) { + synchronized ListenableFuture setRemoteDescription(final SessionDescription sessionDescription) { Log.d(EXTENDED_LOGGING_TAG, "setting remote description:"); logDescription(sessionDescription); return Futures.transformAsync(getPeerConnectionFuture(), peerConnection -> { @@ -482,12 +482,20 @@ public class WebRTCWrapper { private ListenableFuture getPeerConnectionFuture() { final PeerConnection peerConnection = this.peerConnection; if (peerConnection == null) { - return Futures.immediateFailedFuture(new IllegalStateException("initialize PeerConnection first")); + return Futures.immediateFailedFuture(new PeerConnectionNotInitialized()); } else { return Futures.immediateFuture(peerConnection); } } + private PeerConnection requirePeerConnection() { + final PeerConnection peerConnection = this.peerConnection; + if (peerConnection == null) { + throw new PeerConnectionNotInitialized(); + } + return peerConnection; + } + void addIceCandidate(IceCandidate iceCandidate) { requirePeerConnection().addIceCandidate(iceCandidate); } @@ -512,10 +520,15 @@ public class WebRTCWrapper { } } - public PeerConnection.PeerConnectionState getState() { + PeerConnection.PeerConnectionState getState() { return requirePeerConnection().connectionState(); } + public PeerConnection.SignalingState getSignalingState() { + return requirePeerConnection().signalingState(); + } + + EglBase.Context getEglBaseContext() { return this.eglBase.getEglBaseContext(); } @@ -528,14 +541,6 @@ public class WebRTCWrapper { return Optional.fromNullable(this.remoteVideoTrack); } - private PeerConnection requirePeerConnection() { - final PeerConnection peerConnection = this.peerConnection; - if (peerConnection == null) { - throw new PeerConnectionNotInitialized(); - } - return peerConnection; - } - private Context requireContext() { final Context context = this.context; if (context == null) { @@ -552,10 +557,6 @@ public class WebRTCWrapper { executorService.execute(command); } - public PeerConnection.SignalingState getSignalingState() { - return requirePeerConnection().signalingState(); - } - public interface EventCallback { void onIceCandidate(IceCandidate iceCandidate); From 0a18c8613f83d2a6861a4ae4fef4e534f3f122f9 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Tue, 16 Nov 2021 17:08:34 +0100 Subject: [PATCH 18/24] assume credentials are the same for all contents when restarting ICE --- .../conversations/ui/RtpSessionActivity.java | 1 + .../xmpp/jingle/JingleRtpConnection.java | 23 +++++++++++-------- .../xmpp/jingle/RtpContentMap.java | 17 ++++++++++---- .../jingle/stanzas/IceUdpTransportInfo.java | 4 ++-- 4 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java b/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java index d0bdbb788..39ba7429f 100644 --- a/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java +++ b/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java @@ -1003,6 +1003,7 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe RendererCommon.ScalingType.SCALE_ASPECT_FILL, RendererCommon.ScalingType.SCALE_ASPECT_FIT ); + //TODO this should probably only be 'connected' if (STATES_CONSIDERED_CONNECTED.contains(state)) { binding.appBarLayout.setVisibility(View.GONE); getWindow().addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN); 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 66cf5c23b..26c068c08 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -141,7 +141,6 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } private final WebRTCWrapper webRTCWrapper = new WebRTCWrapper(this); - //TODO convert to Queue>? private final Queue> pendingIceCandidates = new LinkedList<>(); private final OmemoVerification omemoVerification = new OmemoVerification(); private final Message message; @@ -295,9 +294,13 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web private boolean checkForIceRestart(final JinglePacket jinglePacket, final RtpContentMap rtpContentMap) { final RtpContentMap existing = getRemoteContentMap(); - final Map existingCredentials = existing.getCredentials(); - final Map newCredentials = rtpContentMap.getCredentials(); - if (!existingCredentials.keySet().equals(newCredentials.keySet())) { + final IceUdpTransportInfo.Credentials existingCredentials; + final IceUdpTransportInfo.Credentials newCredentials; + try { + existingCredentials = existing.getCredentials(); + newCredentials = rtpContentMap.getCredentials(); + } catch (final IllegalStateException e) { + Log.d(Config.LOGTAG, "unable to gather credentials for comparison", e); return false; } if (existingCredentials.equals(newCredentials)) { @@ -307,11 +310,11 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web final RtpContentMap restartContentMap; try { if (isOffer) { - Log.d(Config.LOGTAG, "received offer to restart ICE " + newCredentials.values()); + Log.d(Config.LOGTAG, "received offer to restart ICE " + newCredentials); restartContentMap = existing.modifiedCredentials(newCredentials, IceUdpTransportInfo.Setup.ACTPASS); } else { final IceUdpTransportInfo.Setup setup = getPeerDtlsSetup(); - Log.d(Config.LOGTAG, "received confirmation of ICE restart" + newCredentials.values() + " peer_setup=" + setup); + Log.d(Config.LOGTAG, "received confirmation of ICE restart" + newCredentials + " peer_setup=" + setup); // DTLS setup attribute needs to be rewritten to reflect current peer state // https://groups.google.com/g/discuss-webrtc/c/DfpIMwvUfeM restartContentMap = existing.modifiedCredentials(newCredentials, setup); @@ -319,7 +322,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web if (applyIceRestart(jinglePacket, restartContentMap, isOffer)) { return isOffer; } else { - Log.d(Config.LOGTAG,"ignored ice restart. offer="+isOffer); + Log.d(Config.LOGTAG, "ignoring ICE restart. sending tie-break"); respondWithTieBreak(jinglePacket); return true; } @@ -327,7 +330,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web respondOk(jinglePacket); final Throwable rootCause = Throwables.getRootCause(exception); if (rootCause instanceof WebRTCWrapper.PeerConnectionNotInitialized) { - Log.d(Config.LOGTAG,"ignoring PeerConnectionNotInitialized"); + Log.d(Config.LOGTAG, "ignoring PeerConnectionNotInitialized"); //TODO don’t respond OK but respond with out-of-order return true; } @@ -1451,8 +1454,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web @Override public void onIceCandidate(final IceCandidate iceCandidate) { final RtpContentMap rtpContentMap = isInitiator() ? this.initiatorRtpContentMap : this.responderRtpContentMap; - final Collection currentUfrags = Collections2.transform(rtpContentMap.getCredentials().values(), c -> c.ufrag); - final IceUdpTransportInfo.Candidate candidate = IceUdpTransportInfo.Candidate.fromSdpAttribute(iceCandidate.sdp, currentUfrags); + final String ufrag = rtpContentMap.getCredentials().ufrag; + final IceUdpTransportInfo.Candidate candidate = IceUdpTransportInfo.Candidate.fromSdpAttribute(iceCandidate.sdp, ufrag); if (candidate == null) { Log.d(Config.LOGTAG, "ignoring (not sending) candidate: " + iceCandidate.toString()); return; diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java index 091bf7c10..ea351cb1b 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java @@ -146,14 +146,22 @@ public class RtpContentMap { ); } - public Map getCredentials() { - return Maps.transformValues(contents, dt -> dt.transport.getCredentials()); + public IceUdpTransportInfo.Credentials getCredentials() { + final Set allCredentials = ImmutableSet.copyOf(Collections2.transform( + contents.values(), + dt -> dt.transport.getCredentials() + )); + final IceUdpTransportInfo.Credentials credentials = Iterables.getFirst(allCredentials, null); + if (allCredentials.size() == 1 && credentials != null) { + return credentials; + } + throw new IllegalStateException("Content map does not have distinct credentials"); } public IceUdpTransportInfo.Setup getDtlsSetup() { final Set setups = ImmutableSet.copyOf(Collections2.transform( contents.values(), - dt->dt.transport.getFingerprint().getSetup() + dt -> dt.transport.getFingerprint().getSetup() )); final IceUdpTransportInfo.Setup setup = Iterables.getFirst(setups, null); if (setups.size() == 1 && setup != null) { @@ -170,12 +178,11 @@ public class RtpContentMap { return count == 0; } - public RtpContentMap modifiedCredentials(Map credentialsMap, final IceUdpTransportInfo.Setup setup) { + public RtpContentMap modifiedCredentials(IceUdpTransportInfo.Credentials credentials, final IceUdpTransportInfo.Setup setup) { final ImmutableMap.Builder contentMapBuilder = new ImmutableMap.Builder<>(); for (final Map.Entry content : contents.entrySet()) { final RtpDescription rtpDescription = content.getValue().description; IceUdpTransportInfo transportInfo = content.getValue().transport; - final IceUdpTransportInfo.Credentials credentials = Objects.requireNonNull(credentialsMap.get(content.getKey())); final IceUdpTransportInfo modifiedTransportInfo = transportInfo.modifyCredentials(credentials, setup); contentMapBuilder.put(content.getKey(), new DescriptionTransport(rtpDescription, modifiedTransportInfo)); } diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java index 9af1186fc..45260cafb 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java @@ -146,7 +146,7 @@ public class IceUdpTransportInfo extends GenericTransportInfo { } // https://tools.ietf.org/html/draft-ietf-mmusic-ice-sip-sdp-39#section-5.1 - public static Candidate fromSdpAttribute(final String attribute, Collection currentUfrags) { + public static Candidate fromSdpAttribute(final String attribute, String currentUfrag) { final String[] pair = attribute.split(":", 2); if (pair.length == 2 && "candidate".equals(pair[0])) { final String[] segments = pair[1].split(" "); @@ -163,7 +163,7 @@ public class IceUdpTransportInfo extends GenericTransportInfo { additional.put(segments[i], segments[i + 1]); } final String ufrag = additional.get("ufrag"); - if (ufrag != null && !currentUfrags.contains(ufrag)) { + if (ufrag != null && !ufrag.equals(currentUfrag)) { return null; } final Candidate candidate = new Candidate(); From 1bf2d5dd8f31c4c10199e9133012cc5d1645921a Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Tue, 16 Nov 2021 22:01:48 +0100 Subject: [PATCH 19/24] video calls: leave full screen mode during reconnect --- .../conversations/ui/RtpSessionActivity.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java b/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java index 39ba7429f..65beae35d 100644 --- a/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java +++ b/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java @@ -103,6 +103,11 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe RtpEndUserState.CONNECTED, RtpEndUserState.RECONNECTING ); + private static final List STATES_SHOWING_PIP_PLACEHOLDER = Arrays.asList( + RtpEndUserState.ACCEPTING_CALL, + RtpEndUserState.CONNECTING, + RtpEndUserState.RECONNECTING + ); private static final String PROXIMITY_WAKE_LOCK_TAG = "conversations:in-rtp-session"; private static final int REQUEST_ACCEPT_CALL = 0x1111; private WeakReference rtpConnectionReference; @@ -640,8 +645,8 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe surfaceViewRenderer.setVisibility(View.VISIBLE); try { surfaceViewRenderer.init(requireRtpConnection().getEglBaseContext(), null); - } catch (IllegalStateException e) { - Log.d(Config.LOGTAG, "SurfaceViewRenderer was already initialized"); + } catch (final IllegalStateException e) { + //Log.d(Config.LOGTAG, "SurfaceViewRenderer was already initialized"); } surfaceViewRenderer.setEnableHardwareScaler(true); } @@ -975,7 +980,7 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe getWindow().clearFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN); return; } - if (isPictureInPicture() && (state == RtpEndUserState.CONNECTING || state == RtpEndUserState.ACCEPTING_CALL)) { + if (isPictureInPicture() && STATES_SHOWING_PIP_PLACEHOLDER.contains(state)) { binding.localVideo.setVisibility(View.GONE); binding.remoteVideoWrapper.setVisibility(View.GONE); binding.appBarLayout.setVisibility(View.GONE); @@ -1003,12 +1008,12 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe RendererCommon.ScalingType.SCALE_ASPECT_FILL, RendererCommon.ScalingType.SCALE_ASPECT_FIT ); - //TODO this should probably only be 'connected' - if (STATES_CONSIDERED_CONNECTED.contains(state)) { + if (state == RtpEndUserState.CONNECTED) { binding.appBarLayout.setVisibility(View.GONE); getWindow().addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN); binding.remoteVideoWrapper.setVisibility(View.VISIBLE); } else { + binding.appBarLayout.setVisibility(View.VISIBLE); getWindow().clearFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN); binding.remoteVideoWrapper.setVisibility(View.GONE); } From 61fb38cd8442086596b347b45ef2e4b69f647af2 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 17 Nov 2021 10:49:16 +0100 Subject: [PATCH 20/24] clean up some error handling error ICE restarts --- .../eu/siacs/conversations/xml/Namespace.java | 1 + .../xmpp/jingle/JingleConnectionManager.java | 2 +- .../xmpp/jingle/JingleRtpConnection.java | 101 +++++++++++------- .../xmpp/jingle/RtpContentMap.java | 7 ++ 4 files changed, 71 insertions(+), 40 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/xml/Namespace.java b/src/main/java/eu/siacs/conversations/xml/Namespace.java index b0c4fe85c..09bbda4cd 100644 --- a/src/main/java/eu/siacs/conversations/xml/Namespace.java +++ b/src/main/java/eu/siacs/conversations/xml/Namespace.java @@ -28,6 +28,7 @@ public final class Namespace { public static final String SYNCHRONIZATION = "im.quicksy.synchronization:0"; public static final String AVATAR_CONVERSION = "urn:xmpp:pep-vcard-conversion:0"; public static final String JINGLE = "urn:xmpp:jingle:1"; + public static final String JINGLE_ERRORS = "urn:xmpp:jingle:errors:1"; public static final String JINGLE_MESSAGE = "urn:xmpp:jingle-message:0"; public static final String JINGLE_ENCRYPTED_TRANSPORT = "urn:xmpp:jingle:jet:0"; public static final String JINGLE_ENCRYPTED_TRANSPORT_OMEMO = "urn:xmpp:jingle:jet-omemo:0"; diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnectionManager.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnectionManager.java index 6b94f1f4d..cbf4b85fd 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnectionManager.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnectionManager.java @@ -206,7 +206,7 @@ public class JingleConnectionManager extends AbstractConnectionManager { final Element error = response.addChild("error"); error.setAttribute("type", conditionType); error.addChild(condition, "urn:ietf:params:xml:ns:xmpp-stanzas"); - error.addChild(jingleCondition, "urn:xmpp:jingle:errors:1"); + error.addChild(jingleCondition, Namespace.JINGLE_ERRORS); account.getXmppConnection().sendIqPacket(response, null); } 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 26c068c08..1295bf908 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -306,6 +306,9 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web if (existingCredentials.equals(newCredentials)) { return false; } + //TODO an alternative approach is to check if we already got an iq result to our ICE-restart + // and if that's the case we are seeing an answer. + // This might be more spec compliant but also more error prone potentially final boolean isOffer = rtpContentMap.emptyCandidates(); final RtpContentMap restartContentMap; try { @@ -330,8 +333,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web respondOk(jinglePacket); final Throwable rootCause = Throwables.getRootCause(exception); if (rootCause instanceof WebRTCWrapper.PeerConnectionNotInitialized) { - Log.d(Config.LOGTAG, "ignoring PeerConnectionNotInitialized"); - //TODO don’t respond OK but respond with out-of-order + //If this happens a termination is already in progress + Log.d(Config.LOGTAG, "ignoring PeerConnectionNotInitialized on ICE restart"); return true; } Log.d(Config.LOGTAG, "failure to apply ICE restart", rootCause); @@ -484,8 +487,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web private void receiveSessionInitiate(final JinglePacket jinglePacket, final RtpContentMap contentMap) { try { contentMap.requireContentDescriptions(); - //TODO require actpass - contentMap.requireDTLSFingerprint(); + contentMap.requireDTLSFingerprint(true); } catch (final RuntimeException e) { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": improperly formatted contents", Throwables.getRootCause(e)); respondOk(jinglePacket); @@ -1072,36 +1074,48 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web private synchronized void handleIqResponse(final Account account, final IqPacket response) { if (response.getType() == IqPacket.TYPE.ERROR) { - final String errorCondition = response.getErrorCondition(); - Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": received IQ-error from " + response.getFrom() + " in RTP session. " + errorCondition); - if (isTerminated()) { - Log.i(Config.LOGTAG, id.account.getJid().asBareJid() + ": ignoring error because session was already terminated"); - return; - } - this.webRTCWrapper.close(); - final State target; - if (Arrays.asList( - "service-unavailable", - "recipient-unavailable", - "remote-server-not-found", - "remote-server-timeout" - ).contains(errorCondition)) { - target = State.TERMINATED_CONNECTIVITY_ERROR; - } else { - target = State.TERMINATED_APPLICATION_FAILURE; - } - transitionOrThrow(target); - this.finish(); - } else if (response.getType() == IqPacket.TYPE.TIMEOUT) { - Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": received IQ timeout in RTP session with " + id.with + ". terminating with connectivity error"); - if (isTerminated()) { - Log.i(Config.LOGTAG, id.account.getJid().asBareJid() + ": ignoring error because session was already terminated"); - return; - } - this.webRTCWrapper.close(); - transitionOrThrow(State.TERMINATED_CONNECTIVITY_ERROR); - this.finish(); + handleIqErrorResponse(response); + return; } + if (response.getType() == IqPacket.TYPE.TIMEOUT) { + handleIqTimeoutResponse(response); + } + } + + private void handleIqErrorResponse(final IqPacket response) { + Preconditions.checkArgument(response.getType() == IqPacket.TYPE.ERROR); + final String errorCondition = response.getErrorCondition(); + Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": received IQ-error from " + response.getFrom() + " in RTP session. " + errorCondition); + if (isTerminated()) { + Log.i(Config.LOGTAG, id.account.getJid().asBareJid() + ": ignoring error because session was already terminated"); + return; + } + this.webRTCWrapper.close(); + final State target; + if (Arrays.asList( + "service-unavailable", + "recipient-unavailable", + "remote-server-not-found", + "remote-server-timeout" + ).contains(errorCondition)) { + target = State.TERMINATED_CONNECTIVITY_ERROR; + } else { + target = State.TERMINATED_APPLICATION_FAILURE; + } + transitionOrThrow(target); + this.finish(); + } + + private void handleIqTimeoutResponse(final IqPacket response) { + Preconditions.checkArgument(response.getType() == IqPacket.TYPE.ERROR); + Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": received IQ timeout in RTP session with " + id.with + ". terminating with connectivity error"); + if (isTerminated()) { + Log.i(Config.LOGTAG, id.account.getJid().asBareJid() + ": ignoring error because session was already terminated"); + return; + } + this.webRTCWrapper.close(); + transitionOrThrow(State.TERMINATED_CONNECTIVITY_ERROR); + this.finish(); } private void terminateWithOutOfOrder(final JinglePacket jinglePacket) { @@ -1503,8 +1517,9 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web try { sessionDescription = setLocalSessionDescription(); } catch (final Exception e) { - Log.d(Config.LOGTAG, "failed to renegotiate", e); - //TODO send some sort of failure (comparable to when initiating) + final Throwable cause = Throwables.getRootCause(e); + Log.d(Config.LOGTAG, "failed to renegotiate", cause); + sendSessionTerminate(Reason.FAILED_APPLICATION, cause.getMessage()); return; } final RtpContentMap rtpContentMap = RtpContentMap.of(sessionDescription); @@ -1517,10 +1532,18 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web Log.d(Config.LOGTAG, "received success to our ice restart"); setLocalContentMap(rtpContentMap); webRTCWrapper.setIsReadyToReceiveIceCandidates(true); - } else { - Log.d(Config.LOGTAG, "received failure to our ice restart"); - //TODO ignore tie break (maybe rollback?) - //TODO handle other errors + return; + } + if (response.getType() == IqPacket.TYPE.ERROR) { + final Element error = response.findChild("error"); + if (error != null && error.hasChild("tie-break", Namespace.JINGLE_ERRORS)) { + Log.d(Config.LOGTAG, "received tie-break as result of ice restart"); + return; + } + handleIqErrorResponse(response); + } + if (response.getType() == IqPacket.TYPE.TIMEOUT) { + handleIqTimeoutResponse(response); } }); } diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java index ea351cb1b..21684a165 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java @@ -96,6 +96,10 @@ public class RtpContentMap { } void requireDTLSFingerprint() { + requireDTLSFingerprint(false); + } + + void requireDTLSFingerprint(final boolean requireActPass) { if (this.contents.size() == 0) { throw new IllegalStateException("No contents available"); } @@ -109,6 +113,9 @@ public class RtpContentMap { if (setup == null) { throw new SecurityException(String.format("Use of DTLS-SRTP (XEP-0320) is required for content %s but missing setup attribute", entry.getKey())); } + if (requireActPass && setup != IceUdpTransportInfo.Setup.ACTPASS) { + throw new SecurityException("Initiator needs to offer ACTPASS as setup for DTLS-SRTP (XEP-0320)"); + } } } From a508a81553604b7a6af6f650146f2ccd736b4066 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 17 Nov 2021 11:33:15 +0100 Subject: [PATCH 21/24] externalize rtc config generation into seperate method --- .../xmpp/jingle/JingleRtpConnection.java | 1 + .../xmpp/jingle/WebRTCWrapper.java | 21 +++++++++++++------ 2 files changed, 16 insertions(+), 6 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 1295bf908..e4661a3aa 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -1511,6 +1511,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } private void initiateIceRestart() { + //TODO discover new TURN/STUN credentials this.stateHistory.clear(); this.webRTCWrapper.setIsReadyToReceiveIceCandidates(false); final SessionDescription sessionDescription; 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 0712fa900..6722f9f2c 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java @@ -267,12 +267,7 @@ public class WebRTCWrapper { .createPeerConnectionFactory(); - final PeerConnection.RTCConfiguration rtcConfig = new PeerConnection.RTCConfiguration(iceServers); - rtcConfig.tcpCandidatePolicy = PeerConnection.TcpCandidatePolicy.DISABLED; //XEP-0176 doesn't support tcp - rtcConfig.continualGatheringPolicy = PeerConnection.ContinualGatheringPolicy.GATHER_CONTINUALLY; - rtcConfig.sdpSemantics = PeerConnection.SdpSemantics.UNIFIED_PLAN; - rtcConfig.rtcpMuxPolicy = PeerConnection.RtcpMuxPolicy.NEGOTIATE; - rtcConfig.enableImplicitRollback = true; + final PeerConnection.RTCConfiguration rtcConfig = buildConfiguration(iceServers); final PeerConnection peerConnection = peerConnectionFactory.createPeerConnection(rtcConfig, peerConnectionObserver); if (peerConnection == null) { throw new InitializationException("Unable to create PeerConnection"); @@ -306,6 +301,20 @@ public class WebRTCWrapper { this.peerConnection = peerConnection; } + private static PeerConnection.RTCConfiguration buildConfiguration(final List iceServers) { + final PeerConnection.RTCConfiguration rtcConfig = new PeerConnection.RTCConfiguration(iceServers); + rtcConfig.tcpCandidatePolicy = PeerConnection.TcpCandidatePolicy.DISABLED; //XEP-0176 doesn't support tcp + rtcConfig.continualGatheringPolicy = PeerConnection.ContinualGatheringPolicy.GATHER_CONTINUALLY; + rtcConfig.sdpSemantics = PeerConnection.SdpSemantics.UNIFIED_PLAN; + rtcConfig.rtcpMuxPolicy = PeerConnection.RtcpMuxPolicy.NEGOTIATE; + rtcConfig.enableImplicitRollback = true; + return rtcConfig; + } + + void reconfigurePeerConnection(final List iceServers) { + requirePeerConnection().setConfiguration(buildConfiguration(iceServers)); + } + void restartIce() { executorService.execute(() -> requirePeerConnection().restartIce()); } From 5d526a77e3899714750d5f56536364a5ed9b5149 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 18 Nov 2021 11:24:10 +0100 Subject: [PATCH 22/24] include uncertainty into shared geo uri --- .../ui/ConversationFragment.java | 12 +- .../ui/ShareLocationActivity.java | 357 +++++++++--------- 2 files changed, 189 insertions(+), 180 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/ui/ConversationFragment.java b/src/main/java/eu/siacs/conversations/ui/ConversationFragment.java index 0077684a5..771c99e9c 100644 --- a/src/main/java/eu/siacs/conversations/ui/ConversationFragment.java +++ b/src/main/java/eu/siacs/conversations/ui/ConversationFragment.java @@ -856,9 +856,15 @@ public class ConversationFragment extends XmppFragment implements EditMessage.Ke toggleInputMethod(); break; case ATTACHMENT_CHOICE_LOCATION: - double latitude = data.getDoubleExtra("latitude", 0); - double longitude = data.getDoubleExtra("longitude", 0); - Uri geo = Uri.parse("geo:" + latitude + "," + longitude); + final double latitude = data.getDoubleExtra("latitude", 0); + final double longitude = data.getDoubleExtra("longitude", 0); + final int accuracy = data.getIntExtra("accuracy", 0); + final Uri geo; + if (accuracy > 0) { + geo = Uri.parse(String.format("geo:%s,%s;u=%s", latitude, longitude, accuracy)); + } else { + geo = Uri.parse(String.format("geo:%s,%s", latitude, longitude)); + } mediaPreviewAdapter.addMediaPreviews(Attachment.of(getActivity(), geo, Attachment.Type.LOCATION)); toggleInputMethod(); break; diff --git a/src/main/java/eu/siacs/conversations/ui/ShareLocationActivity.java b/src/main/java/eu/siacs/conversations/ui/ShareLocationActivity.java index 641a01e5c..7e53fe897 100644 --- a/src/main/java/eu/siacs/conversations/ui/ShareLocationActivity.java +++ b/src/main/java/eu/siacs/conversations/ui/ShareLocationActivity.java @@ -13,10 +13,13 @@ import androidx.annotation.NonNull; import androidx.databinding.DataBindingUtil; import com.google.android.material.snackbar.Snackbar; +import com.google.common.math.DoubleMath; import org.osmdroid.api.IGeoPoint; import org.osmdroid.util.GeoPoint; +import java.math.RoundingMode; + import eu.siacs.conversations.Config; import eu.siacs.conversations.R; import eu.siacs.conversations.databinding.ActivityShareLocationBinding; @@ -28,213 +31,213 @@ import eu.siacs.conversations.utils.ThemeHelper; public class ShareLocationActivity extends LocationActivity implements LocationListener { - private Snackbar snackBar; - private ActivityShareLocationBinding binding; - private boolean marker_fixed_to_loc = false; - private static final String KEY_FIXED_TO_LOC = "fixed_to_loc"; - private Boolean noAskAgain = false; + private Snackbar snackBar; + private ActivityShareLocationBinding binding; + private boolean marker_fixed_to_loc = false; + private static final String KEY_FIXED_TO_LOC = "fixed_to_loc"; + private Boolean noAskAgain = false; - @Override - protected void onSaveInstanceState(@NonNull final Bundle outState) { - super.onSaveInstanceState(outState); + @Override + protected void onSaveInstanceState(@NonNull final Bundle outState) { + super.onSaveInstanceState(outState); - outState.putBoolean(KEY_FIXED_TO_LOC, marker_fixed_to_loc); - } + outState.putBoolean(KEY_FIXED_TO_LOC, marker_fixed_to_loc); + } - @Override - protected void onRestoreInstanceState(@NonNull final Bundle savedInstanceState) { - super.onRestoreInstanceState(savedInstanceState); + @Override + protected void onRestoreInstanceState(@NonNull final Bundle savedInstanceState) { + super.onRestoreInstanceState(savedInstanceState); - if (savedInstanceState.containsKey(KEY_FIXED_TO_LOC)) { - this.marker_fixed_to_loc = savedInstanceState.getBoolean(KEY_FIXED_TO_LOC); - } - } + if (savedInstanceState.containsKey(KEY_FIXED_TO_LOC)) { + this.marker_fixed_to_loc = savedInstanceState.getBoolean(KEY_FIXED_TO_LOC); + } + } - @Override - protected void onCreate(final Bundle savedInstanceState) { - super.onCreate(savedInstanceState); + @Override + protected void onCreate(final Bundle savedInstanceState) { + super.onCreate(savedInstanceState); - this.binding = DataBindingUtil.setContentView(this,R.layout.activity_share_location); - setSupportActionBar(binding.toolbar); - configureActionBar(getSupportActionBar()); - setupMapView(binding.map, LocationProvider.getGeoPoint(this)); + this.binding = DataBindingUtil.setContentView(this, R.layout.activity_share_location); + setSupportActionBar(binding.toolbar); + configureActionBar(getSupportActionBar()); + setupMapView(binding.map, LocationProvider.getGeoPoint(this)); - this.binding.cancelButton.setOnClickListener(view -> { - setResult(RESULT_CANCELED); - finish(); - }); + this.binding.cancelButton.setOnClickListener(view -> { + setResult(RESULT_CANCELED); + finish(); + }); - this.snackBar = Snackbar.make(this.binding.snackbarCoordinator, R.string.location_disabled, Snackbar.LENGTH_INDEFINITE); - this.snackBar.setAction(R.string.enable, view -> { - if (isLocationEnabledAndAllowed()) { - updateUi(); - } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M && !hasLocationPermissions()) { - requestPermissions(REQUEST_CODE_SNACKBAR_PRESSED); - } else if (!isLocationEnabled()) { - startActivity(new Intent(android.provider.Settings.ACTION_LOCATION_SOURCE_SETTINGS)); - } - }); - ThemeHelper.fix(this.snackBar); + this.snackBar = Snackbar.make(this.binding.snackbarCoordinator, R.string.location_disabled, Snackbar.LENGTH_INDEFINITE); + this.snackBar.setAction(R.string.enable, view -> { + if (isLocationEnabledAndAllowed()) { + updateUi(); + } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M && !hasLocationPermissions()) { + requestPermissions(REQUEST_CODE_SNACKBAR_PRESSED); + } else if (!isLocationEnabled()) { + startActivity(new Intent(android.provider.Settings.ACTION_LOCATION_SOURCE_SETTINGS)); + } + }); + ThemeHelper.fix(this.snackBar); - this.binding.shareButton.setOnClickListener(view -> { - final Intent result = new Intent(); + this.binding.shareButton.setOnClickListener(this::shareLocation); - if (marker_fixed_to_loc && myLoc != null) { - result.putExtra("latitude", myLoc.getLatitude()); - result.putExtra("longitude", myLoc.getLongitude()); - result.putExtra("altitude", myLoc.getAltitude()); - result.putExtra("accuracy", (int) myLoc.getAccuracy()); - } else { - final IGeoPoint markerPoint = this.binding.map.getMapCenter(); - result.putExtra("latitude", markerPoint.getLatitude()); - result.putExtra("longitude", markerPoint.getLongitude()); - } + this.marker_fixed_to_loc = isLocationEnabledAndAllowed(); - setResult(RESULT_OK, result); - finish(); - }); + this.binding.fab.setOnClickListener(view -> { + if (!marker_fixed_to_loc) { + if (!isLocationEnabled()) { + startActivity(new Intent(android.provider.Settings.ACTION_LOCATION_SOURCE_SETTINGS)); + } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + requestPermissions(REQUEST_CODE_FAB_PRESSED); + } + } + toggleFixedLocation(); + }); + } - this.marker_fixed_to_loc = isLocationEnabledAndAllowed(); + private void shareLocation(final View view) { + final Intent result = new Intent(); + if (marker_fixed_to_loc && myLoc != null) { + result.putExtra("latitude", myLoc.getLatitude()); + result.putExtra("longitude", myLoc.getLongitude()); + result.putExtra("altitude", myLoc.getAltitude()); + result.putExtra("accuracy", DoubleMath.roundToInt(myLoc.getAccuracy(), RoundingMode.HALF_UP)); + } else { + final IGeoPoint markerPoint = this.binding.map.getMapCenter(); + result.putExtra("latitude", markerPoint.getLatitude()); + result.putExtra("longitude", markerPoint.getLongitude()); + } + setResult(RESULT_OK, result); + finish(); + } - this.binding.fab.setOnClickListener(view -> { - if (!marker_fixed_to_loc) { - if (!isLocationEnabled()) { - startActivity(new Intent(android.provider.Settings.ACTION_LOCATION_SOURCE_SETTINGS)); - } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - requestPermissions(REQUEST_CODE_FAB_PRESSED); - } - } - toggleFixedLocation(); - }); - } + @Override + public void onRequestPermissionsResult(final int requestCode, + @NonNull final String[] permissions, + @NonNull final int[] grantResults) { + super.onRequestPermissionsResult(requestCode, permissions, grantResults); - @Override - public void onRequestPermissionsResult(final int requestCode, - @NonNull final String[] permissions, - @NonNull final int[] grantResults) { - super.onRequestPermissionsResult(requestCode, permissions, grantResults); + if (grantResults.length > 0 && + grantResults[0] != PackageManager.PERMISSION_GRANTED && + Build.VERSION.SDK_INT >= 23 && + permissions.length > 0 && + ( + Manifest.permission.LOCATION_HARDWARE.equals(permissions[0]) || + Manifest.permission.ACCESS_FINE_LOCATION.equals(permissions[0]) || + Manifest.permission.ACCESS_COARSE_LOCATION.equals(permissions[0]) + ) && + !shouldShowRequestPermissionRationale(permissions[0])) { + noAskAgain = true; + } - if (grantResults.length > 0 && - grantResults[0] != PackageManager.PERMISSION_GRANTED && - Build.VERSION.SDK_INT >= 23 && - permissions.length > 0 && - ( - Manifest.permission.LOCATION_HARDWARE.equals(permissions[0]) || - Manifest.permission.ACCESS_FINE_LOCATION.equals(permissions[0]) || - Manifest.permission.ACCESS_COARSE_LOCATION.equals(permissions[0]) - ) && - !shouldShowRequestPermissionRationale(permissions[0])) { - noAskAgain = true; - } + if (!noAskAgain && requestCode == REQUEST_CODE_SNACKBAR_PRESSED && !isLocationEnabled() && hasLocationPermissions()) { + startActivity(new Intent(android.provider.Settings.ACTION_LOCATION_SOURCE_SETTINGS)); + } + updateUi(); + } - if (!noAskAgain && requestCode == REQUEST_CODE_SNACKBAR_PRESSED && !isLocationEnabled() && hasLocationPermissions()) { - startActivity(new Intent(android.provider.Settings.ACTION_LOCATION_SOURCE_SETTINGS)); - } - updateUi(); - } + @Override + protected void gotoLoc(final boolean setZoomLevel) { + if (this.myLoc != null && mapController != null) { + if (setZoomLevel) { + mapController.setZoom(Config.Map.FINAL_ZOOM_LEVEL); + } + mapController.animateTo(new GeoPoint(this.myLoc)); + } + } - @Override - protected void gotoLoc(final boolean setZoomLevel) { - if (this.myLoc != null && mapController != null) { - if (setZoomLevel) { - mapController.setZoom(Config.Map.FINAL_ZOOM_LEVEL); - } - mapController.animateTo(new GeoPoint(this.myLoc)); - } - } + @Override + protected void setMyLoc(final Location location) { + this.myLoc = location; + } - @Override - protected void setMyLoc(final Location location) { - this.myLoc = location; - } + @Override + protected void onPause() { + super.onPause(); + } - @Override - protected void onPause() { - super.onPause(); - } + @Override + protected void updateLocationMarkers() { + super.updateLocationMarkers(); + if (this.myLoc != null) { + this.binding.map.getOverlays().add(new MyLocation(this, null, this.myLoc)); + if (this.marker_fixed_to_loc) { + this.binding.map.getOverlays().add(new Marker(marker_icon, new GeoPoint(this.myLoc))); + } else { + this.binding.map.getOverlays().add(new Marker(marker_icon)); + } + } else { + this.binding.map.getOverlays().add(new Marker(marker_icon)); + } + } - @Override - protected void updateLocationMarkers() { - super.updateLocationMarkers(); - if (this.myLoc != null) { - this.binding.map.getOverlays().add(new MyLocation(this, null, this.myLoc)); - if (this.marker_fixed_to_loc) { - this.binding.map.getOverlays().add(new Marker(marker_icon, new GeoPoint(this.myLoc))); - } else { - this.binding.map.getOverlays().add(new Marker(marker_icon)); - } - } else { - this.binding.map.getOverlays().add(new Marker(marker_icon)); - } - } + @Override + public void onLocationChanged(final Location location) { + if (this.myLoc == null) { + this.marker_fixed_to_loc = true; + } + updateUi(); + if (LocationHelper.isBetterLocation(location, this.myLoc)) { + final Location oldLoc = this.myLoc; + this.myLoc = location; - @Override - public void onLocationChanged(final Location location) { - if (this.myLoc == null) { - this.marker_fixed_to_loc = true; - } - updateUi(); - if (LocationHelper.isBetterLocation(location, this.myLoc)) { - final Location oldLoc = this.myLoc; - this.myLoc = location; + // Don't jump back to the users location if they're not moving (more or less). + if (oldLoc == null || (this.marker_fixed_to_loc && this.myLoc.distanceTo(oldLoc) > 1)) { + gotoLoc(); + } - // Don't jump back to the users location if they're not moving (more or less). - if (oldLoc == null || (this.marker_fixed_to_loc && this.myLoc.distanceTo(oldLoc) > 1)) { - gotoLoc(); - } + updateLocationMarkers(); + } + } - updateLocationMarkers(); - } - } + @Override + public void onStatusChanged(final String provider, final int status, final Bundle extras) { - @Override - public void onStatusChanged(final String provider, final int status, final Bundle extras) { + } - } + @Override + public void onProviderEnabled(final String provider) { - @Override - public void onProviderEnabled(final String provider) { + } - } + @Override + public void onProviderDisabled(final String provider) { - @Override - public void onProviderDisabled(final String provider) { + } - } + private boolean isLocationEnabledAndAllowed() { + return this.hasLocationFeature && (Build.VERSION.SDK_INT < Build.VERSION_CODES.M || this.hasLocationPermissions()) && this.isLocationEnabled(); + } - private boolean isLocationEnabledAndAllowed() { - return this.hasLocationFeature && (Build.VERSION.SDK_INT < Build.VERSION_CODES.M || this.hasLocationPermissions()) && this.isLocationEnabled(); - } + private void toggleFixedLocation() { + this.marker_fixed_to_loc = isLocationEnabledAndAllowed() && !this.marker_fixed_to_loc; + if (this.marker_fixed_to_loc) { + gotoLoc(false); + } + updateLocationMarkers(); + updateUi(); + } - private void toggleFixedLocation() { - this.marker_fixed_to_loc = isLocationEnabledAndAllowed() && !this.marker_fixed_to_loc; - if (this.marker_fixed_to_loc) { - gotoLoc(false); - } - updateLocationMarkers(); - updateUi(); - } + @Override + protected void updateUi() { + if (!hasLocationFeature || noAskAgain || isLocationEnabledAndAllowed()) { + this.snackBar.dismiss(); + } else { + this.snackBar.show(); + } - @Override - protected void updateUi() { - if (!hasLocationFeature || noAskAgain || isLocationEnabledAndAllowed()) { - this.snackBar.dismiss(); - } else { - this.snackBar.show(); - } - - if (isLocationEnabledAndAllowed()) { - this.binding.fab.setVisibility(View.VISIBLE); - runOnUiThread(() -> { - this.binding.fab.setImageResource(marker_fixed_to_loc ? R.drawable.ic_gps_fixed_white_24dp : - R.drawable.ic_gps_not_fixed_white_24dp); - this.binding.fab.setContentDescription(getResources().getString( - marker_fixed_to_loc ? R.string.action_unfix_from_location : R.string.action_fix_to_location - )); - this.binding.fab.invalidate(); - }); - } else { - this.binding.fab.setVisibility(View.GONE); - } - } + if (isLocationEnabledAndAllowed()) { + this.binding.fab.setVisibility(View.VISIBLE); + runOnUiThread(() -> { + this.binding.fab.setImageResource(marker_fixed_to_loc ? R.drawable.ic_gps_fixed_white_24dp : + R.drawable.ic_gps_not_fixed_white_24dp); + this.binding.fab.setContentDescription(getResources().getString( + marker_fixed_to_loc ? R.string.action_unfix_from_location : R.string.action_fix_to_location + )); + this.binding.fab.invalidate(); + }); + } else { + this.binding.fab.setVisibility(View.GONE); + } + } } \ No newline at end of file From f8a94161dbc3398e6d355175e45d7b5e407bf32c Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Fri, 19 Nov 2021 12:25:27 +0100 Subject: [PATCH 23/24] don't play tone going from connect->reconnect->connect --- .../java/eu/siacs/conversations/xmpp/jingle/ToneManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/ToneManager.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/ToneManager.java index 4fb9dee16..e368d3b09 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/ToneManager.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/ToneManager.java @@ -51,7 +51,7 @@ class ToneManager { return ToneState.ENDING_CALL; } } - if (state == RtpEndUserState.CONNECTED) { + if (state == RtpEndUserState.CONNECTED || state == RtpEndUserState.RECONNECTING) { if (media.contains(Media.VIDEO)) { return ToneState.NULL; } else { From db834a1f07ee32005d8ba87e11ff03265f986a24 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Fri, 19 Nov 2021 12:26:11 +0100 Subject: [PATCH 24/24] indicate call reconnect in notification --- .../services/NotificationService.java | 17 +++++++++++++---- .../services/XmppConnectionService.java | 18 ++++++++++-------- .../xmpp/jingle/JingleRtpConnection.java | 14 +++++++++++--- src/main/res/values/strings.xml | 2 ++ 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/services/NotificationService.java b/src/main/java/eu/siacs/conversations/services/NotificationService.java index 2f6f36c59..ca4499300 100644 --- a/src/main/java/eu/siacs/conversations/services/NotificationService.java +++ b/src/main/java/eu/siacs/conversations/services/NotificationService.java @@ -488,14 +488,23 @@ public class NotificationService { notify(INCOMING_CALL_NOTIFICATION_ID, notification); } - public Notification getOngoingCallNotification(final AbstractJingleConnection.Id id, final Set media) { + public Notification getOngoingCallNotification(final XmppConnectionService.OngoingCall ongoingCall) { + final AbstractJingleConnection.Id id = ongoingCall.id; final NotificationCompat.Builder builder = new NotificationCompat.Builder(mXmppConnectionService, "ongoing_calls"); - if (media.contains(Media.VIDEO)) { + if (ongoingCall.media.contains(Media.VIDEO)) { builder.setSmallIcon(R.drawable.ic_videocam_white_24dp); - builder.setContentTitle(mXmppConnectionService.getString(R.string.ongoing_video_call)); + if (ongoingCall.reconnecting) { + builder.setContentTitle(mXmppConnectionService.getString(R.string.reconnecting_video_call)); + } else { + builder.setContentTitle(mXmppConnectionService.getString(R.string.ongoing_video_call)); + } } else { builder.setSmallIcon(R.drawable.ic_call_white_24dp); - builder.setContentTitle(mXmppConnectionService.getString(R.string.ongoing_call)); + if (ongoingCall.reconnecting) { + builder.setContentTitle(mXmppConnectionService.getString(R.string.reconnecting_call)); + } else { + builder.setContentTitle(mXmppConnectionService.getString(R.string.ongoing_call)); + } } builder.setContentText(id.account.getRoster().getContact(id.with).getDisplayName()); builder.setVisibility(NotificationCompat.VISIBILITY_PUBLIC); diff --git a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java index 815182680..42b699e46 100644 --- a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java +++ b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java @@ -1298,8 +1298,8 @@ public class XmppConnectionService extends Service { toggleForegroundService(false); } - public void setOngoingCall(AbstractJingleConnection.Id id, Set media) { - ongoingCall.set(new OngoingCall(id, media)); + public void setOngoingCall(AbstractJingleConnection.Id id, Set media, final boolean reconnecting) { + ongoingCall.set(new OngoingCall(id, media, reconnecting)); toggleForegroundService(false); } @@ -1315,7 +1315,7 @@ public class XmppConnectionService extends Service { final Notification notification; final int id; if (ongoing != null) { - notification = this.mNotificationService.getOngoingCallNotification(ongoing.id, ongoing.media); + notification = this.mNotificationService.getOngoingCallNotification(ongoing); id = NotificationService.ONGOING_CALL_NOTIFICATION_ID; startForeground(id, notification); mNotificationService.cancel(NotificationService.FOREGROUND_NOTIFICATION_ID); @@ -4869,12 +4869,14 @@ public class XmppConnectionService extends Service { } public static class OngoingCall { - private final AbstractJingleConnection.Id id; - private final Set media; + public final AbstractJingleConnection.Id id; + public final Set media; + public final boolean reconnecting; - public OngoingCall(AbstractJingleConnection.Id id, Set media) { + public OngoingCall(AbstractJingleConnection.Id id, Set media, final boolean reconnecting) { this.id = id; this.media = media; + this.reconnecting = reconnecting; } @Override @@ -4882,12 +4884,12 @@ public class XmppConnectionService extends Service { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; OngoingCall that = (OngoingCall) o; - return Objects.equal(id, that.id); + return reconnecting == that.reconnecting && Objects.equal(id, that.id) && Objects.equal(media, that.media); } @Override public int hashCode() { - return Objects.hashCode(id); + return Objects.hashCode(id, media, reconnecting); } } } 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 e4661a3aa..12ba35733 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -1484,8 +1484,10 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web this.stateHistory.add(newState); if (newState == PeerConnection.PeerConnectionState.CONNECTED) { this.sessionDuration.start(); + updateOngoingCallNotification(); } else if (this.sessionDuration.isRunning()) { this.sessionDuration.stop(); + updateOngoingCallNotification(); } final boolean neverConnected = !this.stateHistory.contains(PeerConnection.PeerConnectionState.CONNECTED); @@ -1633,8 +1635,15 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } private void updateOngoingCallNotification() { - if (STATES_SHOWING_ONGOING_CALL.contains(this.state)) { - xmppConnectionService.setOngoingCall(id, getMedia()); + final State state = this.state; + if (STATES_SHOWING_ONGOING_CALL.contains(state)) { + final boolean reconnecting; + if (state == State.SESSION_ACCEPTED) { + reconnecting = getPeerConnectionStateAsEndUserState() == RtpEndUserState.RECONNECTING; + } else { + reconnecting = false; + } + xmppConnectionService.setOngoingCall(id, getMedia(), reconnecting); } else { xmppConnectionService.removeOngoingCall(); } @@ -1758,7 +1767,6 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web return webRTCWrapper.getRemoteVideoTrack(); } - public EglBase.Context getEglBaseContext() { return webRTCWrapper.getEglBaseContext(); } diff --git a/src/main/res/values/strings.xml b/src/main/res/values/strings.xml index dae88c606..ff1894533 100644 --- a/src/main/res/values/strings.xml +++ b/src/main/res/values/strings.xml @@ -920,6 +920,8 @@ Hang up Ongoing call Ongoing video call + Reconnecting call + Reconnecting video call Disable Tor to make calls Incoming call Incoming call · %s