From c769ff0fd0407f3e0e50e5753099f062911db111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 22 Apr 2022 02:30:46 +0200 Subject: [PATCH 1/3] Rename method to a more accurate name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../nextcloud/talk/activities/CallActivity.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java index 3435ba0b9..5924fa898 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java @@ -1544,8 +1544,8 @@ public class CallActivity extends CallBaseActivity { private void processMessage(NCSignalingMessage ncSignalingMessage) { if (ncSignalingMessage.getRoomType().equals("video") || ncSignalingMessage.getRoomType().equals("screen")) { PeerConnectionWrapper peerConnectionWrapper = - getPeerConnectionWrapperForSessionIdAndType(ncSignalingMessage.getFrom(), - ncSignalingMessage.getRoomType(), false); + getOrCreatePeerConnectionWrapperForSessionIdAndType(ncSignalingMessage.getFrom(), + ncSignalingMessage.getRoomType(), false); String type = null; if (ncSignalingMessage.getPayload() != null && ncSignalingMessage.getPayload().getType() != null) { @@ -1765,12 +1765,12 @@ public class CallActivity extends CallBaseActivity { if (hasMCU) { // Ensure that own publishing peer is set up. - getPeerConnectionWrapperForSessionIdAndType(webSocketClient.getSessionId(), VIDEO_STREAM_TYPE_VIDEO, true); + getOrCreatePeerConnectionWrapperForSessionIdAndType(webSocketClient.getSessionId(), VIDEO_STREAM_TYPE_VIDEO, true); } for (String sessionId : newSessions) { Log.d(TAG, " newSession joined: " + sessionId); - getPeerConnectionWrapperForSessionIdAndType(sessionId, VIDEO_STREAM_TYPE_VIDEO, false); + getOrCreatePeerConnectionWrapperForSessionIdAndType(sessionId, VIDEO_STREAM_TYPE_VIDEO, false); } if (newSessions.size() > 0 && !currentCallStatus.equals(CallStatus.IN_CONVERSATION)) { @@ -1836,9 +1836,9 @@ public class CallActivity extends CallBaseActivity { return null; } - private PeerConnectionWrapper getPeerConnectionWrapperForSessionIdAndType(String sessionId, - String type, - boolean publisher) { + private PeerConnectionWrapper getOrCreatePeerConnectionWrapperForSessionIdAndType(String sessionId, + String type, + boolean publisher) { PeerConnectionWrapper peerConnectionWrapper; if ((peerConnectionWrapper = getPeerConnectionWrapperForSessionId(sessionId, type)) != null) { return peerConnectionWrapper; @@ -2181,7 +2181,7 @@ public class CallActivity extends CallBaseActivity { if (hasExternalSignalingServer) { nick = webSocketClient.getDisplayNameForSession(session); } else { - nick = getPeerConnectionWrapperForSessionIdAndType(session, videoStreamType, false).getNick(); + nick = getOrCreatePeerConnectionWrapperForSessionIdAndType(session, videoStreamType, false).getNick(); } String userId = ""; From a8045880f975f101e33f37a6c0597e46f79a79c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 22 Apr 2022 02:31:21 +0200 Subject: [PATCH 2/3] Rename method to a more consistent name with its sibling method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../main/java/com/nextcloud/talk/activities/CallActivity.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java index 5924fa898..baea13d83 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java @@ -1825,7 +1825,7 @@ public class CallActivity extends CallBaseActivity { peerConnectionWrapperList.remove(peerConnectionWrapper); } - private PeerConnectionWrapper getPeerConnectionWrapperForSessionId(String sessionId, String type) { + private PeerConnectionWrapper getPeerConnectionWrapperForSessionIdAndType(String sessionId, String type) { for (int i = 0; i < peerConnectionWrapperList.size(); i++) { if (peerConnectionWrapperList.get(i).getSessionId().equals(sessionId) && peerConnectionWrapperList.get(i).getVideoStreamType().equals(type)) { @@ -1840,7 +1840,7 @@ public class CallActivity extends CallBaseActivity { String type, boolean publisher) { PeerConnectionWrapper peerConnectionWrapper; - if ((peerConnectionWrapper = getPeerConnectionWrapperForSessionId(sessionId, type)) != null) { + if ((peerConnectionWrapper = getPeerConnectionWrapperForSessionIdAndType(sessionId, type)) != null) { return peerConnectionWrapper; } else { if (hasMCU && publisher) { From 5bd920142c67eca01274bc3fdec329566eca67a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 22 Apr 2022 02:32:21 +0200 Subject: [PATCH 3/3] Create a connection only for offers instead of for any message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Android app creates a connection with a participant when that participant joins the call and ends it when the participant leaves the call. However, it also created a connection when any signaling message was received from a participant that had no connection yet. Due to this if a signaling message was received from a participant before that participant was in the call the Android app tried to establish a connection too soon, which would be rejected by the HPB. Similarly, if a signaling message was received from a participant after that participant left the call a connection will try to be established. That would fail, but the connection object was not removed, and if that participant joined the call again no connection would be established, as a connection for that participant was already found, even if it was not usable. To solve that now a connection is created when a signaling message is received only if that message is an offer (which is necessary without HPB if the other participant sends the offer before this participant "noticed" that she is in the call); otherwise the message is ignored. Besides that a connection will no longer be created either when setting up the video stream. However, this would be just for correctness and it should not make any difference, as the MediaStreamEvents that cause that are only emitted by changes in peer connections, so they should be already created. Signed-off-by: Daniel Calviño Sánchez --- .../talk/activities/CallActivity.java | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java index baea13d83..cff641560 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java @@ -1543,10 +1543,6 @@ public class CallActivity extends CallBaseActivity { private void processMessage(NCSignalingMessage ncSignalingMessage) { if (ncSignalingMessage.getRoomType().equals("video") || ncSignalingMessage.getRoomType().equals("screen")) { - PeerConnectionWrapper peerConnectionWrapper = - getOrCreatePeerConnectionWrapperForSessionIdAndType(ncSignalingMessage.getFrom(), - ncSignalingMessage.getRoomType(), false); - String type = null; if (ncSignalingMessage.getPayload() != null && ncSignalingMessage.getPayload().getType() != null) { type = ncSignalingMessage.getPayload().getType(); @@ -1554,7 +1550,24 @@ public class CallActivity extends CallBaseActivity { type = ncSignalingMessage.getType(); } - if (type != null) { + PeerConnectionWrapper peerConnectionWrapper = null; + + if ("offer".equals(type)) { + peerConnectionWrapper = + getOrCreatePeerConnectionWrapperForSessionIdAndType(ncSignalingMessage.getFrom(), + ncSignalingMessage.getRoomType(), false); + } else { + peerConnectionWrapper = + getPeerConnectionWrapperForSessionIdAndType(ncSignalingMessage.getFrom(), + ncSignalingMessage.getRoomType()); + } + + if ("unshareScreen".equals(type) || + (("offer".equals(type) || + "answer".equals(type) || + "candidate".equals(type) || + "endOfCandidates".equals(type)) && + peerConnectionWrapper != null)) { switch (type) { case "unshareScreen": endPeerConnection(ncSignalingMessage.getFrom(), true); @@ -2181,7 +2194,9 @@ public class CallActivity extends CallBaseActivity { if (hasExternalSignalingServer) { nick = webSocketClient.getDisplayNameForSession(session); } else { - nick = getOrCreatePeerConnectionWrapperForSessionIdAndType(session, videoStreamType, false).getNick(); + PeerConnectionWrapper peerConnectionWrapper = getPeerConnectionWrapperForSessionIdAndType(session, + videoStreamType); + nick = peerConnectionWrapper != null ? peerConnectionWrapper.getNick() : ""; } String userId = "";