delay discovered ice candidates until JingleRtpConnection is ready to receive

otherwise setLocalDescription and the arrival of first candidates might race
the rtpContentDescription being set
This commit is contained in:
Daniel Gultsch 2021-11-11 21:02:15 +01:00
parent b6dee6da6a
commit 717c83753f
2 changed files with 62 additions and 5 deletions

View file

@ -34,6 +34,7 @@ import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Queue; import java.util.Queue;
import java.util.Set; import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
@ -567,6 +568,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
final SessionDescription sessionDescription = SessionDescription.parse(webRTCSessionDescription.description); final SessionDescription sessionDescription = SessionDescription.parse(webRTCSessionDescription.description);
final RtpContentMap respondingRtpContentMap = RtpContentMap.of(sessionDescription); final RtpContentMap respondingRtpContentMap = RtpContentMap.of(sessionDescription);
this.responderRtpContentMap = respondingRtpContentMap; this.responderRtpContentMap = respondingRtpContentMap;
webRTCWrapper.setIsReadyToReceiveIceCandidates(true);
final ListenableFuture<RtpContentMap> outgoingContentMapFuture = prepareOutgoingContentMap(respondingRtpContentMap); final ListenableFuture<RtpContentMap> outgoingContentMapFuture = prepareOutgoingContentMap(respondingRtpContentMap);
Futures.addCallback(outgoingContentMapFuture, Futures.addCallback(outgoingContentMapFuture,
new FutureCallback<RtpContentMap>() { new FutureCallback<RtpContentMap>() {
@ -872,6 +874,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
final SessionDescription sessionDescription = SessionDescription.parse(webRTCSessionDescription.description); final SessionDescription sessionDescription = SessionDescription.parse(webRTCSessionDescription.description);
final RtpContentMap rtpContentMap = RtpContentMap.of(sessionDescription); final RtpContentMap rtpContentMap = RtpContentMap.of(sessionDescription);
this.initiatorRtpContentMap = rtpContentMap; this.initiatorRtpContentMap = rtpContentMap;
this.webRTCWrapper.setIsReadyToReceiveIceCandidates(true);
final ListenableFuture<RtpContentMap> outgoingContentMapFuture = encryptSessionInitiate(rtpContentMap); final ListenableFuture<RtpContentMap> outgoingContentMapFuture = encryptSessionInitiate(rtpContentMap);
Futures.addCallback(outgoingContentMapFuture, new FutureCallback<RtpContentMap>() { Futures.addCallback(outgoingContentMapFuture, new FutureCallback<RtpContentMap>() {
@Override @Override
@ -1339,7 +1342,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
@Override @Override
public void onConnectionChange(final PeerConnection.PeerConnectionState newState) { 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); this.stateHistory.add(newState);
if (newState == PeerConnection.PeerConnectionState.CONNECTED) { if (newState == PeerConnection.PeerConnectionState.CONNECTED) {
this.sessionDuration.start(); 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 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) { 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); Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": not sending session-terminate after connectivity error because session is already in state " + this.state);
return; return;
} }
new Thread(this::closeWebRTCSessionAfterFailedConnection).start(); webRTCWrapper.execute(this::closeWebRTCSessionAfterFailedConnection);
} else if (newState == PeerConnection.PeerConnectionState.FAILED) { } else if (newState == PeerConnection.PeerConnectionState.FAILED) {
Log.d(Config.LOGTAG, "attempting to restart ICE"); Log.d(Config.LOGTAG, "attempting to restart ICE");
webRTCWrapper.restartIce(); webRTCWrapper.restartIce();
@ -1367,6 +1373,33 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
@Override @Override
public void onRenegotiationNeeded() { public void onRenegotiationNeeded() {
Log.d(Config.LOGTAG, "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() { private void closeWebRTCSessionAfterFailedConnection() {

View file

@ -45,8 +45,13 @@ import org.webrtc.voiceengine.WebRtcAudioEffects;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.LinkedList;
import java.util.List; import java.util.List;
import java.util.Queue;
import java.util.Set; 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.Nonnull;
import javax.annotation.Nullable; import javax.annotation.Nullable;
@ -59,6 +64,8 @@ public class WebRTCWrapper {
private static final String EXTENDED_LOGGING_TAG = WebRTCWrapper.class.getSimpleName(); 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 //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<String> HARDWARE_AEC_BLACKLIST = new ImmutableSet.Builder<String>() private static final Set<String> HARDWARE_AEC_BLACKLIST = new ImmutableSet.Builder<String>()
.add("Pixel") .add("Pixel")
@ -79,6 +86,8 @@ public class WebRTCWrapper {
private static final int CAPTURING_MAX_FRAME_RATE = 30; private static final int CAPTURING_MAX_FRAME_RATE = 30;
private final EventCallback eventCallback; private final EventCallback eventCallback;
private final AtomicBoolean readyToReceivedIceCandidates = new AtomicBoolean(false);
private final Queue<IceCandidate> iceCandidates = new LinkedList<>();
private final AppRTCAudioManager.AudioManagerEvents audioManagerEvents = new AppRTCAudioManager.AudioManagerEvents() { private final AppRTCAudioManager.AudioManagerEvents audioManagerEvents = new AppRTCAudioManager.AudioManagerEvents() {
@Override @Override
public void onAudioDeviceChanged(AppRTCAudioManager.AudioDevice selectedAudioDevice, Set<AppRTCAudioManager.AudioDevice> availableAudioDevices) { public void onAudioDeviceChanged(AppRTCAudioManager.AudioDevice selectedAudioDevice, Set<AppRTCAudioManager.AudioDevice> availableAudioDevices) {
@ -125,7 +134,11 @@ public class WebRTCWrapper {
@Override @Override
public void onIceCandidate(IceCandidate iceCandidate) { public void onIceCandidate(IceCandidate iceCandidate) {
eventCallback.onIceCandidate(iceCandidate); if (readyToReceivedIceCandidates.get()) {
eventCallback.onIceCandidate(iceCandidate);
} else {
iceCandidates.add(iceCandidate);
}
} }
@Override @Override
@ -294,7 +307,14 @@ public class WebRTCWrapper {
} }
void restartIce() { 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() { synchronized void close() {
@ -528,6 +548,10 @@ public class WebRTCWrapper {
return appRTCAudioManager; return appRTCAudioManager;
} }
void execute(final Runnable command) {
executorService.execute(command);
}
public interface EventCallback { public interface EventCallback {
void onIceCandidate(IceCandidate iceCandidate); void onIceCandidate(IceCandidate iceCandidate);