From 65ff4efcb9f20c50c495f47c4f9180896007f888 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 3 Nov 2022 22:01:43 +0100 Subject: [PATCH] Send signaling messages directly from PeerConnectionWrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note that the thread used to send the message does not change; the EventBus subscriber mode was "BACKGROUND", but as the messages were posted from a WebSocket handler (when requesting offers to the HPB) and peer connection observers (when sending offers/answers and candidates, both with and without HPB), which run in worker threads rather than in the main thread, the subscriber was executed in the same thread as the poster. For legacy reasons, when the internal signaling server is used the offers and answers are expected to also provide the nick of the local participant. When the external signaling server is used the field can be included, but it is just ignored and not sent to the other clients. As the local participant nick is a value unrelated to the peer connection and is only needed with one type of signaling server the messages are adjusted as needed before being sent rather than handling this inside the PeerConnectionWrapper. Signed-off-by: Daniel Calviño Sánchez --- .../talk/activities/CallActivity.java | 71 +++++----- .../events/SessionDescriptionSendEvent.java | 129 ------------------ .../talk/webrtc/PeerConnectionWrapper.java | 51 +++++-- 3 files changed, 74 insertions(+), 177 deletions(-) delete mode 100644 app/src/main/java/com/nextcloud/talk/events/SessionDescriptionSendEvent.java 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 cb0b400ae..b184425b5 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.java @@ -66,7 +66,6 @@ import com.nextcloud.talk.events.ConfigurationChangeEvent; import com.nextcloud.talk.events.MediaStreamEvent; import com.nextcloud.talk.events.NetworkEvent; import com.nextcloud.talk.events.PeerConnectionEvent; -import com.nextcloud.talk.events.SessionDescriptionSendEvent; import com.nextcloud.talk.events.WebSocketCommunicationEvent; import com.nextcloud.talk.models.ExternalSignalingServer; import com.nextcloud.talk.models.json.capabilities.CapabilitiesOverall; @@ -1620,18 +1619,6 @@ public class CallActivity extends CallBaseActivity { performCall(); } break; - case "peerReadyForRequestingOffer": - Log.d(TAG, "onMessageEvent 'peerReadyForRequestingOffer'"); - - NCSignalingMessage ncSignalingMessage = new NCSignalingMessage(); - // "to" property is not actually needed in the "requestoffer" signaling message, but it is used to - // set the recipient session ID in the assembled call message. - ncSignalingMessage.setTo(webSocketCommunicationEvent.getHashMap().get("sessionId")); - ncSignalingMessage.setRoomType("video"); - ncSignalingMessage.setType("requestoffer"); - - signalingMessageSender.send(ncSignalingMessage); - break; } } @@ -1967,7 +1954,8 @@ public class CallActivity extends CallBaseActivity { true, true, type, - signalingMessageReceiver); + signalingMessageReceiver, + signalingMessageSender); } else if (hasMCU) { peerConnectionWrapper = new PeerConnectionWrapper(peerConnectionFactory, @@ -1979,7 +1967,8 @@ public class CallActivity extends CallBaseActivity { false, true, type, - signalingMessageReceiver); + signalingMessageReceiver, + signalingMessageSender); } else { if (!"screen".equals(type)) { peerConnectionWrapper = new PeerConnectionWrapper(peerConnectionFactory, @@ -1991,7 +1980,8 @@ public class CallActivity extends CallBaseActivity { false, false, type, - signalingMessageReceiver); + signalingMessageReceiver, + signalingMessageSender); } else { peerConnectionWrapper = new PeerConnectionWrapper(peerConnectionFactory, iceServers, @@ -2002,7 +1992,8 @@ public class CallActivity extends CallBaseActivity { false, false, type, - signalingMessageReceiver); + signalingMessageReceiver, + signalingMessageSender); } } @@ -2246,27 +2237,6 @@ public class CallActivity extends CallBaseActivity { } } - @Subscribe(threadMode = ThreadMode.BACKGROUND) - public void onMessageEvent(SessionDescriptionSendEvent sessionDescriptionSend) { - NCSignalingMessage ncSignalingMessage = new NCSignalingMessage(); - ncSignalingMessage.setTo(sessionDescriptionSend.getPeerId()); - ncSignalingMessage.setRoomType(sessionDescriptionSend.getVideoStreamType()); - ncSignalingMessage.setType(sessionDescriptionSend.getType()); - NCMessagePayload ncMessagePayload = new NCMessagePayload(); - ncMessagePayload.setType(sessionDescriptionSend.getType()); - - if (!"candidate".equals(sessionDescriptionSend.getType())) { - ncMessagePayload.setSdp(sessionDescriptionSend.getSessionDescription().description); - ncMessagePayload.setNick(conversationUser.getDisplayName()); - } else { - ncMessagePayload.setIceCandidate(sessionDescriptionSend.getNcIceCandidate()); - } - - ncSignalingMessage.setPayload(ncMessagePayload); - - signalingMessageSender.send(ncSignalingMessage); - } - @Override public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { @@ -2607,6 +2577,8 @@ public class CallActivity extends CallBaseActivity { @Override public void send(NCSignalingMessage ncSignalingMessage) { + addLocalParticipantNickIfNeeded(ncSignalingMessage); + String serializedNcSignalingMessage; try { serializedNcSignalingMessage = LoganSquare.serialize(ncSignalingMessage); @@ -2663,6 +2635,29 @@ public class CallActivity extends CallBaseActivity { } }); } + + /** + * Adds the local participant nick to offers and answers. + * + * For legacy reasons the offers and answers sent when the internal signaling server is used are expected to + * provide the nick of the local participant. + * + * @param ncSignalingMessage the message to add the nick to + */ + private void addLocalParticipantNickIfNeeded(NCSignalingMessage ncSignalingMessage) { + String type = ncSignalingMessage.getType(); + if (!"offer".equals(type) && !"answer".equals(type)) { + return; + } + + NCMessagePayload payload = ncSignalingMessage.getPayload(); + if (payload == null) { + // Broken message, this should not happen + return; + } + + payload.setNick(conversationUser.getDisplayName()); + } } private class MicrophoneButtonTouchListener implements View.OnTouchListener { diff --git a/app/src/main/java/com/nextcloud/talk/events/SessionDescriptionSendEvent.java b/app/src/main/java/com/nextcloud/talk/events/SessionDescriptionSendEvent.java deleted file mode 100644 index f6fa43e71..000000000 --- a/app/src/main/java/com/nextcloud/talk/events/SessionDescriptionSendEvent.java +++ /dev/null @@ -1,129 +0,0 @@ -/* - * Nextcloud Talk application - * - * @author Mario Danic - * Copyright (C) 2017 Mario Danic - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -package com.nextcloud.talk.events; - -import com.nextcloud.talk.models.json.signaling.NCIceCandidate; - -import org.webrtc.SessionDescription; - -import androidx.annotation.Nullable; - -public class SessionDescriptionSendEvent { - @Nullable - private final SessionDescription sessionDescription; - private final String peerId; - private final String type; - @Nullable - private final NCIceCandidate ncIceCandidate; - private final String videoStreamType; - - public SessionDescriptionSendEvent(@Nullable SessionDescription sessionDescription, String peerId, String type, - @Nullable NCIceCandidate ncIceCandidate, @Nullable String videoStreamType) { - this.sessionDescription = sessionDescription; - this.peerId = peerId; - this.type = type; - this.ncIceCandidate = ncIceCandidate; - this.videoStreamType = videoStreamType; - } - - @Nullable - public SessionDescription getSessionDescription() { - return this.sessionDescription; - } - - public String getPeerId() { - return this.peerId; - } - - public String getType() { - return this.type; - } - - @Nullable - public NCIceCandidate getNcIceCandidate() { - return this.ncIceCandidate; - } - - public String getVideoStreamType() { - return this.videoStreamType; - } - - public boolean equals(final Object o) { - if (o == this) { - return true; - } - if (!(o instanceof SessionDescriptionSendEvent)) { - return false; - } - final SessionDescriptionSendEvent other = (SessionDescriptionSendEvent) o; - if (!other.canEqual((Object) this)) { - return false; - } - final Object this$sessionDescription = this.getSessionDescription(); - final Object other$sessionDescription = other.getSessionDescription(); - if (this$sessionDescription == null ? other$sessionDescription != null : !this$sessionDescription.equals(other$sessionDescription)) { - return false; - } - final Object this$peerId = this.getPeerId(); - final Object other$peerId = other.getPeerId(); - if (this$peerId == null ? other$peerId != null : !this$peerId.equals(other$peerId)) { - return false; - } - final Object this$type = this.getType(); - final Object other$type = other.getType(); - if (this$type == null ? other$type != null : !this$type.equals(other$type)) { - return false; - } - final Object this$ncIceCandidate = this.getNcIceCandidate(); - final Object other$ncIceCandidate = other.getNcIceCandidate(); - if (this$ncIceCandidate == null ? other$ncIceCandidate != null : !this$ncIceCandidate.equals(other$ncIceCandidate)) { - return false; - } - final Object this$videoStreamType = this.getVideoStreamType(); - final Object other$videoStreamType = other.getVideoStreamType(); - - return this$videoStreamType == null ? other$videoStreamType == null : this$videoStreamType.equals(other$videoStreamType); - } - - protected boolean canEqual(final Object other) { - return other instanceof SessionDescriptionSendEvent; - } - - public int hashCode() { - final int PRIME = 59; - int result = 1; - final Object $sessionDescription = this.getSessionDescription(); - result = result * PRIME + ($sessionDescription == null ? 43 : $sessionDescription.hashCode()); - final Object $peerId = this.getPeerId(); - result = result * PRIME + ($peerId == null ? 43 : $peerId.hashCode()); - final Object $type = this.getType(); - result = result * PRIME + ($type == null ? 43 : $type.hashCode()); - final Object $ncIceCandidate = this.getNcIceCandidate(); - result = result * PRIME + ($ncIceCandidate == null ? 43 : $ncIceCandidate.hashCode()); - final Object $videoStreamType = this.getVideoStreamType(); - result = result * PRIME + ($videoStreamType == null ? 43 : $videoStreamType.hashCode()); - return result; - } - - public String toString() { - return "SessionDescriptionSendEvent(sessionDescription=" + this.getSessionDescription() + ", peerId=" + this.getPeerId() + ", type=" + this.getType() + ", ncIceCandidate=" + this.getNcIceCandidate() + ", videoStreamType=" + this.getVideoStreamType() + ")"; - } -} diff --git a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java index d3e2cf40e..5deafad52 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -32,12 +32,13 @@ import com.nextcloud.talk.R; import com.nextcloud.talk.application.NextcloudTalkApplication; import com.nextcloud.talk.events.MediaStreamEvent; import com.nextcloud.talk.events.PeerConnectionEvent; -import com.nextcloud.talk.events.SessionDescriptionSendEvent; -import com.nextcloud.talk.events.WebSocketCommunicationEvent; import com.nextcloud.talk.models.json.signaling.DataChannelMessage; import com.nextcloud.talk.models.json.signaling.DataChannelMessageNick; import com.nextcloud.talk.models.json.signaling.NCIceCandidate; +import com.nextcloud.talk.models.json.signaling.NCMessagePayload; +import com.nextcloud.talk.models.json.signaling.NCSignalingMessage; import com.nextcloud.talk.signaling.SignalingMessageReceiver; +import com.nextcloud.talk.signaling.SignalingMessageSender; import org.greenrobot.eventbus.EventBus; import org.webrtc.AudioTrack; @@ -78,6 +79,8 @@ public class PeerConnectionWrapper { private final SignalingMessageReceiver signalingMessageReceiver; private final WebRtcMessageListener webRtcMessageListener = new WebRtcMessageListener(); + private final SignalingMessageSender signalingMessageSender; + private List iceCandidates = new ArrayList<>(); private PeerConnection peerConnection; private String sessionId; @@ -101,7 +104,8 @@ public class PeerConnectionWrapper { MediaConstraints mediaConstraints, String sessionId, String localSession, @Nullable MediaStream localStream, boolean isMCUPublisher, boolean hasMCU, String videoStreamType, - SignalingMessageReceiver signalingMessageReceiver) { + SignalingMessageReceiver signalingMessageReceiver, + SignalingMessageSender signalingMessageSender) { Objects.requireNonNull(NextcloudTalkApplication.Companion.getSharedApplication()).getComponentApplication().inject(this); @@ -122,6 +126,8 @@ public class PeerConnectionWrapper { this.signalingMessageReceiver = signalingMessageReceiver; this.signalingMessageReceiver.addListener(webRtcMessageListener, sessionId, videoStreamType); + this.signalingMessageSender = signalingMessageSender; + if (peerConnection != null) { if (this.localStream != null) { List localStreamIds = Collections.singletonList(this.localStream.getId()); @@ -143,9 +149,10 @@ public class PeerConnectionWrapper { } else if (hasMCU && this.videoStreamType.equals("video")) { // If the connection type is "screen" the client sharing the screen will send an // offer; offers should be requested only for videos. - HashMap hashMap = new HashMap<>(); - hashMap.put("sessionId", sessionId); - EventBus.getDefault().post(new WebSocketCommunicationEvent("peerReadyForRequestingOffer", hashMap)); + // "to" property is not actually needed in the "requestoffer" signaling message, but it is used to + // set the recipient session ID in the assembled call message. + NCSignalingMessage ncSignalingMessage = createBaseSignalingMessage("requestoffer"); + signalingMessageSender.send(ncSignalingMessage); } else if (!hasMCU && hasInitiated) { peerConnection.createOffer(magicSdpObserver, mediaConstraints); } @@ -269,6 +276,15 @@ public class PeerConnectionWrapper { return false; } + private NCSignalingMessage createBaseSignalingMessage(String type) { + NCSignalingMessage ncSignalingMessage = new NCSignalingMessage(); + ncSignalingMessage.setTo(sessionId); + ncSignalingMessage.setRoomType(videoStreamType); + ncSignalingMessage.setType(type); + + return ncSignalingMessage; + } + private class WebRtcMessageListener implements SignalingMessageReceiver.WebRtcMessageListener { public void onOffer(String sdp, String nick) { @@ -425,12 +441,19 @@ public class PeerConnectionWrapper { @Override public void onIceCandidate(IceCandidate iceCandidate) { + NCSignalingMessage ncSignalingMessage = createBaseSignalingMessage("candidate"); + NCMessagePayload ncMessagePayload = new NCMessagePayload(); + ncMessagePayload.setType("candidate"); + NCIceCandidate ncIceCandidate = new NCIceCandidate(); ncIceCandidate.setSdpMid(iceCandidate.sdpMid); ncIceCandidate.setSdpMLineIndex(iceCandidate.sdpMLineIndex); ncIceCandidate.setCandidate(iceCandidate.sdp); - EventBus.getDefault().post(new SessionDescriptionSendEvent(null, sessionId, - "candidate", ncIceCandidate, videoStreamType)); + ncMessagePayload.setIceCandidate(ncIceCandidate); + + ncSignalingMessage.setPayload(ncMessagePayload); + + signalingMessageSender.send(ncSignalingMessage); } @Override @@ -484,6 +507,12 @@ public class PeerConnectionWrapper { @Override public void onCreateSuccess(SessionDescription sessionDescription) { + String type = sessionDescription.type.canonicalForm(); + + NCSignalingMessage ncSignalingMessage = createBaseSignalingMessage(type); + NCMessagePayload ncMessagePayload = new NCMessagePayload(); + ncMessagePayload.setType(type); + SessionDescription sessionDescriptionWithPreferredCodec; String sessionDescriptionStringWithPreferredCodec = MagicWebRTCUtils.preferCodec (sessionDescription.description, @@ -492,9 +521,11 @@ public class PeerConnectionWrapper { sessionDescription.type, sessionDescriptionStringWithPreferredCodec); + ncMessagePayload.setSdp(sessionDescriptionWithPreferredCodec.description); - EventBus.getDefault().post(new SessionDescriptionSendEvent(sessionDescriptionWithPreferredCodec, sessionId, - sessionDescription.type.canonicalForm(), null, videoStreamType)); + ncSignalingMessage.setPayload(ncMessagePayload); + + signalingMessageSender.send(ncSignalingMessage); if (peerConnection != null) { peerConnection.setLocalDescription(magicSdpObserver, sessionDescriptionWithPreferredCodec);