From 61851e5f843307f2f90de892fd4fefbff5b19d46 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 11 Nov 2021 14:40:15 +0100 Subject: [PATCH] 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