From f9314a2370ed0e1cd6ce213b21c580b3a7c71472 Mon Sep 17 00:00:00 2001 From: bgrozev Date: Mon, 29 Jan 2024 14:05:32 -0600 Subject: [PATCH] Remove backward compat for source names. (#2087) * Remove backward compat for source names. * Remove deprecated sendVideoConstraints. * ref: Remove SenderVideoConstraintsMessage. * ref: Remove "V2" from function name. --- doc/constraints.md | 13 ---- .../org/jitsi/videobridge/Conference.java | 4 +- .../org/jitsi/videobridge/AbstractEndpoint.kt | 15 +--- .../kotlin/org/jitsi/videobridge/Endpoint.kt | 78 +++---------------- .../cc/allocation/AllocationSettings.kt | 63 +++------------ .../cc/allocation/BitrateController.kt | 37 ++------- .../colibri2/Colibri2ConferenceHandler.kt | 8 +- .../load_management/LastNReducer.kt | 2 +- .../message/BridgeChannelMessage.kt | 54 +------------ .../videobridge/relay/RelayedEndpoint.kt | 19 +---- .../org/jitsi/videobridge/ConferenceTest.kt | 2 +- .../cc/allocation/AllocationSettingsTest.kt | 46 +---------- .../allocation/BitrateControllerPerfTest.kt | 6 +- .../cc/allocation/BitrateControllerTest.kt | 4 - .../load_management/LastNReducerTest.kt | 2 +- .../message/BridgeChannelMessageTest.kt | 38 +-------- 16 files changed, 49 insertions(+), 342 deletions(-) diff --git a/doc/constraints.md b/doc/constraints.md index 79875d8125..5cb6adb15f 100644 --- a/doc/constraints.md +++ b/doc/constraints.md @@ -12,16 +12,3 @@ higher than the specified need not be transmitted for a specific video source: "maxHeight": 180 } ``` - -The legacy format prior to the multi-stream support was endpoint scoped. This message is still sent to old clients, but -will be removed in the future. - -``` -{ - "colibriClass": "SenderVideoConstraints", - "videoConstraints": { - "idealHeight": 180 - } -} -``` - diff --git a/jvb/src/main/java/org/jitsi/videobridge/Conference.java b/jvb/src/main/java/org/jitsi/videobridge/Conference.java index a7afe7ee6f..456985d2a5 100644 --- a/jvb/src/main/java/org/jitsi/videobridge/Conference.java +++ b/jvb/src/main/java/org/jitsi/videobridge/Conference.java @@ -718,7 +718,6 @@ public AbstractEndpoint findSourceOwner(@NotNull String sourceName) * @param iceControlling {@code true} if the ICE agent of this endpoint's * transport will be initialized to serve as a controlling ICE agent; * otherwise, {@code false} - * @param sourceNames whether this endpoint signaled the source names support. * @param doSsrcRewriting whether this endpoint signaled SSRC rewriting support. * @return an Endpoint participating in this Conference */ @@ -726,7 +725,6 @@ public AbstractEndpoint findSourceOwner(@NotNull String sourceName) public Endpoint createLocalEndpoint( String id, boolean iceControlling, - boolean sourceNames, boolean doSsrcRewriting, boolean visitor, boolean privateAddresses) @@ -738,7 +736,7 @@ public Endpoint createLocalEndpoint( } final Endpoint endpoint = new Endpoint( - id, this, logger, iceControlling, sourceNames, doSsrcRewriting, visitor, privateAddresses); + id, this, logger, iceControlling, doSsrcRewriting, visitor, privateAddresses); videobridge.localEndpointCreated(visitor); subscribeToEndpointEvents(endpoint); diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/AbstractEndpoint.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/AbstractEndpoint.kt index b52f121552..40816f7eb0 100644 --- a/jvb/src/main/kotlin/org/jitsi/videobridge/AbstractEndpoint.kt +++ b/jvb/src/main/kotlin/org/jitsi/videobridge/AbstractEndpoint.kt @@ -236,7 +236,7 @@ abstract class AbstractEndpoint protected constructor( val newReceiverMaxVideoConstraints = VideoConstraints(newMaxHeight, -1.0) if (newReceiverMaxVideoConstraints != oldReceiverMaxVideoConstraints) { maxReceiverVideoConstraints[sourceName] = newReceiverMaxVideoConstraints - sendVideoConstraintsV2(sourceName, newReceiverMaxVideoConstraints) + sendVideoConstraints(sourceName, newReceiverMaxVideoConstraints) } } @@ -265,17 +265,6 @@ abstract class AbstractEndpoint protected constructor( */ abstract fun setExtmapAllowMixed(allow: Boolean) - /** - * Notifies this instance that the max video constraints that the bridge - * needs to receive from this endpoint has changed. Each implementation - * handles this notification differently. - * - * @param maxVideoConstraints the max video constraints that the bridge - * needs to receive from this endpoint - */ - @Deprecated("use sendVideoConstraintsV2") - protected abstract fun sendVideoConstraints(maxVideoConstraints: VideoConstraints) - /** * Notifies this instance that the max video constraints that the bridge needs to receive from a source of this * endpoint has changed. Each implementation handles this notification differently. @@ -283,7 +272,7 @@ abstract class AbstractEndpoint protected constructor( * @param sourceName the name of the media source * @param maxVideoConstraints the max video constraints that the bridge needs to receive from the source */ - protected abstract fun sendVideoConstraintsV2(sourceName: String, maxVideoConstraints: VideoConstraints) + protected abstract fun sendVideoConstraints(sourceName: String, maxVideoConstraints: VideoConstraints) /** * Notifies this instance that a specified received wants to receive the specified video constraints from the media diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/Endpoint.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/Endpoint.kt index 01cb09af3f..512168c948 100644 --- a/jvb/src/main/kotlin/org/jitsi/videobridge/Endpoint.kt +++ b/jvb/src/main/kotlin/org/jitsi/videobridge/Endpoint.kt @@ -61,11 +61,9 @@ import org.jitsi.videobridge.datachannel.DataChannelStack import org.jitsi.videobridge.datachannel.protocol.DataChannelPacket import org.jitsi.videobridge.datachannel.protocol.DataChannelProtocolConstants import org.jitsi.videobridge.message.BridgeChannelMessage -import org.jitsi.videobridge.message.ForwardedEndpointsMessage import org.jitsi.videobridge.message.ForwardedSourcesMessage import org.jitsi.videobridge.message.ReceiverVideoConstraintsMessage import org.jitsi.videobridge.message.SenderSourceConstraintsMessage -import org.jitsi.videobridge.message.SenderVideoConstraintsMessage import org.jitsi.videobridge.relay.AudioSourceDesc import org.jitsi.videobridge.relay.RelayedEndpoint import org.jitsi.videobridge.rest.root.debug.EndpointDebugFeatures @@ -107,7 +105,6 @@ class Endpoint @JvmOverloads constructor( * as a controlling ICE agent, false otherwise */ iceControlling: Boolean, - private val isUsingSourceNames: Boolean, private val doSsrcRewriting: Boolean, /** * Whether this endpoint is in "visitor" mode, i.e. should be invisible to other endpoints. @@ -205,9 +202,6 @@ class Endpoint @JvmOverloads constructor( // Intentional no-op } - override fun forwardedEndpointsChanged(forwardedEndpoints: Set) = - sendForwardedEndpointsMessage(forwardedEndpoints) - override fun forwardedSourcesChanged(forwardedSources: Set) { sendForwardedSourcesMessage(forwardedSources) } @@ -221,8 +215,7 @@ class Endpoint @JvmOverloads constructor( }, { getOrderedEndpoints() }, diagnosticContext, - logger, - isUsingSourceNames, + logger ) /** Whether any sources are suspended from being sent to this endpoint because of BWE. */ @@ -329,7 +322,7 @@ class Endpoint @JvmOverloads constructor( conference.videobridge.statistics.totalVisitors.inc() } - logger.info("Created new endpoint isUsingSourceNames=$isUsingSourceNames, iceControlling=$iceControlling") + logger.info("Created new endpoint, iceControlling=$iceControlling") } override var mediaSources: Array @@ -514,18 +507,14 @@ class Endpoint @JvmOverloads constructor( // TODO: this should be part of an EndpointMessageTransport.EventHandler interface fun endpointMessageTransportConnected() { sendAllVideoConstraints() - if (isUsingSourceNames) { - sendForwardedSourcesMessage(bitrateController.forwardedSources) - } else { - sendForwardedEndpointsMessage(bitrateController.forwardedEndpoints) - } + sendForwardedSourcesMessage(bitrateController.forwardedSources) videoSsrcs.sendAllMappings() audioSsrcs.sendAllMappings() } private fun sendAllVideoConstraints() { maxReceiverVideoConstraints.forEach { (sourceName, constraints) -> - sendVideoConstraintsV2(sourceName, constraints) + sendVideoConstraints(sourceName, constraints) } } @@ -566,36 +555,17 @@ class Endpoint @JvmOverloads constructor( } } - @Deprecated("use sendVideoConstraintsV2") - override fun sendVideoConstraints(maxVideoConstraints: VideoConstraints) { - // Note that it's up to the client to respect these constraints. - if (mediaSources.isEmpty()) { - logger.cdebug { "Suppressing sending a SenderVideoConstraints message, endpoint has no streams." } - } else { - val senderVideoConstraintsMessage = SenderVideoConstraintsMessage(maxVideoConstraints.maxHeight) - logger.cdebug { "Sender constraints changed: ${senderVideoConstraintsMessage.toJson()}" } - sendMessage(senderVideoConstraintsMessage) - } - } - - override fun sendVideoConstraintsV2(sourceName: String, maxVideoConstraints: VideoConstraints) { + override fun sendVideoConstraints(sourceName: String, maxVideoConstraints: VideoConstraints) { // Note that it's up to the client to respect these constraints. if (findMediaSourceDesc(sourceName) == null) { logger.warn { "Suppressing sending a SenderVideoConstraints message, endpoint has no such source: $sourceName" } } else { - if (isUsingSourceNames) { - val senderSourceConstraintsMessage = - SenderSourceConstraintsMessage(sourceName, maxVideoConstraints.maxHeight) - logger.cdebug { "Sender constraints changed: ${senderSourceConstraintsMessage.toJson()}" } - sendMessage(senderSourceConstraintsMessage) - } else { - maxReceiverVideoConstraints[sourceName]?.let { - sendVideoConstraints(it) - } - ?: logger.error("No max receiver constraints mapping found for: $sourceName") - } + val senderSourceConstraintsMessage = + SenderSourceConstraintsMessage(sourceName, maxVideoConstraints.maxHeight) + logger.cdebug { "Sender constraints changed: ${senderSourceConstraintsMessage.toJson()}" } + sendMessage(senderSourceConstraintsMessage) } } @@ -726,28 +696,6 @@ class Endpoint @JvmOverloads constructor( return true } - /** - * Sends a message to this endpoint in order to notify it that the set of endpoints for which the bridge - * is sending video has changed. - * - * @param forwardedEndpoints the collection of forwarded endpoints. - */ - @Deprecated("", ReplaceWith("sendForwardedSourcesMessage"), DeprecationLevel.WARNING) - fun sendForwardedEndpointsMessage(forwardedEndpoints: Collection) { - if (isUsingSourceNames) { - return - } - - val msg = ForwardedEndpointsMessage(forwardedEndpoints) - TaskPools.IO_POOL.execute { - try { - sendMessage(msg) - } catch (t: Throwable) { - logger.warn("Failed to send message:", t) - } - } - } - /** * Sends a message to this endpoint in order to notify it that the set of media sources for which the bridge * is sending video has changed. @@ -755,10 +703,6 @@ class Endpoint @JvmOverloads constructor( * @param forwardedSources the collection of forwarded media sources (by name). */ fun sendForwardedSourcesMessage(forwardedSources: Collection) { - if (!isUsingSourceNames) { - return - } - val msg = ForwardedSourcesMessage(forwardedSources) TaskPools.IO_POOL.execute { try { @@ -851,9 +795,9 @@ class Endpoint @JvmOverloads constructor( fun isOversending(): Boolean = bitrateController.isOversending() /** - * Returns how many endpoints this Endpoint is currently forwarding video for + * Returns how many video sources are currently forwarding to this endpoint. */ - fun numForwardedEndpoints(): Int = bitrateController.numForwardedEndpoints() + fun numForwardedSources(): Int = bitrateController.numForwardedSources() fun setBandwidthAllocationSettings(message: ReceiverVideoConstraintsMessage) { initialReceiverConstraintsReceived = true diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/cc/allocation/AllocationSettings.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/cc/allocation/AllocationSettings.kt index c644595069..73ca5fb400 100644 --- a/jvb/src/main/kotlin/org/jitsi/videobridge/cc/allocation/AllocationSettings.kt +++ b/jvb/src/main/kotlin/org/jitsi/videobridge/cc/allocation/AllocationSettings.kt @@ -23,7 +23,6 @@ import org.jitsi.utils.logging2.LoggerImpl import org.jitsi.utils.logging2.createChildLogger import org.jitsi.videobridge.cc.config.BitrateControllerConfig.Companion.config import org.jitsi.videobridge.message.ReceiverVideoConstraintsMessage -import org.jitsi.videobridge.util.endpointIdToSourceName /** * This class encapsulates all of the client-controlled settings for bandwidth allocation. @@ -60,17 +59,10 @@ data class AllocationSettings @JvmOverloads constructor( * the overall state changed. */ internal class AllocationSettingsWrapper( - private val useSourceNames: Boolean, parentLogger: Logger = LoggerImpl(AllocationSettingsWrapper::class.java.name) ) { private val logger = createChildLogger(parentLogger) - /** - * The last selected endpoints set signaled by the receiving endpoint. - */ - @Deprecated("", ReplaceWith("selectedSources"), DeprecationLevel.WARNING) - private var selectedEndpoints = emptyList() - /** * The last selected sources set signaled by the receiving endpoint. */ @@ -84,9 +76,6 @@ internal class AllocationSettingsWrapper( private var assumedBandwidthBps: Long = -1 - @Deprecated("", ReplaceWith("onStageSources"), DeprecationLevel.WARNING) - private var onStageEndpoints: List = emptyList() - private var onStageSources: List = emptyList() private var allocationSettings = create() @@ -111,35 +100,16 @@ internal class AllocationSettingsWrapper( changed = true } } - if (useSourceNames) { - message.selectedSources?.let { - if (selectedSources != it) { - selectedSources = it - changed = true - } - } - message.onStageSources?.let { - if (onStageSources != it) { - onStageSources = it - changed = true - } - } - } else { - message.selectedEndpoints?.let { - logger.warn("Setting deprecated selectedEndpoints=$it") - val newSelectedSources = it.map { endpoint -> endpointIdToSourceName(endpoint) } - if (selectedSources != newSelectedSources) { - selectedSources = newSelectedSources - changed = true - } + message.selectedSources?.let { + if (selectedSources != it) { + selectedSources = it + changed = true } - message.onStageEndpoints?.let { - logger.warn("Setting deprecated onStateEndpoints=$it") - val newOnStageSources = it.map { endpoint -> endpointIdToSourceName(endpoint) } - if (onStageSources != newOnStageSources) { - onStageSources = newOnStageSources - changed = true - } + } + message.onStageSources?.let { + if (onStageSources != it) { + onStageSources = it + changed = true } } message.defaultConstraints?.let { @@ -149,19 +119,8 @@ internal class AllocationSettingsWrapper( } } message.constraints?.let { - var newConstraints = it - - // Convert endpoint IDs to source names - if (!useSourceNames) { - newConstraints = HashMap(it.size) - it.entries.forEach { - entry -> - newConstraints[endpointIdToSourceName(entry.key)] = entry.value - } - } - - if (this.videoConstraints != newConstraints) { - this.videoConstraints = newConstraints + if (this.videoConstraints != it) { + this.videoConstraints = it changed = true } } diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/cc/allocation/BitrateController.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/cc/allocation/BitrateController.kt index de2e61400d..48b2ba21ac 100644 --- a/jvb/src/main/kotlin/org/jitsi/videobridge/cc/allocation/BitrateController.kt +++ b/jvb/src/main/kotlin/org/jitsi/videobridge/cc/allocation/BitrateController.kt @@ -48,20 +48,12 @@ class BitrateController @JvmOverloads constructor( endpointsSupplier: Supplier>, private val diagnosticContext: DiagnosticContext, parentLogger: Logger, - private val useSourceNames: Boolean, private val clock: Clock = Clock.systemUTC() ) { val eventEmitter = SyncEventEmitter() private val bitrateAllocatorEventHandler = BitrateAllocatorEventHandler() - /** - * Keep track of the "forwarded" endpoints, i.e. the endpoints for which we are forwarding *some* layer. - */ - @Deprecated("", ReplaceWith("forwardedSources"), DeprecationLevel.WARNING) - var forwardedEndpoints: Set = emptySet() - private set - /** * Keep track of the "forwarded" sources, i.e. the media sources for which we are forwarding *some* layer. */ @@ -99,7 +91,7 @@ class BitrateController @JvmOverloads constructor( ) fun hasSuspendedSources() = bandwidthAllocator.allocation.hasSuspendedSources - private val allocationSettingsWrapper = AllocationSettingsWrapper(useSourceNames, parentLogger) + private val allocationSettingsWrapper = AllocationSettingsWrapper(parentLogger) val allocationSettings get() = allocationSettingsWrapper.get() @@ -127,10 +119,8 @@ class BitrateController @JvmOverloads constructor( fun expire() = bandwidthAllocator.expire() - /** - * Return the number of endpoints whose streams are currently being forwarded. - */ - fun numForwardedEndpoints(): Int = forwardedEndpoints.size + /** Return the number of sources currently being forwarded. */ + fun numForwardedSources(): Int = forwardedSources.size fun getTotalOversendingTime(): Duration = oversendingTimeTracker.totalTimeOn() fun isOversending() = oversendingTimeTracker.state fun bandwidthChanged(newBandwidthBps: Long) { @@ -154,7 +144,6 @@ class BitrateController @JvmOverloads constructor( get() = JSONObject().apply { put("bitrate_allocator", bandwidthAllocator.debugState) put("packet_handler", packetHandler.debugState) - put("forwardedEndpoints", forwardedEndpoints.toString()) put("forwardedSources", forwardedSources.toString()) put("oversending", oversendingTimeTracker.state) put("total_oversending_time_secs", oversendingTimeTracker.totalTimeOn().seconds) @@ -170,7 +159,6 @@ class BitrateController @JvmOverloads constructor( fun setBandwidthAllocationSettings(message: ReceiverVideoConstraintsMessage) { if (allocationSettingsWrapper.setBandwidthAllocationSettings(message)) { - // TODO write a test for a user which uses only the endpoint based constraints bandwidthAllocator.update(allocationSettingsWrapper.get()) } } @@ -256,7 +244,6 @@ class BitrateController @JvmOverloads constructor( } interface EventHandler { - fun forwardedEndpointsChanged(forwardedEndpoints: Set) fun forwardedSourcesChanged(forwardedSources: Set) fun effectiveVideoConstraintsChanged( oldEffectiveConstraints: EffectiveConstraintsMap, @@ -276,18 +263,10 @@ class BitrateController @JvmOverloads constructor( // Actually implement the allocation (configure the packet filter to forward the chosen target layers). packetHandler.allocationChanged(allocation) - if (useSourceNames) { - val newForwardedSources = allocation.forwardedSources - if (forwardedSources != newForwardedSources) { - forwardedSources = newForwardedSources - eventEmitter.fireEvent { forwardedSourcesChanged(newForwardedSources) } - } - } else { - val newForwardedEndpoints = allocation.forwardedEndpoints - if (forwardedEndpoints != newForwardedEndpoints) { - forwardedEndpoints = newForwardedEndpoints - eventEmitter.fireEvent { forwardedEndpointsChanged(newForwardedEndpoints) } - } + val newForwardedSources = allocation.forwardedSources + if (forwardedSources != newForwardedSources) { + forwardedSources = newForwardedSources + eventEmitter.fireEvent { forwardedSourcesChanged(newForwardedSources) } } oversendingTimeTracker.setState(allocation.oversending) @@ -309,7 +288,7 @@ class BitrateController @JvmOverloads constructor( } /** - * Abstracts a source endpoint for the purposes of [BandwidthAllocator]. + * Abstracts a media source for the purposes of [BandwidthAllocator]. */ interface MediaSourceContainer { val id: String diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/colibri2/Colibri2ConferenceHandler.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/colibri2/Colibri2ConferenceHandler.kt index 8e65fb4f1b..0e33906f70 100644 --- a/jvb/src/main/kotlin/org/jitsi/videobridge/colibri2/Colibri2ConferenceHandler.kt +++ b/jvb/src/main/kotlin/org/jitsi/videobridge/colibri2/Colibri2ConferenceHandler.kt @@ -174,13 +174,15 @@ class Colibri2ConferenceHandler( Condition.bad_request, "Attempt to create endpoint ${c2endpoint.id} with no " ) - val sourceNames = c2endpoint.hasCapability(Capability.CAP_SOURCE_NAME_SUPPORT) - val ssrcRewriting = sourceNames && c2endpoint.hasCapability(Capability.CAP_SSRC_REWRITING_SUPPORT) + if (!c2endpoint.hasCapability(Capability.CAP_SOURCE_NAME_SUPPORT)) { + throw IqProcessingException(Condition.bad_request, "Source name support is mandatory.") + } + + val ssrcRewriting = c2endpoint.hasCapability(Capability.CAP_SSRC_REWRITING_SUPPORT) val privateAddresses = c2endpoint.hasCapability(Capability.CAP_PRIVATE_ADDRESS_CONNECTIVITY) conference.createLocalEndpoint( c2endpoint.id, transport.iceControlling, - sourceNames, ssrcRewriting, c2endpoint.mucRole == MUCRole.visitor, privateAddresses diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/load_management/LastNReducer.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/load_management/LastNReducer.kt index 529c4362ee..de4f0f3f32 100644 --- a/jvb/src/main/kotlin/org/jitsi/videobridge/load_management/LastNReducer.kt +++ b/jvb/src/main/kotlin/org/jitsi/videobridge/load_management/LastNReducer.kt @@ -71,7 +71,7 @@ class LastNReducer( .asSequence() .filterIsInstance() .map { - it.numForwardedEndpoints() + it.numForwardedSources() } .maxOrNull() } diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/message/BridgeChannelMessage.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/message/BridgeChannelMessage.kt index 6a0d65f461..71daa008ae 100644 --- a/jvb/src/main/kotlin/org/jitsi/videobridge/message/BridgeChannelMessage.kt +++ b/jvb/src/main/kotlin/org/jitsi/videobridge/message/BridgeChannelMessage.kt @@ -20,7 +20,6 @@ import com.fasterxml.jackson.annotation.JsonAnySetter import com.fasterxml.jackson.annotation.JsonIgnore import com.fasterxml.jackson.annotation.JsonIgnoreProperties import com.fasterxml.jackson.annotation.JsonInclude -import com.fasterxml.jackson.annotation.JsonProperty import com.fasterxml.jackson.annotation.JsonSubTypes import com.fasterxml.jackson.annotation.JsonTypeInfo import com.fasterxml.jackson.core.JsonParser @@ -51,11 +50,9 @@ import java.util.concurrent.atomic.AtomicLong JsonSubTypes.Type(value = LastNMessage::class, name = LastNMessage.TYPE), JsonSubTypes.Type(value = DominantSpeakerMessage::class, name = DominantSpeakerMessage.TYPE), JsonSubTypes.Type(value = EndpointConnectionStatusMessage::class, name = EndpointConnectionStatusMessage.TYPE), - JsonSubTypes.Type(value = ForwardedEndpointsMessage::class, name = ForwardedEndpointsMessage.TYPE), JsonSubTypes.Type(value = ForwardedSourcesMessage::class, name = ForwardedSourcesMessage.TYPE), JsonSubTypes.Type(value = VideoSourcesMap::class, name = VideoSourcesMap.TYPE), JsonSubTypes.Type(value = AudioSourcesMap::class, name = AudioSourcesMap.TYPE), - JsonSubTypes.Type(value = SenderVideoConstraintsMessage::class, name = SenderVideoConstraintsMessage.TYPE), JsonSubTypes.Type(value = SenderSourceConstraintsMessage::class, name = SenderSourceConstraintsMessage.TYPE), JsonSubTypes.Type(value = AddReceiverMessage::class, name = AddReceiverMessage.TYPE), JsonSubTypes.Type(value = RemoveReceiverMessage::class, name = RemoveReceiverMessage.TYPE), @@ -116,11 +113,9 @@ open class MessageHandler { is LastNMessage -> lastN(message) is DominantSpeakerMessage -> dominantSpeaker(message) is EndpointConnectionStatusMessage -> endpointConnectionStatus(message) - is ForwardedEndpointsMessage -> forwardedEndpoints(message) is ForwardedSourcesMessage -> forwardedSources(message) is VideoSourcesMap -> videoSourcesMap(message) is AudioSourcesMap -> audioSourcesMap(message) - is SenderVideoConstraintsMessage -> senderVideoConstraints(message) is SenderSourceConstraintsMessage -> senderSourceConstraints(message) is AddReceiverMessage -> addReceiver(message) is RemoveReceiverMessage -> removeReceiver(message) @@ -143,11 +138,9 @@ open class MessageHandler { open fun lastN(message: LastNMessage) = unhandledMessageReturnNull(message) open fun dominantSpeaker(message: DominantSpeakerMessage) = unhandledMessageReturnNull(message) open fun endpointConnectionStatus(message: EndpointConnectionStatusMessage) = unhandledMessageReturnNull(message) - open fun forwardedEndpoints(message: ForwardedEndpointsMessage) = unhandledMessageReturnNull(message) open fun forwardedSources(message: ForwardedSourcesMessage) = unhandledMessageReturnNull(message) open fun videoSourcesMap(message: VideoSourcesMap) = unhandledMessageReturnNull(message) open fun audioSourcesMap(message: AudioSourcesMap) = unhandledMessageReturnNull(message) - open fun senderVideoConstraints(message: SenderVideoConstraintsMessage) = unhandledMessageReturnNull(message) open fun senderSourceConstraints(message: SenderSourceConstraintsMessage) = unhandledMessageReturnNull(message) open fun addReceiver(message: AddReceiverMessage) = unhandledMessageReturnNull(message) open fun removeReceiver(message: RemoveReceiverMessage) = unhandledMessageReturnNull(message) @@ -310,22 +303,6 @@ class EndpointConnectionStatusMessage( } } -/** - * A message sent from the bridge to a client, indicating the set of endpoints that are currently being forwarded. - */ -@Deprecated("Use ForwardedSourcesMessage", ReplaceWith("ForwardedSourcesMessage"), DeprecationLevel.WARNING) -class ForwardedEndpointsMessage( - /** - * The set of endpoints for which the bridge is currently sending video. - */ - @get:JsonProperty("lastNEndpoints") - val forwardedEndpoints: Collection -) : BridgeChannelMessage() { - companion object { - const val TYPE = "LastNEndpointsChangeEvent" - } -} - /** * A message sent from the bridge to a client, indicating the set of media sources that are currently being forwarded. */ @@ -392,32 +369,6 @@ class AudioSourcesMap( } } -/** - * A message sent from the bridge to a client (sender), indicating constraints for the sender's video streams. - * - * TODO: consider and adjust the format of videoConstraints. Do we need all of the VideoConstraints fields? Document. - * TODO: update https://github.com/jitsi/jitsi-videobridge/blob/master/doc/constraints.md before removing. - */ -@Deprecated("", ReplaceWith("SenderSourceConstraints"), DeprecationLevel.WARNING) -class SenderVideoConstraintsMessage(val videoConstraints: VideoConstraints) : BridgeChannelMessage() { - constructor(maxHeight: Int) : this(VideoConstraints(maxHeight)) - - /** - * Serialize manually because it's faster than Jackson. - * - * We use the "idealHeight" format that the jitsi-meet client expects. - */ - override fun createJson(): String = - """{"colibriClass":"$TYPE", "videoConstraints":{"idealHeight":${videoConstraints.idealHeight}}}""" - - @JsonIgnoreProperties(ignoreUnknown = true) - data class VideoConstraints(val idealHeight: Int) - - companion object { - const val TYPE = "SenderVideoConstraints" - } -} - /** * A message sent from the bridge to a client (sender), indicating constraints for the sender's video stream. */ @@ -482,11 +433,7 @@ class RemoveReceiverMessage( class ReceiverVideoConstraintsMessage( val lastN: Int? = null, - @Deprecated("", ReplaceWith("selectedSources"), DeprecationLevel.WARNING) - val selectedEndpoints: List? = null, val selectedSources: List? = null, - @Deprecated("", ReplaceWith("onStageSources"), DeprecationLevel.WARNING) - val onStageEndpoints: List? = null, val onStageSources: List? = null, val defaultConstraints: VideoConstraints? = null, val constraints: Map? = null, @@ -531,6 +478,7 @@ class SourceVideoTypeMessage( /** * A signaling the type of video stream an endpoint has available. */ +@Deprecated("", ReplaceWith("SourceVideoTypeMessage"), DeprecationLevel.WARNING) class VideoTypeMessage( val videoType: VideoType, /** diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/relay/RelayedEndpoint.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/relay/RelayedEndpoint.kt index 079fe2dd8f..37beba0136 100644 --- a/jvb/src/main/kotlin/org/jitsi/videobridge/relay/RelayedEndpoint.kt +++ b/jvb/src/main/kotlin/org/jitsi/videobridge/relay/RelayedEndpoint.kt @@ -124,8 +124,6 @@ class RelayedEndpoint( // Visitors are never advertised between relays, so relayed endpoints are never visitors. override val visitor = false - fun hasReceiveSsrcs(): Boolean = streamInformationStore.receiveSsrcs.isNotEmpty() - /** Relayed endpoints are not automatically expired. **/ override fun shouldExpire(): Boolean = false @@ -144,20 +142,7 @@ class RelayedEndpoint( override fun setExtmapAllowMixed(allow: Boolean) = streamInformationStore.setExtmapAllowMixed(allow) - @Deprecated("use sendVideoConstraintsV2") - override fun sendVideoConstraints(maxVideoConstraints: VideoConstraints) { - relay.sendMessage( - AddReceiverMessage( - RelayConfig.config.relayId, - id, - // source name - used in multi-stream - null, - maxVideoConstraints - ) - ) - } - - override fun sendVideoConstraintsV2(sourceName: String, maxVideoConstraints: VideoConstraints) { + override fun sendVideoConstraints(sourceName: String, maxVideoConstraints: VideoConstraints) { relay.sendMessage( AddReceiverMessage( RelayConfig.config.relayId, @@ -171,7 +156,7 @@ class RelayedEndpoint( fun relayMessageTransportConnected() { maxReceiverVideoConstraints.forEach { (sourceName, constraints) -> - sendVideoConstraintsV2(sourceName, constraints) + sendVideoConstraints(sourceName, constraints) } } diff --git a/jvb/src/test/kotlin/org/jitsi/videobridge/ConferenceTest.kt b/jvb/src/test/kotlin/org/jitsi/videobridge/ConferenceTest.kt index 95aaebcb24..b0eb96f306 100644 --- a/jvb/src/test/kotlin/org/jitsi/videobridge/ConferenceTest.kt +++ b/jvb/src/test/kotlin/org/jitsi/videobridge/ConferenceTest.kt @@ -38,7 +38,7 @@ class ConferenceTest : ConfigTest() { with(Conference(videobridge, "id", name, null, false)) { endpointCount shouldBe 0 // TODO cover the case when they're true - createLocalEndpoint("abcdabcd", true, false, false, false, false) + createLocalEndpoint("abcdabcd", true, false, false, false) endpointCount shouldBe 1 debugState.shouldBeValidJson() } diff --git a/jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/AllocationSettingsTest.kt b/jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/AllocationSettingsTest.kt index 51668dc9e6..540632c8e4 100644 --- a/jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/AllocationSettingsTest.kt +++ b/jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/AllocationSettingsTest.kt @@ -24,13 +24,11 @@ class AllocationSettingsTest : ShouldSpec() { context("computeVideoConstraints") { context("With client which supports source names") { context("no conversion from endpoint to source takes place") { - val allocationSettings = AllocationSettingsWrapper(true) + val allocationSettings = AllocationSettingsWrapper() allocationSettings.setBandwidthAllocationSettings( ReceiverVideoConstraintsMessage( onStageSources = listOf("S1", "S2"), - onStageEndpoints = listOf("E1", "E2"), selectedSources = listOf("S3", "S4"), - selectedEndpoints = listOf("E3", "E4"), constraints = mapOf( "S1" to VideoConstraints(720), "E1" to VideoConstraints(360) @@ -51,48 +49,6 @@ class AllocationSettingsTest : ShouldSpec() { ) } } - context("With client which doesn't support source names") { - context("Converts onStageEndpoints to onStageSources") { - val allocationSettings = AllocationSettingsWrapper(false) - allocationSettings.setBandwidthAllocationSettings( - ReceiverVideoConstraintsMessage( - onStageEndpoints = listOf("A", "C") - ) - ) - - allocationSettings.get().onStageEndpoints shouldBe emptyList() - allocationSettings.get().onStageSources shouldBe listOf("A-v0", "C-v0") - } - context("Converts selectedEndpoints to selectedSources") { - val allocationSettings = AllocationSettingsWrapper(false) - allocationSettings.setBandwidthAllocationSettings( - ReceiverVideoConstraintsMessage( - selectedEndpoints = listOf("A", "C") - ) - ) - - allocationSettings.get().selectedEndpoints shouldBe emptyList() - allocationSettings.get().selectedSources shouldBe listOf("A-v0", "C-v0") - } - context("Converts endpoints based constraints to source based ones") { - val allocationSettings = AllocationSettingsWrapper(false) - allocationSettings.setBandwidthAllocationSettings( - ReceiverVideoConstraintsMessage( - constraints = mapOf( - "A" to VideoConstraints(720, 15.0), - "B" to VideoConstraints(360, 24.0), - "C" to VideoConstraints(180, 30.0) - ) - ) - ) - - allocationSettings.get().videoConstraints shouldBe mapOf( - "A-v0" to VideoConstraints(720, 15.0), - "B-v0" to VideoConstraints(360, 24.0), - "C-v0" to VideoConstraints(180, 30.0) - ) - } - } } } } diff --git a/jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/BitrateControllerPerfTest.kt b/jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/BitrateControllerPerfTest.kt index 6b7058fbfd..68fc0881a9 100644 --- a/jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/BitrateControllerPerfTest.kt +++ b/jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/BitrateControllerPerfTest.kt @@ -29,6 +29,7 @@ import org.jitsi.utils.nanos import org.jitsi.utils.secs import org.jitsi.utils.time.FakeClock import org.jitsi.videobridge.message.ReceiverVideoConstraintsMessage +import org.jitsi.videobridge.util.endpointIdToSourceName import java.util.function.Supplier import kotlin.random.Random import kotlin.time.ExperimentalTime @@ -67,7 +68,6 @@ class BitrateControllerPerfTest : StringSpec() { private val endpoints: MutableList = createEndpoints(*endpointIds.toTypedArray()) private val bc = BitrateController( object : BitrateController.EventHandler { - override fun forwardedEndpointsChanged(forwardedEndpoints: Set) { } override fun forwardedSourcesChanged(forwardedSources: Set) { } override fun effectiveVideoConstraintsChanged( oldEffectiveConstraints: EffectiveConstraintsMap, @@ -79,8 +79,6 @@ class BitrateControllerPerfTest : StringSpec() { Supplier { endpoints.toList() }, DiagnosticContext(), createLogger(), - // TODO cover the case for true? - false, clock, ).apply { // The BC only starts working 10 seconds after it first received media, so fake that. @@ -122,7 +120,7 @@ class BitrateControllerPerfTest : StringSpec() { bc.setBandwidthAllocationSettings( ReceiverVideoConstraintsMessage( - selectedEndpoints = selectedEndpoints, + selectedSources = selectedEndpoints.map { endpointIdToSourceName(it) }, defaultConstraints = VideoConstraints(maxFrameHeight) ) ) diff --git a/jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/BitrateControllerTest.kt b/jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/BitrateControllerTest.kt index e50c0f7197..4649e80586 100644 --- a/jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/BitrateControllerTest.kt +++ b/jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/BitrateControllerTest.kt @@ -1369,8 +1369,6 @@ class BitrateControllerWrapper(initialEndpoints: List, val val bc = BitrateController( object : BitrateController.EventHandler { - override fun forwardedEndpointsChanged(forwardedEndpoints: Set) { } - override fun forwardedSourcesChanged(forwardedSources: Set) { Event(bwe, forwardedSources, clock.instant()).apply { logger.info("Forwarded sources changed: $this") @@ -1404,8 +1402,6 @@ class BitrateControllerWrapper(initialEndpoints: List, val Supplier { endpoints }, DiagnosticContext(), logger, - // TODO merge BitrateControllerNewTest with old and use this flag - true, clock ) diff --git a/jvb/src/test/kotlin/org/jitsi/videobridge/load_management/LastNReducerTest.kt b/jvb/src/test/kotlin/org/jitsi/videobridge/load_management/LastNReducerTest.kt index 3350e4d582..1ff0a251b9 100644 --- a/jvb/src/test/kotlin/org/jitsi/videobridge/load_management/LastNReducerTest.kt +++ b/jvb/src/test/kotlin/org/jitsi/videobridge/load_management/LastNReducerTest.kt @@ -108,7 +108,7 @@ inline fun withLastNConfig(config: String, block: () -> T): T { */ private fun createMockConference(vararg epNumForwardedVideo: Int): Conference { val eps = epNumForwardedVideo.map { - mockk { every { numForwardedEndpoints() } returns it } + mockk { every { numForwardedSources() } returns it } }.toList() return mockk { every { endpoints } returns eps diff --git a/jvb/src/test/kotlin/org/jitsi/videobridge/message/BridgeChannelMessageTest.kt b/jvb/src/test/kotlin/org/jitsi/videobridge/message/BridgeChannelMessageTest.kt index 9ef13769e8..06854a0f14 100644 --- a/jvb/src/test/kotlin/org/jitsi/videobridge/message/BridgeChannelMessageTest.kt +++ b/jvb/src/test/kotlin/org/jitsi/videobridge/message/BridgeChannelMessageTest.kt @@ -31,7 +31,6 @@ import io.kotest.matchers.types.shouldBeInstanceOf import org.jitsi.nlj.VideoType import org.jitsi.videobridge.cc.allocation.VideoConstraints import org.jitsi.videobridge.message.BridgeChannelMessage.Companion.parse -import org.json.simple.JSONArray import org.json.simple.JSONObject import org.json.simple.parser.JSONParser @@ -201,24 +200,6 @@ class BridgeChannelMessageTest : ShouldSpec() { parsed.active shouldBe "true" } - context("serializing and parsing ForwardedEndpointsMessage") { - val forwardedEndpoints = setOf("a", "b", "c") - - val message = ForwardedEndpointsMessage(forwardedEndpoints) - val parsed = parse(message.toJson()) - - parsed.shouldBeInstanceOf() - - parsed.forwardedEndpoints shouldContainExactly forwardedEndpoints - - // Make sure the forwardedEndpoints field is serialized as lastNEndpoints as the client (presumably) expects - val parsedJson = JSONParser().parse(message.toJson()) - parsedJson.shouldBeInstanceOf() - val parsedForwardedEndpoints = parsedJson["lastNEndpoints"] - parsedForwardedEndpoints.shouldBeInstanceOf() - parsedForwardedEndpoints.toList() shouldContainExactly forwardedEndpoints - } - context("serializing and parsing ForwardedSourcesMessage") { val forwardedSources = setOf("s1", "s2", "s3") @@ -236,18 +217,9 @@ class BridgeChannelMessageTest : ShouldSpec() { videoConstraints.maxFrameRate shouldBe 15.0 } - context("and SenderVideoConstraintsMessage") { - val senderVideoConstraintsMessage = SenderVideoConstraintsMessage(1080) - val parsed = parse(senderVideoConstraintsMessage.toJson()) - - parsed.shouldBeInstanceOf() - - parsed.videoConstraints.idealHeight shouldBe 1080 - } - context("serializing and parsing SenderSourceConstraintsMessage") { - val senderVideoConstraintsMessage = SenderSourceConstraintsMessage("s1", 1080) - val parsed = parse(senderVideoConstraintsMessage.toJson()) + val senderSourceConstraintsMessage = SenderSourceConstraintsMessage("s1", 1080) + val parsed = parse(senderSourceConstraintsMessage.toJson()) parsed.shouldBeInstanceOf() @@ -455,8 +427,6 @@ class BridgeChannelMessageTest : ShouldSpec() { parsed.shouldBeInstanceOf() parsed.lastN shouldBe 3 - parsed.onStageEndpoints shouldBe listOf("onstage1", "onstage2") - parsed.selectedEndpoints shouldBe listOf("selected1", "selected2") parsed.defaultConstraints shouldBe VideoConstraints(0) val constraints = parsed.constraints constraints.shouldNotBeNull() @@ -470,8 +440,6 @@ class BridgeChannelMessageTest : ShouldSpec() { val parsed = parse(RECEIVER_VIDEO_CONSTRAINTS_EMPTY) parsed.shouldBeInstanceOf() parsed.lastN shouldBe null - parsed.onStageEndpoints shouldBe null - parsed.selectedEndpoints shouldBe null parsed.defaultConstraints shouldBe null parsed.constraints shouldBe null } @@ -546,8 +514,6 @@ class BridgeChannelMessageTest : ShouldSpec() { { "colibriClass": "ReceiverVideoConstraints", "lastN": 3, - "selectedEndpoints": [ "selected1", "selected2" ], - "onStageEndpoints": [ "onstage1", "onstage2" ], "defaultConstraints": { "maxHeight": 0 }, "constraints": { "epOnStage": { "maxHeight": 720 },