Skip to content

Commit

Permalink
Remove backward compat for source names. (#2087)
Browse files Browse the repository at this point in the history 
* Remove backward compat for source names.

* Remove deprecated sendVideoConstraints.

* ref: Remove SenderVideoConstraintsMessage.

* ref: Remove "V2" from function name.
  • Loading branch information
@bgrozev
bgrozev authored Jan 29, 2024
1 parent 573abcd commit f9314a2
Show file tree
Hide file tree
Showing 16 changed files with 49 additions and 342 deletions.
13 changes: 0 additions & 13 deletions doc/constraints.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
```

4 changes: 1 addition & 3 deletions jvb/src/main/java/org/jitsi/videobridge/Conference.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -718,15 +718,13 @@ 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 <tt>Endpoint</tt> participating in this <tt>Conference</tt>
*/
@NotNull
public Endpoint createLocalEndpoint(
String id,
boolean iceControlling,
boolean sourceNames,
boolean doSsrcRewriting,
boolean visitor,
boolean privateAddresses)
Expand All @@ -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);
Expand Down
15 changes: 2 additions & 13 deletions jvb/src/main/kotlin/org/jitsi/videobridge/AbstractEndpoint.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -265,25 +265,14 @@ 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.
*
* @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
Expand Down
78 changes: 11 additions & 67 deletions jvb/src/main/kotlin/org/jitsi/videobridge/Endpoint.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -205,9 +202,6 @@ class Endpoint @JvmOverloads constructor(
// Intentional no-op
}

override fun forwardedEndpointsChanged(forwardedEndpoints: Set<String>) =
sendForwardedEndpointsMessage(forwardedEndpoints)

override fun forwardedSourcesChanged(forwardedSources: Set<String>) {
sendForwardedSourcesMessage(forwardedSources)
}
Expand All @@ -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. */
Expand Down Expand Up @@ -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<MediaSourceDesc>
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -726,39 +696,13 @@ 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<String>) {
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.
*
* @param forwardedSources the collection of forwarded media sources (by name).
*/
fun sendForwardedSourcesMessage(forwardedSources: Collection<String>) {
if (!isUsingSourceNames) {
return
}

val msg = ForwardedSourcesMessage(forwardedSources)
TaskPools.IO_POOL.execute {
try {
Expand Down Expand Up @@ -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
Expand Down
63 changes: 11 additions & 52 deletions jvb/src/main/kotlin/org/jitsi/videobridge/cc/allocation/AllocationSettings.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<String>()

/**
* The last selected sources set signaled by the receiving endpoint.
*/
Expand All @@ -84,9 +76,6 @@ internal class AllocationSettingsWrapper(

private var assumedBandwidthBps: Long = -1

@Deprecated("", ReplaceWith("onStageSources"), DeprecationLevel.WARNING)
private var onStageEndpoints: List<String> = emptyList()

private var onStageSources: List<String> = emptyList()

private var allocationSettings = create()
Expand All @@ -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 {
Expand All @@ -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
}
}
Expand Down
Loading

0 comments on commit f9314a2

Please sign in to comment.