From 36a29ed36e170262c2bdfbb12d83af998a8147ba 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] 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 | 42 ++++++++++++++++++- .../talk/call/MessageSenderNoMcuTest.kt | 15 +++++++ 4 files changed, 64 insertions(+), 4 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 2e0820965..a868fc6b8 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 @@ import java.util.List; *

* 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 abstract class MessageSender { 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 7dea72926..6f4306262 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 class MessageSenderNoMcu extends MessageSender { 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 9bdefbfe8..49ca9e864 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 testSendDataChannelMessageToAllWithoutOwnPeerConnection() { + 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 cec1534de..cfd4a850d 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) } }