Make the initial period to ignore BWE configurable. (#2324)

Don't allocate based on an uninitialized bwe.

Author: JonathanLennox

jvb/src/main/kotlin/org/jitsi/videobridge/cc/allocation/BandwidthAllocator.kt

@@ -377,6 +377,9 @@ internal class BandwidthAllocator<T : MediaSourceContainer>(
  * @return true if the bandwidth has changed above the configured threshold, * false otherwise.
  */
 private fun bweChangeIsLargerThanThreshold(previousBwe: Long, currentBwe: Long): Boolean {
+    if (previousBwe == currentBwe) { // Even if we're "changing" -1 to -1
+        return false
+    }
     if (previousBwe == -1L || currentBwe == -1L) {
         return true
     }

jvb/src/main/kotlin/org/jitsi/videobridge/cc/allocation/BitrateController.kt

@@ -28,7 +28,6 @@ import org.jitsi.utils.logging.DiagnosticContext
 import org.jitsi.utils.logging.TimeSeriesLogger
 import org.jitsi.utils.logging2.Logger
 import org.jitsi.utils.logging2.createChildLogger
-import org.jitsi.utils.secs
 import org.jitsi.videobridge.cc.config.BitrateControllerConfig.Companion.config
 import org.jitsi.videobridge.message.ReceiverVideoConstraintsMessage
 import org.jitsi.videobridge.util.BooleanStateTimeTracker
@@ -104,13 +103,15 @@ class BitrateController<T : MediaSourceContainer> @JvmOverloads constructor(
         eventEmitter.addHandler(eventHandler)
     }
 
+    private var bweSet = false
+
     /**
-     * Ignore the bandwidth estimations in the first 10 seconds because the REMBs don't ramp up fast enough. This needs
-     * to go but it's related to our GCC implementation that needs to be brought up to speed.
-     * TODO: Is this comment still accurate?
+     * Ignore the bandwidth estimations for some initial time because the REMBs don't ramp up fast enough.
+     * This shouldn't be needed for other bandwidth estimation algorithms.
      */
     private val trustBwe: Boolean
-        get() = config.trustBwe && supportsRtx && packetHandler.timeSinceFirstMedia() >= 10.secs
+        get() = config.trustBwe && supportsRtx && bweSet &&
+            packetHandler.timeSinceFirstMedia() >= config.initialIgnoreBwePeriod
 
     // Proxy to the allocator
     fun endpointOrderingChanged() = bandwidthAllocator.update()
@@ -129,6 +130,7 @@ class BitrateController<T : MediaSourceContainer> @JvmOverloads constructor(
     fun getTotalOversendingTime(): Duration = oversendingTimeTracker.totalTimeOn()
     fun isOversending() = oversendingTimeTracker.state
     fun bandwidthChanged(newBandwidthBps: Long) {
+        bweSet = true
         timeSeriesLogger?.logBweChange(newBandwidthBps)
         bandwidthAllocator.bandwidthChanged(newBandwidthBps)
     }

jvb/src/main/kotlin/org/jitsi/videobridge/cc/config/BitrateControllerConfig.kt

@@ -114,6 +114,14 @@ class BitrateControllerConfig private constructor() {
         "videobridge.cc.use-vla-target-bitrate".from(JitsiConfig.newConfig)
     }
 
+    /**
+     * How long after first media to ignore bandwidth estimation.  Needed for BWE
+     * algorithms that ramp up slowly; should be set to zero if this isn't a problem.
+     */
+    val initialIgnoreBwePeriod: Duration by config(
+        "videobridge.cc.initial-ignore-bwe-period".from(JitsiConfig.newConfig)
+    )
+
     companion object {
         @JvmField
         val config = BitrateControllerConfig()

jvb/src/main/resources/reference.conf

@@ -81,6 +81,10 @@ videobridge {
     # Whether to use the target bitrate signaled in the VLA extension for allocation. When disabled we use the measured
     # bitrate instead (preserving previous behavior).
     use-vla-target-bitrate = false
+
+    # How long after first media to ignore bandwidth estimation for the purposes of allocation.  Needed
+    # for bwe algorithms that ramp up slowly; should be set to zero if this isn't a problem.
+    initial-ignore-bwe-period = 10 seconds
   }
   # Whether to indicate support for cryptex header extension encryption (RFC 9335)
   cryptex {

jvb/src/test/kotlin/org/jitsi/videobridge/cc/allocation/BitrateControllerTest.kt

@@ -1403,7 +1403,7 @@ class BitrateControllerWrapper(initialEndpoints: List<MediaSourceContainer>, val
         DiagnosticContext(),
         logger,
         clock
-    )
+    ).apply { bandwidthChanged(bwe.bps.toLong()) } // TODO: handle the -1 bps case better
 
     fun setEndpointOrdering(vararg endpoints: TestEndpoint) {
         logger.info("Set endpoints ${endpoints.map{ it.id }.joinToString(",")}")