From 42cb6486cc6d001620a8aefce34bd64cff3d7586 Mon Sep 17 00:00:00 2001 From: bgrozev Date: Wed, 16 Aug 2023 11:55:33 -0500 Subject: [PATCH] fix: Fix audio/video limits since muted stated was moved to SourceInfo. (#1090) * fix: Fix audio/video limits since muted stated was moved to SourceInfo. * Add backward compat (for e.g. jigasi), read mediaType from SourceInfo. --- .../jicofo/xmpp/muc/ChatRoomMemberImpl.kt | 63 +++++++++++-------- .../org/jitsi/jicofo/xmpp/muc/SourceInfo.kt | 29 ++++++++- .../jitsi/jicofo/xmpp/muc/SourceInfoTest.kt | 22 ++++++- 3 files changed, 84 insertions(+), 30 deletions(-) diff --git a/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoomMemberImpl.kt b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoomMemberImpl.kt index 0a4aed698a..f998e8d5f8 100644 --- a/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoomMemberImpl.kt +++ b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoomMemberImpl.kt @@ -21,6 +21,7 @@ import org.jitsi.jicofo.xmpp.Features import org.jitsi.jicofo.xmpp.XmppCapsStats import org.jitsi.jicofo.xmpp.XmppConfig import org.jitsi.jicofo.xmpp.muc.MemberRole.Companion.fromSmack +import org.jitsi.utils.MediaType import org.jitsi.utils.OrderedJsonObject import org.jitsi.utils.logging2.Logger import org.jitsi.utils.logging2.createChildLogger @@ -100,14 +101,43 @@ class ChatRoomMemberImpl( return robot || isJigasi || isJibri } - private fun setSourceInfo(sourceInfoString: String) { - val sourceInfos: Set = try { - parseSourceInfoJson(sourceInfoString) - } catch (e: Exception) { - logger.warn("Ignoring invalid SourceInfo JSON", e) - return + private fun updateSourceInfo(presence: Presence) { + val sourceInfo = presence.getExtension("SourceInfo", "jabber:client") + if (sourceInfo == null) { + sourceInfos = emptySet() + } else { + val sourceInfos: Set = try { + parseSourceInfoJson(sourceInfo.text) + } catch (e: Exception) { + logger.warn("Ignoring invalid SourceInfo JSON", e) + return + } + this.sourceInfos = sourceInfos + } + + val wasAudioMuted = isAudioMuted + isAudioMuted = if (sourceInfo == null) { + // Support the old format, still used by jigasi + presence.getExtension(AudioMutedExtension::class.java)?.isAudioMuted ?: true + } else { + sourceInfos.filter { it.mediaType == MediaType.AUDIO }.none { !it.muted } + } + if (isAudioMuted != wasAudioMuted) { + logger.debug { "isAudioMuted = $isAudioMuted" } + if (isAudioMuted) chatRoom.audioSendersCount-- else chatRoom.audioSendersCount++ + } + + val wasVideoMuted = isVideoMuted + isVideoMuted = if (sourceInfo == null) { + // Support the old format, still used by jigasi + presence.getExtension(VideoMutedExtension::class.java)?.isVideoMuted ?: true + } else { + sourceInfos.filter { it.mediaType == MediaType.VIDEO }.none { !it.muted } + } + if (isVideoMuted != wasVideoMuted) { + logger.debug { "isVideoMuted = $isVideoMuted" } + if (isVideoMuted) chatRoom.videoSendersCount-- else chatRoom.videoSendersCount++ } - this.sourceInfos = sourceInfos } /** @@ -133,8 +163,7 @@ class ChatRoomMemberImpl( capsNodeVer = "${it.node}#${it.ver}" } - val sourceInfo = presence.getExtension("SourceInfo", "jabber:client") - setSourceInfo(if (sourceInfo == null) "{}" else sourceInfo.text) + updateSourceInfo(presence) // We recognize jigasi by the existence of a "feature" extension in its presence. val features = presence.getExtension(FeaturesExtension::class.java) @@ -180,22 +209,6 @@ class ChatRoomMemberImpl( presence.getExtension(StatsId::class.java)?.let { statsId = it.statsId } - - val wasAudioMuted = isAudioMuted - // defaults to true - isAudioMuted = presence.getExtension(AudioMutedExtension::class.java)?.isAudioMuted ?: true - if (isAudioMuted != wasAudioMuted) { - logger.debug { "isAudioMuted = $isAudioMuted" } - if (isAudioMuted) chatRoom.audioSendersCount-- else chatRoom.audioSendersCount++ - } - - val wasVideoMuted = isVideoMuted - // defaults to true - isVideoMuted = presence.getExtension(VideoMutedExtension::class.java)?.isVideoMuted ?: true - if (isVideoMuted != wasVideoMuted) { - logger.debug { "isVideoMuted = $isVideoMuted" } - if (isVideoMuted) chatRoom.videoSendersCount-- else chatRoom.videoSendersCount++ - } } /** diff --git a/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/SourceInfo.kt b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/SourceInfo.kt index 551a2d5bc6..24fa71214d 100644 --- a/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/SourceInfo.kt +++ b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/SourceInfo.kt @@ -16,6 +16,7 @@ package org.jitsi.jicofo.xmpp.muc import org.jitsi.jicofo.conference.source.VideoType +import org.jitsi.utils.MediaType import org.json.simple.JSONObject import org.json.simple.parser.JSONParser @@ -23,7 +24,8 @@ import org.json.simple.parser.JSONParser data class SourceInfo( val name: String, val muted: Boolean, - val videoType: VideoType? + val videoType: VideoType?, + val mediaType: MediaType? ) /** @@ -45,6 +47,29 @@ fun parseSourceInfoJson(s: String): Set { else -> VideoType.parseString(videoTypeValue) } - SourceInfo(name, muted, videoType) + val mediaTypeField = sourceJson["mediaType"] as? String + val mediaType = when { + "audio".equals(mediaTypeField, ignoreCase = true) -> MediaType.AUDIO + "video".equals(mediaTypeField, ignoreCase = true) -> MediaType.VIDEO + else -> parseMediaTypeFromName(name) + } + + SourceInfo(name, muted, videoType, mediaType) }.toSet() } + +/** + * If not explicitly signaled the media type is encoded in the name as in `abcd-a0` (an audio source) and `abcd-v5` + * (a video source). + */ +private fun parseMediaTypeFromName(name: String): MediaType? = name.indexOf('-').let { indexOfDash -> + if (indexOfDash < 0 || indexOfDash == name.length - 1) { + null + } else { + when (name[indexOfDash + 1]) { + 'a' -> MediaType.AUDIO + 'v' -> MediaType.VIDEO + else -> null + } + } +} diff --git a/jicofo-common/src/test/kotlin/org/jitsi/jicofo/xmpp/muc/SourceInfoTest.kt b/jicofo-common/src/test/kotlin/org/jitsi/jicofo/xmpp/muc/SourceInfoTest.kt index a401243928..e420940037 100644 --- a/jicofo-common/src/test/kotlin/org/jitsi/jicofo/xmpp/muc/SourceInfoTest.kt +++ b/jicofo-common/src/test/kotlin/org/jitsi/jicofo/xmpp/muc/SourceInfoTest.kt @@ -19,6 +19,7 @@ import io.kotest.assertions.throwables.shouldThrow import io.kotest.core.spec.style.ShouldSpec import io.kotest.matchers.shouldBe import org.jitsi.jicofo.conference.source.VideoType +import org.jitsi.utils.MediaType import org.json.simple.parser.ParseException class SourceInfoTest : ShouldSpec() { @@ -59,16 +60,31 @@ class SourceInfoTest : ShouldSpec() { "3b554cf4-v1": { "muted": true, "videoType": "desktop" + }, + "3b554cf4-v3": { + "mediaType": "audio" } } """.trimIndent() ).shouldBe( setOf( - SourceInfo("3b554cf4-a0", false, null), - SourceInfo("3b554cf4-v0", true, null), - SourceInfo("3b554cf4-v1", true, VideoType.Desktop), + SourceInfo("3b554cf4-a0", false, null, MediaType.AUDIO), + SourceInfo("3b554cf4-v0", true, null, MediaType.VIDEO), + SourceInfo("3b554cf4-v1", true, VideoType.Desktop, MediaType.VIDEO), + // The explicitly signaled "audio" precedes over "video" inferred from "-v3" in the name. + SourceInfo("3b554cf4-v3", true, null, MediaType.AUDIO), ) ) } + context("Parsing media type") { + parseSourceInfoJson("""{ "abc-a0": {} }""".trimIndent()).first().mediaType shouldBe MediaType.AUDIO + parseSourceInfoJson("""{ "abc-v0": {} }""".trimIndent()).first().mediaType shouldBe MediaType.VIDEO + parseSourceInfoJson("""{ "abc-d0": {} }""".trimIndent()).first().mediaType shouldBe null + parseSourceInfoJson("""{ "abc0": {} }""".trimIndent()).first().mediaType shouldBe null + parseSourceInfoJson("""{ "abc0": { "mediaType": "audio" } }""".trimIndent()).first().mediaType shouldBe + MediaType.AUDIO + parseSourceInfoJson("""{ "abc0": { "mediaType": "VIDEO" } }""".trimIndent()).first().mediaType shouldBe + MediaType.VIDEO + } } }