From 416563760fc345ab306c3cac90ae9d0358bf05ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 5 Dec 2024 11:32:51 +0100 Subject: [PATCH 01/27] Remove Dagger related code from PeerConnectionWrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PeerConnectionWrapper does not need to be injected in the application, nor the Context needs to be injected in the PeerConnectionWrapper. This all seems to be leftovers from the past, and removing them would ease adding unit tests for the PeerConnectionWrapper. Signed-off-by: Daniel Calviño Sánchez --- .../talk/webrtc/PeerConnectionWrapper.java | 13 ------------- 1 file changed, 13 deletions(-) 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 fe44e797d8..b77950ec38 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -7,11 +7,9 @@ */ package com.nextcloud.talk.webrtc; -import android.content.Context; import android.util.Log; import com.bluelinelabs.logansquare.LoganSquare; -import com.nextcloud.talk.application.NextcloudTalkApplication; import com.nextcloud.talk.models.json.signaling.DataChannelMessage; import com.nextcloud.talk.models.json.signaling.NCIceCandidate; import com.nextcloud.talk.models.json.signaling.NCMessagePayload; @@ -38,19 +36,11 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Objects; - -import javax.inject.Inject; import androidx.annotation.Nullable; -import autodagger.AutoInjector; -@AutoInjector(NextcloudTalkApplication.class) public class PeerConnectionWrapper { - @Inject - Context context; - private static final String TAG = PeerConnectionWrapper.class.getCanonicalName(); private final SignalingMessageReceiver signalingMessageReceiver; @@ -117,9 +107,6 @@ public PeerConnectionWrapper(PeerConnectionFactory peerConnectionFactory, boolean isMCUPublisher, boolean hasMCU, String videoStreamType, SignalingMessageReceiver signalingMessageReceiver, SignalingMessageSender signalingMessageSender) { - - Objects.requireNonNull(NextcloudTalkApplication.Companion.getSharedApplication()).getComponentApplication().inject(this); - this.localStream = localStream; this.videoStreamType = videoStreamType; From acf0db136e6a7624419352d05c43d1287ff57cbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 5 Dec 2024 14:13:36 +0100 Subject: [PATCH 02/27] Add dummy Log implementation to be used in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Log methods are static, so they can not be mocked using Mockito. Although it might be possible to use PowerMockito a dummy implementation was added instead, as Log uses are widespread and it is not something worth mocking anyway. Signed-off-by: Daniel Calviño Sánchez --- app/src/test/java/android/util/Log.java | 51 +++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 app/src/test/java/android/util/Log.java diff --git a/app/src/test/java/android/util/Log.java b/app/src/test/java/android/util/Log.java new file mode 100644 index 0000000000..7090f73cd5 --- /dev/null +++ b/app/src/test/java/android/util/Log.java @@ -0,0 +1,51 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package android.util; + +/** + * Dummy implementation of android.util.Log to be used in unit tests. + *

+ * The Android Gradle plugin provides a library with the APIs of the Android framework that throws an exception if any + * of them are called. This class is loaded before that library and therefore becomes the implementation used during the + * tests, simply printing the messages to the system console. + */ +public class Log { + + public static int d(String tag, String msg) { + System.out.println("DEBUG: " + tag + ": " + msg); + + return 1; + } + + public static int e(String tag, String msg) { + System.out.println("ERROR: " + tag + ": " + msg); + + return 1; + } + + public static int i(String tag, String msg) { + System.out.println("INFO: " + tag + ": " + msg); + + return 1; + } + + public static boolean isLoggable(String tag, int level) { + return true; + } + + public static int v(String tag, String msg) { + System.out.println("VERBOSE: " + tag + ": " + msg); + + return 1; + } + + public static int w(String tag, String msg) { + System.out.println("WARN: " + tag + ": " + msg); + + return 1; + } +} From bc29b0e25816835ff07e0c96ef78562189142b6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 5 Dec 2024 14:34:06 +0100 Subject: [PATCH 03/27] Add unit tests for receiving data channel messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../talk/webrtc/PeerConnectionWrapper.java | 3 + .../talk/webrtc/PeerConnectionWrapperTest.kt | 194 ++++++++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt 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 b77950ec38..fdc211e743 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -71,6 +71,9 @@ public class PeerConnectionWrapper { /** * Listener for data channel messages. *

+ * Messages might have been received on any data channel, independently of its label or whether it was open by the + * local or the remote peer. + *

* The messages are bound to a specific peer connection, so each listener is expected to handle messages only for * a single peer connection. *

diff --git a/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt new file mode 100644 index 0000000000..98e2de4ae0 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt @@ -0,0 +1,194 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.webrtc + +import com.bluelinelabs.logansquare.LoganSquare +import com.nextcloud.talk.models.json.signaling.DataChannelMessage +import com.nextcloud.talk.signaling.SignalingMessageReceiver +import com.nextcloud.talk.signaling.SignalingMessageSender +import com.nextcloud.talk.webrtc.PeerConnectionWrapper.DataChannelMessageListener +import org.junit.Before +import org.junit.Test +import org.mockito.ArgumentCaptor +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.eq +import org.mockito.Mockito +import org.mockito.Mockito.doNothing +import org.webrtc.DataChannel +import org.webrtc.MediaConstraints +import org.webrtc.PeerConnection +import org.webrtc.PeerConnectionFactory +import java.nio.ByteBuffer +import java.util.HashMap + +class PeerConnectionWrapperTest { + + private var peerConnectionWrapper: PeerConnectionWrapper? = null + private var mockedPeerConnection: PeerConnection? = null + private var mockedPeerConnectionFactory: PeerConnectionFactory? = null + private var mockedSignalingMessageReceiver: SignalingMessageReceiver? = null + private var mockedSignalingMessageSender: SignalingMessageSender? = null + + private fun dataChannelMessageToBuffer(dataChannelMessage: DataChannelMessage): DataChannel.Buffer { + return DataChannel.Buffer( + ByteBuffer.wrap(LoganSquare.serialize(dataChannelMessage).toByteArray()), + false + ) + } + + @Before + fun setUp() { + mockedPeerConnection = Mockito.mock(PeerConnection::class.java) + mockedPeerConnectionFactory = Mockito.mock(PeerConnectionFactory::class.java) + mockedSignalingMessageReceiver = Mockito.mock(SignalingMessageReceiver::class.java) + mockedSignalingMessageSender = Mockito.mock(SignalingMessageSender::class.java) + } + + @Test + fun testReceiveDataChannelMessage() { + Mockito.`when`( + mockedPeerConnectionFactory!!.createPeerConnection( + any(PeerConnection.RTCConfiguration::class.java), + any(PeerConnection.Observer::class.java) + ) + ).thenReturn(mockedPeerConnection) + + val mockedStatusDataChannel = Mockito.mock(DataChannel::class.java) + Mockito.`when`(mockedStatusDataChannel.label()).thenReturn("status") + Mockito.`when`(mockedStatusDataChannel.state()).thenReturn(DataChannel.State.OPEN) + Mockito.`when`(mockedPeerConnection!!.createDataChannel(eq("status"), any())) + .thenReturn(mockedStatusDataChannel) + + val statusDataChannelObserverArgumentCaptor: ArgumentCaptor = + ArgumentCaptor.forClass(DataChannel.Observer::class.java) + + doNothing().`when`(mockedStatusDataChannel).registerObserver(statusDataChannelObserverArgumentCaptor.capture()) + + peerConnectionWrapper = PeerConnectionWrapper( + mockedPeerConnectionFactory, + ArrayList(), + MediaConstraints(), + "the-session-id", + "the-local-session-id", + null, + true, + true, + "video", + mockedSignalingMessageReceiver, + mockedSignalingMessageSender + ) + + val mockedDataChannelMessageListener = Mockito.mock(DataChannelMessageListener::class.java) + peerConnectionWrapper!!.addListener(mockedDataChannelMessageListener) + + // The payload must be a map to be able to serialize it and, therefore, generate the data that would have been + // received from another participant, so it is not possible to test receiving the nick as a String payload. + val payloadMap = HashMap() + payloadMap["name"] = "the-nick-in-map" + + statusDataChannelObserverArgumentCaptor.value.onMessage( + dataChannelMessageToBuffer(DataChannelMessage("nickChanged", null, payloadMap)) + ) + + Mockito.verify(mockedDataChannelMessageListener).onNickChanged("the-nick-in-map") + Mockito.verifyNoMoreInteractions(mockedDataChannelMessageListener) + + statusDataChannelObserverArgumentCaptor.value.onMessage( + dataChannelMessageToBuffer(DataChannelMessage("audioOn")) + ) + + Mockito.verify(mockedDataChannelMessageListener).onAudioOn() + Mockito.verifyNoMoreInteractions(mockedDataChannelMessageListener) + + statusDataChannelObserverArgumentCaptor.value.onMessage( + dataChannelMessageToBuffer(DataChannelMessage("audioOff")) + ) + + Mockito.verify(mockedDataChannelMessageListener).onAudioOff() + Mockito.verifyNoMoreInteractions(mockedDataChannelMessageListener) + + statusDataChannelObserverArgumentCaptor.value.onMessage( + dataChannelMessageToBuffer(DataChannelMessage("videoOn")) + ) + + Mockito.verify(mockedDataChannelMessageListener).onVideoOn() + Mockito.verifyNoMoreInteractions(mockedDataChannelMessageListener) + + statusDataChannelObserverArgumentCaptor.value.onMessage( + dataChannelMessageToBuffer(DataChannelMessage("videoOff")) + ) + + Mockito.verify(mockedDataChannelMessageListener).onVideoOff() + Mockito.verifyNoMoreInteractions(mockedDataChannelMessageListener) + } + + @Test + fun testReceiveDataChannelMessageWithOpenRemoteDataChannel() { + val peerConnectionObserverArgumentCaptor: ArgumentCaptor = + ArgumentCaptor.forClass(PeerConnection.Observer::class.java) + + Mockito.`when`( + mockedPeerConnectionFactory!!.createPeerConnection( + any(PeerConnection.RTCConfiguration::class.java), + peerConnectionObserverArgumentCaptor.capture() + ) + ).thenReturn(mockedPeerConnection) + + val mockedStatusDataChannel = Mockito.mock(DataChannel::class.java) + Mockito.`when`(mockedStatusDataChannel.label()).thenReturn("status") + Mockito.`when`(mockedStatusDataChannel.state()).thenReturn(DataChannel.State.OPEN) + Mockito.`when`(mockedPeerConnection!!.createDataChannel(eq("status"), any())) + .thenReturn(mockedStatusDataChannel) + + val statusDataChannelObserverArgumentCaptor: ArgumentCaptor = + ArgumentCaptor.forClass(DataChannel.Observer::class.java) + + doNothing().`when`(mockedStatusDataChannel).registerObserver(statusDataChannelObserverArgumentCaptor.capture()) + + peerConnectionWrapper = PeerConnectionWrapper( + mockedPeerConnectionFactory, + ArrayList(), + MediaConstraints(), + "the-session-id", + "the-local-session-id", + null, + true, + true, + "video", + mockedSignalingMessageReceiver, + mockedSignalingMessageSender + ) + + val randomIdDataChannelObserverArgumentCaptor: ArgumentCaptor = + ArgumentCaptor.forClass(DataChannel.Observer::class.java) + + val mockedRandomIdDataChannel = Mockito.mock(DataChannel::class.java) + Mockito.`when`(mockedRandomIdDataChannel.label()).thenReturn("random-id") + Mockito.`when`(mockedRandomIdDataChannel.state()).thenReturn(DataChannel.State.OPEN) + doNothing().`when`(mockedRandomIdDataChannel).registerObserver( + randomIdDataChannelObserverArgumentCaptor.capture() + ) + peerConnectionObserverArgumentCaptor.value.onDataChannel(mockedRandomIdDataChannel) + + val mockedDataChannelMessageListener = Mockito.mock(DataChannelMessageListener::class.java) + peerConnectionWrapper!!.addListener(mockedDataChannelMessageListener) + + statusDataChannelObserverArgumentCaptor.value.onMessage( + dataChannelMessageToBuffer(DataChannelMessage("audioOn")) + ) + + Mockito.verify(mockedDataChannelMessageListener).onAudioOn() + Mockito.verifyNoMoreInteractions(mockedDataChannelMessageListener) + + randomIdDataChannelObserverArgumentCaptor.value.onMessage( + dataChannelMessageToBuffer(DataChannelMessage("audioOff")) + ) + + Mockito.verify(mockedDataChannelMessageListener).onAudioOff() + Mockito.verifyNoMoreInteractions(mockedDataChannelMessageListener) + } +} From 1b4d80dad6568e9ca6b8d47fbfa8ef1106853e9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 5 Dec 2024 14:54:00 +0100 Subject: [PATCH 04/27] Unify log messages for received data channel messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 fdc211e743..34ea47eaa9 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -379,7 +379,7 @@ public void onStateChange() { @Override public void onMessage(DataChannel.Buffer buffer) { if (buffer.binary) { - Log.d(TAG, "Received binary msg over " + TAG + " " + sessionId); + Log.d(TAG, "Received binary data channel message over " + TAG + " " + sessionId); return; } @@ -387,7 +387,7 @@ public void onMessage(DataChannel.Buffer buffer) { final byte[] bytes = new byte[data.capacity()]; data.get(bytes); String strData = new String(bytes); - Log.d(TAG, "Got msg: " + strData + " over " + TAG + " " + sessionId); + Log.d(TAG, "Received data channel message (" + strData + ") over " + TAG + " " + sessionId); DataChannelMessage dataChannelMessage; try { From f2046629e44c05409dbfb9f6c9016a9fcf7ae068 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 5 Dec 2024 14:55:57 +0100 Subject: [PATCH 05/27] Include data channel label in log message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This implicitly fixes trying to send the initial state on the latest remote data channel found (which is the one stored in the "dataChannel" attribute of the "PeerConnectionWrapper") when any other existing data channel changes its status to open. Nevertheless, as all this will be reworked, no unit test was added for it. Signed-off-by: Daniel Calviño Sánchez --- .../talk/webrtc/PeerConnectionWrapper.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) 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 34ea47eaa9..2d033d08e6 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -36,6 +36,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import androidx.annotation.Nullable; @@ -144,7 +145,7 @@ public PeerConnectionWrapper(PeerConnectionFactory peerConnectionFactory, DataChannel.Init init = new DataChannel.Init(); init.negotiated = false; dataChannel = peerConnection.createDataChannel("status", init); - dataChannel.registerObserver(new DataChannelObserver()); + dataChannel.registerObserver(new DataChannelObserver(dataChannel)); if (isMCUPublisher) { peerConnection.createOffer(sdpObserver, mediaConstraints); } else if (hasMCU && "video".equals(this.videoStreamType)) { @@ -363,6 +364,12 @@ public void onEndOfCandidates() { private class DataChannelObserver implements DataChannel.Observer { + private final DataChannel dataChannel; + + public DataChannelObserver(DataChannel dataChannel) { + this.dataChannel = Objects.requireNonNull(dataChannel); + } + @Override public void onBufferedAmountChange(long l) { @@ -370,8 +377,7 @@ public void onBufferedAmountChange(long l) { @Override public void onStateChange() { - if (dataChannel != null && - dataChannel.state() == DataChannel.State.OPEN) { + if (dataChannel.state() == DataChannel.State.OPEN) { sendInitialMediaStatus(); } } @@ -379,7 +385,7 @@ public void onStateChange() { @Override public void onMessage(DataChannel.Buffer buffer) { if (buffer.binary) { - Log.d(TAG, "Received binary data channel message over " + TAG + " " + sessionId); + Log.d(TAG, "Received binary data channel message over " + dataChannel.label() + " " + sessionId); return; } @@ -387,7 +393,7 @@ public void onMessage(DataChannel.Buffer buffer) { final byte[] bytes = new byte[data.capacity()]; data.get(bytes); String strData = new String(bytes); - Log.d(TAG, "Received data channel message (" + strData + ") over " + TAG + " " + sessionId); + Log.d(TAG, "Received data channel message (" + strData + ") over " + dataChannel.label() + " " + sessionId); DataChannelMessage dataChannelMessage; try { @@ -512,7 +518,7 @@ public void onDataChannel(DataChannel dataChannel) { + " exists, but received onDataChannel event for DataChannel with label " + dataChannel.label()); } PeerConnectionWrapper.this.dataChannel = dataChannel; - PeerConnectionWrapper.this.dataChannel.registerObserver(new DataChannelObserver()); + PeerConnectionWrapper.this.dataChannel.registerObserver(new DataChannelObserver(dataChannel)); } @Override From e9daa6ec5fae88f6b9732cc6c3197389f02e579e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 6 Dec 2024 14:00:57 +0100 Subject: [PATCH 06/27] Rename "sendChannelData" to "send" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legacy name was a bit strange, so now it is renamed to just "send" as the parameter type ("DataChannelMessage") gives enough context. Signed-off-by: Daniel Calviño Sánchez --- .../java/com/nextcloud/talk/activities/CallActivity.kt | 10 +++++----- .../nextcloud/talk/webrtc/PeerConnectionWrapper.java | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt index 9220952c0d..3bcc7611b0 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt @@ -1174,12 +1174,12 @@ class CallActivity : CallBaseActivity() { if (isConnectionEstablished && othersInCall) { if (!hasMCU) { for (peerConnectionWrapper in peerConnectionWrapperList) { - peerConnectionWrapper.sendChannelData(DataChannelMessage(isSpeakingMessage)) + peerConnectionWrapper.send(DataChannelMessage(isSpeakingMessage)) } } else { for (peerConnectionWrapper in peerConnectionWrapperList) { if (peerConnectionWrapper.sessionId == webSocketClient!!.sessionId) { - peerConnectionWrapper.sendChannelData(DataChannelMessage(isSpeakingMessage)) + peerConnectionWrapper.send(DataChannelMessage(isSpeakingMessage)) break } } @@ -1370,12 +1370,12 @@ class CallActivity : CallBaseActivity() { if (isConnectionEstablished) { if (!hasMCU) { for (peerConnectionWrapper in peerConnectionWrapperList) { - peerConnectionWrapper.sendChannelData(DataChannelMessage(message)) + peerConnectionWrapper.send(DataChannelMessage(message)) } } else { for (peerConnectionWrapper in peerConnectionWrapperList) { if (peerConnectionWrapper.sessionId == webSocketClient!!.sessionId) { - peerConnectionWrapper.sendChannelData(DataChannelMessage(message)) + peerConnectionWrapper.send(DataChannelMessage(message)) break } } @@ -2563,7 +2563,7 @@ class CallActivity : CallBaseActivity() { } override fun onNext(aLong: Long) { - peerConnectionWrapper.sendChannelData(dataChannelMessage) + peerConnectionWrapper.send(dataChannelMessage) } override fun onError(e: Throwable) { 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 2d033d08e6..bf95befb82 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -269,7 +269,7 @@ private void addCandidate(IceCandidate iceCandidate) { } } - public void sendChannelData(DataChannelMessage dataChannelMessage) { + public void send(DataChannelMessage dataChannelMessage) { ByteBuffer buffer; if (dataChannel != null && dataChannelMessage != null) { try { @@ -292,15 +292,15 @@ public String getSessionId() { private void sendInitialMediaStatus() { if (localStream != null) { if (localStream.videoTracks.size() == 1 && localStream.videoTracks.get(0).enabled()) { - sendChannelData(new DataChannelMessage("videoOn")); + send(new DataChannelMessage("videoOn")); } else { - sendChannelData(new DataChannelMessage("videoOff")); + send(new DataChannelMessage("videoOff")); } if (localStream.audioTracks.size() == 1 && localStream.audioTracks.get(0).enabled()) { - sendChannelData(new DataChannelMessage("audioOn")); + send(new DataChannelMessage("audioOn")); } else { - sendChannelData(new DataChannelMessage("audioOff")); + send(new DataChannelMessage("audioOff")); } } } From 9384ef6ced95ad44bdb8d9032086f8fa6aef10a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 5 Dec 2024 17:42:34 +0100 Subject: [PATCH 07/27] Send data channel messages using "status" data channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Data channel messages are expected to be sent using the "status" data channel that is locally created. However, if another data channel was opened by the remote peer the reference to the "status" data channel was overwritten with the new data channel, and messages were sent instead on the remote data channel. In current Talk versions that was not a problem, and the change makes no difference either, because since the support for Janus 1.x was added data channel messages are listened on all data channels, independently of their label or whether they were created by the local or remote peer. However, in older Talk versions this fixes a regression introduced with the support for Janus 1.x. In those versions only messages coming from the "status" or "JanusDataChannel" data channels were taken into account. When Janus is not used the WebUI opens the legacy "simplewebrtc" data channel, so that data channel may be the one used to send data channel messages (if it is open after the "status" data channel), but the messages received on that data channel were ignored by the WebUI. Nevertheless, at this point this is more an academic problem than a real world problem, as it is unlikely that there are many Nextcloud servers with Talk < 16 and without HPB being used. Independently of all that, when the peer connection is removed only the "status" data channel is disposed, but none of the remote data channels are. This is just a variation of an already existing bug (the last open data channel was the one disposed due to being the last saved reference, but the rest were not) and it will be fixed in another commit. Signed-off-by: Daniel Calviño Sánchez --- .../talk/webrtc/PeerConnectionWrapper.java | 35 ++++--- .../talk/webrtc/PeerConnectionWrapperTest.kt | 97 +++++++++++++++++++ 2 files changed, 118 insertions(+), 14 deletions(-) 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 bf95befb82..6413f0344b 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -57,7 +57,7 @@ public class PeerConnectionWrapper { private PeerConnection peerConnection; private String sessionId; private final MediaConstraints mediaConstraints; - private DataChannel dataChannel; + private DataChannel statusDataChannel; private final SdpObserver sdpObserver; private final boolean hasInitiated; @@ -144,8 +144,8 @@ public PeerConnectionWrapper(PeerConnectionFactory peerConnectionFactory, if (hasMCU || hasInitiated) { DataChannel.Init init = new DataChannel.Init(); init.negotiated = false; - dataChannel = peerConnection.createDataChannel("status", init); - dataChannel.registerObserver(new DataChannelObserver(dataChannel)); + statusDataChannel = peerConnection.createDataChannel("status", init); + statusDataChannel.registerObserver(new DataChannelObserver(statusDataChannel)); if (isMCUPublisher) { peerConnection.createOffer(sdpObserver, mediaConstraints); } else if (hasMCU && "video".equals(this.videoStreamType)) { @@ -233,9 +233,9 @@ public MediaStream getStream() { public void removePeerConnection() { signalingMessageReceiver.removeListener(webRtcMessageListener); - if (dataChannel != null) { - dataChannel.dispose(); - dataChannel = null; + if (statusDataChannel != null) { + statusDataChannel.dispose(); + statusDataChannel = null; Log.d(TAG, "Disposed DataChannel"); } else { Log.d(TAG, "DataChannel is null."); @@ -269,12 +269,24 @@ private void addCandidate(IceCandidate iceCandidate) { } } + /** + * Sends a data channel message. + *

+ * Data channel messages are always sent on the "status" data channel locally opened. However, if Janus is used, + * messages can be sent only on publisher connections, even if subscriber connections have a "status" data channel; + * messages sent on subscriber connections will be simply ignored. Moreover, even if the message is sent on the + * "status" data channel subscriber connections will receive it on a data channel with a different label, as + * Janus opens its own data channel on subscriber connections and "multiplexes" all the received data channel + * messages on it, independently of on which data channel they were originally sent. + * + * @param dataChannelMessage the message to send + */ public void send(DataChannelMessage dataChannelMessage) { ByteBuffer buffer; - if (dataChannel != null && dataChannelMessage != null) { + if (statusDataChannel != null && dataChannelMessage != null) { try { buffer = ByteBuffer.wrap(LoganSquare.serialize(dataChannelMessage).getBytes()); - dataChannel.send(new DataChannel.Buffer(buffer, false)); + statusDataChannel.send(new DataChannel.Buffer(buffer, false)); } catch (Exception e) { Log.d(TAG, "Failed to send channel data, attempting regular " + dataChannelMessage); } @@ -513,12 +525,7 @@ public void onRemoveStream(MediaStream mediaStream) { @Override public void onDataChannel(DataChannel dataChannel) { - if (PeerConnectionWrapper.this.dataChannel != null) { - Log.w(TAG, "Data channel with label " + PeerConnectionWrapper.this.dataChannel.label() - + " exists, but received onDataChannel event for DataChannel with label " + dataChannel.label()); - } - PeerConnectionWrapper.this.dataChannel = dataChannel; - PeerConnectionWrapper.this.dataChannel.registerObserver(new DataChannelObserver(dataChannel)); + dataChannel.registerObserver(new DataChannelObserver(dataChannel)); } @Override diff --git a/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt index 98e2de4ae0..be628c2afd 100644 --- a/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt +++ b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt @@ -14,10 +14,13 @@ import com.nextcloud.talk.webrtc.PeerConnectionWrapper.DataChannelMessageListene import org.junit.Before import org.junit.Test import org.mockito.ArgumentCaptor +import org.mockito.ArgumentMatcher import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.argThat import org.mockito.ArgumentMatchers.eq import org.mockito.Mockito import org.mockito.Mockito.doNothing +import org.mockito.Mockito.never import org.webrtc.DataChannel import org.webrtc.MediaConstraints import org.webrtc.PeerConnection @@ -33,6 +36,19 @@ class PeerConnectionWrapperTest { private var mockedSignalingMessageReceiver: SignalingMessageReceiver? = null private var mockedSignalingMessageSender: SignalingMessageSender? = null + /** + * Helper matcher for DataChannelMessages. + */ + private inner class MatchesDataChannelMessage( + private val expectedDataChannelMessage: DataChannelMessage + ) : ArgumentMatcher { + override fun matches(buffer: DataChannel.Buffer): Boolean { + // DataChannel.Buffer does not implement "equals", so the comparison needs to be done on the ByteBuffer + // instead. + return dataChannelMessageToBuffer(expectedDataChannelMessage).data.equals(buffer.data) + } + } + private fun dataChannelMessageToBuffer(dataChannelMessage: DataChannelMessage): DataChannel.Buffer { return DataChannel.Buffer( ByteBuffer.wrap(LoganSquare.serialize(dataChannelMessage).toByteArray()), @@ -48,6 +64,87 @@ class PeerConnectionWrapperTest { mockedSignalingMessageSender = Mockito.mock(SignalingMessageSender::class.java) } + @Test + fun testSendDataChannelMessage() { + Mockito.`when`( + mockedPeerConnectionFactory!!.createPeerConnection( + any(PeerConnection.RTCConfiguration::class.java), + any(PeerConnection.Observer::class.java) + ) + ).thenReturn(mockedPeerConnection) + + val mockedStatusDataChannel = Mockito.mock(DataChannel::class.java) + Mockito.`when`(mockedStatusDataChannel.label()).thenReturn("status") + Mockito.`when`(mockedStatusDataChannel.state()).thenReturn(DataChannel.State.OPEN) + Mockito.`when`(mockedPeerConnection!!.createDataChannel(eq("status"), any())) + .thenReturn(mockedStatusDataChannel) + + peerConnectionWrapper = PeerConnectionWrapper( + mockedPeerConnectionFactory, + ArrayList(), + MediaConstraints(), + "the-session-id", + "the-local-session-id", + null, + true, + true, + "video", + mockedSignalingMessageReceiver, + mockedSignalingMessageSender + ) + + peerConnectionWrapper!!.send(DataChannelMessage("the-message-type")) + + Mockito.verify(mockedStatusDataChannel).send( + argThat(MatchesDataChannelMessage(DataChannelMessage("the-message-type"))) + ) + } + + @Test + fun testSendDataChannelMessageWithOpenRemoteDataChannel() { + val peerConnectionObserverArgumentCaptor: ArgumentCaptor = + ArgumentCaptor.forClass(PeerConnection.Observer::class.java) + + Mockito.`when`( + mockedPeerConnectionFactory!!.createPeerConnection( + any(PeerConnection.RTCConfiguration::class.java), + peerConnectionObserverArgumentCaptor.capture() + ) + ).thenReturn(mockedPeerConnection) + + val mockedStatusDataChannel = Mockito.mock(DataChannel::class.java) + Mockito.`when`(mockedStatusDataChannel.label()).thenReturn("status") + Mockito.`when`(mockedStatusDataChannel.state()).thenReturn(DataChannel.State.OPEN) + Mockito.`when`(mockedPeerConnection!!.createDataChannel(eq("status"), any())) + .thenReturn(mockedStatusDataChannel) + + peerConnectionWrapper = PeerConnectionWrapper( + mockedPeerConnectionFactory, + ArrayList(), + MediaConstraints(), + "the-session-id", + "the-local-session-id", + null, + true, + true, + "video", + mockedSignalingMessageReceiver, + mockedSignalingMessageSender + ) + + val mockedRandomIdDataChannel = Mockito.mock(DataChannel::class.java) + Mockito.`when`(mockedRandomIdDataChannel.label()).thenReturn("random-id") + Mockito.`when`(mockedRandomIdDataChannel.state()).thenReturn(DataChannel.State.OPEN) + peerConnectionObserverArgumentCaptor.value.onDataChannel(mockedRandomIdDataChannel) + + peerConnectionWrapper!!.send(DataChannelMessage("the-message-type")) + + Mockito.verify(mockedStatusDataChannel).send( + argThat(MatchesDataChannelMessage(DataChannelMessage("the-message-type"))) + ) + Mockito.verify(mockedRandomIdDataChannel, never()).send(any()) + } + @Test fun testReceiveDataChannelMessage() { Mockito.`when`( From 1ede0689df34c9e874d2f0dd2a9ad9f60a983115 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 6 Dec 2024 03:08:15 +0100 Subject: [PATCH 08/27] Fix remote data channels not disposed when removing peer connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../talk/webrtc/PeerConnectionWrapper.java | 36 ++++++++++++---- .../talk/webrtc/PeerConnectionWrapperTest.kt | 43 +++++++++++++++++++ 2 files changed, 71 insertions(+), 8 deletions(-) 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 6413f0344b..27ad9e439a 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -34,6 +34,7 @@ import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -57,7 +58,7 @@ public class PeerConnectionWrapper { private PeerConnection peerConnection; private String sessionId; private final MediaConstraints mediaConstraints; - private DataChannel statusDataChannel; + private final Map dataChannels = new HashMap<>(); private final SdpObserver sdpObserver; private final boolean hasInitiated; @@ -144,8 +145,11 @@ public PeerConnectionWrapper(PeerConnectionFactory peerConnectionFactory, if (hasMCU || hasInitiated) { DataChannel.Init init = new DataChannel.Init(); init.negotiated = false; - statusDataChannel = peerConnection.createDataChannel("status", init); + + DataChannel statusDataChannel = peerConnection.createDataChannel("status", init); statusDataChannel.registerObserver(new DataChannelObserver(statusDataChannel)); + dataChannels.put("status", statusDataChannel); + if (isMCUPublisher) { peerConnection.createOffer(sdpObserver, mediaConstraints); } else if (hasMCU && "video".equals(this.videoStreamType)) { @@ -233,13 +237,12 @@ public MediaStream getStream() { public void removePeerConnection() { signalingMessageReceiver.removeListener(webRtcMessageListener); - if (statusDataChannel != null) { - statusDataChannel.dispose(); - statusDataChannel = null; - Log.d(TAG, "Disposed DataChannel"); - } else { - Log.d(TAG, "DataChannel is null."); + for (DataChannel dataChannel: dataChannels.values()) { + Log.d(TAG, "Disposed DataChannel " + dataChannel.label()); + + dataChannel.dispose(); } + dataChannels.clear(); if (peerConnection != null) { peerConnection.close(); @@ -283,6 +286,7 @@ private void addCandidate(IceCandidate iceCandidate) { */ public void send(DataChannelMessage dataChannelMessage) { ByteBuffer buffer; + DataChannel statusDataChannel = dataChannels.get("status"); if (statusDataChannel != null && dataChannelMessage != null) { try { buffer = ByteBuffer.wrap(LoganSquare.serialize(dataChannelMessage).getBytes()); @@ -525,7 +529,23 @@ public void onRemoveStream(MediaStream mediaStream) { @Override public void onDataChannel(DataChannel dataChannel) { + // Another data channel with the same label, no matter if the same instance or a different one, should not + // be added, but just in case. + DataChannel oldDataChannel = dataChannels.get(dataChannel.label()); + if (oldDataChannel == dataChannel) { + Log.w(TAG, "Data channel with label " + dataChannel.label() + " added again"); + + return; + } + + if (oldDataChannel != null) { + Log.w(TAG, "Data channel with label " + dataChannel.label() + " exists"); + + oldDataChannel.dispose(); + } + dataChannel.registerObserver(new DataChannelObserver(dataChannel)); + dataChannels.put(dataChannel.label(), dataChannel); } @Override diff --git a/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt index be628c2afd..450cc4fa83 100644 --- a/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt +++ b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt @@ -288,4 +288,47 @@ class PeerConnectionWrapperTest { Mockito.verify(mockedDataChannelMessageListener).onAudioOff() Mockito.verifyNoMoreInteractions(mockedDataChannelMessageListener) } + + @Test + fun testRemovePeerConnectionWithOpenRemoteDataChannel() { + val peerConnectionObserverArgumentCaptor: ArgumentCaptor = + ArgumentCaptor.forClass(PeerConnection.Observer::class.java) + + Mockito.`when`( + mockedPeerConnectionFactory!!.createPeerConnection( + any(PeerConnection.RTCConfiguration::class.java), + peerConnectionObserverArgumentCaptor.capture() + ) + ).thenReturn(mockedPeerConnection) + + val mockedStatusDataChannel = Mockito.mock(DataChannel::class.java) + Mockito.`when`(mockedStatusDataChannel.label()).thenReturn("status") + Mockito.`when`(mockedStatusDataChannel.state()).thenReturn(DataChannel.State.OPEN) + Mockito.`when`(mockedPeerConnection!!.createDataChannel(eq("status"), any())) + .thenReturn(mockedStatusDataChannel) + + peerConnectionWrapper = PeerConnectionWrapper( + mockedPeerConnectionFactory, + ArrayList(), + MediaConstraints(), + "the-session-id", + "the-local-session-id", + null, + true, + true, + "video", + mockedSignalingMessageReceiver, + mockedSignalingMessageSender + ) + + val mockedRandomIdDataChannel = Mockito.mock(DataChannel::class.java) + Mockito.`when`(mockedRandomIdDataChannel.label()).thenReturn("random-id") + Mockito.`when`(mockedRandomIdDataChannel.state()).thenReturn(DataChannel.State.OPEN) + peerConnectionObserverArgumentCaptor.value.onDataChannel(mockedRandomIdDataChannel) + + peerConnectionWrapper!!.removePeerConnection() + + Mockito.verify(mockedStatusDataChannel).dispose() + Mockito.verify(mockedRandomIdDataChannel).dispose() + } } From 2f051033ee47e08a459a011e09ef9506f1354eb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 6 Dec 2024 04:13:49 +0100 Subject: [PATCH 09/27] Move variable declaration into try block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 27ad9e439a..08c6b8fcf4 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -285,11 +285,10 @@ private void addCandidate(IceCandidate iceCandidate) { * @param dataChannelMessage the message to send */ public void send(DataChannelMessage dataChannelMessage) { - ByteBuffer buffer; DataChannel statusDataChannel = dataChannels.get("status"); if (statusDataChannel != null && dataChannelMessage != null) { try { - buffer = ByteBuffer.wrap(LoganSquare.serialize(dataChannelMessage).getBytes()); + ByteBuffer buffer = ByteBuffer.wrap(LoganSquare.serialize(dataChannelMessage).getBytes()); statusDataChannel.send(new DataChannel.Buffer(buffer, false)); } catch (Exception e) { Log.d(TAG, "Failed to send channel data, attempting regular " + dataChannelMessage); From 0223d6700d6ea39b3380bcf9c11b0f9e16e5fdde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 6 Dec 2024 04:14:58 +0100 Subject: [PATCH 10/27] Rewrite method to return early MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../talk/webrtc/PeerConnectionWrapper.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) 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 08c6b8fcf4..9474471fea 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -286,13 +286,15 @@ private void addCandidate(IceCandidate iceCandidate) { */ public void send(DataChannelMessage dataChannelMessage) { DataChannel statusDataChannel = dataChannels.get("status"); - if (statusDataChannel != null && dataChannelMessage != null) { - try { - ByteBuffer buffer = ByteBuffer.wrap(LoganSquare.serialize(dataChannelMessage).getBytes()); - statusDataChannel.send(new DataChannel.Buffer(buffer, false)); - } catch (Exception e) { - Log.d(TAG, "Failed to send channel data, attempting regular " + dataChannelMessage); - } + if (statusDataChannel == null || dataChannelMessage == null) { + return; + } + + try { + ByteBuffer buffer = ByteBuffer.wrap(LoganSquare.serialize(dataChannelMessage).getBytes()); + statusDataChannel.send(new DataChannel.Buffer(buffer, false)); + } catch (Exception e) { + Log.d(TAG, "Failed to send channel data, attempting regular " + dataChannelMessage); } } From 3ebcd261dc320d4ac89b61910d007e246ea9a3bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 6 Dec 2024 04:15:46 +0100 Subject: [PATCH 11/27] Split condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../com/nextcloud/talk/webrtc/PeerConnectionWrapper.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 9474471fea..26f09d31e7 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -285,8 +285,12 @@ private void addCandidate(IceCandidate iceCandidate) { * @param dataChannelMessage the message to send */ public void send(DataChannelMessage dataChannelMessage) { + if (dataChannelMessage == null) { + return; + } + DataChannel statusDataChannel = dataChannels.get("status"); - if (statusDataChannel == null || dataChannelMessage == null) { + if (statusDataChannel == null) { return; } From c77b38598ab5c7247affdb88edefaf8c7ebfd546 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 6 Dec 2024 04:29:22 +0100 Subject: [PATCH 12/27] Queue data channel messages sent when data channel is not open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Data channel messages can be sent only when the data channel is open. Otherwise the message is simply lost. Clients of the PeerConnectionWrapper do not need to be aware of that detail or keep track of whether the data channel was open already or not, so now data channel messages sent before the data channel is open are queued and sent once the data channel is opened. Signed-off-by: Daniel Calviño Sánchez --- .../talk/webrtc/PeerConnectionWrapper.java | 19 ++++++- .../talk/webrtc/PeerConnectionWrapperTest.kt | 50 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) 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 26f09d31e7..be55a68635 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -59,6 +59,7 @@ public class PeerConnectionWrapper { private String sessionId; private final MediaConstraints mediaConstraints; private final Map dataChannels = new HashMap<>(); + private final List pendingDataChannelMessages = new ArrayList<>(); private final SdpObserver sdpObserver; private final boolean hasInitiated; @@ -281,6 +282,13 @@ private void addCandidate(IceCandidate iceCandidate) { * "status" data channel subscriber connections will receive it on a data channel with a different label, as * Janus opens its own data channel on subscriber connections and "multiplexes" all the received data channel * messages on it, independently of on which data channel they were originally sent. + *

+ * Data channel messages can be sent at any time; if the "status" data channel is not open yet the messages will be + * queued and sent once it is opened. Nevertheless, if Janus is used, it is not guaranteed that the messages will + * be received by other participants, as it is only known when the data channel of the publisher was opened, but + * not if the data channel of the subscribers was. However, in general this should be a concern only during the + * first seconds after a participant joins; after some time the subscriber connections should be established and + * their data channels open. * * @param dataChannelMessage the message to send */ @@ -290,7 +298,9 @@ public void send(DataChannelMessage dataChannelMessage) { } DataChannel statusDataChannel = dataChannels.get("status"); - if (statusDataChannel == null) { + if (statusDataChannel == null || statusDataChannel.state() != DataChannel.State.OPEN) { + pendingDataChannelMessages.add(dataChannelMessage); + return; } @@ -398,6 +408,13 @@ public void onBufferedAmountChange(long l) { @Override public void onStateChange() { + if (dataChannel.state() == DataChannel.State.OPEN && "status".equals(dataChannel.label())) { + for (DataChannelMessage dataChannelMessage: pendingDataChannelMessages) { + send(dataChannelMessage); + } + pendingDataChannelMessages.clear(); + } + if (dataChannel.state() == DataChannel.State.OPEN) { sendInitialMediaStatus(); } diff --git a/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt index 450cc4fa83..61c5f5daa4 100644 --- a/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt +++ b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt @@ -145,6 +145,56 @@ class PeerConnectionWrapperTest { Mockito.verify(mockedRandomIdDataChannel, never()).send(any()) } + @Test + fun testSendDataChannelMessageBeforeOpeningDataChannel() { + Mockito.`when`( + mockedPeerConnectionFactory!!.createPeerConnection( + any(PeerConnection.RTCConfiguration::class.java), + any(PeerConnection.Observer::class.java) + ) + ).thenReturn(mockedPeerConnection) + + val mockedStatusDataChannel = Mockito.mock(DataChannel::class.java) + Mockito.`when`(mockedStatusDataChannel.label()).thenReturn("status") + Mockito.`when`(mockedStatusDataChannel.state()).thenReturn(DataChannel.State.CONNECTING) + Mockito.`when`(mockedPeerConnection!!.createDataChannel(eq("status"), any())) + .thenReturn(mockedStatusDataChannel) + + val statusDataChannelObserverArgumentCaptor: ArgumentCaptor = + ArgumentCaptor.forClass(DataChannel.Observer::class.java) + + doNothing().`when`(mockedStatusDataChannel).registerObserver(statusDataChannelObserverArgumentCaptor.capture()) + + peerConnectionWrapper = PeerConnectionWrapper( + mockedPeerConnectionFactory, + ArrayList(), + MediaConstraints(), + "the-session-id", + "the-local-session-id", + null, + true, + true, + "video", + mockedSignalingMessageReceiver, + mockedSignalingMessageSender + ) + + peerConnectionWrapper!!.send(DataChannelMessage("the-message-type")) + peerConnectionWrapper!!.send(DataChannelMessage("another-message-type")) + + Mockito.verify(mockedStatusDataChannel, never()).send(any()) + + Mockito.`when`(mockedStatusDataChannel.state()).thenReturn(DataChannel.State.OPEN) + statusDataChannelObserverArgumentCaptor.value.onStateChange() + + Mockito.verify(mockedStatusDataChannel).send( + argThat(MatchesDataChannelMessage(DataChannelMessage("the-message-type"))) + ) + Mockito.verify(mockedStatusDataChannel).send( + argThat(MatchesDataChannelMessage(DataChannelMessage("another-message-type"))) + ) + } + @Test fun testReceiveDataChannelMessage() { Mockito.`when`( From fbb7a71fb0633ae97c0dcf9ee8bad431b71ecac7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 6 Dec 2024 04:37:13 +0100 Subject: [PATCH 13/27] Add logs for sending data channel messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../com/nextcloud/talk/webrtc/PeerConnectionWrapper.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 be55a68635..7bc1f04695 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -299,16 +299,20 @@ public void send(DataChannelMessage dataChannelMessage) { DataChannel statusDataChannel = dataChannels.get("status"); if (statusDataChannel == null || statusDataChannel.state() != DataChannel.State.OPEN) { + Log.d(TAG, "Queuing data channel message (" + dataChannelMessage + ") " + sessionId); + pendingDataChannelMessages.add(dataChannelMessage); return; } try { + Log.d(TAG, "Sending data channel message (" + dataChannelMessage + ") " + sessionId); + ByteBuffer buffer = ByteBuffer.wrap(LoganSquare.serialize(dataChannelMessage).getBytes()); statusDataChannel.send(new DataChannel.Buffer(buffer, false)); } catch (Exception e) { - Log.d(TAG, "Failed to send channel data, attempting regular " + dataChannelMessage); + Log.w(TAG, "Failed to send data channel message"); } } From b7a9bf7033341955bbb276653ed4d25f25748515 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 9 Dec 2024 03:11:54 +0100 Subject: [PATCH 14/27] Store data channel label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Getting the label is no longer possible once the data channel has been disposed. This will help to make the observer thread-safe. Signed-off-by: Daniel Calviño Sánchez --- .../com/nextcloud/talk/webrtc/PeerConnectionWrapper.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 7bc1f04695..08da1f7267 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -400,9 +400,11 @@ public void onEndOfCandidates() { private class DataChannelObserver implements DataChannel.Observer { private final DataChannel dataChannel; + private final String dataChannelLabel; public DataChannelObserver(DataChannel dataChannel) { this.dataChannel = Objects.requireNonNull(dataChannel); + this.dataChannelLabel = dataChannel.label(); } @Override @@ -412,7 +414,7 @@ public void onBufferedAmountChange(long l) { @Override public void onStateChange() { - if (dataChannel.state() == DataChannel.State.OPEN && "status".equals(dataChannel.label())) { + if (dataChannel.state() == DataChannel.State.OPEN && "status".equals(dataChannelLabel)) { for (DataChannelMessage dataChannelMessage: pendingDataChannelMessages) { send(dataChannelMessage); } @@ -427,7 +429,7 @@ public void onStateChange() { @Override public void onMessage(DataChannel.Buffer buffer) { if (buffer.binary) { - Log.d(TAG, "Received binary data channel message over " + dataChannel.label() + " " + sessionId); + Log.d(TAG, "Received binary data channel message over " + dataChannelLabel + " " + sessionId); return; } @@ -435,7 +437,7 @@ public void onMessage(DataChannel.Buffer buffer) { final byte[] bytes = new byte[data.capacity()]; data.get(bytes); String strData = new String(bytes); - Log.d(TAG, "Received data channel message (" + strData + ") over " + dataChannel.label() + " " + sessionId); + Log.d(TAG, "Received data channel message (" + strData + ") over " + dataChannelLabel + " " + sessionId); DataChannelMessage dataChannelMessage; try { From f410b4658c823bc094478eaf84a107545147a411 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 8 Dec 2024 05:23:11 +0100 Subject: [PATCH 15/27] Fix "removePeerConnection" not being thread-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding and disposing remote data channels is done from different threads; they are added from the WebRTC signaling thread when "onDataChannel" is called, while they can be disposed potentially from any thread when "removePeerConnection" is called. To prevent race conditions between them now both operations are synchronized. However, as "onDataChannel" belongs to an inner class it needs to use a synchronized statement with the outer class lock. This could still cause a race condition if the same data channel was added again; this should not happen, but it is handled just in case. Moreover, once a data channel is disposed it can be no longer used, and trying to call any of its methods throws an "IllegalStateException". Due to this, as sending can be also done potentially from any thread, it needs to be synchronized too with removing the peer connection. State changes on data channels as well as receiving messages are also done in the WebRTC signaling thread. State changes needs synchronization as well, although receiving messages should not, as it does not directly use the data channel (and it is assumed that using the buffers of a disposed data channel is safe). Nevertheless a little check (which in this case requires synchronization) was added to ignore the received messages if the peer connection was removed already. Finally, the synchronization added to "send" and "onStateChange" had the nice side effect of making the pending data channel messages thread-safe too, as before it could happen that a message was enqueued when the pending messages were being sent, which caused a "ConcurrentModificationException". Signed-off-by: Daniel Calviño Sánchez --- .../talk/webrtc/PeerConnectionWrapper.java | 84 +++- .../talk/webrtc/PeerConnectionWrapperTest.kt | 376 ++++++++++++++++++ 2 files changed, 438 insertions(+), 22 deletions(-) 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 08da1f7267..13bb5f2f7b 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -235,7 +235,7 @@ public MediaStream getStream() { return stream; } - public void removePeerConnection() { + public synchronized void removePeerConnection() { signalingMessageReceiver.removeListener(webRtcMessageListener); for (DataChannel dataChannel: dataChannels.values()) { @@ -292,7 +292,7 @@ private void addCandidate(IceCandidate iceCandidate) { * * @param dataChannelMessage the message to send */ - public void send(DataChannelMessage dataChannelMessage) { + public synchronized void send(DataChannelMessage dataChannelMessage) { if (dataChannelMessage == null) { return; } @@ -414,20 +414,38 @@ public void onBufferedAmountChange(long l) { @Override public void onStateChange() { - if (dataChannel.state() == DataChannel.State.OPEN && "status".equals(dataChannelLabel)) { - for (DataChannelMessage dataChannelMessage: pendingDataChannelMessages) { - send(dataChannelMessage); + synchronized (PeerConnectionWrapper.this) { + // The PeerConnection could have been removed in parallel even with the synchronization (as just after + // "onStateChange" was called "removePeerConnection" could have acquired the lock). + if (peerConnection == null) { + return; } - pendingDataChannelMessages.clear(); - } - if (dataChannel.state() == DataChannel.State.OPEN) { - sendInitialMediaStatus(); + if (dataChannel.state() == DataChannel.State.OPEN && "status".equals(dataChannelLabel)) { + for (DataChannelMessage dataChannelMessage : pendingDataChannelMessages) { + send(dataChannelMessage); + } + pendingDataChannelMessages.clear(); + } + + if (dataChannel.state() == DataChannel.State.OPEN) { + sendInitialMediaStatus(); + } } } @Override public void onMessage(DataChannel.Buffer buffer) { + synchronized (PeerConnectionWrapper.this) { + // It is assumed that, even if its data channel was disposed, its buffers can be used while there is + // a reference to them, so it would not be necessary to check this from a thread-safety point of view. + // Nevertheless, if the remote peer connection was removed it would not make sense to notify the + // listeners anyway. + if (peerConnection == null) { + return; + } + } + if (buffer.binary) { Log.d(TAG, "Received binary data channel message over " + dataChannelLabel + " " + sessionId); return; @@ -557,23 +575,45 @@ public void onRemoveStream(MediaStream mediaStream) { @Override public void onDataChannel(DataChannel dataChannel) { - // Another data channel with the same label, no matter if the same instance or a different one, should not - // be added, but just in case. - DataChannel oldDataChannel = dataChannels.get(dataChannel.label()); - if (oldDataChannel == dataChannel) { - Log.w(TAG, "Data channel with label " + dataChannel.label() + " added again"); + synchronized (PeerConnectionWrapper.this) { + // Another data channel with the same label, no matter if the same instance or a different one, should + // not be added, but this is handled just in case. + // Moreover, if it were possible that an already added data channel was added again there would be a + // potential race condition with "removePeerConnection", even with the synchronization, as it would + // be possible that "onDataChannel" was called, then "removePeerConnection" disposed the data + // channel, and then "onDataChannel" continued in the synchronized statements and tried to get the + // label, which would throw an exception due to the data channel having been disposed already. + String dataChannelLabel; + try { + dataChannelLabel = dataChannel.label(); + } catch (IllegalStateException e) { + // The data channel was disposed already, nothing to do. + return; + } - return; - } + DataChannel oldDataChannel = dataChannels.get(dataChannelLabel); + if (oldDataChannel == dataChannel) { + Log.w(TAG, "Data channel with label " + dataChannel.label() + " added again"); - if (oldDataChannel != null) { - Log.w(TAG, "Data channel with label " + dataChannel.label() + " exists"); + return; + } - oldDataChannel.dispose(); - } + if (oldDataChannel != null) { + Log.w(TAG, "Data channel with label " + dataChannel.label() + " exists"); + + oldDataChannel.dispose(); + } + + // If the peer connection was removed in parallel dispose the data channel instead of adding it. + if (peerConnection == null) { + dataChannel.dispose(); - dataChannel.registerObserver(new DataChannelObserver(dataChannel)); - dataChannels.put(dataChannel.label(), dataChannel); + return; + } + + dataChannel.registerObserver(new DataChannelObserver(dataChannel)); + dataChannels.put(dataChannel.label(), dataChannel); + } } @Override diff --git a/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt index 61c5f5daa4..f4f7e6443f 100644 --- a/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt +++ b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt @@ -19,15 +19,22 @@ import org.mockito.ArgumentMatchers.any import org.mockito.ArgumentMatchers.argThat import org.mockito.ArgumentMatchers.eq import org.mockito.Mockito +import org.mockito.Mockito.atLeast +import org.mockito.Mockito.atMostOnce +import org.mockito.Mockito.doAnswer import org.mockito.Mockito.doNothing import org.mockito.Mockito.never +import org.mockito.invocation.InvocationOnMock +import org.mockito.stubbing.Answer import org.webrtc.DataChannel import org.webrtc.MediaConstraints import org.webrtc.PeerConnection import org.webrtc.PeerConnectionFactory import java.nio.ByteBuffer import java.util.HashMap +import kotlin.concurrent.thread +@Suppress("LongMethod", "TooGenericExceptionCaught") class PeerConnectionWrapperTest { private var peerConnectionWrapper: PeerConnectionWrapper? = null @@ -36,6 +43,23 @@ class PeerConnectionWrapperTest { private var mockedSignalingMessageReceiver: SignalingMessageReceiver? = null private var mockedSignalingMessageSender: SignalingMessageSender? = null + /** + * Helper answer for DataChannel methods. + */ + private class ReturnValueOrThrowIfDisposed(val value: T) : + Answer { + override fun answer(currentInvocation: InvocationOnMock): T { + if (Mockito.mockingDetails(currentInvocation.mock).invocations.find { + it!!.method.name === "dispose" + } !== null + ) { + throw IllegalStateException("DataChannel has been disposed") + } + + return value + } + } + /** * Helper matcher for DataChannelMessages. */ @@ -195,6 +219,83 @@ class PeerConnectionWrapperTest { ) } + @Test + fun testSendDataChannelMessageBeforeOpeningDataChannelWithDifferentThreads() { + // A brute force approach is used to test race conditions between different threads just repeating the test + // several times. Due to this the test passing could be a false positive, as it could have been a matter of + // luck, but even if the test may wrongly pass sometimes it is better than nothing (although, in general, with + // that number of reruns, it fails when it should). + for (i in 1..1000) { + Mockito.`when`( + mockedPeerConnectionFactory!!.createPeerConnection( + any(PeerConnection.RTCConfiguration::class.java), + any(PeerConnection.Observer::class.java) + ) + ).thenReturn(mockedPeerConnection) + + val mockedStatusDataChannel = Mockito.mock(DataChannel::class.java) + Mockito.`when`(mockedStatusDataChannel.label()).thenReturn("status") + Mockito.`when`(mockedStatusDataChannel.state()).thenReturn(DataChannel.State.CONNECTING) + Mockito.`when`(mockedPeerConnection!!.createDataChannel(eq("status"), any())) + .thenReturn(mockedStatusDataChannel) + + val statusDataChannelObserverArgumentCaptor: ArgumentCaptor = + ArgumentCaptor.forClass(DataChannel.Observer::class.java) + + doNothing().`when`(mockedStatusDataChannel) + .registerObserver(statusDataChannelObserverArgumentCaptor.capture()) + + peerConnectionWrapper = PeerConnectionWrapper( + mockedPeerConnectionFactory, + ArrayList(), + MediaConstraints(), + "the-session-id", + "the-local-session-id", + null, + true, + true, + "video", + mockedSignalingMessageReceiver, + mockedSignalingMessageSender + ) + + val dataChannelMessageCount = 5 + + val sendThread = thread { + for (j in 1..dataChannelMessageCount) { + peerConnectionWrapper!!.send(DataChannelMessage("the-message-type-$j")) + } + } + + // Exceptions thrown in threads are not propagated to the main thread, so it needs to be explicitly done + // (for example, for ConcurrentModificationExceptions when iterating over the data channel messages). + var exceptionOnStateChange: Exception? = null + + val openDataChannelThread = thread { + Mockito.`when`(mockedStatusDataChannel.state()).thenReturn(DataChannel.State.OPEN) + + try { + statusDataChannelObserverArgumentCaptor.value.onStateChange() + } catch (e: Exception) { + exceptionOnStateChange = e + } + } + + sendThread.join() + openDataChannelThread.join() + + if (exceptionOnStateChange !== null) { + throw exceptionOnStateChange!! + } + + for (j in 1..dataChannelMessageCount) { + Mockito.verify(mockedStatusDataChannel).send( + argThat(MatchesDataChannelMessage(DataChannelMessage("the-message-type-$j"))) + ) + } + } + } + @Test fun testReceiveDataChannelMessage() { Mockito.`when`( @@ -381,4 +482,279 @@ class PeerConnectionWrapperTest { Mockito.verify(mockedStatusDataChannel).dispose() Mockito.verify(mockedRandomIdDataChannel).dispose() } + + @Test + fun testRemovePeerConnectionWhileAddingRemoteDataChannelsWithDifferentThreads() { + // A brute force approach is used to test race conditions between different threads just repeating the test + // several times. Due to this the test passing could be a false positive, as it could have been a matter of + // luck, but even if the test may wrongly pass sometimes it is better than nothing (although, in general, with + // that number of reruns, it fails when it should). + for (i in 1..1000) { + val peerConnectionObserverArgumentCaptor: ArgumentCaptor = + ArgumentCaptor.forClass(PeerConnection.Observer::class.java) + + Mockito.`when`( + mockedPeerConnectionFactory!!.createPeerConnection( + any(PeerConnection.RTCConfiguration::class.java), + peerConnectionObserverArgumentCaptor.capture() + ) + ).thenReturn(mockedPeerConnection) + + val mockedStatusDataChannel = Mockito.mock(DataChannel::class.java) + Mockito.`when`(mockedStatusDataChannel.label()).thenAnswer(ReturnValueOrThrowIfDisposed("status")) + Mockito.`when`(mockedStatusDataChannel.state()).thenAnswer( + ReturnValueOrThrowIfDisposed(DataChannel.State.OPEN) + ) + Mockito.`when`(mockedPeerConnection!!.createDataChannel(eq("status"), any())) + .thenReturn(mockedStatusDataChannel) + + peerConnectionWrapper = PeerConnectionWrapper( + mockedPeerConnectionFactory, + ArrayList(), + MediaConstraints(), + "the-session-id", + "the-local-session-id", + null, + true, + true, + "video", + mockedSignalingMessageReceiver, + mockedSignalingMessageSender + ) + + val dataChannelCount = 5 + + val mockedRandomIdDataChannels: MutableList = ArrayList() + val dataChannelObservers: MutableList = ArrayList() + for (j in 0.. + if (Mockito.mockingDetails(invocation.mock).invocations.find { + it!!.method.name === "dispose" + } !== null + ) { + throw IllegalStateException("DataChannel has been disposed") + } + + dataChannelObservers[j] = invocation.getArgument(0, DataChannel.Observer::class.java) + + null + }.`when`(mockedRandomIdDataChannels[j]).registerObserver(any()) + } + + val onDataChannelThread = thread { + // Add again "status" data channel to test that it is correctly disposed also in that case (which + // should not happen anyway even if it was added by the remote peer, but just in case) + peerConnectionObserverArgumentCaptor.value.onDataChannel(mockedStatusDataChannel) + + for (j in 0.. = + ArgumentCaptor.forClass(PeerConnection.Observer::class.java) + + Mockito.`when`( + mockedPeerConnectionFactory!!.createPeerConnection( + any(PeerConnection.RTCConfiguration::class.java), + peerConnectionObserverArgumentCaptor.capture() + ) + ).thenReturn(mockedPeerConnection) + + val mockedStatusDataChannel = Mockito.mock(DataChannel::class.java) + + Mockito.`when`(mockedStatusDataChannel.label()).thenAnswer(ReturnValueOrThrowIfDisposed("status")) + Mockito.`when`(mockedStatusDataChannel.state()) + .thenAnswer(ReturnValueOrThrowIfDisposed(DataChannel.State.OPEN)) + Mockito.`when`(mockedStatusDataChannel.send(any())).thenAnswer(ReturnValueOrThrowIfDisposed(true)) + Mockito.`when`(mockedPeerConnection!!.createDataChannel(eq("status"), any())) + .thenReturn(mockedStatusDataChannel) + + peerConnectionWrapper = PeerConnectionWrapper( + mockedPeerConnectionFactory, + ArrayList(), + MediaConstraints(), + "the-session-id", + "the-local-session-id", + null, + true, + true, + "video", + mockedSignalingMessageReceiver, + mockedSignalingMessageSender + ) + + val dataChannelMessageCount = 5 + + // Exceptions thrown in threads are not propagated to the main thread, so it needs to be explicitly done + // (for example, for IllegalStateExceptions when using a disposed data channel). + var exceptionSend: Exception? = null + + val sendThread = thread { + try { + for (j in 0.. = + ArgumentCaptor.forClass(PeerConnection.Observer::class.java) + + Mockito.`when`( + mockedPeerConnectionFactory!!.createPeerConnection( + any(PeerConnection.RTCConfiguration::class.java), + peerConnectionObserverArgumentCaptor.capture() + ) + ).thenReturn(mockedPeerConnection) + + val mockedStatusDataChannel = Mockito.mock(DataChannel::class.java) + Mockito.`when`(mockedStatusDataChannel.label()).thenAnswer(ReturnValueOrThrowIfDisposed("status")) + Mockito.`when`(mockedStatusDataChannel.state()).thenAnswer( + ReturnValueOrThrowIfDisposed(DataChannel.State.OPEN) + ) + Mockito.`when`(mockedPeerConnection!!.createDataChannel(eq("status"), any())) + .thenReturn(mockedStatusDataChannel) + + val statusDataChannelObserverArgumentCaptor: ArgumentCaptor = + ArgumentCaptor.forClass(DataChannel.Observer::class.java) + + doNothing().`when`(mockedStatusDataChannel) + .registerObserver(statusDataChannelObserverArgumentCaptor.capture()) + + peerConnectionWrapper = PeerConnectionWrapper( + mockedPeerConnectionFactory, + ArrayList(), + MediaConstraints(), + "the-session-id", + "the-local-session-id", + null, + true, + true, + "video", + mockedSignalingMessageReceiver, + mockedSignalingMessageSender + ) + + val mockedDataChannelMessageListener = Mockito.mock(DataChannelMessageListener::class.java) + peerConnectionWrapper!!.addListener(mockedDataChannelMessageListener) + + // Exceptions thrown in threads are not propagated to the main thread, so it needs to be explicitly done + // (for example, for IllegalStateExceptions when using a disposed data channel). + var exceptionOnMessage: Exception? = null + + val onMessageThread = thread { + try { + // It is assumed that, even if its data channel was disposed, its buffers can be used while there + // is a reference to them, so no special mock behaviour is added to throw an exception in that case. + statusDataChannelObserverArgumentCaptor.value.onMessage( + dataChannelMessageToBuffer(DataChannelMessage("audioOn")) + ) + + statusDataChannelObserverArgumentCaptor.value.onMessage( + dataChannelMessageToBuffer(DataChannelMessage("audioOff")) + ) + } catch (e: Exception) { + exceptionOnMessage = e + } + } + + val removePeerConnectionThread = thread { + peerConnectionWrapper!!.removePeerConnection() + } + + onMessageThread.join() + removePeerConnectionThread.join() + + if (exceptionOnMessage !== null) { + throw exceptionOnMessage!! + } + + Mockito.verify(mockedStatusDataChannel).registerObserver(any()) + Mockito.verify(mockedStatusDataChannel).dispose() + Mockito.verify(mockedStatusDataChannel, atLeast(0)).label() + Mockito.verify(mockedStatusDataChannel, atLeast(0)).state() + Mockito.verifyNoMoreInteractions(mockedStatusDataChannel) + Mockito.verify(mockedDataChannelMessageListener, atMostOnce()).onAudioOn() + Mockito.verify(mockedDataChannelMessageListener, atMostOnce()).onAudioOff() + Mockito.verifyNoMoreInteractions(mockedDataChannelMessageListener) + } + } } From b338693d937016611de818e0adf2991247691324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 9 Dec 2024 18:36:22 +0100 Subject: [PATCH 16/27] Fix "send" not respecting order of pending messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the data channel is not open yet data channel messages are queued and then sent once opened. "onStateChange" is called from the WebRTC signaling thread, while "send" can be called potentially from any thread, so to send the data channel messages in the same order that they were added new messages need to be enqueued until all the pending messages have been sent. Otherwise, even if there is synchronization already, it could happen that "onStateChange" was called but, before getting the lock, "send" gets it and sends the new message before the pending messages were sent. Signed-off-by: Daniel Calviño Sánchez --- .../com/nextcloud/talk/webrtc/PeerConnectionWrapper.java | 9 +++++++-- .../nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt | 5 ++++- 2 files changed, 11 insertions(+), 3 deletions(-) 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 13bb5f2f7b..9f2a90f859 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -298,7 +298,8 @@ public synchronized void send(DataChannelMessage dataChannelMessage) { } DataChannel statusDataChannel = dataChannels.get("status"); - if (statusDataChannel == null || statusDataChannel.state() != DataChannel.State.OPEN) { + if (statusDataChannel == null || statusDataChannel.state() != DataChannel.State.OPEN || + !pendingDataChannelMessages.isEmpty()) { Log.d(TAG, "Queuing data channel message (" + dataChannelMessage + ") " + sessionId); pendingDataChannelMessages.add(dataChannelMessage); @@ -306,6 +307,10 @@ public synchronized void send(DataChannelMessage dataChannelMessage) { return; } + sendWithoutQueuing(statusDataChannel, dataChannelMessage); + } + + private void sendWithoutQueuing(DataChannel statusDataChannel, DataChannelMessage dataChannelMessage) { try { Log.d(TAG, "Sending data channel message (" + dataChannelMessage + ") " + sessionId); @@ -423,7 +428,7 @@ public void onStateChange() { if (dataChannel.state() == DataChannel.State.OPEN && "status".equals(dataChannelLabel)) { for (DataChannelMessage dataChannelMessage : pendingDataChannelMessages) { - send(dataChannelMessage); + sendWithoutQueuing(dataChannel, dataChannelMessage); } pendingDataChannelMessages.clear(); } diff --git a/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt index f4f7e6443f..b6dd409626 100644 --- a/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt +++ b/app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt @@ -23,6 +23,7 @@ import org.mockito.Mockito.atLeast import org.mockito.Mockito.atMostOnce import org.mockito.Mockito.doAnswer import org.mockito.Mockito.doNothing +import org.mockito.Mockito.inOrder import org.mockito.Mockito.never import org.mockito.invocation.InvocationOnMock import org.mockito.stubbing.Answer @@ -288,8 +289,10 @@ class PeerConnectionWrapperTest { throw exceptionOnStateChange!! } + val inOrder = inOrder(mockedStatusDataChannel) + for (j in 1..dataChannelMessageCount) { - Mockito.verify(mockedStatusDataChannel).send( + inOrder.verify(mockedStatusDataChannel).send( argThat(MatchesDataChannelMessage(DataChannelMessage("the-message-type-$j"))) ) } From a9c6947fec844a4697c8a0885b5d94a4ef0dc5fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 24 Oct 2024 21:37:50 +0200 Subject: [PATCH 17/27] Set "hasMCU" once its value is known MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "hasMCU" (which has always been the wrong name, because it is an SFU rather than an MCU, but it is wrong even in the signaling server so for now the legacy name is kept) was set again and again whenever the call participant list changed. Now it is set instead once its value is known, that is, when it is known that the internal signaling server is used (as no "MCU" is used in that case), or when the connection with the external signaling server is established, as its supported features are not known until then. This change should have no effect in the usages of "hasMCU", as it is used when the call participant list change, which will happen only after joining the call in the signaling server, or when sending "isSpeaking" and toggling media, in both cases guarded by "isConnectionEstablished", which will be true only once "performCall" was called or if the call is active with other participants. Signed-off-by: Daniel Calviño Sánchez --- .../com/nextcloud/talk/activities/CallActivity.kt | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt index 3bcc7611b0..2d36413d62 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt @@ -1618,6 +1618,9 @@ class CallActivity : CallBaseActivity() { signalingMessageReceiver!!.addListener(localParticipantMessageListener) signalingMessageReceiver!!.addListener(offerMessageListener) signalingMessageSender = internalSignalingMessageSender + + hasMCU = false + joinRoomAndCall() } } @@ -1903,6 +1906,11 @@ class CallActivity : CallBaseActivity() { signalingMessageReceiver!!.addListener(localParticipantMessageListener) signalingMessageReceiver!!.addListener(offerMessageListener) signalingMessageSender = webSocketClient!!.signalingMessageSender + + // If the connection with the signaling server was not established yet the value will be false, but it will + // be overwritten with the right value once the response to the "hello" message is received. + hasMCU = webSocketClient!!.hasMCU() + Log.d(TAG, "hasMCU is $hasMCU") } else { if (webSocketClient!!.isConnected && currentCallStatus === CallStatus.PUBLISHER_FAILED) { webSocketClient!!.restartWebSocket() @@ -1928,6 +1936,10 @@ class CallActivity : CallBaseActivity() { when (webSocketCommunicationEvent.getType()) { "hello" -> { Log.d(TAG, "onMessageEvent 'hello'") + + hasMCU = webSocketClient!!.hasMCU() + Log.d(TAG, "hasMCU is $hasMCU") + if (!webSocketCommunicationEvent.getHashMap()!!.containsKey("oldResumeId")) { if (currentCallStatus === CallStatus.RECONNECTING) { hangup(false, false) @@ -2136,8 +2148,6 @@ class CallActivity : CallBaseActivity() { unchanged: Collection ) { Log.d(TAG, "handleCallParticipantsChanged") - hasMCU = hasExternalSignalingServer && webSocketClient != null && webSocketClient!!.hasMCU() - Log.d(TAG, " hasMCU is $hasMCU") // The signaling session is the same as the Nextcloud session only when the MCU is not used. var currentSessionId = callSession From c21596e758779b7a566513c11899499125d54693 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 25 Oct 2024 14:37:37 +0200 Subject: [PATCH 18/27] Add helper class to send messages to call participants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For now it just provides support for sending a data channel message to all participants, so notifying all participants when the media is toggled or the speaking status change can be directly refactored to use it. While it would have been fine to use a single class for both MCU and no MCU they were split for easier and cleaner unit testing in future stages. Signed-off-by: Daniel Calviño Sánchez --- .../nextcloud/talk/activities/CallActivity.kt | 56 ++++++++------- .../nextcloud/talk/call/MessageSender.java | 47 +++++++++++++ .../nextcloud/talk/call/MessageSenderMcu.java | 34 ++++++++++ .../talk/call/MessageSenderNoMcu.java | 28 ++++++++ .../talk/call/MessageSenderMcuTest.kt | 68 +++++++++++++++++++ .../talk/call/MessageSenderNoMcuTest.kt | 48 +++++++++++++ 6 files changed, 257 insertions(+), 24 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/talk/call/MessageSender.java create mode 100644 app/src/main/java/com/nextcloud/talk/call/MessageSenderMcu.java create mode 100644 app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java create mode 100644 app/src/test/java/com/nextcloud/talk/call/MessageSenderMcuTest.kt create mode 100644 app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt index 2d36413d62..fa478fa763 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt @@ -63,6 +63,9 @@ import com.nextcloud.talk.application.NextcloudTalkApplication.Companion.sharedA import com.nextcloud.talk.call.CallParticipant import com.nextcloud.talk.call.CallParticipantList import com.nextcloud.talk.call.CallParticipantModel +import com.nextcloud.talk.call.MessageSender +import com.nextcloud.talk.call.MessageSenderMcu +import com.nextcloud.talk.call.MessageSenderNoMcu import com.nextcloud.talk.call.ReactionAnimator import com.nextcloud.talk.chat.ChatActivity import com.nextcloud.talk.data.user.model.User @@ -242,6 +245,7 @@ class CallActivity : CallBaseActivity() { private var signalingMessageReceiver: SignalingMessageReceiver? = null private val internalSignalingMessageSender = InternalSignalingMessageSender() private var signalingMessageSender: SignalingMessageSender? = null + private var messageSender: MessageSender? = null private val offerAnswerNickProviders: MutableMap = HashMap() private val callParticipantMessageListeners: MutableMap = HashMap() private val selfPeerConnectionObserver: PeerConnectionObserver = CallActivitySelfPeerConnectionObserver() @@ -1172,18 +1176,7 @@ class CallActivity : CallBaseActivity() { if (isSpeaking) SIGNALING_MESSAGE_SPEAKING_STARTED else SIGNALING_MESSAGE_SPEAKING_STOPPED if (isConnectionEstablished && othersInCall) { - if (!hasMCU) { - for (peerConnectionWrapper in peerConnectionWrapperList) { - peerConnectionWrapper.send(DataChannelMessage(isSpeakingMessage)) - } - } else { - for (peerConnectionWrapper in peerConnectionWrapperList) { - if (peerConnectionWrapper.sessionId == webSocketClient!!.sessionId) { - peerConnectionWrapper.send(DataChannelMessage(isSpeakingMessage)) - break - } - } - } + messageSender!!.sendToAll(DataChannelMessage(isSpeakingMessage)) } } @@ -1368,18 +1361,7 @@ class CallActivity : CallBaseActivity() { } } if (isConnectionEstablished) { - if (!hasMCU) { - for (peerConnectionWrapper in peerConnectionWrapperList) { - peerConnectionWrapper.send(DataChannelMessage(message)) - } - } else { - for (peerConnectionWrapper in peerConnectionWrapperList) { - if (peerConnectionWrapper.sessionId == webSocketClient!!.sessionId) { - peerConnectionWrapper.send(DataChannelMessage(message)) - break - } - } - } + messageSender!!.sendToAll(DataChannelMessage(message)) } } @@ -1621,6 +1603,10 @@ class CallActivity : CallBaseActivity() { hasMCU = false + messageSender = MessageSenderNoMcu( + peerConnectionWrapperList + ) + joinRoomAndCall() } } @@ -1911,6 +1897,17 @@ class CallActivity : CallBaseActivity() { // be overwritten with the right value once the response to the "hello" message is received. hasMCU = webSocketClient!!.hasMCU() Log.d(TAG, "hasMCU is $hasMCU") + + if (hasMCU) { + messageSender = MessageSenderMcu( + peerConnectionWrapperList, + webSocketClient!!.sessionId + ) + } else { + messageSender = MessageSenderNoMcu( + peerConnectionWrapperList + ) + } } else { if (webSocketClient!!.isConnected && currentCallStatus === CallStatus.PUBLISHER_FAILED) { webSocketClient!!.restartWebSocket() @@ -1940,6 +1937,17 @@ class CallActivity : CallBaseActivity() { hasMCU = webSocketClient!!.hasMCU() Log.d(TAG, "hasMCU is $hasMCU") + if (hasMCU) { + messageSender = MessageSenderMcu( + peerConnectionWrapperList, + webSocketClient!!.sessionId + ) + } else { + messageSender = MessageSenderNoMcu( + peerConnectionWrapperList + ) + } + if (!webSocketCommunicationEvent.getHashMap()!!.containsKey("oldResumeId")) { if (currentCallStatus === CallStatus.RECONNECTING) { hangup(false, false) diff --git a/app/src/main/java/com/nextcloud/talk/call/MessageSender.java b/app/src/main/java/com/nextcloud/talk/call/MessageSender.java new file mode 100644 index 0000000000..2e0820965e --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/call/MessageSender.java @@ -0,0 +1,47 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call; + +import com.nextcloud.talk.models.json.signaling.DataChannelMessage; +import com.nextcloud.talk.webrtc.PeerConnectionWrapper; + +import java.util.List; + +/** + * Helper class to send messages to participants in a call. + *

+ * A specific subclass has to be created depending on whether an MCU is being used or not. + *

+ * Note that, unlike signaling messages, data channel messages require a peer connection. Therefore data channel + * messages may not be received by a participant if there is no peer connection with that participant (for example, if + * neither the local and remote participants have publishing rights). + */ +public abstract class MessageSender { + + protected final List peerConnectionWrappers; + + public MessageSender(List peerConnectionWrappers) { + this.peerConnectionWrappers = peerConnectionWrappers; + } + + /** + * Sends the given data channel message to all the participants in the call. + * + * @param dataChannelMessage the message to send + */ + public abstract void sendToAll(DataChannelMessage dataChannelMessage); + + protected PeerConnectionWrapper getPeerConnectionWrapper(String sessionId) { + for (PeerConnectionWrapper peerConnectionWrapper: peerConnectionWrappers) { + if (peerConnectionWrapper.getSessionId().equals(sessionId)) { + return peerConnectionWrapper; + } + } + + return null; + } +} diff --git a/app/src/main/java/com/nextcloud/talk/call/MessageSenderMcu.java b/app/src/main/java/com/nextcloud/talk/call/MessageSenderMcu.java new file mode 100644 index 0000000000..803becf57b --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/call/MessageSenderMcu.java @@ -0,0 +1,34 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call; + +import com.nextcloud.talk.models.json.signaling.DataChannelMessage; +import com.nextcloud.talk.webrtc.PeerConnectionWrapper; + +import java.util.List; + +/** + * Helper class to send messages to participants in a call when an MCU is used. + */ +public class MessageSenderMcu extends MessageSender { + + private final String ownSessionId; + + public MessageSenderMcu(List peerConnectionWrappers, + String ownSessionId) { + super(peerConnectionWrappers); + + this.ownSessionId = ownSessionId; + } + + public void sendToAll(DataChannelMessage dataChannelMessage) { + PeerConnectionWrapper ownPeerConnectionWrapper = getPeerConnectionWrapper(ownSessionId); + if (ownPeerConnectionWrapper != null) { + ownPeerConnectionWrapper.send(dataChannelMessage); + } + } +} diff --git a/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java b/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java new file mode 100644 index 0000000000..7dea72926d --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java @@ -0,0 +1,28 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call; + +import com.nextcloud.talk.models.json.signaling.DataChannelMessage; +import com.nextcloud.talk.webrtc.PeerConnectionWrapper; + +import java.util.List; + +/** + * Helper class to send messages to participants in a call when an MCU is not used. + */ +public class MessageSenderNoMcu extends MessageSender { + + public MessageSenderNoMcu(List peerConnectionWrappers) { + super(peerConnectionWrappers); + } + + public void sendToAll(DataChannelMessage dataChannelMessage) { + for (PeerConnectionWrapper peerConnectionWrapper: peerConnectionWrappers) { + peerConnectionWrapper.send(dataChannelMessage); + } + } +} diff --git a/app/src/test/java/com/nextcloud/talk/call/MessageSenderMcuTest.kt b/app/src/test/java/com/nextcloud/talk/call/MessageSenderMcuTest.kt new file mode 100644 index 0000000000..9bdefbfe89 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/call/MessageSenderMcuTest.kt @@ -0,0 +1,68 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call + +import com.nextcloud.talk.models.json.signaling.DataChannelMessage +import com.nextcloud.talk.webrtc.PeerConnectionWrapper +import org.junit.Before +import org.junit.Test +import org.mockito.Mockito +import org.mockito.Mockito.never + +class MessageSenderMcuTest { + + private var peerConnectionWrappers: MutableList? = null + private var peerConnectionWrapper1: PeerConnectionWrapper? = null + private var peerConnectionWrapper2: PeerConnectionWrapper? = null + private var ownPeerConnectionWrapper: PeerConnectionWrapper? = null + + private var messageSender: MessageSenderMcu? = null + + @Before + fun setUp() { + peerConnectionWrappers = ArrayList() + + peerConnectionWrapper1 = Mockito.mock(PeerConnectionWrapper::class.java) + Mockito.`when`(peerConnectionWrapper1!!.sessionId).thenReturn("theSessionId1") + Mockito.`when`(peerConnectionWrapper1!!.videoStreamType).thenReturn("video") + peerConnectionWrappers!!.add(peerConnectionWrapper1) + + peerConnectionWrapper2 = Mockito.mock(PeerConnectionWrapper::class.java) + Mockito.`when`(peerConnectionWrapper2!!.sessionId).thenReturn("theSessionId2") + Mockito.`when`(peerConnectionWrapper2!!.videoStreamType).thenReturn("video") + peerConnectionWrappers!!.add(peerConnectionWrapper2) + + ownPeerConnectionWrapper = Mockito.mock(PeerConnectionWrapper::class.java) + Mockito.`when`(ownPeerConnectionWrapper!!.sessionId).thenReturn("ownSessionId") + Mockito.`when`(ownPeerConnectionWrapper!!.videoStreamType).thenReturn("video") + peerConnectionWrappers!!.add(ownPeerConnectionWrapper) + + messageSender = MessageSenderMcu(peerConnectionWrappers, "ownSessionId") + } + + @Test + fun testSendDataChannelMessageToAll() { + val message = DataChannelMessage() + messageSender!!.sendToAll(message) + + Mockito.verify(ownPeerConnectionWrapper!!).send(message) + Mockito.verify(peerConnectionWrapper1!!, never()).send(message) + Mockito.verify(peerConnectionWrapper2!!, never()).send(message) + } + + @Test + fun testSendDataChannelMessageToAllWithoutOwnPeerConnection() { + peerConnectionWrappers!!.remove(ownPeerConnectionWrapper) + + val message = DataChannelMessage() + messageSender!!.sendToAll(message) + + Mockito.verify(ownPeerConnectionWrapper!!, never()).send(message) + Mockito.verify(peerConnectionWrapper1!!, never()).send(message) + Mockito.verify(peerConnectionWrapper2!!, never()).send(message) + } +} diff --git a/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt b/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt new file mode 100644 index 0000000000..cec1534de7 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt @@ -0,0 +1,48 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call + +import com.nextcloud.talk.models.json.signaling.DataChannelMessage +import com.nextcloud.talk.webrtc.PeerConnectionWrapper +import org.junit.Before +import org.junit.Test +import org.mockito.Mockito + +class MessageSenderNoMcuTest { + + private var peerConnectionWrappers: MutableList? = null + private var peerConnectionWrapper1: PeerConnectionWrapper? = null + private var peerConnectionWrapper2: PeerConnectionWrapper? = null + + private var messageSender: MessageSenderNoMcu? = null + + @Before + fun setUp() { + peerConnectionWrappers = ArrayList() + + peerConnectionWrapper1 = Mockito.mock(PeerConnectionWrapper::class.java) + Mockito.`when`(peerConnectionWrapper1!!.sessionId).thenReturn("theSessionId1") + Mockito.`when`(peerConnectionWrapper1!!.videoStreamType).thenReturn("video") + peerConnectionWrappers!!.add(peerConnectionWrapper1) + + peerConnectionWrapper2 = Mockito.mock(PeerConnectionWrapper::class.java) + Mockito.`when`(peerConnectionWrapper2!!.sessionId).thenReturn("theSessionId2") + Mockito.`when`(peerConnectionWrapper2!!.videoStreamType).thenReturn("video") + peerConnectionWrappers!!.add(peerConnectionWrapper2) + + messageSender = MessageSenderNoMcu(peerConnectionWrappers) + } + + @Test + fun testSendDataChannelMessageToAll() { + val message = DataChannelMessage() + messageSender!!.sendToAll(message) + + Mockito.verify(peerConnectionWrapper1!!).send(message) + Mockito.verify(peerConnectionWrapper2!!).send(message) + } +} From 846404021eb5e310e34d4dbd1311b7279d8aef1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 25 Oct 2024 14:38:59 +0200 Subject: [PATCH 19/27] Send data channel messages only to "video" peer connections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Data channel messages are expected to be sent only to peer connections with "video" type, which provide the audio and video tracks of the participant (and, in fact, peer connections for screen shares do not even have data channels enabled in the WebUI). Note that this could change if at some point several audio/video tracks are sent in the same peer connection, or if "speaking" messages are added to screen shares, but that will be addressed if/when that happens. Signed-off-by: Daniel Calviño Sánchez --- .../nextcloud/talk/call/MessageSender.java | 7 +++- .../talk/call/MessageSenderNoMcu.java | 4 +- .../talk/call/MessageSenderMcuTest.kt | 40 +++++++++++++++++++ .../talk/call/MessageSenderNoMcuTest.kt | 15 +++++++ 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/call/MessageSender.java b/app/src/main/java/com/nextcloud/talk/call/MessageSender.java index 2e0820965e..a868fc6b81 100644 --- a/app/src/main/java/com/nextcloud/talk/call/MessageSender.java +++ b/app/src/main/java/com/nextcloud/talk/call/MessageSender.java @@ -18,7 +18,9 @@ *

* Note that, unlike signaling messages, data channel messages require a peer connection. Therefore data channel * messages may not be received by a participant if there is no peer connection with that participant (for example, if - * neither the local and remote participants have publishing rights). + * neither the local and remote participants have publishing rights). Moreover, data channel messages are expected to + * be received only on peer connections with type "video", so data channel messages will not be sent on other peer + * connections. */ public abstract class MessageSender { @@ -37,7 +39,8 @@ public MessageSender(List peerConnectionWrappers) { protected PeerConnectionWrapper getPeerConnectionWrapper(String sessionId) { for (PeerConnectionWrapper peerConnectionWrapper: peerConnectionWrappers) { - if (peerConnectionWrapper.getSessionId().equals(sessionId)) { + if (peerConnectionWrapper.getSessionId().equals(sessionId) + && "video".equals(peerConnectionWrapper.getVideoStreamType())) { return peerConnectionWrapper; } } diff --git a/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java b/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java index 7dea72926d..6f4306262e 100644 --- a/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java +++ b/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java @@ -22,7 +22,9 @@ public MessageSenderNoMcu(List peerConnectionWrappers) { public void sendToAll(DataChannelMessage dataChannelMessage) { for (PeerConnectionWrapper peerConnectionWrapper: peerConnectionWrappers) { - peerConnectionWrapper.send(dataChannelMessage); + if ("video".equals(peerConnectionWrapper.getVideoStreamType())){ + peerConnectionWrapper.send(dataChannelMessage); + } } } } diff --git a/app/src/test/java/com/nextcloud/talk/call/MessageSenderMcuTest.kt b/app/src/test/java/com/nextcloud/talk/call/MessageSenderMcuTest.kt index 9bdefbfe89..49ca9e864d 100644 --- a/app/src/test/java/com/nextcloud/talk/call/MessageSenderMcuTest.kt +++ b/app/src/test/java/com/nextcloud/talk/call/MessageSenderMcuTest.kt @@ -18,7 +18,10 @@ class MessageSenderMcuTest { private var peerConnectionWrappers: MutableList? = null private var peerConnectionWrapper1: PeerConnectionWrapper? = null private var peerConnectionWrapper2: PeerConnectionWrapper? = null + private var peerConnectionWrapper2Screen: PeerConnectionWrapper? = null + private var peerConnectionWrapper4Screen: PeerConnectionWrapper? = null private var ownPeerConnectionWrapper: PeerConnectionWrapper? = null + private var ownPeerConnectionWrapperScreen: PeerConnectionWrapper? = null private var messageSender: MessageSenderMcu? = null @@ -36,11 +39,26 @@ class MessageSenderMcuTest { Mockito.`when`(peerConnectionWrapper2!!.videoStreamType).thenReturn("video") peerConnectionWrappers!!.add(peerConnectionWrapper2) + peerConnectionWrapper2Screen = Mockito.mock(PeerConnectionWrapper::class.java) + Mockito.`when`(peerConnectionWrapper2Screen!!.sessionId).thenReturn("theSessionId2") + Mockito.`when`(peerConnectionWrapper2Screen!!.videoStreamType).thenReturn("screen") + peerConnectionWrappers!!.add(peerConnectionWrapper2Screen) + + peerConnectionWrapper4Screen = Mockito.mock(PeerConnectionWrapper::class.java) + Mockito.`when`(peerConnectionWrapper4Screen!!.sessionId).thenReturn("theSessionId4") + Mockito.`when`(peerConnectionWrapper4Screen!!.videoStreamType).thenReturn("screen") + peerConnectionWrappers!!.add(peerConnectionWrapper4Screen) + ownPeerConnectionWrapper = Mockito.mock(PeerConnectionWrapper::class.java) Mockito.`when`(ownPeerConnectionWrapper!!.sessionId).thenReturn("ownSessionId") Mockito.`when`(ownPeerConnectionWrapper!!.videoStreamType).thenReturn("video") peerConnectionWrappers!!.add(ownPeerConnectionWrapper) + ownPeerConnectionWrapperScreen = Mockito.mock(PeerConnectionWrapper::class.java) + Mockito.`when`(ownPeerConnectionWrapperScreen!!.sessionId).thenReturn("ownSessionId") + Mockito.`when`(ownPeerConnectionWrapperScreen!!.videoStreamType).thenReturn("screen") + peerConnectionWrappers!!.add(ownPeerConnectionWrapperScreen) + messageSender = MessageSenderMcu(peerConnectionWrappers, "ownSessionId") } @@ -50,19 +68,41 @@ class MessageSenderMcuTest { messageSender!!.sendToAll(message) Mockito.verify(ownPeerConnectionWrapper!!).send(message) + Mockito.verify(ownPeerConnectionWrapperScreen!!, never()).send(message) + Mockito.verify(peerConnectionWrapper1!!, never()).send(message) + Mockito.verify(peerConnectionWrapper2!!, never()).send(message) + Mockito.verify(peerConnectionWrapper2Screen!!, never()).send(message) + Mockito.verify(peerConnectionWrapper4Screen!!, never()).send(message) + } + + @Test + fun testSendDataChannelMessageToAllIfOwnScreenPeerConnection() { + peerConnectionWrappers!!.remove(ownPeerConnectionWrapper) + + val message = DataChannelMessage() + messageSender!!.sendToAll(message) + + Mockito.verify(ownPeerConnectionWrapper!!, never()).send(message) + Mockito.verify(ownPeerConnectionWrapperScreen!!, never()).send(message) Mockito.verify(peerConnectionWrapper1!!, never()).send(message) Mockito.verify(peerConnectionWrapper2!!, never()).send(message) + Mockito.verify(peerConnectionWrapper2Screen!!, never()).send(message) + Mockito.verify(peerConnectionWrapper4Screen!!, never()).send(message) } @Test fun testSendDataChannelMessageToAllWithoutOwnPeerConnection() { peerConnectionWrappers!!.remove(ownPeerConnectionWrapper) + peerConnectionWrappers!!.remove(ownPeerConnectionWrapperScreen) val message = DataChannelMessage() messageSender!!.sendToAll(message) Mockito.verify(ownPeerConnectionWrapper!!, never()).send(message) + Mockito.verify(ownPeerConnectionWrapperScreen!!, never()).send(message) Mockito.verify(peerConnectionWrapper1!!, never()).send(message) Mockito.verify(peerConnectionWrapper2!!, never()).send(message) + Mockito.verify(peerConnectionWrapper2Screen!!, never()).send(message) + Mockito.verify(peerConnectionWrapper4Screen!!, never()).send(message) } } diff --git a/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt b/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt index cec1534de7..cfd4a850d0 100644 --- a/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt +++ b/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt @@ -11,12 +11,15 @@ import com.nextcloud.talk.webrtc.PeerConnectionWrapper import org.junit.Before import org.junit.Test import org.mockito.Mockito +import org.mockito.Mockito.never class MessageSenderNoMcuTest { private var peerConnectionWrappers: MutableList? = null private var peerConnectionWrapper1: PeerConnectionWrapper? = null private var peerConnectionWrapper2: PeerConnectionWrapper? = null + private var peerConnectionWrapper2Screen: PeerConnectionWrapper? = null + private var peerConnectionWrapper4Screen: PeerConnectionWrapper? = null private var messageSender: MessageSenderNoMcu? = null @@ -34,6 +37,16 @@ class MessageSenderNoMcuTest { Mockito.`when`(peerConnectionWrapper2!!.videoStreamType).thenReturn("video") peerConnectionWrappers!!.add(peerConnectionWrapper2) + peerConnectionWrapper2Screen = Mockito.mock(PeerConnectionWrapper::class.java) + Mockito.`when`(peerConnectionWrapper2Screen!!.sessionId).thenReturn("theSessionId2") + Mockito.`when`(peerConnectionWrapper2Screen!!.videoStreamType).thenReturn("screen") + peerConnectionWrappers!!.add(peerConnectionWrapper2Screen) + + peerConnectionWrapper4Screen = Mockito.mock(PeerConnectionWrapper::class.java) + Mockito.`when`(peerConnectionWrapper4Screen!!.sessionId).thenReturn("theSessionId4") + Mockito.`when`(peerConnectionWrapper4Screen!!.videoStreamType).thenReturn("screen") + peerConnectionWrappers!!.add(peerConnectionWrapper4Screen) + messageSender = MessageSenderNoMcu(peerConnectionWrappers) } @@ -44,5 +57,7 @@ class MessageSenderNoMcuTest { Mockito.verify(peerConnectionWrapper1!!).send(message) Mockito.verify(peerConnectionWrapper2!!).send(message) + Mockito.verify(peerConnectionWrapper2Screen!!, never()).send(message) + Mockito.verify(peerConnectionWrapper4Screen!!, never()).send(message) } } From 79ed2e0242db7f9c2af32a76f08fc5a6bbb2778b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 25 Oct 2024 19:43:17 +0200 Subject: [PATCH 20/27] Add data model for local call participants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the counterpart of CallParticipantModel for the local participant. For now it just stores whether audio and video are enabled or not, and whether the local participant is speaking or not, but it will be eventually extended with further properties. It is also expected that the views, like the button with the microphone state, will update themselves based on the model. Similarly the model should be moved from the CallActivity to a class similar to CallParticipant but for the local participant. In any case, all that is something for the future; the immediate use of the model will be to know when the local state changes to notify other participants. Signed-off-by: Daniel Calviño Sánchez --- .../nextcloud/talk/activities/CallActivity.kt | 8 + .../talk/call/LocalCallParticipantModel.java | 114 ++++++++++++ .../LocalCallParticipantModelNotifier.java | 73 ++++++++ .../MutableLocalCallParticipantModel.java | 51 ++++++ .../call/LocalCallParticipantModelTest.kt | 168 ++++++++++++++++++ 5 files changed, 414 insertions(+) create mode 100644 app/src/main/java/com/nextcloud/talk/call/LocalCallParticipantModel.java create mode 100644 app/src/main/java/com/nextcloud/talk/call/LocalCallParticipantModelNotifier.java create mode 100644 app/src/main/java/com/nextcloud/talk/call/MutableLocalCallParticipantModel.java create mode 100644 app/src/test/java/com/nextcloud/talk/call/LocalCallParticipantModelTest.kt diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt index fa478fa763..91be62eafe 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt @@ -66,6 +66,7 @@ import com.nextcloud.talk.call.CallParticipantModel import com.nextcloud.talk.call.MessageSender import com.nextcloud.talk.call.MessageSenderMcu import com.nextcloud.talk.call.MessageSenderNoMcu +import com.nextcloud.talk.call.MutableLocalCallParticipantModel import com.nextcloud.talk.call.ReactionAnimator import com.nextcloud.talk.chat.ChatActivity import com.nextcloud.talk.data.user.model.User @@ -246,6 +247,7 @@ class CallActivity : CallBaseActivity() { private val internalSignalingMessageSender = InternalSignalingMessageSender() private var signalingMessageSender: SignalingMessageSender? = null private var messageSender: MessageSender? = null + private val localCallParticipantModel: MutableLocalCallParticipantModel = MutableLocalCallParticipantModel() private val offerAnswerNickProviders: MutableMap = HashMap() private val callParticipantMessageListeners: MutableMap = HashMap() private val selfPeerConnectionObserver: PeerConnectionObserver = CallActivitySelfPeerConnectionObserver() @@ -1123,6 +1125,7 @@ class CallActivity : CallBaseActivity() { localStream!!.addTrack(localVideoTrack) localVideoTrack!!.setEnabled(false) localVideoTrack!!.addSink(binding!!.selfVideoRenderer) + localCallParticipantModel.isVideoEnabled = false } private fun microphoneInitialization() { @@ -1133,6 +1136,7 @@ class CallActivity : CallBaseActivity() { localAudioTrack = peerConnectionFactory!!.createAudioTrack("NCa0", audioSource) localAudioTrack!!.setEnabled(false) localStream!!.addTrack(localAudioTrack) + localCallParticipantModel.isAudioEnabled = false } @SuppressLint("MissingPermission") @@ -1157,9 +1161,11 @@ class CallActivity : CallBaseActivity() { if (microphoneOn && isCurrentlySpeaking && !isSpeakingLongTerm) { isSpeakingLongTerm = true + localCallParticipantModel.isSpeaking = true sendIsSpeakingMessage(true) } else if (!isCurrentlySpeaking && isSpeakingLongTerm) { isSpeakingLongTerm = false + localCallParticipantModel.isSpeaking = false sendIsSpeakingMessage(false) } Thread.sleep(MICROPHONE_VALUE_SLEEP) @@ -1342,6 +1348,7 @@ class CallActivity : CallBaseActivity() { } if (localStream != null && localStream!!.videoTracks.size > 0) { localStream!!.videoTracks[0].setEnabled(enable) + localCallParticipantModel.isVideoEnabled = enable } if (enable) { binding!!.selfVideoRenderer.visibility = View.VISIBLE @@ -1358,6 +1365,7 @@ class CallActivity : CallBaseActivity() { } if (localStream != null && localStream!!.audioTracks.size > 0) { localStream!!.audioTracks[0].setEnabled(enable) + localCallParticipantModel.isAudioEnabled = enable } } if (isConnectionEstablished) { diff --git a/app/src/main/java/com/nextcloud/talk/call/LocalCallParticipantModel.java b/app/src/main/java/com/nextcloud/talk/call/LocalCallParticipantModel.java new file mode 100644 index 0000000000..b1dcececc4 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/call/LocalCallParticipantModel.java @@ -0,0 +1,114 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call; + +import android.os.Handler; + +import java.util.Objects; + +/** + * Read-only data model for local call participants. + *

+ * Clients of the model can observe it with LocalCallParticipantModel.Observer to be notified when any value changes. + * Getters called after receiving a notification are guaranteed to provide at least the value that triggered the + * notification, but it may return even a more up to date one (so getting the value again on the following notification + * may return the same value as before). + */ +public class LocalCallParticipantModel { + + protected final LocalCallParticipantModelNotifier localCallParticipantModelNotifier = + new LocalCallParticipantModelNotifier(); + + protected Data audioEnabled; + protected Data speaking; + protected Data speakingWhileMuted; + protected Data videoEnabled; + + public interface Observer { + void onChange(); + } + + protected class Data { + + private T value; + + public Data() { + } + + public Data(T value) { + this.value = value; + } + + public T getValue() { + return value; + } + + public void setValue(T value) { + if (Objects.equals(this.value, value)) { + return; + } + + this.value = value; + + localCallParticipantModelNotifier.notifyChange(); + } + } + + public LocalCallParticipantModel() { + this.audioEnabled = new Data<>(Boolean.FALSE); + this.speaking = new Data<>(Boolean.FALSE); + this.speakingWhileMuted = new Data<>(Boolean.FALSE); + this.videoEnabled = new Data<>(Boolean.FALSE); + } + + public Boolean isAudioEnabled() { + return audioEnabled.getValue(); + } + + public Boolean isSpeaking() { + return speaking.getValue(); + } + + public Boolean isSpeakingWhileMuted() { + return speakingWhileMuted.getValue(); + } + + public Boolean isVideoEnabled() { + return videoEnabled.getValue(); + } + + /** + * Adds an Observer to be notified when any value changes. + * + * @param observer the Observer + * @see LocalCallParticipantModel#addObserver(Observer, Handler) + */ + public void addObserver(Observer observer) { + addObserver(observer, null); + } + + /** + * Adds an observer to be notified when any value changes. + *

+ * The observer will be notified on the thread associated to the given handler. If no handler is given the + * observer will be immediately notified on the same thread that changed the value; the observer will be + * immediately notified too if the thread of the handler is the same thread that changed the value. + *

+ * An observer is expected to be added only once. If the same observer is added again it will be notified just + * once on the thread of the last handler. + * + * @param observer the Observer + * @param handler a Handler for the thread to be notified on + */ + public void addObserver(Observer observer, Handler handler) { + localCallParticipantModelNotifier.addObserver(observer, handler); + } + + public void removeObserver(Observer observer) { + localCallParticipantModelNotifier.removeObserver(observer); + } +} diff --git a/app/src/main/java/com/nextcloud/talk/call/LocalCallParticipantModelNotifier.java b/app/src/main/java/com/nextcloud/talk/call/LocalCallParticipantModelNotifier.java new file mode 100644 index 0000000000..b46f1f0a7e --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/call/LocalCallParticipantModelNotifier.java @@ -0,0 +1,73 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call; + +import android.os.Handler; +import android.os.Looper; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +/** + * Helper class to register and notify LocalCallParticipantModel.Observers. + *

+ * This class is only meant for internal use by LocalCallParticipantModel; observers must register themselves against a + * LocalCallParticipantModel rather than against a LocalCallParticipantModelNotifier. + */ +class LocalCallParticipantModelNotifier { + + private final List localCallParticipantModelObserversOn = new ArrayList<>(); + + /** + * Helper class to associate a LocalCallParticipantModel.Observer with a Handler. + */ + private static class LocalCallParticipantModelObserverOn { + public final LocalCallParticipantModel.Observer observer; + public final Handler handler; + + private LocalCallParticipantModelObserverOn(LocalCallParticipantModel.Observer observer, Handler handler) { + this.observer = observer; + this.handler = handler; + } + } + + public synchronized void addObserver(LocalCallParticipantModel.Observer observer, Handler handler) { + if (observer == null) { + throw new IllegalArgumentException("LocalCallParticipantModel.Observer can not be null"); + } + + removeObserver(observer); + + localCallParticipantModelObserversOn.add(new LocalCallParticipantModelObserverOn(observer, handler)); + } + + public synchronized void removeObserver(LocalCallParticipantModel.Observer observer) { + Iterator it = localCallParticipantModelObserversOn.iterator(); + while (it.hasNext()) { + LocalCallParticipantModelObserverOn observerOn = it.next(); + + if (observerOn.observer == observer) { + it.remove(); + + return; + } + } + } + + public synchronized void notifyChange() { + for (LocalCallParticipantModelObserverOn observerOn : new ArrayList<>(localCallParticipantModelObserversOn)) { + if (observerOn.handler == null || observerOn.handler.getLooper() == Looper.myLooper()) { + observerOn.observer.onChange(); + } else { + observerOn.handler.post(() -> { + observerOn.observer.onChange(); + }); + } + } + } +} diff --git a/app/src/main/java/com/nextcloud/talk/call/MutableLocalCallParticipantModel.java b/app/src/main/java/com/nextcloud/talk/call/MutableLocalCallParticipantModel.java new file mode 100644 index 0000000000..91bbbfc9ff --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/call/MutableLocalCallParticipantModel.java @@ -0,0 +1,51 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call; + +import java.util.Objects; + +/** + * Mutable data model for local call participants. + *

+ * Setting "speaking" will automatically set "speaking" or "speakingWhileMuted" as needed, depending on whether audio is + * enabled or not. Similarly, setting whether the audio is enabled or disabled will automatically switch between + * "speaking" and "speakingWhileMuted" as needed. + *

+ * There is no synchronization when setting the values; if needed, it should be handled by the clients of the model. + */ +public class MutableLocalCallParticipantModel extends LocalCallParticipantModel { + + public void setAudioEnabled(Boolean audioEnabled) { + if (Objects.equals(this.audioEnabled.getValue(), audioEnabled)) { + return; + } + + if (audioEnabled == null || !audioEnabled) { + this.speakingWhileMuted.setValue(this.speaking.getValue()); + this.speaking.setValue(Boolean.FALSE); + } + + this.audioEnabled.setValue(audioEnabled); + + if (audioEnabled != null && audioEnabled) { + this.speaking.setValue(this.speakingWhileMuted.getValue()); + this.speakingWhileMuted.setValue(Boolean.FALSE); + } + } + + public void setSpeaking(Boolean speaking) { + if (this.audioEnabled.getValue() != null && this.audioEnabled.getValue()) { + this.speaking.setValue(speaking); + } else { + this.speakingWhileMuted.setValue(speaking); + } + } + + public void setVideoEnabled(Boolean videoEnabled) { + this.videoEnabled.setValue(videoEnabled); + } +} diff --git a/app/src/test/java/com/nextcloud/talk/call/LocalCallParticipantModelTest.kt b/app/src/test/java/com/nextcloud/talk/call/LocalCallParticipantModelTest.kt new file mode 100644 index 0000000000..2440fd0c1b --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/call/LocalCallParticipantModelTest.kt @@ -0,0 +1,168 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.mockito.Mockito + +class LocalCallParticipantModelTest { + private var localCallParticipantModel: MutableLocalCallParticipantModel? = null + private var mockedLocalCallParticipantModelObserver: LocalCallParticipantModel.Observer? = null + + @Before + fun setUp() { + localCallParticipantModel = MutableLocalCallParticipantModel() + mockedLocalCallParticipantModelObserver = Mockito.mock(LocalCallParticipantModel.Observer::class.java) + } + + @Test + fun testSetAudioEnabled() { + localCallParticipantModel!!.addObserver(mockedLocalCallParticipantModelObserver) + + localCallParticipantModel!!.isAudioEnabled = true + + assertTrue(localCallParticipantModel!!.isAudioEnabled) + assertFalse(localCallParticipantModel!!.isSpeaking) + assertFalse(localCallParticipantModel!!.isSpeakingWhileMuted) + Mockito.verify(mockedLocalCallParticipantModelObserver, Mockito.only())?.onChange() + } + + @Test + fun testSetAudioEnabledWhileSpeakingWhileMuted() { + localCallParticipantModel!!.isSpeaking = true + + localCallParticipantModel!!.addObserver(mockedLocalCallParticipantModelObserver) + + localCallParticipantModel!!.isAudioEnabled = true + + assertTrue(localCallParticipantModel!!.isAudioEnabled) + assertTrue(localCallParticipantModel!!.isSpeaking) + assertFalse(localCallParticipantModel!!.isSpeakingWhileMuted) + Mockito.verify(mockedLocalCallParticipantModelObserver, Mockito.times(3))?.onChange() + } + + @Test + fun testSetAudioEnabledTwiceWhileSpeakingWhileMuted() { + localCallParticipantModel!!.isSpeaking = true + + localCallParticipantModel!!.addObserver(mockedLocalCallParticipantModelObserver) + + localCallParticipantModel!!.isAudioEnabled = true + localCallParticipantModel!!.isAudioEnabled = true + + assertTrue(localCallParticipantModel!!.isAudioEnabled) + assertTrue(localCallParticipantModel!!.isSpeaking) + assertFalse(localCallParticipantModel!!.isSpeakingWhileMuted) + Mockito.verify(mockedLocalCallParticipantModelObserver, Mockito.times(3))?.onChange() + } + + @Test + fun testSetAudioDisabled() { + localCallParticipantModel!!.isAudioEnabled = true + + localCallParticipantModel!!.addObserver(mockedLocalCallParticipantModelObserver) + + localCallParticipantModel!!.isAudioEnabled = false + + assertFalse(localCallParticipantModel!!.isAudioEnabled) + assertFalse(localCallParticipantModel!!.isSpeaking) + assertFalse(localCallParticipantModel!!.isSpeakingWhileMuted) + Mockito.verify(mockedLocalCallParticipantModelObserver, Mockito.only())?.onChange() + } + + @Test + fun testSetAudioDisabledWhileSpeaking() { + localCallParticipantModel!!.isAudioEnabled = true + localCallParticipantModel!!.isSpeaking = true + + localCallParticipantModel!!.addObserver(mockedLocalCallParticipantModelObserver) + + localCallParticipantModel!!.isAudioEnabled = false + + assertFalse(localCallParticipantModel!!.isAudioEnabled) + assertFalse(localCallParticipantModel!!.isSpeaking) + assertTrue(localCallParticipantModel!!.isSpeakingWhileMuted) + Mockito.verify(mockedLocalCallParticipantModelObserver, Mockito.times(3))?.onChange() + } + + @Test + fun testSetAudioDisabledTwiceWhileSpeaking() { + localCallParticipantModel!!.isAudioEnabled = true + localCallParticipantModel!!.isSpeaking = true + + localCallParticipantModel!!.addObserver(mockedLocalCallParticipantModelObserver) + + localCallParticipantModel!!.isAudioEnabled = false + localCallParticipantModel!!.isAudioEnabled = false + + assertFalse(localCallParticipantModel!!.isAudioEnabled) + assertFalse(localCallParticipantModel!!.isSpeaking) + assertTrue(localCallParticipantModel!!.isSpeakingWhileMuted) + Mockito.verify(mockedLocalCallParticipantModelObserver, Mockito.times(3))?.onChange() + } + + @Test + fun testSetSpeakingWhileAudioEnabled() { + localCallParticipantModel!!.isAudioEnabled = true + + localCallParticipantModel!!.addObserver(mockedLocalCallParticipantModelObserver) + + localCallParticipantModel!!.isSpeaking = true + + assertTrue(localCallParticipantModel!!.isAudioEnabled) + assertTrue(localCallParticipantModel!!.isSpeaking) + assertFalse(localCallParticipantModel!!.isSpeakingWhileMuted) + Mockito.verify(mockedLocalCallParticipantModelObserver, Mockito.only())?.onChange() + } + + @Test + fun testSetNotSpeakingWhileAudioEnabled() { + localCallParticipantModel!!.isAudioEnabled = true + localCallParticipantModel!!.isSpeaking = true + + localCallParticipantModel!!.addObserver(mockedLocalCallParticipantModelObserver) + + localCallParticipantModel!!.isSpeaking = false + + assertTrue(localCallParticipantModel!!.isAudioEnabled) + assertFalse(localCallParticipantModel!!.isSpeaking) + assertFalse(localCallParticipantModel!!.isSpeakingWhileMuted) + Mockito.verify(mockedLocalCallParticipantModelObserver, Mockito.only())?.onChange() + } + + @Test + fun testSetSpeakingWhileAudioDisabled() { + localCallParticipantModel!!.isAudioEnabled = false + + localCallParticipantModel!!.addObserver(mockedLocalCallParticipantModelObserver) + + localCallParticipantModel!!.isSpeaking = true + + assertFalse(localCallParticipantModel!!.isAudioEnabled) + assertFalse(localCallParticipantModel!!.isSpeaking) + assertTrue(localCallParticipantModel!!.isSpeakingWhileMuted) + Mockito.verify(mockedLocalCallParticipantModelObserver, Mockito.only())?.onChange() + } + + @Test + fun testSetNotSpeakingWhileAudioDisabled() { + localCallParticipantModel!!.isAudioEnabled = false + localCallParticipantModel!!.isSpeaking = true + + localCallParticipantModel!!.addObserver(mockedLocalCallParticipantModelObserver) + + localCallParticipantModel!!.isSpeaking = false + + assertFalse(localCallParticipantModel!!.isAudioEnabled) + assertFalse(localCallParticipantModel!!.isSpeaking) + assertFalse(localCallParticipantModel!!.isSpeakingWhileMuted) + Mockito.verify(mockedLocalCallParticipantModelObserver, Mockito.only())?.onChange() + } +} From e8705e613b707619fa94e2de13212b97cf8df49a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sat, 30 Nov 2024 06:25:26 +0100 Subject: [PATCH 21/27] Add helper class to broadcast the local participant state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LocalStateBroadcaster observes changes in the LocalCallParticipantModel and notifies other participants in the call as needed. Although it is created right before joining the call there is a slim chance of the state changing before the local participant is actually in the call, but even in that case other participants would not be notified about the state due to the MessageSender depending on the list of call participants / peer connections passed to it, which should not be initialized before the local participant is actually in the call. There is, however, a race condition that could cause participants to not be added to the participant list if they join at the same time as the local participant and a signaling message listing them but not the local participant as in the call is received once the CallParticipantList was created, but that is unrelated to the broadcaster and will be fixed in another commit. Currently only changes in the audio, speaking and video state are notified, although in the future it should also notify about the nick, the raised hand or any other state (but not one-time events, like reactions). The notifications right now are sent only through data channels, but at a later point they will be sent also through signaling messages as needed. Similarly, although right now it only notifies of changes in the state it will also take care of notifying other participants about the current state when they join the call (or the local participant joins). Signed-off-by: Daniel Calviño Sánchez --- .../nextcloud/talk/activities/CallActivity.kt | 44 +-- .../talk/call/LocalStateBroadcaster.java | 102 +++++++ .../talk/call/LocalStateBroadcasterTest.kt | 260 ++++++++++++++++++ 3 files changed, 371 insertions(+), 35 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java create mode 100644 app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterTest.kt diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt index 91be62eafe..45c21f6142 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt @@ -63,6 +63,7 @@ import com.nextcloud.talk.application.NextcloudTalkApplication.Companion.sharedA import com.nextcloud.talk.call.CallParticipant import com.nextcloud.talk.call.CallParticipantList import com.nextcloud.talk.call.CallParticipantModel +import com.nextcloud.talk.call.LocalStateBroadcaster import com.nextcloud.talk.call.MessageSender import com.nextcloud.talk.call.MessageSenderMcu import com.nextcloud.talk.call.MessageSenderNoMcu @@ -248,6 +249,7 @@ class CallActivity : CallBaseActivity() { private var signalingMessageSender: SignalingMessageSender? = null private var messageSender: MessageSender? = null private val localCallParticipantModel: MutableLocalCallParticipantModel = MutableLocalCallParticipantModel() + private var localStateBroadcaster: LocalStateBroadcaster? = null private val offerAnswerNickProviders: MutableMap = HashMap() private val callParticipantMessageListeners: MutableMap = HashMap() private val selfPeerConnectionObserver: PeerConnectionObserver = CallActivitySelfPeerConnectionObserver() @@ -1142,7 +1144,6 @@ class CallActivity : CallBaseActivity() { @SuppressLint("MissingPermission") private fun startMicInputDetection() { if (permissionUtil!!.isMicrophonePermissionGranted() && micInputAudioRecordThread == null) { - var isSpeakingLongTerm = false micInputAudioRecorder = AudioRecord( MediaRecorder.AudioSource.MIC, SAMPLE_RATE, @@ -1159,15 +1160,8 @@ class CallActivity : CallBaseActivity() { micInputAudioRecorder.read(byteArr, 0, byteArr.size) val isCurrentlySpeaking = abs(byteArr[0].toDouble()) > MICROPHONE_VALUE_THRESHOLD - if (microphoneOn && isCurrentlySpeaking && !isSpeakingLongTerm) { - isSpeakingLongTerm = true - localCallParticipantModel.isSpeaking = true - sendIsSpeakingMessage(true) - } else if (!isCurrentlySpeaking && isSpeakingLongTerm) { - isSpeakingLongTerm = false - localCallParticipantModel.isSpeaking = false - sendIsSpeakingMessage(false) - } + localCallParticipantModel.isSpeaking = isCurrentlySpeaking + Thread.sleep(MICROPHONE_VALUE_SLEEP) } } @@ -1176,16 +1170,6 @@ class CallActivity : CallBaseActivity() { } } - @Suppress("Detekt.NestedBlockDepth") - private fun sendIsSpeakingMessage(isSpeaking: Boolean) { - val isSpeakingMessage: String = - if (isSpeaking) SIGNALING_MESSAGE_SPEAKING_STARTED else SIGNALING_MESSAGE_SPEAKING_STOPPED - - if (isConnectionEstablished && othersInCall) { - messageSender!!.sendToAll(DataChannelMessage(isSpeakingMessage)) - } - } - private fun createCameraCapturer(enumerator: CameraEnumerator?): VideoCapturer? { val deviceNames = enumerator!!.deviceNames @@ -1329,12 +1313,9 @@ class CallActivity : CallBaseActivity() { } private fun toggleMedia(enable: Boolean, video: Boolean) { - var message: String if (video) { - message = SIGNALING_MESSAGE_VIDEO_OFF if (enable) { binding!!.cameraButton.alpha = OPACITY_ENABLED - message = SIGNALING_MESSAGE_VIDEO_ON startVideoCapture() } else { binding!!.cameraButton.alpha = OPACITY_DISABLED @@ -1356,9 +1337,7 @@ class CallActivity : CallBaseActivity() { binding!!.selfVideoRenderer.visibility = View.INVISIBLE } } else { - message = SIGNALING_MESSAGE_AUDIO_OFF if (enable) { - message = SIGNALING_MESSAGE_AUDIO_ON binding!!.microphoneButton.alpha = OPACITY_ENABLED } else { binding!!.microphoneButton.alpha = OPACITY_DISABLED @@ -1368,9 +1347,6 @@ class CallActivity : CallBaseActivity() { localCallParticipantModel.isAudioEnabled = enable } } - if (isConnectionEstablished) { - messageSender!!.sendToAll(DataChannelMessage(message)) - } } fun clickRaiseOrLowerHandButton() { @@ -1752,6 +1728,8 @@ class CallActivity : CallBaseActivity() { callParticipantList = CallParticipantList(signalingMessageReceiver) callParticipantList!!.addObserver(callParticipantListObserver) + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, messageSender) + val apiVersion = ApiUtils.getCallApiVersion(conversationUser, intArrayOf(ApiUtils.API_V4, 1)) ncApi!!.joinCall( credentials, @@ -2104,6 +2082,9 @@ class CallActivity : CallBaseActivity() { private fun hangupNetworkCalls(shutDownView: Boolean, endCallForAll: Boolean) { Log.d(TAG, "hangupNetworkCalls. shutDownView=$shutDownView") val apiVersion = ApiUtils.getCallApiVersion(conversationUser, intArrayOf(ApiUtils.API_V4, 1)) + if (localStateBroadcaster != null) { + localStateBroadcaster!!.destroy() + } if (callParticipantList != null) { callParticipantList!!.removeObserver(callParticipantListObserver) callParticipantList!!.destroy() @@ -3286,12 +3267,5 @@ class CallActivity : CallBaseActivity() { private const val Y_POS_NO_CALL_INFO: Float = 20f private const val SESSION_ID_PREFFIX_END: Int = 4 - - private const val SIGNALING_MESSAGE_SPEAKING_STARTED = "speaking" - private const val SIGNALING_MESSAGE_SPEAKING_STOPPED = "stoppedSpeaking" - private const val SIGNALING_MESSAGE_VIDEO_ON = "videoOn" - private const val SIGNALING_MESSAGE_VIDEO_OFF = "videoOff" - private const val SIGNALING_MESSAGE_AUDIO_ON = "audioOn" - private const val SIGNALING_MESSAGE_AUDIO_OFF = "audioOff" } } diff --git a/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java new file mode 100644 index 0000000000..9ad0093f19 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java @@ -0,0 +1,102 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call; + +import com.nextcloud.talk.models.json.signaling.DataChannelMessage; + +import java.util.Objects; + +/** + * Helper class to send the local participant state to the other participants in the call. + *

+ * Once created, and until destroyed, the LocalStateBroadcaster will send the changes in the local participant state to + * all the participants in the call. Note that the LocalStateBroadcaster does not check whether the local participant + * is actually in the call or not; it is expected that the LocalStateBroadcaster will be created and destroyed when the + * local participant joins and leaves the call. + */ +public class LocalStateBroadcaster { + + private final LocalCallParticipantModel localCallParticipantModel; + + private final LocalCallParticipantModelObserver localCallParticipantModelObserver; + + private final MessageSender messageSender; + + private class LocalCallParticipantModelObserver implements LocalCallParticipantModel.Observer { + + private Boolean audioEnabled; + private Boolean speaking; + private Boolean videoEnabled; + + public LocalCallParticipantModelObserver(LocalCallParticipantModel localCallParticipantModel) { + audioEnabled = localCallParticipantModel.isAudioEnabled(); + speaking = localCallParticipantModel.isSpeaking(); + videoEnabled = localCallParticipantModel.isVideoEnabled(); + } + + @Override + public void onChange() { + if (!Objects.equals(audioEnabled, localCallParticipantModel.isAudioEnabled())) { + audioEnabled = localCallParticipantModel.isAudioEnabled(); + + messageSender.sendToAll(getDataChannelMessageForAudioState()); + } + + if (!Objects.equals(speaking, localCallParticipantModel.isSpeaking())) { + speaking = localCallParticipantModel.isSpeaking(); + + messageSender.sendToAll(getDataChannelMessageForSpeakingState()); + } + + if (!Objects.equals(videoEnabled, localCallParticipantModel.isVideoEnabled())) { + videoEnabled = localCallParticipantModel.isVideoEnabled(); + + messageSender.sendToAll(getDataChannelMessageForVideoState()); + } + } + } + + public LocalStateBroadcaster(LocalCallParticipantModel localCallParticipantModel, + MessageSender messageSender) { + this.localCallParticipantModel = localCallParticipantModel; + this.localCallParticipantModelObserver = new LocalCallParticipantModelObserver(localCallParticipantModel); + this.messageSender = messageSender; + + this.localCallParticipantModel.addObserver(localCallParticipantModelObserver); + } + + public void destroy() { + this.localCallParticipantModel.removeObserver(localCallParticipantModelObserver); + } + + private DataChannelMessage getDataChannelMessageForAudioState() { + String type = "audioOff"; + if (localCallParticipantModel.isAudioEnabled() != null && localCallParticipantModel.isAudioEnabled()) { + type = "audioOn"; + } + + return new DataChannelMessage(type); + } + + private DataChannelMessage getDataChannelMessageForSpeakingState() { + String type = "stoppedSpeaking"; + if (localCallParticipantModel.isSpeaking() != null && localCallParticipantModel.isSpeaking()) { + type = "speaking"; + } + + return new DataChannelMessage(type); + } + + private DataChannelMessage getDataChannelMessageForVideoState() { + String type = "videoOff"; + if (localCallParticipantModel.isVideoEnabled() != null && localCallParticipantModel.isVideoEnabled()) { + type = "videoOn"; + } + + return new DataChannelMessage(type); + } +} diff --git a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterTest.kt b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterTest.kt new file mode 100644 index 0000000000..a070ef946c --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterTest.kt @@ -0,0 +1,260 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call + +import com.nextcloud.talk.models.json.signaling.DataChannelMessage +import org.junit.Before +import org.junit.Test +import org.mockito.Mockito + +@Suppress("TooManyFunctions") +class LocalStateBroadcasterTest { + + private var localCallParticipantModel: MutableLocalCallParticipantModel? = null + private var mockedMessageSender: MessageSender? = null + + private var localStateBroadcaster: LocalStateBroadcaster? = null + + @Before + fun setUp() { + localCallParticipantModel = MutableLocalCallParticipantModel() + mockedMessageSender = Mockito.mock(MessageSender::class.java) + } + + @Test + fun testEnableAudio() { + localCallParticipantModel!!.isAudioEnabled = false + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isAudioEnabled = true + + val expectedAudioOn = DataChannelMessage("audioOn") + + Mockito.verify(mockedMessageSender!!).sendToAll(expectedAudioOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testEnableAudioTwice() { + localCallParticipantModel!!.isAudioEnabled = true + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isAudioEnabled = true + + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testDisableAudio() { + localCallParticipantModel!!.isAudioEnabled = true + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isAudioEnabled = false + + val expectedAudioOff = DataChannelMessage("audioOff") + + Mockito.verify(mockedMessageSender!!).sendToAll(expectedAudioOff) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testDisableAudioTwice() { + localCallParticipantModel!!.isAudioEnabled = false + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isAudioEnabled = false + + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testEnableSpeaking() { + localCallParticipantModel!!.isAudioEnabled = true + localCallParticipantModel!!.isSpeaking = false + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isSpeaking = true + + val expectedSpeaking = DataChannelMessage("speaking") + + Mockito.verify(mockedMessageSender!!).sendToAll(expectedSpeaking) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testEnableSpeakingTwice() { + localCallParticipantModel!!.isAudioEnabled = true + localCallParticipantModel!!.isSpeaking = true + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isSpeaking = true + + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testEnableSpeakingWithAudioDisabled() { + localCallParticipantModel!!.isAudioEnabled = false + localCallParticipantModel!!.isSpeaking = false + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isSpeaking = true + + Mockito.verifyNoInteractions(mockedMessageSender) + } + + @Test + fun testEnableAudioWhileSpeaking() { + localCallParticipantModel!!.isAudioEnabled = false + localCallParticipantModel!!.isSpeaking = false + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isSpeaking = true + localCallParticipantModel!!.isAudioEnabled = true + + val expectedAudioOn = DataChannelMessage("audioOn") + val expectedSpeaking = DataChannelMessage("speaking") + + val inOrder = Mockito.inOrder(mockedMessageSender) + + inOrder.verify(mockedMessageSender!!).sendToAll(expectedAudioOn) + inOrder.verify(mockedMessageSender!!).sendToAll(expectedSpeaking) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testDisableSpeaking() { + localCallParticipantModel!!.isAudioEnabled = true + localCallParticipantModel!!.isSpeaking = true + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isSpeaking = false + + val expectedStoppedSpeaking = DataChannelMessage("stoppedSpeaking") + + Mockito.verify(mockedMessageSender!!).sendToAll(expectedStoppedSpeaking) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testDisableSpeakingTwice() { + localCallParticipantModel!!.isAudioEnabled = true + localCallParticipantModel!!.isSpeaking = false + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isSpeaking = false + + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testDisableAudioWhileSpeaking() { + localCallParticipantModel!!.isAudioEnabled = true + localCallParticipantModel!!.isSpeaking = true + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isAudioEnabled = false + + val expectedStoppedSpeaking = DataChannelMessage("stoppedSpeaking") + val expectedAudioOff = DataChannelMessage("audioOff") + + val inOrder = Mockito.inOrder(mockedMessageSender) + + inOrder.verify(mockedMessageSender!!).sendToAll(expectedStoppedSpeaking) + inOrder.verify(mockedMessageSender!!).sendToAll(expectedAudioOff) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testDisableSpeakingWithAudioDisabled() { + localCallParticipantModel!!.isAudioEnabled = false + localCallParticipantModel!!.isSpeaking = true + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isSpeaking = false + + Mockito.verifyNoInteractions(mockedMessageSender) + } + + @Test + fun testEnableVideo() { + localCallParticipantModel!!.isVideoEnabled = false + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isVideoEnabled = true + + val expectedVideoOn = DataChannelMessage("videoOn") + + Mockito.verify(mockedMessageSender!!).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testEnableVideoTwice() { + localCallParticipantModel!!.isVideoEnabled = true + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isVideoEnabled = true + + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testDisableVideo() { + localCallParticipantModel!!.isVideoEnabled = true + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isVideoEnabled = false + + val expectedVideoOff = DataChannelMessage("videoOff") + + Mockito.verify(mockedMessageSender!!).sendToAll(expectedVideoOff) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testDisableVideoTwice() { + localCallParticipantModel!!.isVideoEnabled = false + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localCallParticipantModel!!.isVideoEnabled = false + + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testChangeStateAfterDestroying() { + localCallParticipantModel!!.isAudioEnabled = false + localCallParticipantModel!!.isSpeaking = false + localCallParticipantModel!!.isVideoEnabled = false + + localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, mockedMessageSender) + + localStateBroadcaster!!.destroy() + localCallParticipantModel!!.isAudioEnabled = true + localCallParticipantModel!!.isSpeaking = true + localCallParticipantModel!!.isVideoEnabled = true + + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } +} From 5fe9eec3227a5d798b5e4acbdc0e8c594366a812 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 5 Dec 2024 03:40:11 +0100 Subject: [PATCH 22/27] Add support for sending data channel messages to a single participant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is not possible when Janus is used, as Janus only allows broadcasting data channel messages to all the subscribers of the publisher connection. Signed-off-by: Daniel Calviño Sánchez --- .../nextcloud/talk/call/MessageSenderMcu.java | 3 ++ .../talk/call/MessageSenderNoMcu.java | 13 ++++++++ .../talk/call/MessageSenderNoMcuTest.kt | 33 +++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/app/src/main/java/com/nextcloud/talk/call/MessageSenderMcu.java b/app/src/main/java/com/nextcloud/talk/call/MessageSenderMcu.java index 803becf57b..f3278fe4c1 100644 --- a/app/src/main/java/com/nextcloud/talk/call/MessageSenderMcu.java +++ b/app/src/main/java/com/nextcloud/talk/call/MessageSenderMcu.java @@ -13,6 +13,9 @@ /** * Helper class to send messages to participants in a call when an MCU is used. + *

+ * Note that when Janus is used it is not possible to send a data channel message to a specific participant. Any data + * channel message will be broadcast to all the subscribers of the publisher peer connection (the own peer connection). */ public class MessageSenderMcu extends MessageSender { diff --git a/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java b/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java index 6f4306262e..b5b9035032 100644 --- a/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java +++ b/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java @@ -20,6 +20,19 @@ public MessageSenderNoMcu(List peerConnectionWrappers) { super(peerConnectionWrappers); } + /** + * Sends the given data channel message to the given signaling session ID. + * + * @param dataChannelMessage the message to send + * @param sessionId the signaling session ID of the participant to send the message to + */ + public void send(DataChannelMessage dataChannelMessage, String sessionId) { + PeerConnectionWrapper peerConnectionWrapper = getPeerConnectionWrapper(sessionId); + if (peerConnectionWrapper != null) { + peerConnectionWrapper.send(dataChannelMessage); + } + } + public void sendToAll(DataChannelMessage dataChannelMessage) { for (PeerConnectionWrapper peerConnectionWrapper: peerConnectionWrappers) { if ("video".equals(peerConnectionWrapper.getVideoStreamType())){ diff --git a/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt b/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt index cfd4a850d0..6784bcef37 100644 --- a/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt +++ b/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt @@ -50,6 +50,39 @@ class MessageSenderNoMcuTest { messageSender = MessageSenderNoMcu(peerConnectionWrappers) } + @Test + fun testSendDataChannelMessage() { + val message = DataChannelMessage() + messageSender!!.send(message, "theSessionId2") + + Mockito.verify(peerConnectionWrapper2!!).send(message) + Mockito.verify(peerConnectionWrapper1!!, never()).send(message) + Mockito.verify(peerConnectionWrapper2Screen!!, never()).send(message) + Mockito.verify(peerConnectionWrapper4Screen!!, never()).send(message) + } + + @Test + fun testSendDataChannelMessageIfScreenPeerConnection() { + val message = DataChannelMessage() + messageSender!!.send(message, "theSessionId4") + + Mockito.verify(peerConnectionWrapper1!!, never()).send(message) + Mockito.verify(peerConnectionWrapper2!!, never()).send(message) + Mockito.verify(peerConnectionWrapper2Screen!!, never()).send(message) + Mockito.verify(peerConnectionWrapper4Screen!!, never()).send(message) + } + + @Test + fun testSendDataChannelMessageIfNoPeerConnection() { + val message = DataChannelMessage() + messageSender!!.send(message, "theSessionId3") + + Mockito.verify(peerConnectionWrapper1!!, never()).send(message) + Mockito.verify(peerConnectionWrapper2!!, never()).send(message) + Mockito.verify(peerConnectionWrapper2Screen!!, never()).send(message) + Mockito.verify(peerConnectionWrapper4Screen!!, never()).send(message) + } + @Test fun testSendDataChannelMessageToAll() { val message = DataChannelMessage() From c1303b1ce659447aaf8e67dc5564ac014a3e0bfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 11 Dec 2024 05:53:15 +0100 Subject: [PATCH 23/27] Send current state to remote participants when they join MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note that this implicitly send the current state to remote participants when the local participant joins, as in that case all the remote participants already in the call join from the point of view of the local participant Signed-off-by: Daniel Calviño Sánchez --- .../nextcloud/talk/activities/CallActivity.kt | 17 +- .../talk/call/LocalStateBroadcaster.java | 15 +- .../talk/call/LocalStateBroadcasterMcu.java | 72 +++ .../talk/call/LocalStateBroadcasterNoMcu.java | 119 +++++ .../talk/webrtc/PeerConnectionWrapper.java | 25 - .../talk/call/LocalStateBroadcasterMcuTest.kt | 439 ++++++++++++++++++ .../call/LocalStateBroadcasterNoMcuTest.kt | 304 ++++++++++++ .../talk/call/LocalStateBroadcasterTest.kt | 14 + 8 files changed, 975 insertions(+), 30 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterMcu.java create mode 100644 app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcu.java create mode 100644 app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterMcuTest.kt create mode 100644 app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcuTest.kt diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt index 45c21f6142..4a4d8e874f 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt @@ -64,6 +64,8 @@ import com.nextcloud.talk.call.CallParticipant import com.nextcloud.talk.call.CallParticipantList import com.nextcloud.talk.call.CallParticipantModel import com.nextcloud.talk.call.LocalStateBroadcaster +import com.nextcloud.talk.call.LocalStateBroadcasterMcu +import com.nextcloud.talk.call.LocalStateBroadcasterNoMcu import com.nextcloud.talk.call.MessageSender import com.nextcloud.talk.call.MessageSenderMcu import com.nextcloud.talk.call.MessageSenderNoMcu @@ -1728,7 +1730,14 @@ class CallActivity : CallBaseActivity() { callParticipantList = CallParticipantList(signalingMessageReceiver) callParticipantList!!.addObserver(callParticipantListObserver) - localStateBroadcaster = LocalStateBroadcaster(localCallParticipantModel, messageSender) + if (hasMCU) { + localStateBroadcaster = LocalStateBroadcasterMcu(localCallParticipantModel, messageSender) + } else { + localStateBroadcaster = LocalStateBroadcasterNoMcu( + localCallParticipantModel, + messageSender as MessageSenderNoMcu + ) + } val apiVersion = ApiUtils.getCallApiVersion(conversationUser, intArrayOf(ApiUtils.API_V4, 1)) ncApi!!.joinCall( @@ -2425,6 +2434,9 @@ class CallActivity : CallBaseActivity() { callParticipantEventDisplayers[sessionId] = callParticipantEventDisplayer callParticipantModel.addObserver(callParticipantEventDisplayer, callParticipantEventDisplayersHandler) runOnUiThread { addParticipantDisplayItem(callParticipantModel, "video") } + + localStateBroadcaster!!.handleCallParticipantAdded(callParticipant.callParticipantModel) + return callParticipant } @@ -2450,6 +2462,9 @@ class CallActivity : CallBaseActivity() { private fun removeCallParticipant(sessionId: String?) { val callParticipant = callParticipants.remove(sessionId) ?: return + + localStateBroadcaster!!.handleCallParticipantRemoved(callParticipant.callParticipantModel) + val screenParticipantDisplayItemManager = screenParticipantDisplayItemManagers.remove(sessionId) callParticipant.callParticipantModel.removeObserver(screenParticipantDisplayItemManager) val callParticipantEventDisplayer = callParticipantEventDisplayers.remove(sessionId) diff --git a/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java index 9ad0093f19..0376312970 100644 --- a/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java +++ b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java @@ -17,8 +17,12 @@ * all the participants in the call. Note that the LocalStateBroadcaster does not check whether the local participant * is actually in the call or not; it is expected that the LocalStateBroadcaster will be created and destroyed when the * local participant joins and leaves the call. + *

+ * The LocalStateBroadcaster also sends the current state to remote participants when they join (which implicitly + * sends it to all remote participants when the local participant joins the call) so they can set an initial state + * for the local participant. */ -public class LocalStateBroadcaster { +public abstract class LocalStateBroadcaster { private final LocalCallParticipantModel localCallParticipantModel; @@ -73,7 +77,10 @@ public void destroy() { this.localCallParticipantModel.removeObserver(localCallParticipantModelObserver); } - private DataChannelMessage getDataChannelMessageForAudioState() { + public abstract void handleCallParticipantAdded(CallParticipantModel callParticipantModel); + public abstract void handleCallParticipantRemoved(CallParticipantModel callParticipantModel); + + protected DataChannelMessage getDataChannelMessageForAudioState() { String type = "audioOff"; if (localCallParticipantModel.isAudioEnabled() != null && localCallParticipantModel.isAudioEnabled()) { type = "audioOn"; @@ -82,7 +89,7 @@ private DataChannelMessage getDataChannelMessageForAudioState() { return new DataChannelMessage(type); } - private DataChannelMessage getDataChannelMessageForSpeakingState() { + protected DataChannelMessage getDataChannelMessageForSpeakingState() { String type = "stoppedSpeaking"; if (localCallParticipantModel.isSpeaking() != null && localCallParticipantModel.isSpeaking()) { type = "speaking"; @@ -91,7 +98,7 @@ private DataChannelMessage getDataChannelMessageForSpeakingState() { return new DataChannelMessage(type); } - private DataChannelMessage getDataChannelMessageForVideoState() { + protected DataChannelMessage getDataChannelMessageForVideoState() { String type = "videoOff"; if (localCallParticipantModel.isVideoEnabled() != null && localCallParticipantModel.isVideoEnabled()) { type = "videoOn"; diff --git a/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterMcu.java b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterMcu.java new file mode 100644 index 0000000000..628b615375 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterMcu.java @@ -0,0 +1,72 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call; + +import java.util.concurrent.TimeUnit; + +import io.reactivex.Observable; +import io.reactivex.disposables.Disposable; +import io.reactivex.schedulers.Schedulers; + +/** + * Helper class to send the local participant state to the other participants in the call when an MCU is used. + *

+ * Sending the state when it changes is handled by the base class; this subclass only handles sending the initial + * state when a remote participant is added. + *

+ * When Janus is used data channel messages are sent to all remote participants (with a peer connection to receive from + * the local participant). Moreover, it is not possible to know when the remote participants open the data channel to + * receive the messages, or even when they establish the receiver connection; it is only possible to know when the + * data channel is open for the publisher connection of the local participant. Due to all that the state is sent + * several times with an increasing delay whenever a participant joins the call (which implicitly broadcasts the + * initial state when the local participant joins the call, as all the remote participants joined from the point of + * view of the local participant). If the state was already being sent the sending is restarted with each new + * participant that joins. + */ +public class LocalStateBroadcasterMcu extends LocalStateBroadcaster { + + private final MessageSender messageSender; + + private Disposable sendStateWithRepetition; + + public LocalStateBroadcasterMcu(LocalCallParticipantModel localCallParticipantModel, + MessageSender messageSender) { + super(localCallParticipantModel, messageSender); + + this.messageSender = messageSender; + } + + public void destroy() { + super.destroy(); + + if (sendStateWithRepetition != null) { + sendStateWithRepetition.dispose(); + } + } + + @Override + public void handleCallParticipantAdded(CallParticipantModel callParticipantModel) { + if (sendStateWithRepetition != null) { + sendStateWithRepetition.dispose(); + } + + sendStateWithRepetition = Observable + .fromArray(new Integer[]{0, 1, 2, 4, 8, 16}) + .concatMap(i -> Observable.just(i).delay(i, TimeUnit.SECONDS, Schedulers.io())) + .subscribe(value -> sendState()); + } + + @Override + public void handleCallParticipantRemoved(CallParticipantModel callParticipantModel) { + } + + private void sendState() { + messageSender.sendToAll(getDataChannelMessageForAudioState()); + messageSender.sendToAll(getDataChannelMessageForSpeakingState()); + messageSender.sendToAll(getDataChannelMessageForVideoState()); + } +} diff --git a/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcu.java b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcu.java new file mode 100644 index 0000000000..2a1bf04ea0 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcu.java @@ -0,0 +1,119 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call; + +import org.webrtc.PeerConnection; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +/** + * Helper class to send the local participant state to the other participants in the call when an MCU is not used. + *

+ * Sending the state when it changes is handled by the base class; this subclass only handles sending the initial + * state when a remote participant is added. + *

+ * The state is sent when a connection with another participant is first established (which implicitly broadcasts the + * initial state when the local participant joins the call, as a connection is established with all the remote + * participants). Note that, as long as that participant stays in the call, the initial state is not sent again, even + * after a temporary disconnection; data channels use a reliable transport by default, so even if the state changes + * while the connection is temporarily interrupted the normal state update messages should be received by the other + * participant once the connection is restored. + */ +public class LocalStateBroadcasterNoMcu extends LocalStateBroadcaster { + + private final MessageSenderNoMcu messageSender; + + private final Map iceConnectionStateObservers = new HashMap<>(); + + private class IceConnectionStateObserver implements CallParticipantModel.Observer { + + private final CallParticipantModel callParticipantModel; + + private PeerConnection.IceConnectionState iceConnectionState; + + public IceConnectionStateObserver(CallParticipantModel callParticipantModel) { + this.callParticipantModel = callParticipantModel; + + callParticipantModel.addObserver(this); + iceConnectionStateObservers.put(callParticipantModel.getSessionId(), this); + } + + @Override + public void onChange() { + if (Objects.equals(iceConnectionState, callParticipantModel.getIceConnectionState())) { + return; + } + + iceConnectionState = callParticipantModel.getIceConnectionState(); + + if (iceConnectionState == PeerConnection.IceConnectionState.CONNECTED || + iceConnectionState == PeerConnection.IceConnectionState.COMPLETED) { + remove(); + + sendState(callParticipantModel.getSessionId()); + } + } + + @Override + public void onReaction(String reaction) { + } + + public void remove() { + callParticipantModel.removeObserver(this); + iceConnectionStateObservers.remove(callParticipantModel.getSessionId()); + } + } + + public LocalStateBroadcasterNoMcu(LocalCallParticipantModel localCallParticipantModel, + MessageSenderNoMcu messageSender) { + super(localCallParticipantModel, messageSender); + + this.messageSender = messageSender; + } + + public void destroy() { + super.destroy(); + + // The observers remove themselves from the map, so a copy is needed to remove them while iterating. + List iceConnectionStateObserversCopy = + new ArrayList<>(iceConnectionStateObservers.values()); + for (IceConnectionStateObserver iceConnectionStateObserver : iceConnectionStateObserversCopy) { + iceConnectionStateObserver.remove(); + } + } + + @Override + public void handleCallParticipantAdded(CallParticipantModel callParticipantModel) { + IceConnectionStateObserver iceConnectionStateObserver = + iceConnectionStateObservers.get(callParticipantModel.getSessionId()); + if (iceConnectionStateObserver != null) { + iceConnectionStateObserver.remove(); + } + + iceConnectionStateObserver = new IceConnectionStateObserver(callParticipantModel); + iceConnectionStateObservers.put(callParticipantModel.getSessionId(), iceConnectionStateObserver); + } + + @Override + public void handleCallParticipantRemoved(CallParticipantModel callParticipantModel) { + IceConnectionStateObserver iceConnectionStateObserver = + iceConnectionStateObservers.get(callParticipantModel.getSessionId()); + if (iceConnectionStateObserver != null) { + iceConnectionStateObserver.remove(); + } + } + + private void sendState(String sessionId) { + messageSender.send(getDataChannelMessageForAudioState(), sessionId); + messageSender.send(getDataChannelMessageForSpeakingState(), sessionId); + messageSender.send(getDataChannelMessageForVideoState(), sessionId); + } +} 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 9f2a90f859..8929af1c0d 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -329,22 +329,6 @@ public String getSessionId() { return sessionId; } - private void sendInitialMediaStatus() { - if (localStream != null) { - if (localStream.videoTracks.size() == 1 && localStream.videoTracks.get(0).enabled()) { - send(new DataChannelMessage("videoOn")); - } else { - send(new DataChannelMessage("videoOff")); - } - - if (localStream.audioTracks.size() == 1 && localStream.audioTracks.get(0).enabled()) { - send(new DataChannelMessage("audioOn")); - } else { - send(new DataChannelMessage("audioOff")); - } - } - } - public boolean isMCUPublisher() { return isMCUPublisher; } @@ -432,10 +416,6 @@ public void onStateChange() { } pendingDataChannelMessages.clear(); } - - if (dataChannel.state() == DataChannel.State.OPEN) { - sendInitialMediaStatus(); - } } } @@ -523,11 +503,6 @@ public void onSignalingChange(PeerConnection.SignalingState signalingState) { public void onIceConnectionChange(PeerConnection.IceConnectionState iceConnectionState) { Log.d("iceConnectionChangeTo: ", iceConnectionState.name() + " over " + peerConnection.hashCode() + " " + sessionId); - if (iceConnectionState == PeerConnection.IceConnectionState.CONNECTED) { - if (hasInitiated) { - sendInitialMediaStatus(); - } - } peerConnectionNotifier.notifyIceConnectionStateChanged(iceConnectionState); } diff --git a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterMcuTest.kt b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterMcuTest.kt new file mode 100644 index 0000000000..aafeb2e512 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterMcuTest.kt @@ -0,0 +1,439 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call + +import com.nextcloud.talk.models.json.signaling.DataChannelMessage +import io.reactivex.plugins.RxJavaPlugins +import io.reactivex.schedulers.TestScheduler +import org.junit.Before +import org.junit.Test +import org.mockito.Mockito +import org.mockito.Mockito.times +import java.util.concurrent.TimeUnit + +@Suppress("LongMethod") +class LocalStateBroadcasterMcuTest { + + private var localCallParticipantModel: MutableLocalCallParticipantModel? = null + private var mockedMessageSender: MessageSender? = null + + private var localStateBroadcasterMcu: LocalStateBroadcasterMcu? = null + + @Before + fun setUp() { + localCallParticipantModel = MutableLocalCallParticipantModel() + localCallParticipantModel!!.isAudioEnabled = true + localCallParticipantModel!!.isSpeaking = true + localCallParticipantModel!!.isVideoEnabled = true + mockedMessageSender = Mockito.mock(MessageSender::class.java) + } + + @Test + fun testStateSentWithExponentialBackoffWhenParticipantAdded() { + val testScheduler = TestScheduler() + RxJavaPlugins.setIoSchedulerHandler { testScheduler } + + localStateBroadcasterMcu = LocalStateBroadcasterMcu( + localCallParticipantModel, + mockedMessageSender + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + + localStateBroadcasterMcu!!.handleCallParticipantAdded(callParticipantModel) + + // Sending will be done in another thread, so just adding the participant does not send anything until that + // other thread could run. + Mockito.verifyNoInteractions(mockedMessageSender) + + val expectedAudioOn = DataChannelMessage("audioOn") + val expectedSpeaking = DataChannelMessage("speaking") + val expectedVideoOn = DataChannelMessage("videoOn") + + testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) + + var messageCount = 1 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) + + messageCount = 2 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) + + messageCount = 3 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) + + messageCount = 4 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(8, TimeUnit.SECONDS) + + messageCount = 5 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(16, TimeUnit.SECONDS) + + messageCount = 6 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) + + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testStateSentWithExponentialBackoffIsTheCurrentState() { + // This test could have been included in "testStateSentWithExponentialBackoffWhenParticipantAdded", but was + // kept separate for clarity. + + val testScheduler = TestScheduler() + RxJavaPlugins.setIoSchedulerHandler { testScheduler } + + localStateBroadcasterMcu = LocalStateBroadcasterMcu( + localCallParticipantModel, + mockedMessageSender + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + + localStateBroadcasterMcu!!.handleCallParticipantAdded(callParticipantModel) + + // Sending will be done in another thread, so just adding the participant does not send anything until that + // other thread could run. + Mockito.verifyNoInteractions(mockedMessageSender) + + val expectedAudioOn = DataChannelMessage("audioOn") + val expectedSpeaking = DataChannelMessage("speaking") + val expectedVideoOn = DataChannelMessage("videoOn") + + testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) + + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + localCallParticipantModel!!.isSpeaking = false + + val expectedStoppedSpeaking = DataChannelMessage("stoppedSpeaking") + + // Changing the state causes the normal state update to be sent, independently of the initial state + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedStoppedSpeaking) + + testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) + + Mockito.verify(mockedMessageSender!!, times(2)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(2)).sendToAll(expectedStoppedSpeaking) + Mockito.verify(mockedMessageSender!!, times(2)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + localCallParticipantModel!!.isAudioEnabled = false + + val expectedAudioOff = DataChannelMessage("audioOff") + + // Changing the state causes the normal state update to be sent, independently of the initial state + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedAudioOff) + + testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) + + Mockito.verify(mockedMessageSender!!, times(2)).sendToAll(expectedAudioOff) + Mockito.verify(mockedMessageSender!!, times(3)).sendToAll(expectedStoppedSpeaking) + Mockito.verify(mockedMessageSender!!, times(3)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + localCallParticipantModel!!.isVideoEnabled = false + + val expectedVideoOff = DataChannelMessage("videoOff") + + // Changing the state causes the normal state update to be sent, independently of the initial state + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedVideoOff) + + testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) + + Mockito.verify(mockedMessageSender!!, times(3)).sendToAll(expectedAudioOff) + Mockito.verify(mockedMessageSender!!, times(4)).sendToAll(expectedStoppedSpeaking) + Mockito.verify(mockedMessageSender!!, times(2)).sendToAll(expectedVideoOff) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + localCallParticipantModel!!.isVideoEnabled = true + + // Changing the state causes the normal state update to be sent, independently of the initial state + Mockito.verify(mockedMessageSender!!, times(4)).sendToAll(expectedVideoOn) + + testScheduler.advanceTimeBy(8, TimeUnit.SECONDS) + + Mockito.verify(mockedMessageSender!!, times(4)).sendToAll(expectedAudioOff) + Mockito.verify(mockedMessageSender!!, times(5)).sendToAll(expectedStoppedSpeaking) + Mockito.verify(mockedMessageSender!!, times(5)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testStateSentWithExponentialBackoffRestartedWhenAnotherParticipantAdded() { + val testScheduler = TestScheduler() + RxJavaPlugins.setIoSchedulerHandler { testScheduler } + + localStateBroadcasterMcu = LocalStateBroadcasterMcu( + localCallParticipantModel, + mockedMessageSender + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + + localStateBroadcasterMcu!!.handleCallParticipantAdded(callParticipantModel) + + // Sending will be done in another thread, so just adding the participant does not send anything until that + // other thread could run. + Mockito.verifyNoInteractions(mockedMessageSender) + + val expectedAudioOn = DataChannelMessage("audioOn") + val expectedSpeaking = DataChannelMessage("speaking") + val expectedVideoOn = DataChannelMessage("videoOn") + + testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) + + var messageCount = 1 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) + + messageCount = 2 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) + + messageCount = 3 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) + + messageCount = 4 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + val callParticipantModel2 = MutableCallParticipantModel("theSessionId2") + + localStateBroadcasterMcu!!.handleCallParticipantAdded(callParticipantModel2) + + testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) + + messageCount = 5 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) + + messageCount = 6 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) + + messageCount = 7 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) + + messageCount = 8 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(8, TimeUnit.SECONDS) + + messageCount = 9 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(16, TimeUnit.SECONDS) + + messageCount = 10 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) + + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testStateStillSentWithExponentialBackoffWhenParticipantRemoved() { + // For simplicity the exponential backoff is not aborted when the participant that triggered it is removed. + + val testScheduler = TestScheduler() + RxJavaPlugins.setIoSchedulerHandler { testScheduler } + + localStateBroadcasterMcu = LocalStateBroadcasterMcu( + localCallParticipantModel, + mockedMessageSender + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + + localStateBroadcasterMcu!!.handleCallParticipantAdded(callParticipantModel) + + // Sending will be done in another thread, so just adding the participant does not send anything until that + // other thread could run. + Mockito.verifyNoInteractions(mockedMessageSender) + + val expectedAudioOn = DataChannelMessage("audioOn") + val expectedSpeaking = DataChannelMessage("speaking") + val expectedVideoOn = DataChannelMessage("videoOn") + + testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) + + var messageCount = 1 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) + + messageCount = 2 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) + + messageCount = 3 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) + + messageCount = 4 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + localStateBroadcasterMcu!!.handleCallParticipantRemoved(callParticipantModel) + + testScheduler.advanceTimeBy(8, TimeUnit.SECONDS) + + messageCount = 5 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(16, TimeUnit.SECONDS) + + messageCount = 6 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) + + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } + + @Test + fun testStateNoLongerSentOnceDestroyed() { + val testScheduler = TestScheduler() + RxJavaPlugins.setIoSchedulerHandler { testScheduler } + + localStateBroadcasterMcu = LocalStateBroadcasterMcu( + localCallParticipantModel, + mockedMessageSender + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + + localStateBroadcasterMcu!!.handleCallParticipantAdded(callParticipantModel) + + // Sending will be done in another thread, so just adding the participant does not send anything until that + // other thread could run. + Mockito.verifyNoInteractions(mockedMessageSender) + + val expectedAudioOn = DataChannelMessage("audioOn") + val expectedSpeaking = DataChannelMessage("speaking") + val expectedVideoOn = DataChannelMessage("videoOn") + + testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) + + var messageCount = 1 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) + + messageCount = 2 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) + + messageCount = 3 + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + localStateBroadcasterMcu!!.destroy() + + testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) + + Mockito.verifyNoMoreInteractions(mockedMessageSender) + } +} diff --git a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcuTest.kt b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcuTest.kt new file mode 100644 index 0000000000..2075785fdd --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcuTest.kt @@ -0,0 +1,304 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call + +import com.nextcloud.talk.models.json.signaling.DataChannelMessage +import org.junit.Before +import org.junit.Test +import org.mockito.Mockito +import org.webrtc.PeerConnection + +class LocalStateBroadcasterNoMcuTest { + + private var localCallParticipantModel: MutableLocalCallParticipantModel? = null + private var mockedMessageSenderNoMcu: MessageSenderNoMcu? = null + + private var localStateBroadcasterNoMcu: LocalStateBroadcasterNoMcu? = null + + @Before + fun setUp() { + localCallParticipantModel = MutableLocalCallParticipantModel() + localCallParticipantModel!!.isAudioEnabled = true + localCallParticipantModel!!.isSpeaking = true + localCallParticipantModel!!.isVideoEnabled = true + mockedMessageSenderNoMcu = Mockito.mock(MessageSenderNoMcu::class.java) + } + + @Test + fun testStateSentWhenIceConnected() { + localStateBroadcasterNoMcu = LocalStateBroadcasterNoMcu( + localCallParticipantModel, + mockedMessageSenderNoMcu + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + + localStateBroadcasterNoMcu!!.handleCallParticipantAdded(callParticipantModel) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CHECKING) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CONNECTED) + + val expectedAudioOn = DataChannelMessage("audioOn") + val expectedSpeaking = DataChannelMessage("speaking") + val expectedVideoOn = DataChannelMessage("videoOn") + + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedAudioOn, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedSpeaking, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedVideoOn, "theSessionId") + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + } + + @Test + fun testStateSentWhenIceCompleted() { + localStateBroadcasterNoMcu = LocalStateBroadcasterNoMcu( + localCallParticipantModel, + mockedMessageSenderNoMcu + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + + localStateBroadcasterNoMcu!!.handleCallParticipantAdded(callParticipantModel) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CHECKING) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.COMPLETED) + + val expectedAudioOn = DataChannelMessage("audioOn") + val expectedSpeaking = DataChannelMessage("speaking") + val expectedVideoOn = DataChannelMessage("videoOn") + + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedAudioOn, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedSpeaking, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedVideoOn, "theSessionId") + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + } + + @Test + fun testStateNotSentWhenIceCompletedAfterConnected() { + localStateBroadcasterNoMcu = LocalStateBroadcasterNoMcu( + localCallParticipantModel, + mockedMessageSenderNoMcu + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + + localStateBroadcasterNoMcu!!.handleCallParticipantAdded(callParticipantModel) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CHECKING) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CONNECTED) + + val expectedAudioOn = DataChannelMessage("audioOn") + val expectedSpeaking = DataChannelMessage("speaking") + val expectedVideoOn = DataChannelMessage("videoOn") + + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedAudioOn, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedSpeaking, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedVideoOn, "theSessionId") + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.COMPLETED) + + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + } + + @Test + fun testStateNotSentWhenIceConnectedAgain() { + localStateBroadcasterNoMcu = LocalStateBroadcasterNoMcu( + localCallParticipantModel, + mockedMessageSenderNoMcu + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + + localStateBroadcasterNoMcu!!.handleCallParticipantAdded(callParticipantModel) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CHECKING) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CONNECTED) + + val expectedAudioOn = DataChannelMessage("audioOn") + val expectedSpeaking = DataChannelMessage("speaking") + val expectedVideoOn = DataChannelMessage("videoOn") + + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedAudioOn, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedSpeaking, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedVideoOn, "theSessionId") + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.COMPLETED) + + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + + // Completed -> Connected could happen with an ICE restart + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CONNECTED) + + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.DISCONNECTED) + + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CONNECTED) + + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + + // Failed -> Checking could happen with an ICE restart + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.FAILED) + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CHECKING) + + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CONNECTED) + + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + } + + @Test + fun testStateNotSentToOtherParticipantsWhenIceConnected() { + localStateBroadcasterNoMcu = LocalStateBroadcasterNoMcu( + localCallParticipantModel, + mockedMessageSenderNoMcu + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + val callParticipantModel2 = MutableCallParticipantModel("theSessionId2") + + localStateBroadcasterNoMcu!!.handleCallParticipantAdded(callParticipantModel) + localStateBroadcasterNoMcu!!.handleCallParticipantAdded(callParticipantModel2) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CHECKING) + callParticipantModel2.setIceConnectionState(PeerConnection.IceConnectionState.CHECKING) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CONNECTED) + + val expectedAudioOn = DataChannelMessage("audioOn") + val expectedSpeaking = DataChannelMessage("speaking") + val expectedVideoOn = DataChannelMessage("videoOn") + + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedAudioOn, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedSpeaking, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedVideoOn, "theSessionId") + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + + callParticipantModel2.setIceConnectionState(PeerConnection.IceConnectionState.CONNECTED) + + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedAudioOn, "theSessionId2") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedSpeaking, "theSessionId2") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedVideoOn, "theSessionId2") + Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) + } + + @Test + fun testStateNotSentWhenIceConnectedAfterParticipantIsRemoved() { + // This should not happen, as peer connections are expected to be ended when a call participant is removed, but + // just in case. + + localStateBroadcasterNoMcu = LocalStateBroadcasterNoMcu( + localCallParticipantModel, + mockedMessageSenderNoMcu + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + + localStateBroadcasterNoMcu!!.handleCallParticipantAdded(callParticipantModel) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CHECKING) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + + localStateBroadcasterNoMcu!!.handleCallParticipantRemoved(callParticipantModel) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CONNECTED) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + } + + @Test + fun testStateNotSentWhenIceCompletedAfterParticipantIsRemoved() { + // This should not happen, as peer connections are expected to be ended when a call participant is removed, but + // just in case. + + localStateBroadcasterNoMcu = LocalStateBroadcasterNoMcu( + localCallParticipantModel, + mockedMessageSenderNoMcu + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + + localStateBroadcasterNoMcu!!.handleCallParticipantAdded(callParticipantModel) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CHECKING) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + + localStateBroadcasterNoMcu!!.handleCallParticipantRemoved(callParticipantModel) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.COMPLETED) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + } + + @Test + fun testStateNotSentWhenIceConnectedAfterDestroyed() { + localStateBroadcasterNoMcu = LocalStateBroadcasterNoMcu( + localCallParticipantModel, + mockedMessageSenderNoMcu + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + val callParticipantModel2 = MutableCallParticipantModel("theSessionId2") + + localStateBroadcasterNoMcu!!.handleCallParticipantAdded(callParticipantModel) + localStateBroadcasterNoMcu!!.handleCallParticipantAdded(callParticipantModel2) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CHECKING) + callParticipantModel2.setIceConnectionState(PeerConnection.IceConnectionState.CHECKING) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + + localStateBroadcasterNoMcu!!.destroy() + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CONNECTED) + callParticipantModel2.setIceConnectionState(PeerConnection.IceConnectionState.CONNECTED) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + } + + @Test + fun testStateNotSentWhenIceCompletedAfterDestroyed() { + localStateBroadcasterNoMcu = LocalStateBroadcasterNoMcu( + localCallParticipantModel, + mockedMessageSenderNoMcu + ) + + val callParticipantModel = MutableCallParticipantModel("theSessionId") + + localStateBroadcasterNoMcu!!.handleCallParticipantAdded(callParticipantModel) + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.CHECKING) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + + localStateBroadcasterNoMcu!!.destroy() + + callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.COMPLETED) + + Mockito.verifyNoInteractions(mockedMessageSenderNoMcu) + } +} diff --git a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterTest.kt b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterTest.kt index a070ef946c..29b205cda5 100644 --- a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterTest.kt +++ b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterTest.kt @@ -14,6 +14,20 @@ import org.mockito.Mockito @Suppress("TooManyFunctions") class LocalStateBroadcasterTest { + private class LocalStateBroadcaster( + localCallParticipantModel: LocalCallParticipantModel?, + messageSender: MessageSender? + ) : com.nextcloud.talk.call.LocalStateBroadcaster(localCallParticipantModel, messageSender) { + + override fun handleCallParticipantAdded(callParticipantModel: CallParticipantModel) { + // Not used in base class tests + } + + override fun handleCallParticipantRemoved(callParticipantModel: CallParticipantModel) { + // Not used in base class tests + } + } + private var localCallParticipantModel: MutableLocalCallParticipantModel? = null private var mockedMessageSender: MessageSender? = null From 1508acc5c44d29004d87151067023a47e7f73b4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 19 Dec 2024 10:39:42 +0100 Subject: [PATCH 24/27] Move attributes to local variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../talk/webrtc/PeerConnectionWrapper.java | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) 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 8929af1c0d..70d59e11fa 100644 --- a/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java +++ b/app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java @@ -62,9 +62,6 @@ public class PeerConnectionWrapper { private final List pendingDataChannelMessages = new ArrayList<>(); private final SdpObserver sdpObserver; - private final boolean hasInitiated; - - private final MediaStream localStream; private final boolean isMCUPublisher; private final String videoStreamType; @@ -113,14 +110,13 @@ public PeerConnectionWrapper(PeerConnectionFactory peerConnectionFactory, boolean isMCUPublisher, boolean hasMCU, String videoStreamType, SignalingMessageReceiver signalingMessageReceiver, SignalingMessageSender signalingMessageSender) { - this.localStream = localStream; this.videoStreamType = videoStreamType; this.sessionId = sessionId; this.mediaConstraints = mediaConstraints; sdpObserver = new SdpObserver(); - hasInitiated = sessionId.compareTo(localSession) < 0; + boolean hasInitiated = sessionId.compareTo(localSession) < 0; this.isMCUPublisher = isMCUPublisher; PeerConnection.RTCConfiguration configuration = new PeerConnection.RTCConfiguration(iceServerList); @@ -133,12 +129,12 @@ public PeerConnectionWrapper(PeerConnectionFactory peerConnectionFactory, this.signalingMessageSender = signalingMessageSender; if (peerConnection != null) { - if (this.localStream != null) { - List localStreamIds = Collections.singletonList(this.localStream.getId()); - for(AudioTrack track : this.localStream.audioTracks) { + if (localStream != null) { + List localStreamIds = Collections.singletonList(localStream.getId()); + for(AudioTrack track : localStream.audioTracks) { peerConnection.addTrack(track, localStreamIds); } - for(VideoTrack track : this.localStream.videoTracks) { + for(VideoTrack track : localStream.videoTracks) { peerConnection.addTrack(track, localStreamIds); } } From 811b7c0fb8113c56d8dc0c86e63dbf244df90f47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 19 Dec 2024 12:17:41 +0100 Subject: [PATCH 25/27] Add support for sending signaling messages in the MessageSender 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.kt | 10 ++ .../nextcloud/talk/call/MessageSender.java | 47 +++++- .../nextcloud/talk/call/MessageSenderMcu.java | 8 +- .../talk/call/MessageSenderNoMcu.java | 8 +- .../talk/call/MessageSenderMcuTest.kt | 12 +- .../talk/call/MessageSenderNoMcuTest.kt | 7 +- .../nextcloud/talk/call/MessageSenderTest.kt | 134 ++++++++++++++++++ 7 files changed, 218 insertions(+), 8 deletions(-) create mode 100644 app/src/test/java/com/nextcloud/talk/call/MessageSenderTest.kt diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt index 4a4d8e874f..ba707b909d 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt @@ -1590,6 +1590,8 @@ class CallActivity : CallBaseActivity() { hasMCU = false messageSender = MessageSenderNoMcu( + signalingMessageSender, + callParticipants.keys, peerConnectionWrapperList ) @@ -1895,11 +1897,15 @@ class CallActivity : CallBaseActivity() { if (hasMCU) { messageSender = MessageSenderMcu( + signalingMessageSender, + callParticipants.keys, peerConnectionWrapperList, webSocketClient!!.sessionId ) } else { messageSender = MessageSenderNoMcu( + signalingMessageSender, + callParticipants.keys, peerConnectionWrapperList ) } @@ -1934,11 +1940,15 @@ class CallActivity : CallBaseActivity() { if (hasMCU) { messageSender = MessageSenderMcu( + signalingMessageSender, + callParticipants.keys, peerConnectionWrapperList, webSocketClient!!.sessionId ) } else { messageSender = MessageSenderNoMcu( + signalingMessageSender, + callParticipants.keys, peerConnectionWrapperList ) } diff --git a/app/src/main/java/com/nextcloud/talk/call/MessageSender.java b/app/src/main/java/com/nextcloud/talk/call/MessageSender.java index a868fc6b81..dd3eb149ad 100644 --- a/app/src/main/java/com/nextcloud/talk/call/MessageSender.java +++ b/app/src/main/java/com/nextcloud/talk/call/MessageSender.java @@ -7,16 +7,22 @@ package com.nextcloud.talk.call; import com.nextcloud.talk.models.json.signaling.DataChannelMessage; +import com.nextcloud.talk.models.json.signaling.NCSignalingMessage; +import com.nextcloud.talk.signaling.SignalingMessageSender; import com.nextcloud.talk.webrtc.PeerConnectionWrapper; import java.util.List; +import java.util.Set; /** * Helper class to send messages to participants in a call. *

* A specific subclass has to be created depending on whether an MCU is being used or not. *

- * Note that, unlike signaling messages, data channel messages require a peer connection. Therefore data channel + * Note that recipients of signaling messages are not validated, so no error will be triggered if trying to send a + * message to a participant with a session ID that does not exist or is not in the call. + *

+ * Also note that, unlike signaling messages, data channel messages require a peer connection. Therefore data channel * messages may not be received by a participant if there is no peer connection with that participant (for example, if * neither the local and remote participants have publishing rights). Moreover, data channel messages are expected to * be received only on peer connections with type "video", so data channel messages will not be sent on other peer @@ -24,9 +30,17 @@ */ public abstract class MessageSender { + private final SignalingMessageSender signalingMessageSender; + + private final Set callParticipantSessionIds; + protected final List peerConnectionWrappers; - public MessageSender(List peerConnectionWrappers) { + public MessageSender(SignalingMessageSender signalingMessageSender, + Set callParticipantSessionIds, + List peerConnectionWrappers) { + this.signalingMessageSender = signalingMessageSender; + this.callParticipantSessionIds = callParticipantSessionIds; this.peerConnectionWrappers = peerConnectionWrappers; } @@ -37,6 +51,35 @@ public MessageSender(List peerConnectionWrappers) { */ public abstract void sendToAll(DataChannelMessage dataChannelMessage); + /** + * Sends the given signaling message to the given session ID. + *

+ * Note that the signaling message will be modified to set the recipient in the "to" field. + * + * @param ncSignalingMessage the message to send + * @param sessionId the signaling session ID of the participant to send the message to + */ + public void send(NCSignalingMessage ncSignalingMessage, String sessionId) { + ncSignalingMessage.setTo(sessionId); + + signalingMessageSender.send(ncSignalingMessage); + } + + /** + * Sends the given signaling message to all the participants in the call. + *

+ * Note that the signaling message will be modified to set each of the recipients in the "to" field. + * + * @param ncSignalingMessage the message to send + */ + public void sendToAll(NCSignalingMessage ncSignalingMessage) { + for (String sessionId: callParticipantSessionIds) { + ncSignalingMessage.setTo(sessionId); + + signalingMessageSender.send(ncSignalingMessage); + } + } + protected PeerConnectionWrapper getPeerConnectionWrapper(String sessionId) { for (PeerConnectionWrapper peerConnectionWrapper: peerConnectionWrappers) { if (peerConnectionWrapper.getSessionId().equals(sessionId) diff --git a/app/src/main/java/com/nextcloud/talk/call/MessageSenderMcu.java b/app/src/main/java/com/nextcloud/talk/call/MessageSenderMcu.java index f3278fe4c1..0b7d3eaeef 100644 --- a/app/src/main/java/com/nextcloud/talk/call/MessageSenderMcu.java +++ b/app/src/main/java/com/nextcloud/talk/call/MessageSenderMcu.java @@ -7,9 +7,11 @@ package com.nextcloud.talk.call; import com.nextcloud.talk.models.json.signaling.DataChannelMessage; +import com.nextcloud.talk.signaling.SignalingMessageSender; import com.nextcloud.talk.webrtc.PeerConnectionWrapper; import java.util.List; +import java.util.Set; /** * Helper class to send messages to participants in a call when an MCU is used. @@ -21,9 +23,11 @@ public class MessageSenderMcu extends MessageSender { private final String ownSessionId; - public MessageSenderMcu(List peerConnectionWrappers, + public MessageSenderMcu(SignalingMessageSender signalingMessageSender, + Set callParticipantSessionIds, + List peerConnectionWrappers, String ownSessionId) { - super(peerConnectionWrappers); + super(signalingMessageSender, callParticipantSessionIds, peerConnectionWrappers); this.ownSessionId = ownSessionId; } diff --git a/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java b/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java index b5b9035032..d6c837bb73 100644 --- a/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java +++ b/app/src/main/java/com/nextcloud/talk/call/MessageSenderNoMcu.java @@ -7,17 +7,21 @@ package com.nextcloud.talk.call; import com.nextcloud.talk.models.json.signaling.DataChannelMessage; +import com.nextcloud.talk.signaling.SignalingMessageSender; import com.nextcloud.talk.webrtc.PeerConnectionWrapper; import java.util.List; +import java.util.Set; /** * Helper class to send messages to participants in a call when an MCU is not used. */ public class MessageSenderNoMcu extends MessageSender { - public MessageSenderNoMcu(List peerConnectionWrappers) { - super(peerConnectionWrappers); + public MessageSenderNoMcu(SignalingMessageSender signalingMessageSender, + Set callParticipantSessionIds, + List peerConnectionWrappers) { + super(signalingMessageSender, callParticipantSessionIds, peerConnectionWrappers); } /** diff --git a/app/src/test/java/com/nextcloud/talk/call/MessageSenderMcuTest.kt b/app/src/test/java/com/nextcloud/talk/call/MessageSenderMcuTest.kt index 49ca9e864d..9fd8d6289e 100644 --- a/app/src/test/java/com/nextcloud/talk/call/MessageSenderMcuTest.kt +++ b/app/src/test/java/com/nextcloud/talk/call/MessageSenderMcuTest.kt @@ -7,6 +7,7 @@ package com.nextcloud.talk.call import com.nextcloud.talk.models.json.signaling.DataChannelMessage +import com.nextcloud.talk.signaling.SignalingMessageSender import com.nextcloud.talk.webrtc.PeerConnectionWrapper import org.junit.Before import org.junit.Test @@ -27,6 +28,10 @@ class MessageSenderMcuTest { @Before fun setUp() { + val signalingMessageSender = Mockito.mock(SignalingMessageSender::class.java) + + val callParticipants = HashMap() + peerConnectionWrappers = ArrayList() peerConnectionWrapper1 = Mockito.mock(PeerConnectionWrapper::class.java) @@ -59,7 +64,12 @@ class MessageSenderMcuTest { Mockito.`when`(ownPeerConnectionWrapperScreen!!.videoStreamType).thenReturn("screen") peerConnectionWrappers!!.add(ownPeerConnectionWrapperScreen) - messageSender = MessageSenderMcu(peerConnectionWrappers, "ownSessionId") + messageSender = MessageSenderMcu( + signalingMessageSender, + callParticipants.keys, + peerConnectionWrappers, + "ownSessionId" + ) } @Test diff --git a/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt b/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt index 6784bcef37..303108ed97 100644 --- a/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt +++ b/app/src/test/java/com/nextcloud/talk/call/MessageSenderNoMcuTest.kt @@ -7,6 +7,7 @@ package com.nextcloud.talk.call import com.nextcloud.talk.models.json.signaling.DataChannelMessage +import com.nextcloud.talk.signaling.SignalingMessageSender import com.nextcloud.talk.webrtc.PeerConnectionWrapper import org.junit.Before import org.junit.Test @@ -25,6 +26,10 @@ class MessageSenderNoMcuTest { @Before fun setUp() { + val signalingMessageSender = Mockito.mock(SignalingMessageSender::class.java) + + val callParticipants = HashMap() + peerConnectionWrappers = ArrayList() peerConnectionWrapper1 = Mockito.mock(PeerConnectionWrapper::class.java) @@ -47,7 +52,7 @@ class MessageSenderNoMcuTest { Mockito.`when`(peerConnectionWrapper4Screen!!.videoStreamType).thenReturn("screen") peerConnectionWrappers!!.add(peerConnectionWrapper4Screen) - messageSender = MessageSenderNoMcu(peerConnectionWrappers) + messageSender = MessageSenderNoMcu(signalingMessageSender, callParticipants.keys, peerConnectionWrappers) } @Test diff --git a/app/src/test/java/com/nextcloud/talk/call/MessageSenderTest.kt b/app/src/test/java/com/nextcloud/talk/call/MessageSenderTest.kt new file mode 100644 index 0000000000..46915ef40a --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/call/MessageSenderTest.kt @@ -0,0 +1,134 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Daniel Calviño Sánchez + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.call + +import com.nextcloud.talk.models.json.signaling.DataChannelMessage +import com.nextcloud.talk.models.json.signaling.NCSignalingMessage +import com.nextcloud.talk.signaling.SignalingMessageSender +import com.nextcloud.talk.webrtc.PeerConnectionWrapper +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.mockito.Mockito +import org.mockito.Mockito.any +import org.mockito.Mockito.doAnswer +import org.mockito.Mockito.times +import org.mockito.invocation.InvocationOnMock + +class MessageSenderTest { + + private class MessageSender( + signalingMessageSender: SignalingMessageSender?, + callParticipantSessionIds: Set?, + peerConnectionWrappers: List? + ) : com.nextcloud.talk.call.MessageSender( + signalingMessageSender, + callParticipantSessionIds, + peerConnectionWrappers + ) { + + override fun sendToAll(dataChannelMessage: DataChannelMessage?) { + // Not used in base class tests + } + } + + private var signalingMessageSender: SignalingMessageSender? = null + + private var callParticipants: MutableMap? = null + + private var messageSender: MessageSender? = null + + @Before + fun setUp() { + signalingMessageSender = Mockito.mock(SignalingMessageSender::class.java) + + callParticipants = HashMap() + + val callParticipant1: CallParticipant = Mockito.mock(CallParticipant::class.java) + callParticipants!!["theSessionId1"] = callParticipant1 + + val callParticipant2: CallParticipant = Mockito.mock(CallParticipant::class.java) + callParticipants!!["theSessionId2"] = callParticipant2 + + val callParticipant3: CallParticipant = Mockito.mock(CallParticipant::class.java) + callParticipants!!["theSessionId3"] = callParticipant3 + + val callParticipant4: CallParticipant = Mockito.mock(CallParticipant::class.java) + callParticipants!!["theSessionId4"] = callParticipant4 + + val peerConnectionWrappers = ArrayList() + + messageSender = MessageSender(signalingMessageSender, callParticipants!!.keys, peerConnectionWrappers) + } + + @Test + fun testSendSignalingMessage() { + val message: NCSignalingMessage = Mockito.mock(NCSignalingMessage::class.java) + messageSender!!.send(message, "theSessionId2") + + Mockito.verify(message).to = "theSessionId2" + Mockito.verify(signalingMessageSender!!).send(message) + } + + @Test + fun testSendSignalingMessageIfUnknownSessionId() { + val message: NCSignalingMessage = Mockito.mock(NCSignalingMessage::class.java) + messageSender!!.send(message, "unknownSessionId") + + Mockito.verify(message).to = "unknownSessionId" + Mockito.verify(signalingMessageSender!!).send(message) + } + + @Test + fun testSendSignalingMessageToAll() { + val sentTo: MutableList = ArrayList() + doAnswer { invocation: InvocationOnMock -> + val arguments = invocation.arguments + val message = (arguments[0] as NCSignalingMessage) + + sentTo.add(message.to) + null + }.`when`(signalingMessageSender!!).send(any()) + + val message = NCSignalingMessage() + messageSender!!.sendToAll(message) + + assertTrue(sentTo.contains("theSessionId1")) + assertTrue(sentTo.contains("theSessionId2")) + assertTrue(sentTo.contains("theSessionId3")) + assertTrue(sentTo.contains("theSessionId4")) + Mockito.verify(signalingMessageSender!!, times(4)).send(message) + Mockito.verifyNoMoreInteractions(signalingMessageSender) + } + + @Test + fun testSendSignalingMessageToAllWhenParticipantsWereUpdated() { + val callParticipant5: CallParticipant = Mockito.mock(CallParticipant::class.java) + callParticipants!!["theSessionId5"] = callParticipant5 + + callParticipants!!.remove("theSessionId2") + callParticipants!!.remove("theSessionId3") + + val sentTo: MutableList = ArrayList() + doAnswer { invocation: InvocationOnMock -> + val arguments = invocation.arguments + val message = (arguments[0] as NCSignalingMessage) + + sentTo.add(message.to) + null + }.`when`(signalingMessageSender!!).send(any()) + + val message = NCSignalingMessage() + messageSender!!.sendToAll(message) + + assertTrue(sentTo.contains("theSessionId1")) + assertTrue(sentTo.contains("theSessionId4")) + assertTrue(sentTo.contains("theSessionId5")) + Mockito.verify(signalingMessageSender!!, times(3)).send(message) + Mockito.verifyNoMoreInteractions(signalingMessageSender) + } +} From 7cfc8b98eb38cad3fe907ce3023f80bb6317233c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 20 Dec 2024 15:00:30 +0100 Subject: [PATCH 26/27] Rename variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will be used to have separate counts for data channel and signaling messages. Signed-off-by: Daniel Calviño Sánchez --- .../talk/call/LocalStateBroadcasterMcuTest.kt | 128 +++++++++--------- 1 file changed, 64 insertions(+), 64 deletions(-) diff --git a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterMcuTest.kt b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterMcuTest.kt index aafeb2e512..a20fea8437 100644 --- a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterMcuTest.kt +++ b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterMcuTest.kt @@ -218,34 +218,34 @@ class LocalStateBroadcasterMcuTest { testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) - var messageCount = 1 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + var dataChannelMessageCount = 1 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) - messageCount = 2 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 2 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) - messageCount = 3 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 3 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) - messageCount = 4 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 4 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) val callParticipantModel2 = MutableCallParticipantModel("theSessionId2") @@ -254,50 +254,50 @@ class LocalStateBroadcasterMcuTest { testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) - messageCount = 5 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 5 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) - messageCount = 6 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 6 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) - messageCount = 7 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 7 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) - messageCount = 8 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 8 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(8, TimeUnit.SECONDS) - messageCount = 9 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 9 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(16, TimeUnit.SECONDS) - messageCount = 10 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 10 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) @@ -331,52 +331,52 @@ class LocalStateBroadcasterMcuTest { testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) - var messageCount = 1 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + var dataChannelMessageCount = 1 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) - messageCount = 2 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 2 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) - messageCount = 3 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 3 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) - messageCount = 4 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 4 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) localStateBroadcasterMcu!!.handleCallParticipantRemoved(callParticipantModel) testScheduler.advanceTimeBy(8, TimeUnit.SECONDS) - messageCount = 5 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 5 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(16, TimeUnit.SECONDS) - messageCount = 6 - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) - Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + dataChannelMessageCount = 6 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) From d094dc55c957e7be540354e3e443175cf60e8a3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 19 Dec 2024 12:41:07 +0100 Subject: [PATCH 27/27] Send state also through signaling messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The speaking state is still sent only through data channels, as it is not currently handled by other clients when sent through signaling messages. Signed-off-by: Daniel Calviño Sánchez --- .../talk/call/LocalStateBroadcaster.java | 61 +++++ .../talk/call/LocalStateBroadcasterMcu.java | 46 ++++ .../talk/call/LocalStateBroadcasterNoMcu.java | 9 + .../talk/call/LocalStateBroadcasterMcuTest.kt | 210 +++++++++++++++++- .../call/LocalStateBroadcasterNoMcuTest.kt | 53 +++++ .../talk/call/LocalStateBroadcasterTest.kt | 50 +++++ 6 files changed, 425 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java index 0376312970..1022d39e12 100644 --- a/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java +++ b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java @@ -7,6 +7,8 @@ package com.nextcloud.talk.call; import com.nextcloud.talk.models.json.signaling.DataChannelMessage; +import com.nextcloud.talk.models.json.signaling.NCMessagePayload; +import com.nextcloud.talk.models.json.signaling.NCSignalingMessage; import java.util.Objects; @@ -48,6 +50,7 @@ public void onChange() { audioEnabled = localCallParticipantModel.isAudioEnabled(); messageSender.sendToAll(getDataChannelMessageForAudioState()); + messageSender.sendToAll(getSignalingMessageForAudioState()); } if (!Objects.equals(speaking, localCallParticipantModel.isSpeaking())) { @@ -60,6 +63,7 @@ public void onChange() { videoEnabled = localCallParticipantModel.isVideoEnabled(); messageSender.sendToAll(getDataChannelMessageForVideoState()); + messageSender.sendToAll(getSignalingMessageForVideoState()); } } } @@ -106,4 +110,61 @@ protected DataChannelMessage getDataChannelMessageForVideoState() { return new DataChannelMessage(type); } + + /** + * Returns a signaling message with the common fields set (type and room type). + * + * @param type the type of the signaling message + * @return the signaling message + */ + private NCSignalingMessage createBaseSignalingMessage(String type) { + NCSignalingMessage ncSignalingMessage = new NCSignalingMessage(); + // "roomType" is not really relevant without a peer or when referring to the whole participant, but it is + // nevertheless expected in the message. As most of the signaling messages currently sent to all participants + // are related to audio/video state "video" is used as the room type. + ncSignalingMessage.setRoomType("video"); + ncSignalingMessage.setType(type); + + return ncSignalingMessage; + } + + /** + * Returns a signaling message to notify current audio state. + * + * @return the signaling message + */ + protected NCSignalingMessage getSignalingMessageForAudioState() { + String type = "mute"; + if (localCallParticipantModel.isAudioEnabled() != null && localCallParticipantModel.isAudioEnabled()) { + type = "unmute"; + } + + NCSignalingMessage ncSignalingMessage = createBaseSignalingMessage(type); + + NCMessagePayload ncMessagePayload = new NCMessagePayload(); + ncMessagePayload.setName("audio"); + ncSignalingMessage.setPayload(ncMessagePayload); + + return ncSignalingMessage; + } + + /** + * Returns a signaling message to notify current video state. + * + * @return the signaling message + */ + protected NCSignalingMessage getSignalingMessageForVideoState() { + String type = "mute"; + if (localCallParticipantModel.isVideoEnabled() != null && localCallParticipantModel.isVideoEnabled()) { + type = "unmute"; + } + + NCSignalingMessage ncSignalingMessage = createBaseSignalingMessage(type); + + NCMessagePayload ncMessagePayload = new NCMessagePayload(); + ncMessagePayload.setName("video"); + ncSignalingMessage.setPayload(ncMessagePayload); + + return ncSignalingMessage; + } } diff --git a/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterMcu.java b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterMcu.java index 628b615375..911bf1bf39 100644 --- a/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterMcu.java +++ b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterMcu.java @@ -6,6 +6,8 @@ */ package com.nextcloud.talk.call; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.TimeUnit; import io.reactivex.Observable; @@ -26,11 +28,30 @@ * initial state when the local participant joins the call, as all the remote participants joined from the point of * view of the local participant). If the state was already being sent the sending is restarted with each new * participant that joins. + *

+ * Similarly, in the case of signaling messages it is not possible either to know when the remote participants have + * "seen" the local participant and thus are ready to handle signaling messages about the state. However, in the case + * of signaling messages it is possible to send them to a specific participant, so the initial state is sent several + * times with an increasing delay directly to the participant that was added. Moreover, if the participant is removed + * the state is no longer directly sent. + *

+ * In any case, note that the state is sent only when the remote participant joins the call. Even in case of + * temporary disconnections the normal state updates sent when the state changes are expected to be received by the + * other participant, as signaling messages are sent through a WebSocket and are therefore reliable. Moreover, even + * if the WebSocket is restarted and the connection resumed (rather than joining with a new session ID) the messages + * would be also received, as in that case they would be queued until the WebSocket is connected again. + *

+ * Data channel messages, on the other hand, could be lost if the remote participant restarts the peer receiver + * connection (although they would be received in case of temporary disconnections, as data channels use a reliable + * transport by default). Therefore, as the speaking state is sent only through data channels, updates of the speaking + * state could be not received by remote participants. */ public class LocalStateBroadcasterMcu extends LocalStateBroadcaster { private final MessageSender messageSender; + private final Map sendStateWithRepetitionByParticipant = new HashMap<>(); + private Disposable sendStateWithRepetition; public LocalStateBroadcasterMcu(LocalCallParticipantModel localCallParticipantModel, @@ -46,6 +67,10 @@ public void destroy() { if (sendStateWithRepetition != null) { sendStateWithRepetition.dispose(); } + + for (Disposable sendStateWithRepetitionForParticipant: sendStateWithRepetitionByParticipant.values()) { + sendStateWithRepetitionForParticipant.dispose(); + } } @Override @@ -58,10 +83,26 @@ public void handleCallParticipantAdded(CallParticipantModel callParticipantModel .fromArray(new Integer[]{0, 1, 2, 4, 8, 16}) .concatMap(i -> Observable.just(i).delay(i, TimeUnit.SECONDS, Schedulers.io())) .subscribe(value -> sendState()); + + String sessionId = callParticipantModel.getSessionId(); + Disposable sendStateWithRepetitionForParticipant = sendStateWithRepetitionByParticipant.get(sessionId); + if (sendStateWithRepetitionForParticipant != null) { + sendStateWithRepetitionForParticipant.dispose(); + } + + sendStateWithRepetitionByParticipant.put(sessionId, Observable + .fromArray(new Integer[]{0, 1, 2, 4, 8, 16}) + .concatMap(i -> Observable.just(i).delay(i, TimeUnit.SECONDS, Schedulers.io())) + .subscribe(value -> sendState(sessionId))); } @Override public void handleCallParticipantRemoved(CallParticipantModel callParticipantModel) { + String sessionId = callParticipantModel.getSessionId(); + Disposable sendStateWithRepetitionForParticipant = sendStateWithRepetitionByParticipant.get(sessionId); + if (sendStateWithRepetitionForParticipant != null) { + sendStateWithRepetitionForParticipant.dispose(); + } } private void sendState() { @@ -69,4 +110,9 @@ private void sendState() { messageSender.sendToAll(getDataChannelMessageForSpeakingState()); messageSender.sendToAll(getDataChannelMessageForVideoState()); } + + private void sendState(String sessionId) { + messageSender.send(getSignalingMessageForAudioState(), sessionId); + messageSender.send(getSignalingMessageForVideoState(), sessionId); + } } diff --git a/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcu.java b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcu.java index 2a1bf04ea0..1377e626b4 100644 --- a/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcu.java +++ b/app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcu.java @@ -26,6 +26,12 @@ * after a temporary disconnection; data channels use a reliable transport by default, so even if the state changes * while the connection is temporarily interrupted the normal state update messages should be received by the other * participant once the connection is restored. + *

+ * Nevertheless, in case of a failed connection and an ICE restart it is unclear whether the data channel messages + * would be received or not (as the data channel transport may be the one that failed and needs to be restarted). + * However, the state (except the speaking state) is also sent through signaling messages, which need to be + * explicitly fetched from the internal signaling server, so even in case of a failed connection they will be + * eventually received once the remote participant connects again. */ public class LocalStateBroadcasterNoMcu extends LocalStateBroadcaster { @@ -115,5 +121,8 @@ private void sendState(String sessionId) { messageSender.send(getDataChannelMessageForAudioState(), sessionId); messageSender.send(getDataChannelMessageForSpeakingState(), sessionId); messageSender.send(getDataChannelMessageForVideoState(), sessionId); + + messageSender.send(getSignalingMessageForAudioState(), sessionId); + messageSender.send(getSignalingMessageForVideoState(), sessionId); } } diff --git a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterMcuTest.kt b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterMcuTest.kt index a20fea8437..03e32457e9 100644 --- a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterMcuTest.kt +++ b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterMcuTest.kt @@ -7,6 +7,8 @@ package com.nextcloud.talk.call import com.nextcloud.talk.models.json.signaling.DataChannelMessage +import com.nextcloud.talk.models.json.signaling.NCMessagePayload +import com.nextcloud.talk.models.json.signaling.NCSignalingMessage import io.reactivex.plugins.RxJavaPlugins import io.reactivex.schedulers.TestScheduler import org.junit.Before @@ -32,6 +34,54 @@ class LocalStateBroadcasterMcuTest { mockedMessageSender = Mockito.mock(MessageSender::class.java) } + private fun getExpectedUnmuteAudio(): NCSignalingMessage { + val expectedUnmuteAudio = NCSignalingMessage() + expectedUnmuteAudio.roomType = "video" + expectedUnmuteAudio.type = "unmute" + + val payload = NCMessagePayload() + payload.name = "audio" + expectedUnmuteAudio.payload = payload + + return expectedUnmuteAudio + } + + private fun getExpectedMuteAudio(): NCSignalingMessage { + val expectedMuteAudio = NCSignalingMessage() + expectedMuteAudio.roomType = "video" + expectedMuteAudio.type = "mute" + + val payload = NCMessagePayload() + payload.name = "audio" + expectedMuteAudio.payload = payload + + return expectedMuteAudio + } + + private fun getExpectedUnmuteVideo(): NCSignalingMessage { + val expectedUnmuteVideo = NCSignalingMessage() + expectedUnmuteVideo.roomType = "video" + expectedUnmuteVideo.type = "unmute" + + val payload = NCMessagePayload() + payload.name = "video" + expectedUnmuteVideo.payload = payload + + return expectedUnmuteVideo + } + + private fun getExpectedMuteVideo(): NCSignalingMessage { + val expectedMuteVideo = NCSignalingMessage() + expectedMuteVideo.roomType = "video" + expectedMuteVideo.type = "mute" + + val payload = NCMessagePayload() + payload.name = "video" + expectedMuteVideo.payload = payload + + return expectedMuteVideo + } + @Test fun testStateSentWithExponentialBackoffWhenParticipantAdded() { val testScheduler = TestScheduler() @@ -54,12 +104,17 @@ class LocalStateBroadcasterMcuTest { val expectedSpeaking = DataChannelMessage("speaking") val expectedVideoOn = DataChannelMessage("videoOn") + val expectedUnmuteAudio = getExpectedUnmuteAudio() + val expectedUnmuteVideo = getExpectedUnmuteVideo() + testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) var messageCount = 1 Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) @@ -68,6 +123,8 @@ class LocalStateBroadcasterMcuTest { Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) @@ -76,6 +133,8 @@ class LocalStateBroadcasterMcuTest { Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) @@ -84,6 +143,8 @@ class LocalStateBroadcasterMcuTest { Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(8, TimeUnit.SECONDS) @@ -92,6 +153,8 @@ class LocalStateBroadcasterMcuTest { Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(16, TimeUnit.SECONDS) @@ -100,6 +163,8 @@ class LocalStateBroadcasterMcuTest { Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) @@ -132,11 +197,16 @@ class LocalStateBroadcasterMcuTest { val expectedSpeaking = DataChannelMessage("speaking") val expectedVideoOn = DataChannelMessage("videoOn") + val expectedUnmuteAudio = getExpectedUnmuteAudio() + val expectedUnmuteVideo = getExpectedUnmuteVideo() + testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(1)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) localCallParticipantModel!!.isSpeaking = false @@ -151,51 +221,73 @@ class LocalStateBroadcasterMcuTest { Mockito.verify(mockedMessageSender!!, times(2)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(2)).sendToAll(expectedStoppedSpeaking) Mockito.verify(mockedMessageSender!!, times(2)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(2)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(2)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) localCallParticipantModel!!.isAudioEnabled = false val expectedAudioOff = DataChannelMessage("audioOff") + val expectedMuteAudio = getExpectedMuteAudio() // Changing the state causes the normal state update to be sent, independently of the initial state Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedAudioOff) + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedMuteAudio) testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) Mockito.verify(mockedMessageSender!!, times(2)).sendToAll(expectedAudioOff) Mockito.verify(mockedMessageSender!!, times(3)).sendToAll(expectedStoppedSpeaking) Mockito.verify(mockedMessageSender!!, times(3)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedMuteAudio) + Mockito.verify(mockedMessageSender!!, times(1)).send(expectedMuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(3)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) localCallParticipantModel!!.isVideoEnabled = false val expectedVideoOff = DataChannelMessage("videoOff") + val expectedMuteVideo = getExpectedMuteVideo() // Changing the state causes the normal state update to be sent, independently of the initial state Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedVideoOff) + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedMuteVideo) testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) Mockito.verify(mockedMessageSender!!, times(3)).sendToAll(expectedAudioOff) Mockito.verify(mockedMessageSender!!, times(4)).sendToAll(expectedStoppedSpeaking) Mockito.verify(mockedMessageSender!!, times(2)).sendToAll(expectedVideoOff) + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedMuteAudio) + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedMuteVideo) + Mockito.verify(mockedMessageSender!!, times(2)).send(expectedMuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(1)).send(expectedMuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) localCallParticipantModel!!.isVideoEnabled = true // Changing the state causes the normal state update to be sent, independently of the initial state Mockito.verify(mockedMessageSender!!, times(4)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedUnmuteVideo) testScheduler.advanceTimeBy(8, TimeUnit.SECONDS) Mockito.verify(mockedMessageSender!!, times(4)).sendToAll(expectedAudioOff) Mockito.verify(mockedMessageSender!!, times(5)).sendToAll(expectedStoppedSpeaking) Mockito.verify(mockedMessageSender!!, times(5)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedMuteAudio) + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedMuteVideo) + Mockito.verify(mockedMessageSender!!, times(1)).sendToAll(expectedUnmuteVideo) + Mockito.verify(mockedMessageSender!!, times(3)).send(expectedMuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(4)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) } @Test - fun testStateSentWithExponentialBackoffRestartedWhenAnotherParticipantAdded() { + fun testStateSentWithExponentialBackoffWhenAnotherParticipantAdded() { + // The state sent through data channels should be restarted, although the state sent through signaling + // messages should be independent for each participant. + val testScheduler = TestScheduler() RxJavaPlugins.setIoSchedulerHandler { testScheduler } @@ -216,36 +308,51 @@ class LocalStateBroadcasterMcuTest { val expectedSpeaking = DataChannelMessage("speaking") val expectedVideoOn = DataChannelMessage("videoOn") + val expectedUnmuteAudio = getExpectedUnmuteAudio() + val expectedUnmuteVideo = getExpectedUnmuteVideo() + testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) var dataChannelMessageCount = 1 + var signalingMessageCount1 = 1 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) dataChannelMessageCount = 2 + signalingMessageCount1 = 2 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) dataChannelMessageCount = 3 + signalingMessageCount1 = 3 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) dataChannelMessageCount = 4 + signalingMessageCount1 = 4 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) val callParticipantModel2 = MutableCallParticipantModel("theSessionId2") @@ -255,49 +362,107 @@ class LocalStateBroadcasterMcuTest { testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) dataChannelMessageCount = 5 + var signalingMessageCount2 = 1 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteVideo, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteAudio, "theSessionId2") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteVideo, "theSessionId2") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) dataChannelMessageCount = 6 + signalingMessageCount2 = 2 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteVideo, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteAudio, "theSessionId2") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteVideo, "theSessionId2") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) dataChannelMessageCount = 7 + signalingMessageCount2 = 3 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteVideo, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteAudio, "theSessionId2") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteVideo, "theSessionId2") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) dataChannelMessageCount = 8 + signalingMessageCount2 = 4 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteVideo, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteAudio, "theSessionId2") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteVideo, "theSessionId2") Mockito.verifyNoMoreInteractions(mockedMessageSender) - testScheduler.advanceTimeBy(8, TimeUnit.SECONDS) + // 0+1+2+4+1=8 seconds since last signaling messages for participant 1 + testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) + + signalingMessageCount1 = 5 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteVideo, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteAudio, "theSessionId2") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteVideo, "theSessionId2") + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + // 1+7=8 seconds since last data channel messages and signaling messages for participant 2 + testScheduler.advanceTimeBy(7, TimeUnit.SECONDS) dataChannelMessageCount = 9 + signalingMessageCount2 = 5 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteVideo, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteAudio, "theSessionId2") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteVideo, "theSessionId2") Mockito.verifyNoMoreInteractions(mockedMessageSender) - testScheduler.advanceTimeBy(16, TimeUnit.SECONDS) + // 7+9=16 seconds since last signaling messages for participant 1 + testScheduler.advanceTimeBy(9, TimeUnit.SECONDS) + + signalingMessageCount1 = 6 + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteVideo, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteAudio, "theSessionId2") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteVideo, "theSessionId2") + Mockito.verifyNoMoreInteractions(mockedMessageSender) + + // 9+7=16 seconds since last data channel messages and signaling messages for participant 2 + testScheduler.advanceTimeBy(7, TimeUnit.SECONDS) dataChannelMessageCount = 10 + signalingMessageCount2 = 6 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount1)).send(expectedUnmuteVideo, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteAudio, "theSessionId2") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount2)).send(expectedUnmuteVideo, "theSessionId2") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) @@ -306,8 +471,9 @@ class LocalStateBroadcasterMcuTest { } @Test - fun testStateStillSentWithExponentialBackoffWhenParticipantRemoved() { + fun testStateSentWithExponentialBackoffWhenParticipantRemoved() { // For simplicity the exponential backoff is not aborted when the participant that triggered it is removed. + // However, the signaling messages are stopped when the participant is removed. val testScheduler = TestScheduler() RxJavaPlugins.setIoSchedulerHandler { testScheduler } @@ -329,36 +495,51 @@ class LocalStateBroadcasterMcuTest { val expectedSpeaking = DataChannelMessage("speaking") val expectedVideoOn = DataChannelMessage("videoOn") + val expectedUnmuteAudio = getExpectedUnmuteAudio() + val expectedUnmuteVideo = getExpectedUnmuteVideo() + testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) var dataChannelMessageCount = 1 + var signalingMessageCount = 1 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) dataChannelMessageCount = 2 + signalingMessageCount = 2 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) dataChannelMessageCount = 3 + signalingMessageCount = 3 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(4, TimeUnit.SECONDS) dataChannelMessageCount = 4 + signalingMessageCount = 4 Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) localStateBroadcasterMcu!!.handleCallParticipantRemoved(callParticipantModel) @@ -369,6 +550,8 @@ class LocalStateBroadcasterMcuTest { Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(16, TimeUnit.SECONDS) @@ -377,6 +560,8 @@ class LocalStateBroadcasterMcuTest { Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(dataChannelMessageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(signalingMessageCount)).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(100, TimeUnit.SECONDS) @@ -395,8 +580,10 @@ class LocalStateBroadcasterMcuTest { ) val callParticipantModel = MutableCallParticipantModel("theSessionId") + val callParticipantModel2 = MutableCallParticipantModel("theSessionId2") localStateBroadcasterMcu!!.handleCallParticipantAdded(callParticipantModel) + localStateBroadcasterMcu!!.handleCallParticipantAdded(callParticipantModel2) // Sending will be done in another thread, so just adding the participant does not send anything until that // other thread could run. @@ -406,12 +593,19 @@ class LocalStateBroadcasterMcuTest { val expectedSpeaking = DataChannelMessage("speaking") val expectedVideoOn = DataChannelMessage("videoOn") + val expectedUnmuteAudio = getExpectedUnmuteAudio() + val expectedUnmuteVideo = getExpectedUnmuteVideo() + testScheduler.advanceTimeBy(0, TimeUnit.SECONDS) var messageCount = 1 Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteVideo, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteAudio, "theSessionId2") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteVideo, "theSessionId2") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) @@ -420,6 +614,10 @@ class LocalStateBroadcasterMcuTest { Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteVideo, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteAudio, "theSessionId2") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteVideo, "theSessionId2") Mockito.verifyNoMoreInteractions(mockedMessageSender) testScheduler.advanceTimeBy(2, TimeUnit.SECONDS) @@ -428,6 +626,10 @@ class LocalStateBroadcasterMcuTest { Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedAudioOn) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedSpeaking) Mockito.verify(mockedMessageSender!!, times(messageCount)).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteVideo, "theSessionId") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteAudio, "theSessionId2") + Mockito.verify(mockedMessageSender!!, times(messageCount)).send(expectedUnmuteVideo, "theSessionId2") Mockito.verifyNoMoreInteractions(mockedMessageSender) localStateBroadcasterMcu!!.destroy() diff --git a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcuTest.kt b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcuTest.kt index 2075785fdd..f225ff7394 100644 --- a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcuTest.kt +++ b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcuTest.kt @@ -7,6 +7,8 @@ package com.nextcloud.talk.call import com.nextcloud.talk.models.json.signaling.DataChannelMessage +import com.nextcloud.talk.models.json.signaling.NCMessagePayload +import com.nextcloud.talk.models.json.signaling.NCSignalingMessage import org.junit.Before import org.junit.Test import org.mockito.Mockito @@ -28,6 +30,30 @@ class LocalStateBroadcasterNoMcuTest { mockedMessageSenderNoMcu = Mockito.mock(MessageSenderNoMcu::class.java) } + private fun getExpectedUnmuteAudio(): NCSignalingMessage { + val expectedUnmuteAudio = NCSignalingMessage() + expectedUnmuteAudio.roomType = "video" + expectedUnmuteAudio.type = "unmute" + + val payload = NCMessagePayload() + payload.name = "audio" + expectedUnmuteAudio.payload = payload + + return expectedUnmuteAudio + } + + private fun getExpectedUnmuteVideo(): NCSignalingMessage { + val expectedUnmuteVideo = NCSignalingMessage() + expectedUnmuteVideo.roomType = "video" + expectedUnmuteVideo.type = "unmute" + + val payload = NCMessagePayload() + payload.name = "video" + expectedUnmuteVideo.payload = payload + + return expectedUnmuteVideo + } + @Test fun testStateSentWhenIceConnected() { localStateBroadcasterNoMcu = LocalStateBroadcasterNoMcu( @@ -49,9 +75,14 @@ class LocalStateBroadcasterNoMcuTest { val expectedSpeaking = DataChannelMessage("speaking") val expectedVideoOn = DataChannelMessage("videoOn") + val expectedUnmuteAudio = getExpectedUnmuteAudio() + val expectedUnmuteVideo = getExpectedUnmuteVideo() + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedAudioOn, "theSessionId") Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedSpeaking, "theSessionId") Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedVideoOn, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) } @@ -76,9 +107,14 @@ class LocalStateBroadcasterNoMcuTest { val expectedSpeaking = DataChannelMessage("speaking") val expectedVideoOn = DataChannelMessage("videoOn") + val expectedUnmuteAudio = getExpectedUnmuteAudio() + val expectedUnmuteVideo = getExpectedUnmuteVideo() + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedAudioOn, "theSessionId") Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedSpeaking, "theSessionId") Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedVideoOn, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) } @@ -103,9 +139,14 @@ class LocalStateBroadcasterNoMcuTest { val expectedSpeaking = DataChannelMessage("speaking") val expectedVideoOn = DataChannelMessage("videoOn") + val expectedUnmuteAudio = getExpectedUnmuteAudio() + val expectedUnmuteVideo = getExpectedUnmuteVideo() + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedAudioOn, "theSessionId") Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedSpeaking, "theSessionId") Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedVideoOn, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.COMPLETED) @@ -134,9 +175,14 @@ class LocalStateBroadcasterNoMcuTest { val expectedSpeaking = DataChannelMessage("speaking") val expectedVideoOn = DataChannelMessage("videoOn") + val expectedUnmuteAudio = getExpectedUnmuteAudio() + val expectedUnmuteVideo = getExpectedUnmuteVideo() + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedAudioOn, "theSessionId") Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedSpeaking, "theSessionId") Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedVideoOn, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) callParticipantModel.setIceConnectionState(PeerConnection.IceConnectionState.COMPLETED) @@ -191,9 +237,14 @@ class LocalStateBroadcasterNoMcuTest { val expectedSpeaking = DataChannelMessage("speaking") val expectedVideoOn = DataChannelMessage("videoOn") + val expectedUnmuteAudio = getExpectedUnmuteAudio() + val expectedUnmuteVideo = getExpectedUnmuteVideo() + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedAudioOn, "theSessionId") Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedSpeaking, "theSessionId") Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedVideoOn, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedUnmuteAudio, "theSessionId") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedUnmuteVideo, "theSessionId") Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) callParticipantModel2.setIceConnectionState(PeerConnection.IceConnectionState.CONNECTED) @@ -201,6 +252,8 @@ class LocalStateBroadcasterNoMcuTest { Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedAudioOn, "theSessionId2") Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedSpeaking, "theSessionId2") Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedVideoOn, "theSessionId2") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedUnmuteAudio, "theSessionId2") + Mockito.verify(mockedMessageSenderNoMcu!!).send(expectedUnmuteVideo, "theSessionId2") Mockito.verifyNoMoreInteractions(mockedMessageSenderNoMcu) } diff --git a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterTest.kt b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterTest.kt index 29b205cda5..34ca59e7e7 100644 --- a/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterTest.kt +++ b/app/src/test/java/com/nextcloud/talk/call/LocalStateBroadcasterTest.kt @@ -7,6 +7,8 @@ package com.nextcloud.talk.call import com.nextcloud.talk.models.json.signaling.DataChannelMessage +import com.nextcloud.talk.models.json.signaling.NCMessagePayload +import com.nextcloud.talk.models.json.signaling.NCSignalingMessage import org.junit.Before import org.junit.Test import org.mockito.Mockito @@ -49,7 +51,15 @@ class LocalStateBroadcasterTest { val expectedAudioOn = DataChannelMessage("audioOn") + val expectedUnmuteAudio = NCSignalingMessage() + expectedUnmuteAudio.roomType = "video" + expectedUnmuteAudio.type = "unmute" + val payload = NCMessagePayload() + payload.name = "audio" + expectedUnmuteAudio.payload = payload + Mockito.verify(mockedMessageSender!!).sendToAll(expectedAudioOn) + Mockito.verify(mockedMessageSender!!).sendToAll(expectedUnmuteAudio) Mockito.verifyNoMoreInteractions(mockedMessageSender) } @@ -74,7 +84,15 @@ class LocalStateBroadcasterTest { val expectedAudioOff = DataChannelMessage("audioOff") + val expectedMuteAudio = NCSignalingMessage() + expectedMuteAudio.roomType = "video" + expectedMuteAudio.type = "mute" + val payload = NCMessagePayload() + payload.name = "audio" + expectedMuteAudio.payload = payload + Mockito.verify(mockedMessageSender!!).sendToAll(expectedAudioOff) + Mockito.verify(mockedMessageSender!!).sendToAll(expectedMuteAudio) Mockito.verifyNoMoreInteractions(mockedMessageSender) } @@ -141,10 +159,18 @@ class LocalStateBroadcasterTest { val expectedAudioOn = DataChannelMessage("audioOn") val expectedSpeaking = DataChannelMessage("speaking") + val expectedUnmuteAudio = NCSignalingMessage() + expectedUnmuteAudio.roomType = "video" + expectedUnmuteAudio.type = "unmute" + val payload = NCMessagePayload() + payload.name = "audio" + expectedUnmuteAudio.payload = payload + val inOrder = Mockito.inOrder(mockedMessageSender) inOrder.verify(mockedMessageSender!!).sendToAll(expectedAudioOn) inOrder.verify(mockedMessageSender!!).sendToAll(expectedSpeaking) + Mockito.verify(mockedMessageSender!!).sendToAll(expectedUnmuteAudio) Mockito.verifyNoMoreInteractions(mockedMessageSender) } @@ -187,10 +213,18 @@ class LocalStateBroadcasterTest { val expectedStoppedSpeaking = DataChannelMessage("stoppedSpeaking") val expectedAudioOff = DataChannelMessage("audioOff") + val expectedMuteAudio = NCSignalingMessage() + expectedMuteAudio.roomType = "video" + expectedMuteAudio.type = "mute" + val payload = NCMessagePayload() + payload.name = "audio" + expectedMuteAudio.payload = payload + val inOrder = Mockito.inOrder(mockedMessageSender) inOrder.verify(mockedMessageSender!!).sendToAll(expectedStoppedSpeaking) inOrder.verify(mockedMessageSender!!).sendToAll(expectedAudioOff) + Mockito.verify(mockedMessageSender!!).sendToAll(expectedMuteAudio) Mockito.verifyNoMoreInteractions(mockedMessageSender) } @@ -216,7 +250,15 @@ class LocalStateBroadcasterTest { val expectedVideoOn = DataChannelMessage("videoOn") + val expectedUnmuteVideo = NCSignalingMessage() + expectedUnmuteVideo.roomType = "video" + expectedUnmuteVideo.type = "unmute" + val payload = NCMessagePayload() + payload.name = "video" + expectedUnmuteVideo.payload = payload + Mockito.verify(mockedMessageSender!!).sendToAll(expectedVideoOn) + Mockito.verify(mockedMessageSender!!).sendToAll(expectedUnmuteVideo) Mockito.verifyNoMoreInteractions(mockedMessageSender) } @@ -241,7 +283,15 @@ class LocalStateBroadcasterTest { val expectedVideoOff = DataChannelMessage("videoOff") + val expectedMuteVideo = NCSignalingMessage() + expectedMuteVideo.roomType = "video" + expectedMuteVideo.type = "mute" + val payload = NCMessagePayload() + payload.name = "video" + expectedMuteVideo.payload = payload + Mockito.verify(mockedMessageSender!!).sendToAll(expectedVideoOff) + Mockito.verify(mockedMessageSender!!).sendToAll(expectedMuteVideo) Mockito.verifyNoMoreInteractions(mockedMessageSender) }