Skip to content

Commit

Permalink
Fixed #684: Consider enforcing policy for ICE candidate timeouts
Browse files Browse the repository at this point in the history
  • Loading branch information
Antonis committed Oct 9, 2017
1 parent e5e2576 commit e345484
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 16 deletions.
17 changes: 17 additions & 0 deletions Examples/restcomm-olympus/app/src/main/java/org/restcomm/android/olympus/CallActivity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,10 @@ private void handleCall(Intent intent) {
connectParams.put(RCConnection.ParameterKeys.CONNECTION_PREFERRED_VIDEO_FRAME_RATE,
frameRateString2Enum(prefs.getString(RCConnection.ParameterKeys.CONNECTION_PREFERRED_VIDEO_FRAME_RATE, ""))
);
// Needed until we implement Trickle ICE
connectParams.put(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT,
Integer.parseInt(prefs.getString(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT, "0"))
);

// Here's how to set manually
//connectParams.put(RCConnection.ParameterKeys.CONNECTION_PREFERRED_VIDEO_CODEC, RCConnection.VideoCodec.VIDEO_CODEC_VP8);
Expand Down Expand Up @@ -389,6 +393,11 @@ private void handleCall(Intent intent) {
acceptParams.put(RCConnection.ParameterKeys.CONNECTION_PREFERRED_AUDIO_CODEC,
audioCodecString2Enum(prefs.getString(RCConnection.ParameterKeys.CONNECTION_PREFERRED_AUDIO_CODEC, ""))
);
// Needed until we implement Trickle ICE
acceptParams.put(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT,
Integer.parseInt(prefs.getString(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT, "0"))
);


if (intent.getAction().equals(RCDevice.ACTION_INCOMING_CALL_ANSWER_VIDEO)) {
acceptParams.put(RCConnection.ParameterKeys.CONNECTION_PREFERRED_VIDEO_CODEC,
Expand Down Expand Up @@ -495,6 +504,10 @@ public void onClick(View view) {
acceptParams.put(RCConnection.ParameterKeys.CONNECTION_PREFERRED_VIDEO_FRAME_RATE,
frameRateString2Enum(prefs.getString(RCConnection.ParameterKeys.CONNECTION_PREFERRED_VIDEO_FRAME_RATE, ""))
);
acceptParams.put(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT,
Integer.parseInt(prefs.getString(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT, "0"))
);


// Check permissions asynchronously and then accept the call
handlePermissions(true);
Expand All @@ -513,6 +526,10 @@ public void onClick(View view) {
audioCodecString2Enum(prefs.getString(RCConnection.ParameterKeys.CONNECTION_PREFERRED_AUDIO_CODEC, ""))
);

acceptParams.put(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT,
Integer.parseInt(prefs.getString(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT, "0"))
);

// Check permissions asynchronously and then accept the call
handlePermissions(false);
}
Expand Down
36 changes: 36 additions & 0 deletions ...les/restcomm-olympus/app/src/main/java/org/restcomm/android/olympus/SettingsActivity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ protected void onResume()
int iceServersDiscoveryType = Integer.parseInt(prefs.getString(RCDevice.ParameterKeys.MEDIA_ICE_SERVERS_DISCOVERY_TYPE, "0"));
updatedPref.setSummary(getResources().getStringArray(R.array.ice_servers_discovery_types_entries)[iceServersDiscoveryType]);

updatedPref = settingsFragment.findPreference(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT);
//int candidateTimeout = Integer.parseInt(prefs.getString(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT, "0"));
//updatedPref.setSummary(getResources().getStringArray(R.array.candidate_timeout_entries)[candidateTimeout]);
updatedPref.setSummary(candidateTimeoutValue2Summary(prefs.getString(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT, "0")));

updated = false;
}

Expand Down Expand Up @@ -187,6 +192,15 @@ public boolean onOptionsItemSelected(MenuItem item)
RCDevice.MediaIceServersDiscoveryType.values()[Integer.parseInt(iceServersDiscoveryType)]
);

// Same for candidate timeout
String candidateTimeout = "0";
if (prefHashMap.containsKey(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT)) {
candidateTimeout = (String) prefHashMap.get(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT);
prefHashMap.remove(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT);
}
prefHashMap.put(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT,
Integer.parseInt(candidateTimeout)
);

RCUtils.validateSettingsParms(prefHashMap);
if (!device.updateParams(params)) {
Expand Down Expand Up @@ -272,6 +286,17 @@ else if (key.equals(RCDevice.ParameterKeys.MEDIA_ICE_SERVERS_DISCOVERY_TYPE)) {
}
updated = true;
}
else if (key.equals(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT)) {
params.put(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT,
Integer.parseInt(prefs.getString(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT, "0")));
Preference updatedPref = settingsFragment.findPreference(key);
if (updatedPref != null) {
//int candidateTimeout = Integer.parseInt(prefs.getString(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT, "0"));
//updatedPref.setSummary(getResources().getStringArray(R.array.candidate_timeout_entries)[candidateTimeout]);
updatedPref.setSummary(candidateTimeoutValue2Summary(prefs.getString(RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT, "0")));
}
updated = true;
}
else if (key.equals(RCConnection.ParameterKeys.CONNECTION_PREFERRED_AUDIO_CODEC)) {
params.put(RCConnection.ParameterKeys.CONNECTION_PREFERRED_AUDIO_CODEC, prefs.getString(RCConnection.ParameterKeys.CONNECTION_PREFERRED_AUDIO_CODEC, "Default"));
Preference updatedPref = settingsFragment.findPreference(key);
Expand Down Expand Up @@ -318,4 +343,15 @@ public void onClick(DialogInterface dialog, int which)
});
alertDialog.show();
}

