Skip to content

Fix handling offers in already established connections #1823

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
danxuliu wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Fix handling offers in already established connections #1823

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7b619e5
Add and remove nick provider listeners based on the peer connections
danxuliu Nov 30, 2022
c1f77b1
Handle "sid" in peer connections
danxuliu Nov 30, 2022
1b5be43
Handle offers to renegotiate an already established connection
danxuliu Feb 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 37 additions & 11 deletions app/src/main/java/com/nextcloud/talk/activities/CallActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,15 @@ public void onCallEndedForAll() {

private SignalingMessageReceiver.OfferMessageListener offerMessageListener = new SignalingMessageReceiver.OfferMessageListener() {
@Override
public void onOffer(String sessionId, String roomType, String sdp, String nick) {
getOrCreatePeerConnectionWrapperForSessionIdAndType(sessionId, roomType, false);
public void onOffer(String sessionId, String roomType, String sid, String sdp, String nick) {
// If there is already a peer connection but a new offer is received with a different sid the existing
// peer connection is stale, so it needs to be removed and a new one created instead.
PeerConnectionWrapper peerConnectionWrapper = getPeerConnectionWrapperForSessionIdAndType(sessionId, roomType);
if (peerConnectionWrapper != null && sid != null && !sid.equals(peerConnectionWrapper.getSid())) {
endPeerConnection(sessionId, roomType);
}

getOrCreatePeerConnectionWrapperForSessionIdAndType(sessionId, roomType, sid, false);
}
};

Expand Down Expand Up @@ -1850,7 +1857,7 @@ private void handleCallParticipantsChanged(Collection<Participant> joined, Colle

if (hasMCU) {
// Ensure that own publishing peer is set up.
getOrCreatePeerConnectionWrapperForSessionIdAndType(webSocketClient.getSessionId(), VIDEO_STREAM_TYPE_VIDEO, true);
getOrCreatePeerConnectionWrapperForSessionIdAndType(webSocketClient.getSessionId(), VIDEO_STREAM_TYPE_VIDEO, null, true);
}

boolean selfJoined = false;
Expand Down Expand Up @@ -1894,7 +1901,7 @@ private void handleCallParticipantsChanged(Collection<Participant> joined, Colle
// higher session ID but is not publishing media.
if ((hasMCU && participantHasAudioOrVideo) ||
(!hasMCU && selfParticipantHasAudioOrVideo && (!participantHasAudioOrVideo || sessionId.compareTo(currentSessionId) < 0))) {
getOrCreatePeerConnectionWrapperForSessionIdAndType(sessionId, VIDEO_STREAM_TYPE_VIDEO, false);
getOrCreatePeerConnectionWrapperForSessionIdAndType(sessionId, VIDEO_STREAM_TYPE_VIDEO, null, false);
}
}

Expand Down Expand Up @@ -1934,6 +1941,7 @@ private PeerConnectionWrapper getPeerConnectionWrapperForSessionIdAndType(String

private PeerConnectionWrapper getOrCreatePeerConnectionWrapperForSessionIdAndType(String sessionId,
String type,
String sid,
boolean publisher) {
PeerConnectionWrapper peerConnectionWrapper;
if ((peerConnectionWrapper = getPeerConnectionWrapperForSessionIdAndType(sessionId, type)) != null) {
Expand All @@ -1952,6 +1960,7 @@ private PeerConnectionWrapper getOrCreatePeerConnectionWrapperForSessionIdAndTyp
iceServers,
sdpConstraintsForMCU,
sessionId,
sid,
callSession,
localStream,
true,
Expand All @@ -1965,6 +1974,7 @@ private PeerConnectionWrapper getOrCreatePeerConnectionWrapperForSessionIdAndTyp
iceServers,
sdpConstraints,
sessionId,
sid,
callSession,
null,
false,
Expand All @@ -1978,6 +1988,7 @@ private PeerConnectionWrapper getOrCreatePeerConnectionWrapperForSessionIdAndTyp
iceServers,
sdpConstraints,
sessionId,
sid,
callSession,
localStream,
false,
Expand All @@ -1990,6 +2001,7 @@ private PeerConnectionWrapper getOrCreatePeerConnectionWrapperForSessionIdAndTyp
iceServers,
sdpConstraints,
sessionId,
sid,
callSession,
null,
false,
Expand All @@ -2013,6 +2025,17 @@ private PeerConnectionWrapper getOrCreatePeerConnectionWrapperForSessionIdAndTyp
} else {
callParticipant.setPeerConnectionWrapper(peerConnectionWrapper);
}

if (!hasExternalSignalingServer) {
OfferAnswerNickProvider offerAnswerNickProvider = offerAnswerNickProviders.get(sessionId);
if ("screen".equals(type)) {
signalingMessageReceiver.addListener(offerAnswerNickProvider.getScreenWebRtcMessageListener(),
sessionId, "screen", peerConnectionWrapper.getSid());
} else {
signalingMessageReceiver.addListener(offerAnswerNickProvider.getVideoWebRtcMessageListener(),
sessionId, "video", peerConnectionWrapper.getSid());
}
}
}

if (publisher) {
Expand All @@ -2037,8 +2060,6 @@ private CallParticipant addCallParticipant(String sessionId) {
if (!hasExternalSignalingServer) {
OfferAnswerNickProvider offerAnswerNickProvider = new OfferAnswerNickProvider(sessionId);
offerAnswerNickProviders.put(sessionId, offerAnswerNickProvider);
signalingMessageReceiver.addListener(offerAnswerNickProvider.getVideoWebRtcMessageListener(), sessionId, "video");
signalingMessageReceiver.addListener(offerAnswerNickProvider.getScreenWebRtcMessageListener(), sessionId, "screen");
}

final CallParticipantModel callParticipantModel = callParticipant.getCallParticipantModel();
Expand Down Expand Up @@ -2074,6 +2095,15 @@ private void endPeerConnection(String sessionId, String type) {
}
}

if (!hasExternalSignalingServer) {
OfferAnswerNickProvider offerAnswerNickProvider = offerAnswerNickProviders.get(sessionId);
if ("screen".equals(type)) {
signalingMessageReceiver.removeListener(offerAnswerNickProvider.getScreenWebRtcMessageListener());
} else {
signalingMessageReceiver.removeListener(offerAnswerNickProvider.getVideoWebRtcMessageListener());
}
}

peerConnectionWrapper.removePeerConnection();
peerConnectionWrapperList.remove(peerConnectionWrapper);
}
Expand All @@ -2093,11 +2123,7 @@ private void removeCallParticipant(String sessionId) {
SignalingMessageReceiver.CallParticipantMessageListener listener = callParticipantMessageListeners.remove(sessionId);
signalingMessageReceiver.removeListener(listener);

OfferAnswerNickProvider offerAnswerNickProvider = offerAnswerNickProviders.remove(sessionId);
if (offerAnswerNickProvider != null) {
signalingMessageReceiver.removeListener(offerAnswerNickProvider.getVideoWebRtcMessageListener());
signalingMessageReceiver.removeListener(offerAnswerNickProvider.getScreenWebRtcMessageListener());
}
offerAnswerNickProviders.remove(sessionId);

runOnUiThread(() -> removeParticipantDisplayItem(sessionId, "video"));
}
Expand Down
4 changes: 2 additions & 2 deletions app/src/main/java/com/nextcloud/talk/signaling/OfferMessageNotifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ public synchronized void removeListener(SignalingMessageReceiver.OfferMessageLis
offerMessageListeners.remove(listener);
}

public synchronized void notifyOffer(String sessionId, String roomType, String sdp, String nick) {
public synchronized void notifyOffer(String sessionId, String roomType, String sid, String sdp, String nick) {
for (SignalingMessageReceiver.OfferMessageListener listener : new ArrayList<>(offerMessageListeners)) {
listener.onOffer(sessionId, roomType, sdp, nick);
listener.onOffer(sessionId, roomType, sid, sdp, nick);
}
}
}
24 changes: 17 additions & 7 deletions app/src/main/java/com/nextcloud/talk/signaling/SignalingMessageReceiver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public interface CallParticipantMessageListener {
* When an offer is received all OfferMessageListeners are notified before any WebRtcMessageListener is notified.
*/
public interface OfferMessageListener {
void onOffer(String sessionId, String roomType, String sdp, String nick);
void onOffer(String sessionId, String roomType, String sid, String sdp, String nick);
}

/**
Expand Down Expand Up @@ -218,12 +218,19 @@ public void removeListener(OfferMessageListener listener) {
* A listener is expected to be added only once. If the same listener is added again it will no longer be notified
* for the messages from the previous session ID or room type.
*
* At any given time there will be just one peer connection with the same session ID and room type. The peer
* connection ID is an additional guarantee to ensure that both ends are using the same peer connection. However,
* in some cases (older external signaling server or app versions) the WebRTC messages may not contain a peer
* connection ID. In those cases the listener will still receive the messages, even if it was registered against
* a specific peer connection ID.
*
* @param listener the WebRtcMessageListener
* @param sessionId the ID of the session that messages come from
* @param roomType the room type that messages come from
* @param sid the ID of the peer connection
*/
public void addListener(WebRtcMessageListener listener, String sessionId, String roomType) {
webRtcMessageNotifier.addListener(listener, sessionId, roomType);
public void addListener(WebRtcMessageListener listener, String sessionId, String roomType, String sid) {
webRtcMessageNotifier.addListener(listener, sessionId, roomType, sid);
}

public void removeListener(WebRtcMessageListener listener) {
Expand Down Expand Up @@ -452,6 +459,8 @@ protected void processSignalingMessage(NCSignalingMessage signalingMessage) {
return;
}

String sid = signalingMessage.getSid();

if ("offer".equals(type)) {
// Message schema (external signaling server):
// {
Expand Down Expand Up @@ -504,8 +513,8 @@ protected void processSignalingMessage(NCSignalingMessage signalingMessage) {
// although extremely unlikely, that the WebRtcMessageListeners for the second offer are notified before the
// WebRtcMessageListeners for the first offer. This should not be a problem, though, so for simplicity
// the statements are not synchronized.
offerMessageNotifier.notifyOffer(sessionId, roomType, sdp, nick);
webRtcMessageNotifier.notifyOffer(sessionId, roomType, sdp, nick);
offerMessageNotifier.notifyOffer(sessionId, roomType, sid, sdp, nick);
webRtcMessageNotifier.notifyOffer(sessionId, roomType, sid, sdp, nick);

return;
}
Expand All @@ -522,7 +531,7 @@ protected void processSignalingMessage(NCSignalingMessage signalingMessage) {
String sdp = payload.getSdp();
String nick = payload.getNick();

webRtcMessageNotifier.notifyAnswer(sessionId, roomType, sdp, nick);
webRtcMessageNotifier.notifyAnswer(sessionId, roomType, sid, sdp, nick);

return;
}
Expand Down Expand Up @@ -585,6 +594,7 @@ protected void processSignalingMessage(NCSignalingMessage signalingMessage) {

webRtcMessageNotifier.notifyCandidate(sessionId,
roomType,
sid,
ncIceCandidate.getSdpMid(),
ncIceCandidate.getSdpMLineIndex(),
ncIceCandidate.getCandidate());
Expand All @@ -593,7 +603,7 @@ protected void processSignalingMessage(NCSignalingMessage signalingMessage) {
}

if ("endOfCandidates".equals(type)) {
webRtcMessageNotifier.notifyEndOfCandidates(sessionId, roomType);
webRtcMessageNotifier.notifyEndOfCandidates(sessionId, roomType, sid);

return;
}
Expand Down
34 changes: 21 additions & 13 deletions app/src/main/java/com/nextcloud/talk/signaling/WebRtcMessageNotifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,22 @@ private static class WebRtcMessageListenerFrom {
public final SignalingMessageReceiver.WebRtcMessageListener listener;
public final String sessionId;
public final String roomType;
public final String sid;

private WebRtcMessageListenerFrom(SignalingMessageReceiver.WebRtcMessageListener listener,
String sessionId,
String roomType) {
String roomType,
String sid) {
this.listener = listener;
this.sessionId = sessionId;
this.roomType = roomType;
this.sid = sid;
}
}

private final List<WebRtcMessageListenerFrom> webRtcMessageListenersFrom = new ArrayList<>();

public synchronized void addListener(SignalingMessageReceiver.WebRtcMessageListener listener, String sessionId, String roomType) {
public synchronized void addListener(SignalingMessageReceiver.WebRtcMessageListener listener, String sessionId, String roomType, String sid) {
if (listener == null) {
throw new IllegalArgumentException("WebRtcMessageListener can not be null");
}
Expand All @@ -63,9 +66,13 @@ public synchronized void addListener(SignalingMessageReceiver.WebRtcMessageListe
throw new IllegalArgumentException("roomType can not be null");
}

if (sid == null) {
throw new IllegalArgumentException("sid can not be null");
}

removeListener(listener);

webRtcMessageListenersFrom.add(new WebRtcMessageListenerFrom(listener, sessionId, roomType));
webRtcMessageListenersFrom.add(new WebRtcMessageListenerFrom(listener, sessionId, roomType, sid));
}

public synchronized void removeListener(SignalingMessageReceiver.WebRtcMessageListener listener) {
Expand All @@ -81,39 +88,40 @@ public synchronized void removeListener(SignalingMessageReceiver.WebRtcMessageLi
}
}

private List<SignalingMessageReceiver.WebRtcMessageListener> getListenersFor(String sessionId, String roomType) {
private List<SignalingMessageReceiver.WebRtcMessageListener> getListenersFor(String sessionId, String roomType, String sid) {
List<SignalingMessageReceiver.WebRtcMessageListener> webRtcMessageListeners =
new ArrayList<>(webRtcMessageListenersFrom.size());

for (WebRtcMessageListenerFrom listenerFrom : webRtcMessageListenersFrom) {
if (listenerFrom.sessionId.equals(sessionId) && listenerFrom.roomType.equals(roomType)) {
if (listenerFrom.sessionId.equals(sessionId) && listenerFrom.roomType.equals(roomType) &&
(sid == null || listenerFrom.sid.equals(sid))) {
webRtcMessageListeners.add(listenerFrom.listener);
}
}

return webRtcMessageListeners;
}

public synchronized void notifyOffer(String sessionId, String roomType, String sdp, String nick) {
for (SignalingMessageReceiver.WebRtcMessageListener listener : getListenersFor(sessionId, roomType)) {
public synchronized void notifyOffer(String sessionId, String roomType, String sid, String sdp, String nick) {
for (SignalingMessageReceiver.WebRtcMessageListener listener : getListenersFor(sessionId, roomType, sid)) {
listener.onOffer(sdp, nick);
}
}

public synchronized void notifyAnswer(String sessionId, String roomType, String sdp, String nick) {
for (SignalingMessageReceiver.WebRtcMessageListener listener : getListenersFor(sessionId, roomType)) {
public synchronized void notifyAnswer(String sessionId, String roomType, String sid, String sdp, String nick) {
for (SignalingMessageReceiver.WebRtcMessageListener listener : getListenersFor(sessionId, roomType, sid)) {
listener.onAnswer(sdp, nick);
}
}

public synchronized void notifyCandidate(String sessionId, String roomType, String sdpMid, int sdpMLineIndex, String sdp) {
for (SignalingMessageReceiver.WebRtcMessageListener listener : getListenersFor(sessionId, roomType)) {
public synchronized void notifyCandidate(String sessionId, String roomType, String sid, String sdpMid, int sdpMLineIndex, String sdp) {
for (SignalingMessageReceiver.WebRtcMessageListener listener : getListenersFor(sessionId, roomType, sid)) {
listener.onCandidate(sdpMid, sdpMLineIndex, sdp);
}
}

public synchronized void notifyEndOfCandidates(String sessionId, String roomType) {
for (SignalingMessageReceiver.WebRtcMessageListener listener : getListenersFor(sessionId, roomType)) {
public synchronized void notifyEndOfCandidates(String sessionId, String roomType, String sid) {
for (SignalingMessageReceiver.WebRtcMessageListener listener : getListenersFor(sessionId, roomType, sid)) {
listener.onEndOfCandidates();
}
}
Expand Down
Loading