Use RateLimit to limit keyframe request sending rate to endpoints. (#2349)

* Use RateLimit to limit keyframe request sending rate to endpoints.

* Also limit keyframe request rate to the RTT.

Author: JonathanLennox

jitsi-media-transform/src/main/kotlin/org/jitsi/nlj/rtcp/KeyframeRequester.kt

@@ -16,6 +16,8 @@
 
 package org.jitsi.nlj.rtcp
 
+import org.jitsi.config.JitsiConfig
+import org.jitsi.metaconfig.config
 import org.jitsi.nlj.Event
 import org.jitsi.nlj.PacketInfo
 import org.jitsi.nlj.SetLocalSsrcEvent
@@ -28,15 +30,16 @@ import org.jitsi.rtp.rtcp.rtcpfb.payload_specific_fb.RtcpFbFirPacketBuilder
 import org.jitsi.rtp.rtcp.rtcpfb.payload_specific_fb.RtcpFbPliPacket
 import org.jitsi.rtp.rtcp.rtcpfb.payload_specific_fb.RtcpFbPliPacketBuilder
 import org.jitsi.utils.MediaType
-import org.jitsi.utils.NEVER
+import org.jitsi.utils.RateLimit
+import org.jitsi.utils.durationOfDoubleSeconds
 import org.jitsi.utils.logging2.Logger
 import org.jitsi.utils.logging2.cdebug
 import org.jitsi.utils.logging2.createChildLogger
+import org.jitsi.utils.min
 import java.time.Clock
 import java.time.Duration
 import java.time.Instant
 import java.util.concurrent.atomic.AtomicInteger
-import kotlin.math.min
 
 /**
  * [KeyframeRequester] handles a few things around keyframes:
@@ -54,11 +57,11 @@ class KeyframeRequester @JvmOverloads constructor(
     private val logger = createChildLogger(parentLogger)
 
     // Map a SSRC to the timestamp (represented as an [Instant]) of when we last requested a keyframe for it
-    private val keyframeRequests = mutableMapOf<Long, Instant>()
+    private val keyframeLimiter = mutableMapOf<Long, RateLimit>()
+    private val keyframeLimiterSyncRoot = Any()
     private val firCommandSequenceNumber: AtomicInteger = AtomicInteger(0)
-    private val keyframeRequestsSyncRoot = Any()
     private var localSsrc: Long? = null
-    private var waitInterval = DEFAULT_WAIT_INTERVAL
+    private var waitInterval = minInterval
 
     // Stats
 
@@ -127,14 +130,14 @@ class KeyframeRequester @JvmOverloads constructor(
         if (!streamInformationStore.supportsPli && !streamInformationStore.supportsFir) {
             return false
         }
-        synchronized(keyframeRequestsSyncRoot) {
-            return if (Duration.between(keyframeRequests.getOrDefault(mediaSsrc, NEVER), now) < waitInterval) {
-                logger.cdebug {
-                    "Sent a keyframe request less than $waitInterval ago for $mediaSsrc, ignoring request"
-                }
+        synchronized(keyframeLimiterSyncRoot) {
+            val limiter = keyframeLimiter.computeIfAbsent(mediaSsrc) {
+                RateLimit(defaultMinInterval = minInterval, maxRequests = maxRequests, interval = maxRequestInterval)
+            }
+            return if (!limiter.accept(now, waitInterval)) {
+                logger.cdebug { "Ignoring keyframe request for $mediaSsrc, rate limited" }
                 false
             } else {
-                keyframeRequests[mediaSsrc] = now
                 logger.cdebug { "Keyframe requester requesting keyframe for $mediaSsrc" }
                 true
             }
@@ -217,11 +220,19 @@ class KeyframeRequester @JvmOverloads constructor(
 
     fun onRttUpdate(newRtt: Double) {
         // avg(rtt) + stddev(rtt) would be more accurate than rtt + 10.
-        waitInterval = Duration.ofMillis(min(DEFAULT_WAIT_INTERVAL.toMillis(), newRtt.toLong() + 10))
+        waitInterval = min(minInterval, durationOfDoubleSeconds((newRtt + 10) / 1e3))
     }
 
     companion object {
-        private val DEFAULT_WAIT_INTERVAL = Duration.ofMillis(100)
+        private val minInterval: Duration by config {
+            "jmt.keyframe.min-interval".from(JitsiConfig.newConfig)
+        }
+        private val maxRequests: Int by config {
+            "jmt.keyframe.max-requests".from(JitsiConfig.newConfig)
+        }
+        private val maxRequestInterval: Duration by config {
+            "jmt.keyframe.max-request-interval".from(JitsiConfig.newConfig)
+        }
     }
 }
 

jitsi-media-transform/src/main/resources/reference.conf

@@ -66,6 +66,15 @@ jmt {
     accepted-fingerprint-hash-functions = [ sha-512, sha-384, sha-256, sha-1 ]
 
   }
+  keyframe {
+    // The minimum interval between consecutive keyframe requests for a media source.
+    // (This interval will also be limited by the current round-trip time to the sender of that source.)
+    min-interval = 200 ms
+    // The maximum number of requests for a media source per max-request-interval
+    max-requests = 3
+    // The interval over which to compute max-requests
+    max-request-interval = 10 s
+  }
   srtp {
     // The maximum number of packets that can be discarded early (without going through the SRTP stack for
     // authentication), or -1 to authenticate all packets.

pom.xml

@@ -27,7 +27,7 @@
         <kotlin.version>2.0.0</kotlin.version>
         <kotest.version>5.9.1</kotest.version>
         <junit.version>5.10.2</junit.version>
-        <jitsi.utils.version>1.0-143-g65e2bd7</jitsi.utils.version>
+        <jitsi.utils.version>1.0-145-g6673f0f</jitsi.utils.version>
         <jicoco.version>1.1-159-gf9c2712</jicoco.version>
         <mockk.version>1.13.11</mockk.version>
         <ktlint-maven-plugin.version>3.2.0</ktlint-maven-plugin.version>