private String candidateTimeoutValue2Summary(String value)
{
String summary = "No timeout";
if (Integer.parseInt(value) > 0) {
summary = value + " " + "seconds";
}
return summary;
}


}
15 changes: 14 additions & 1 deletion Examples/restcomm-olympus/app/src/main/res/values/arrays.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,20 @@
<item>2</item>
</string-array>


<string-array name="candidate_timeout_entries">
<item>No timeout</item>
<item>3 seconds</item>
<item>5 seconds</item>
<item>10 seconds</item>
<item>20 seconds</item>
</string-array>
<string-array name="candidate_timeout_values">
<item>0</item>
<item>3</item>
<item>5</item>
<item>10</item>
<item>20</item>
</string-array>

</resources>

7 changes: 7 additions & 0 deletions Examples/restcomm-olympus/app/src/main/res/xml/preferences.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@
android:dialogTitle="@string/pref_ice_servers_discovery_type_dlg"
android:entries="@array/ice_servers_discovery_types_entries"
android:entryValues="@array/ice_servers_discovery_types_values"/>
<ListPreference
android:key="debug-connection-candidate-timeout"
android:title="@string/pref_candidate_timeout"
android:defaultValue="@string/pref_candidate_timeout_default"
android:dialogTitle="@string/pref_candidate_timeout_dlg"
android:entries="@array/candidate_timeout_entries"
android:entryValues="@array/candidate_timeout_values"/>
<EditTextPreference
android:defaultValue="https://es.xirsys.com/_turn"
android:key="turn-url"
Expand Down
4 changes: 4 additions & 0 deletions restcomm.android.sdk/src/main/java/org/restcomm/android/sdk/RCClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public enum ErrorCodes {
ERROR_CONNECTION_AUDIO_CALL_VIDEO_CODEC_FORBIDDEN,
ERROR_CONNECTION_AUDIO_CALL_VIDEO_RESOLUTION_FORBIDDEN,
ERROR_CONNECTION_AUDIO_CALL_VIDEO_FRAME_RATE_FORBIDDEN,
ERROR_CONNECTION_WEBRTC_CANDIDATES_TIMED_OUT,

ERROR_MESSAGE_AUTHENTICATION_FORBIDDEN,
ERROR_MESSAGE_URI_INVALID,
Expand Down Expand Up @@ -271,6 +272,9 @@ else if (errorCode == ErrorCodes.ERROR_CONNECTION_AUDIO_CALL_VIDEO_RESOLUTION_FO
else if (errorCode == ErrorCodes.ERROR_CONNECTION_AUDIO_CALL_VIDEO_FRAME_RATE_FORBIDDEN) {
return "Failed to initiate connection due to parameter validation error; video frame rate not allowed to be specified in an audio call";
}
else if (errorCode == ErrorCodes.ERROR_CONNECTION_WEBRTC_CANDIDATES_TIMED_OUT) {
return "Failed to collect any candidates on time; please check your network settings and connectivity or consider increasing candidate timeout";
}

else if (errorCode == ErrorCodes.ERROR_MESSAGE_AUTHENTICATION_FORBIDDEN) {
return "Message failed to authenticate with Service";
Expand Down
97 changes: 82 additions & 15 deletions restcomm.android.sdk/src/main/java/org/restcomm/android/sdk/RCConnection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,9 @@ public static class ParameterKeys {
// Incoming headers from Restcomm both for incoming and outgoing calls
public static final String CONNECTION_CUSTOM_INCOMING_SIP_HEADERS = "sip-headers-incoming";
public static final String CONNECTION_SIP_HEADER_KEY_CALL_SID = "X-RestComm-CallSid";

// Until we have trickle, as a way to timeout sooner than 40 seconds (webrtc default timeout)
public static final String DEBUG_CONNECTION_CANDIDATE_TIMEOUT = "debug-connection-candidate-timeout";
}

/**
Expand Down Expand Up @@ -382,6 +385,8 @@ public RCConnection build()
private Handler timeoutHandler = null;
// call times out if it hasn't been established after 15 seconds
private final int CALL_TIMEOUT_DURATION_MILIS = 15 * 1000;
private Handler candidateTimeoutHandler = null;
private boolean iceGatheringCompleteCalled = false;
// Device was already busy with another Connection when this Connection arrived. If so we need to set this so that we have custom behavior later
private boolean deviceAlreadyBusy = false;

Expand Down Expand Up @@ -445,6 +450,7 @@ private RCConnection(Builder builder)
peer = builder.peer;
deviceAlreadyBusy = builder.deviceAlreadyBusy;
timeoutHandler = new Handler(device.getMainLooper());
candidateTimeoutHandler = new Handler(device.getMainLooper());

callParams = new HashMap<>();
if (builder.customHeaders != null) {
Expand Down Expand Up @@ -536,6 +542,11 @@ public void open(Map<String, Object> parameters)
* <b>RCConnection.ParameterKeys.CONNECTION_PREFERRED_VIDEO_FRAME_RATE</b>: Preferred frame rate to use. Default is 30fps. Possible values are enumerated at <i>RCConnection.VideoFrameRate</i> <br>
* <b>RCConnection.ParameterKeys.CONNECTION_CUSTOM_SIP_HEADERS</b>: An optional HashMap&lt;String,String&gt; of custom SIP headers we want to add. For an example
* please check restcomm-helloworld or restcomm-olympus sample Apps (optional) <br>
* <b>RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT</b>: An optional Integer denoting how long to wait for ICE candidates. Zero means default behaviour which is
* to depend on onIceGatheringComplete from Peer Connection facilities. Any other integer value means to wait at most that amount of time no matter if onIceGatheringComplete has fired.
* The problem we are addressing here is the new Peer Connection ICE gathering timeout which is 40 seconds which is way too long. Notice that the root cause here is in reality
* lack of support for Trickle ICE, so once it is supported we won't be needing such workarounds.
* please check restcomm-helloworld or restcomm-olympus sample Apps (optional) <br>
*/
public void accept(Map<String, Object> parameters)
{
Expand Down Expand Up @@ -1062,6 +1073,7 @@ public void onCallErrorEvent(String jobId, RCClient.ErrorCodes errorCode, String
private void handleDisconnected(String jobId, boolean haveDisconnectedLocally)
{
timeoutHandler.removeCallbacksAndMessages(null);
candidateTimeoutHandler.removeCallbacksAndMessages(null);

// Device was already busy with another Connection, skip all handling here
if (deviceAlreadyBusy) {
Expand Down Expand Up @@ -1110,6 +1122,7 @@ private void handleDisconnect(String reason)
{
RCLogger.i(TAG, "handleDisconnect(): reason: " + reason);
timeoutHandler.removeCallbacksAndMessages(null);
candidateTimeoutHandler.removeCallbacksAndMessages(null);

audioManager.stop();

Expand Down Expand Up @@ -1179,6 +1192,22 @@ public void onIceServersReady(final LinkedList<PeerConnection.IceServer> iceServ
@Override
public void run()
{

if (RCConnection.this.callParams.containsKey(ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT) &&
(Integer) RCConnection.this.callParams.get(ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT) != 0) {
// cancel any pending timers before we start new one
candidateTimeoutHandler.removeCallbacksAndMessages(null);
Runnable runnable = new Runnable() {
@Override
public void run()
{
onCandidatesTimeout();
}
};
candidateTimeoutHandler.postDelayed(runnable, (Integer) RCConnection.this.callParams.get(ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT) * 1000);
//candidateTimeoutHandler.postDelayed(runnable, 150);
}

if (!RCConnection.this.incoming) {
// we are the initiator

Expand Down Expand Up @@ -1392,6 +1421,35 @@ private void onCallTimeout()
sendQoSDisconnectErrorIntent(errorCode.ordinal(), RCClient.errorText(errorCode));
}

private void onCandidatesTimeout()
{
RCLogger.e(TAG, "onCandidatesTimeout: Candidates timed out after: " + RCConnection.this.callParams.get(ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT) + " seconds");

if (signalingParameters != null && signalingParameters.iceCandidates != null &&
signalingParameters.iceCandidates.size() > 0) {
RCLogger.w(TAG, "onCandidatesTimeout: Managed to collect: " + signalingParameters.iceCandidates.size() + " candidates");
onIceGatheringComplete();
}
else {
// no candidates are gathered
handleDisconnect(null);

if (RCDevice.state == RCDevice.DeviceState.BUSY) {
RCDevice.state = RCDevice.DeviceState.READY;
}

if (device.isAttached()) {
RCConnection.this.listener.onError(RCConnection.this, RCClient.ErrorCodes.ERROR_CONNECTION_WEBRTC_CANDIDATES_TIMED_OUT.ordinal(),
RCClient.errorText(RCClient.ErrorCodes.ERROR_CONNECTION_WEBRTC_CANDIDATES_TIMED_OUT));
}
else {
RCLogger.w(TAG, "RCConnectionListener event suppressed since Restcomm Client Service not attached: onDisconnected()");
}

}
}


/*
// DEBUG (Issue #380)
public void off()
Expand Down Expand Up @@ -1920,24 +1978,33 @@ public void onIceGatheringComplete()
public void run()
{
RCLogger.i(TAG, "onIceGatheringComplete");
if (peerConnectionClient == null) {
// if the user hangs up the call before its setup we need to bail
return;
}
if (signalingParameters.initiator) {
HashMap<String, Object> parameters = new HashMap<String, Object>();
parameters.put(RCConnection.ParameterKeys.CONNECTION_PEER, signalingParameters.sipUrl);
parameters.put("sdp", connection.signalingParameters.generateSipSdp(connection.signalingParameters.offerSdp, connection.signalingParameters.iceCandidates));
parameters.put(ParameterKeys.CONNECTION_CUSTOM_SIP_HEADERS, connection.signalingParameters.sipHeaders);

signalingClient.call(jobId, parameters);
candidateTimeoutHandler.removeCallbacksAndMessages(null);

if (!iceGatheringCompleteCalled) {
iceGatheringCompleteCalled = true;

if (peerConnectionClient == null) {
// if the user hangs up the call before its setup we need to bail
return;
}
if (signalingParameters.initiator) {
HashMap<String, Object> parameters = new HashMap<String, Object>();
parameters.put(RCConnection.ParameterKeys.CONNECTION_PEER, signalingParameters.sipUrl);
parameters.put("sdp", connection.signalingParameters.generateSipSdp(connection.signalingParameters.offerSdp, connection.signalingParameters.iceCandidates));
parameters.put(ParameterKeys.CONNECTION_CUSTOM_SIP_HEADERS, connection.signalingParameters.sipHeaders);

signalingClient.call(jobId, parameters);
} else {
HashMap<String, Object> parameters = new HashMap<>();
parameters.put("sdp", connection.signalingParameters.generateSipSdp(connection.signalingParameters.answerSdp,
connection.signalingParameters.iceCandidates));
signalingClient.accept(jobId, parameters);
//connection.state = ConnectionState.CONNECTING;
}
}
else {
HashMap<String, Object> parameters = new HashMap<>();
parameters.put("sdp", connection.signalingParameters.generateSipSdp(connection.signalingParameters.answerSdp,
connection.signalingParameters.iceCandidates));
signalingClient.accept(jobId, parameters);
//connection.state = ConnectionState.CONNECTING;
RCLogger.w(TAG, "onIceGatheringComplete() already called, skipping");
}
}
};
Expand Down
5 changes: 5 additions & 0 deletions restcomm.android.sdk/src/main/java/org/restcomm/android/sdk/RCDevice.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,11 @@ public void updateCapabilityToken(String token)
* <b>RCConnection.ParameterKeys.CONNECTION_PREFERRED_VIDEO_FRAME_RATE</b>: Preferred frame rate to use. Default is 30fps. Possible values are enumerated at <i>RCConnection.VideoFrameRate</i> (optional) <br>
* <b>RCConnection.ParameterKeys.CONNECTION_CUSTOM_SIP_HEADERS</b>: An optional HashMap&lt;String,String&gt; of custom SIP headers we want to add. For an example
* please check restcomm-helloworld or restcomm-olympus sample Apps (optional) <br>
* <b>RCConnection.ParameterKeys.DEBUG_CONNECTION_CANDIDATE_TIMEOUT</b>: An optional Integer denoting how long to wait for ICE candidates. Zero means default behaviour which is
* to depend on onIceGatheringComplete from Peer Connection facilities. Any other integer value means to wait at most that amount of time no matter if onIceGatheringComplete has fired.
* The problem we are addressing here is the new Peer Connection ICE gathering timeout which is 40 seconds which is way too long. Notice that the root cause here is in reality
* lack of support for Trickle ICE, so once it is supported we won't be needing such workarounds.
* please check restcomm-helloworld or restcomm-olympus sample Apps (optional) <br>
* @param listener The listener object that will receive events when the connection state changes
* @return An RCConnection object representing the new connection or null in case of error. Error
* means that RCDevice.state not ready to make a call (this usually means no WiFi available)
Expand Down
4 changes: 4 additions & 0 deletions restcomm.android.sdk/src/main/res/values/strings.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@
<string name="pref_ice_servers_discovery_type_dlg">Select ICE Servers Discovery Type</string>
<string name="pref_ice_servers_discovery_type_default">1</string>

<string name="pref_candidate_timeout">Candidate timeout</string>
<string name="pref_candidate_timeout_dlg">Select candidate timeout</string>
<string name="pref_candidate_timeout_default">0</string>

<string name="pref_startvideobitrate_key">startvideobitrate_preference</string>
<string name="pref_startvideobitrate_title">Start video bitrate setting.</string>
<string name="pref_startvideobitrate_dlg">Start video bitrate setting.</string>
Expand Down

0 comments on commit e345484

Please sign in to comment.