Enforce an interval at which BitrateController#update runs (#1397)

* use Clock in BitrateController * enforce minimum interval at which BitrateController#update runs * fix initial value
jitsi · Aug 24, 2020 · 8488f77 · 8488f77
1 parent 63fe425
commit 8488f77
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 6 deletions.
diff --git a/jvb/src/main/java/org/jitsi/videobridge/cc/BitrateController.java b/jvb/src/main/java/org/jitsi/videobridge/cc/BitrateController.java
@@ -27,10 +27,13 @@
 import org.jitsi.utils.logging2.Logger;
 import org.jitsi.videobridge.*;
 import org.jitsi.videobridge.cc.config.*;
+import org.jitsi.videobridge.util.*;
 import org.json.simple.*;
 
+import javax.annotation.CheckReturnValue;
 import java.lang.*;
 import java.lang.SuppressWarnings;
+import java.time.*;
 import java.util.*;
 import java.util.concurrent.*;
 import java.util.stream.*;
@@ -212,23 +215,40 @@ public class BitrateController
     private final AtomicInteger numDroppedPacketsUnknownSsrc =
         new AtomicInteger(0);
 
+    private final Clock clock;
+
+    /**
+     * The last time {@link BitrateController#update()} was called
+     */
+    private Instant lastUpdateTime = Instant.MIN;
+
     /**
      * Initializes a new {@link BitrateController} instance which is to
      * belong to a particular {@link Endpoint}.
      */
     public BitrateController(
             Endpoint destinationEndpoint,
             @NotNull DiagnosticContext diagnosticContext,
-            Logger parentLogger
+            Logger parentLogger,
+            Clock clock
     )
     {
         this.destinationEndpoint = destinationEndpoint;
         this.diagnosticContext = diagnosticContext;
         this.logger = parentLogger.createChildLogger(BitrateController.class.getName());
+        this.clock = clock;
 
         enableVideoQualityTracing = timeSeriesLogger.isTraceEnabled();
     }
 
+    public BitrateController(
+        Endpoint destinationEndpoint,
+        @NotNull DiagnosticContext diagnosticContext,
+        Logger parentLogger
+    ) {
+        this(destinationEndpoint, diagnosticContext, parentLogger, Clock.systemUTC());
+    }
+
     /**
      * Returns a boolean that indicates whether or not the current bandwidth
      * estimation (in bps) has changed above the configured threshold (in
@@ -414,6 +434,11 @@ public boolean accept(@NotNull PacketInfo packetInfo)
      */
     public boolean accept(RtcpSrPacket rtcpSrPacket)
     {
+        if (Duration.between(lastUpdateTime, clock.instant())
+                .compareTo(BitrateControllerConfig.maxTimeBetweenCalculations()) > 0) {
+            logger.debug("Forcing an update");
+            TaskPools.CPU_POOL.submit(this::update);
+        }
         long ssrc = rtcpSrPacket.getSenderSsrc();
 
         AdaptiveSourceProjection adaptiveSourceProjection
@@ -431,7 +456,6 @@ public boolean accept(RtcpSrPacket rtcpSrPacket)
         return ssrc == adaptiveSourceProjection.getTargetSsrc();
     }
 
-
     public boolean transformRtcp(RtcpSrPacket rtcpSrPacket)
     {
         long ssrc = rtcpSrPacket.getSenderSsrc();
@@ -520,7 +544,7 @@ StatusSnapshot getStatusSnapshot()
         }
         List<Long> activeSsrcs = new ArrayList<>();
         long totalTargetBps = 0, totalIdealBps = 0;
-        long nowMs = System.currentTimeMillis();
+        long nowMs = clock.instant().toEpochMilli();
         for (MediaSourceDesc incomingSource : destinationEndpoint
             .getConference().getEndpoints().stream()
             .filter(e -> !destinationEndpoint.equals(e))
@@ -663,7 +687,9 @@ public synchronized void endpointOrderingChanged(List<String> conferenceEndpoint
      */
     private synchronized void update()
     {
-        long nowMs = System.currentTimeMillis();
+        Instant now = clock.instant();
+        lastUpdateTime = now;
+        long nowMs = now.toEpochMilli();
 
         long bweBps = getAvailableBandwidth(nowMs);
 
@@ -1120,7 +1146,7 @@ public boolean transformRtp(@NotNull PacketInfo packetInfo)
         VideoRtpPacket videoPacket = (VideoRtpPacket)packetInfo.getPacket();
         if (firstMediaMs == -1)
         {
-            firstMediaMs = System.currentTimeMillis();
+            firstMediaMs = clock.instant().toEpochMilli();
         }
 
         Long ssrc = videoPacket.getSsrc();
@@ -1302,7 +1328,7 @@ private SourceBitrateAllocation(
                 return;
             }
 
-            long nowMs = System.currentTimeMillis();
+            long nowMs = clock.instant().toEpochMilli();
             List<RateSnapshot> ratesList = new ArrayList<>();
             // Initialize the list of flows that we will consider for sending
             // for this source. For example, for the on-stage participant we

diff --git a/jvb/src/main/kotlin/org/jitsi/videobridge/cc/config/BitrateControllerConfig.kt b/jvb/src/main/kotlin/org/jitsi/videobridge/cc/config/BitrateControllerConfig.kt
@@ -19,6 +19,7 @@ package org.jitsi.videobridge.cc.config
 import org.jitsi.config.JitsiConfig
 import org.jitsi.metaconfig.config
 import org.jitsi.metaconfig.from
+import java.time.Duration
 
 class BitrateControllerConfig {
     companion object {
@@ -108,5 +109,15 @@ class BitrateControllerConfig {
 
         @JvmStatic
         fun onstageIdealHeightPx() = onstageIdealHeightPx
+
+        /**
+         * The maximum amount of time we'll run before recalculating which streams we'll
+         * forward.
+         */
+        private val maxTimeBetweenCalculations: Duration by
+            config("videobridge.cc.max-time-between-calculations".from(JitsiConfig.newConfig))
+
+        @JvmStatic
+        fun maxTimeBetweenCalculations() = maxTimeBetweenCalculations
     }
 }
diff --git a/jvb/src/main/resources/reference.conf b/jvb/src/main/resources/reference.conf
@@ -44,6 +44,10 @@ videobridge {
 
     # How often we check to send probing data
     padding-period=15ms
+
+    # How often we'll force recalculations of forwarded
+    # streams
+    max-time-between-calculations = 15 seconds
   }
   # The APIs by which the JVB can be controlled
   apis {