From 7ae1013670c6a060e4b890b962a9c71d38be2904 Mon Sep 17 00:00:00 2001 From: bgrozev Date: Mon, 12 Feb 2024 08:36:03 -0600 Subject: [PATCH] feat: Enable transcription based on MUC config form (#1135) Read room_metadata from the MUC config form, and enable transcription if it is requested. --- jicofo-common/pom.xml | 5 ++ .../org/jitsi/jicofo/xmpp/muc/ChatRoom.kt | 3 + .../org/jitsi/jicofo/xmpp/muc/ChatRoomImpl.kt | 32 ++++++- .../jitsi/jicofo/xmpp/muc/ChatRoomListener.kt | 1 + .../org/jitsi/jicofo/xmpp/muc/RoomMetadata.kt | 54 ++++++++++++ .../jitsi/jicofo/xmpp/muc/RoomMetadataTest.kt | 84 +++++++++++++++++++ .../conference/JitsiMeetConferenceImpl.java | 5 ++ .../jicofo/jigasi/TranscriberManager.java | 62 ++++++++------ 8 files changed, 217 insertions(+), 29 deletions(-) create mode 100644 jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/RoomMetadata.kt create mode 100644 jicofo-common/src/test/kotlin/org/jitsi/jicofo/xmpp/muc/RoomMetadataTest.kt diff --git a/jicofo-common/pom.xml b/jicofo-common/pom.xml index ab5853facb..b8980b53e2 100644 --- a/jicofo-common/pom.xml +++ b/jicofo-common/pom.xml @@ -103,6 +103,11 @@ org.glassfish.jersey.media jersey-media-json-jackson + + com.fasterxml.jackson.module + jackson-module-kotlin + + com.github.spotbugs diff --git a/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoom.kt b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoom.kt index 9e786a6111..5427896806 100644 --- a/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoom.kt +++ b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoom.kt @@ -62,6 +62,9 @@ interface ChatRoom { * Read from the MUC config form. */ val participantsSoftLimit: Int? + /** Whether the room is configured to require transcription. */ + val transcriptionRequested: Boolean + val debugState: OrderedJsonObject /** Returns the number of members that currently have their audio sources unmuted. */ diff --git a/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoomImpl.kt b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoomImpl.kt index 855cafd31f..b7e55e38db 100644 --- a/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoomImpl.kt +++ b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoomImpl.kt @@ -156,6 +156,15 @@ class ChatRoomImpl( } } + override var transcriptionRequested: Boolean = false + private set(value) { + if (value != field) { + logger.info("transcriptionRequested is now $value.") + field = value + eventEmitter.fireEvent { transcriptionRequestedChanged(value) } + } + } + private val avModerationByMediaType = ConcurrentHashMap() /** The emitter used to fire events. */ @@ -278,10 +287,25 @@ class ChatRoomImpl( private fun parseConfigForm(configForm: Form) { lobbyEnabled = configForm.getField(MucConfigFormManager.MUC_ROOMCONFIG_MEMBERSONLY)?.firstValue?.toBoolean() ?: false - visitorsEnabled = - configForm.getField(MucConfigFields.VISITORS_ENABLED)?.firstValue?.toBoolean() - participantsSoftLimit = - configForm.getField(MucConfigFields.PARTICIPANTS_SOFT_LIMIT)?.firstValue?.toInt() + visitorsEnabled = configForm.getField(MucConfigFields.VISITORS_ENABLED)?.firstValue?.toBoolean() + participantsSoftLimit = configForm.getField(MucConfigFields.PARTICIPANTS_SOFT_LIMIT)?.firstValue?.toInt() + // Default to false unless specified. + val roomMetadata = configForm.getRoomMetadata() + if (roomMetadata != null) { + transcriptionRequested = roomMetadata.recording?.isTranscribingEnabled == true + } + } + + private fun Form.getRoomMetadata(): RoomMetadata.Metadata? { + getField("muc#roominfo_jitsimetadata")?.firstValue?.let { + try { + return RoomMetadata.parse(it).metadata + } catch (e: Exception) { + logger.warn("Invalid room metadata content", e) + return null + } + } + return null } override fun leave() { diff --git a/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoomListener.kt b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoomListener.kt index 6a2af2706f..79f6f20591 100644 --- a/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoomListener.kt +++ b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/ChatRoomListener.kt @@ -29,6 +29,7 @@ interface ChatRoomListener { fun localRoleChanged(newRole: MemberRole) {} fun numAudioSendersChanged(numAudioSenders: Int) {} fun numVideoSendersChanged(numVideoSenders: Int) {} + fun transcriptionRequestedChanged(transcriptionRequested: Boolean) {} } /** A class with the default kotlin method implementations (to avoid using @JvmDefault) **/ diff --git a/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/RoomMetadata.kt b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/RoomMetadata.kt new file mode 100644 index 0000000000..4b0f63d438 --- /dev/null +++ b/jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/muc/RoomMetadata.kt @@ -0,0 +1,54 @@ +/* + * Jicofo, the Jitsi Conference Focus. + * + * Copyright @ 2024-Present 8x8, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jitsi.jicofo.xmpp.muc + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties +import com.fasterxml.jackson.core.JsonParser +import com.fasterxml.jackson.core.JsonProcessingException +import com.fasterxml.jackson.databind.JsonMappingException +import com.fasterxml.jackson.databind.MapperFeature +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import com.fasterxml.jackson.module.kotlin.readValue + +/** + * The JSON structure included in the MUC config form from the room_metadata prosody module in jitsi-meet. Includes + * only the fields that we need here in jicofo. + */ +@JsonIgnoreProperties(ignoreUnknown = true) +data class RoomMetadata( + val type: String, + val metadata: Metadata? +) { + @JsonIgnoreProperties(ignoreUnknown = true) + data class Metadata(val recording: Recording?) { + @JsonIgnoreProperties(ignoreUnknown = true) + data class Recording(val isTranscribingEnabled: Boolean?) + } + + companion object { + private val mapper = jacksonObjectMapper().apply { + enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS) + enable(JsonParser.Feature.STRICT_DUPLICATE_DETECTION) + } + + @Throws(JsonProcessingException::class, JsonMappingException::class) + fun parse(string: String): RoomMetadata { + return mapper.readValue(string) + } + } +} diff --git a/jicofo-common/src/test/kotlin/org/jitsi/jicofo/xmpp/muc/RoomMetadataTest.kt b/jicofo-common/src/test/kotlin/org/jitsi/jicofo/xmpp/muc/RoomMetadataTest.kt new file mode 100644 index 0000000000..07079b1155 --- /dev/null +++ b/jicofo-common/src/test/kotlin/org/jitsi/jicofo/xmpp/muc/RoomMetadataTest.kt @@ -0,0 +1,84 @@ +/* + * Jicofo, the Jitsi Conference Focus. + * + * Copyright @ 2024-Present 8x8, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jitsi.jicofo.xmpp.muc + +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.core.spec.style.ShouldSpec +import io.kotest.matchers.nulls.shouldNotBeNull +import io.kotest.matchers.shouldBe +import io.kotest.matchers.types.shouldBeInstanceOf + +class RoomMetadataTest : ShouldSpec() { + init { + context("Valid") { + context("With isTranscribingEnabled set") { + val parsed = RoomMetadata.parse( + """ + { + "type": "room_metadata", + "metadata": { + "recording": { + "isTranscribingEnabled": true, + "anotherField": 123 + }, + "anotherField": {} + } + } + """.trimIndent() + ) + parsed.shouldBeInstanceOf() + parsed.metadata!!.recording!!.isTranscribingEnabled shouldBe true + } + context("With no recording included") { + + val parsed = RoomMetadata.parse( + """ + { + "type": "room_metadata", + "metadata": { + "key": { + "key2": "value2" + }, + "anotherField": {} + } + } + """.trimIndent() + ) + parsed.shouldBeInstanceOf() + parsed.metadata.shouldNotBeNull() + parsed.metadata?.recording shouldBe null + } + } + context("Invalid") { + context("Missing type") { + shouldThrow { + RoomMetadata.parse( + """ + { "key": 123 } + """.trimIndent() + ) + } + } + context("Invalid JSON") { + shouldThrow { + RoomMetadata.parse("{") + } + } + } + } +} diff --git a/jicofo/src/main/java/org/jitsi/jicofo/conference/JitsiMeetConferenceImpl.java b/jicofo/src/main/java/org/jitsi/jicofo/conference/JitsiMeetConferenceImpl.java index b11c5e6a94..3e6c200057 100644 --- a/jicofo/src/main/java/org/jitsi/jicofo/conference/JitsiMeetConferenceImpl.java +++ b/jicofo/src/main/java/org/jitsi/jicofo/conference/JitsiMeetConferenceImpl.java @@ -2336,6 +2336,11 @@ public void memberPresenceChanged(@NotNull ChatRoomMember member) { } + @Override + public void transcriptionRequestedChanged(boolean transcriptionRequested) + { + } + @Override public void numAudioSendersChanged(int numAudioSenders) { diff --git a/jicofo/src/main/java/org/jitsi/jicofo/jigasi/TranscriberManager.java b/jicofo/src/main/java/org/jitsi/jicofo/jigasi/TranscriberManager.java index cf7887bf4a..dfca0a0a24 100644 --- a/jicofo/src/main/java/org/jitsi/jicofo/jigasi/TranscriberManager.java +++ b/jicofo/src/main/java/org/jitsi/jicofo/jigasi/TranscriberManager.java @@ -77,7 +77,8 @@ public class TranscriberManager /** * A single-threaded {@link ExecutorService} to offload inviting the - * Transcriber from the smack thread updating presence. + * Transcriber from the smack thread updating presence. It's important that requests are handled sequentially to + * prevent multiple jigasis being invited. */ private final ExecutorService executorService = Executors.newSingleThreadExecutor(); @@ -125,20 +126,37 @@ private void memberPresenceChanged(@NotNull ChatRoomMember member) if (transcriptionStatusExtension != null && TranscriptionStatusExtension.Status.OFF.equals(transcriptionStatusExtension.getStatus())) { - // puts the stopping in the single threaded executor - // so we can order the events and avoid indicating active = false - // while we are starting due to concurrent presences processed - executorService.execute(this::stopTranscribing); + active = false; + logger.info("detected transcription status being turned off."); } if (isRequestingTranscriber(presence) && !active) { - if (jigasiDetector == null) + tryToStart(); + } + } + + private void tryToStart() + { + if (jigasiDetector == null) + { + logger.warn("Transcription requested, but jigasiDetector is not configured."); + return; + } + + if (active) + { + return; + } + + executorService.execute(() -> { + if (active) { - logger.warn("Transcription requested, but jigasiDetector is not configured."); return; } - executorService.execute(() -> this.startTranscribing(conference.getBridgeRegions())); - } + + // We need a modifiable list for the "exclude" parameter. + selectTranscriber(2, new ArrayList<>(), conference.getBridgeRegions()); + }); } /** @@ -159,13 +177,6 @@ private TranscriptionStatusExtension getTranscriptionStatus(Presence p) */ private void startTranscribing(@NotNull Collection preferredRegions) { - if (active) - { - return; - } - - // We need a modifiable list for the "exclude" parameter. - selectTranscriber(2, new ArrayList<>(), preferredRegions); } /** @@ -234,15 +245,6 @@ private void selectTranscriber( } } - /** - * Indicate transcription has stopped and sets {@link this#active} to false. - */ - private void stopTranscribing() - { - active = false; - logger.info("detected transcription status being turned off."); - } - /** * Checks whether the given {@link Presence} indicates a conference * participant is requesting transcription @@ -275,5 +277,15 @@ public void memberPresenceChanged(@NotNull ChatRoomMember member) { TranscriberManager.this.memberPresenceChanged(member); } + + @Override + public void transcriptionRequestedChanged(boolean transcriptionRequested) + { + if (transcriptionRequested) + { + logger.info("Transcription requested from the room."); + tryToStart(); + } + } } }