diff --git a/src/main/kotlin/org/jitsi/nlj/rtp/TransportCcEngine.kt b/src/main/kotlin/org/jitsi/nlj/rtp/TransportCcEngine.kt index a8c98573c..88d700991 100644 --- a/src/main/kotlin/org/jitsi/nlj/rtp/TransportCcEngine.kt +++ b/src/main/kotlin/org/jitsi/nlj/rtp/TransportCcEngine.kt @@ -157,6 +157,8 @@ class TransportCcEngine( } } } + bandwidthEstimator.feedbackComplete(now) + if (missingPacketDetailSeqNums.isNotEmpty()) { logger.warn("TCC packet contained received sequence numbers: " + "${tccPacket.iterator().asSequence() diff --git a/src/main/kotlin/org/jitsi/nlj/rtp/bandwidthestimation/BandwidthEstimator.kt b/src/main/kotlin/org/jitsi/nlj/rtp/bandwidthestimation/BandwidthEstimator.kt index 817e6ddd2..e80f3ec91 100644 --- a/src/main/kotlin/org/jitsi/nlj/rtp/bandwidthestimation/BandwidthEstimator.kt +++ b/src/main/kotlin/org/jitsi/nlj/rtp/bandwidthestimation/BandwidthEstimator.kt @@ -73,6 +73,10 @@ abstract class BandwidthEstimator( * necessarily have any relationship to each other, but must be consistent * within themselves across all calls to functions of this [BandwidthEstimator]. * + * All arrival and loss reports based on a single feedback message should have the + * same [now] value. [feedbackComplete] should be called once all feedback reports + * based on a single feedback message have been processed. + * * @param[now] The current time, when this function is called. * @param[sendTime] The time the packet was sent, if known, or null. * @param[recvTime] The time the packet was received, if known, or null. @@ -125,6 +129,10 @@ abstract class BandwidthEstimator( /** * Inform the bandwidth estimator that a packet was lost. * + * All arrival and loss reports based on a single feedback message should have the + * same [now] value. [feedbackComplete] should be called once all feedback reports + * based on a single feedback message have been processed. + * * @param[now] The current time, when this function is called. * @param[sendTime] The time the packet was sent, if known, or null. * @param[seq] A 16-bit sequence number of packets processed by this @@ -150,6 +158,28 @@ abstract class BandwidthEstimator( */ protected abstract fun doProcessPacketLoss(now: Instant, sendTime: Instant?, seq: Int) + /** + * Inform the bandwidth estimator that a block of feedback is complete. + * + * @param[now] The current time, when this function is called. This should match + * the value passed to [processPacketArrival] and [processPacketLoss]. + */ + fun feedbackComplete(now: Instant) { + if (timeSeriesLogger.isTraceEnabled) { + val point = diagnosticContext.makeTimeSeriesPoint("bwe_feedback_complete", now) + timeSeriesLogger.trace(point) + } + + doFeedbackComplete(now) + } + + /** + * A subclass's implementation of [feedbackComplete]. + * + * See that function for parameter details. + */ + protected abstract fun doFeedbackComplete(now: Instant) + /** * Inform the bandwidth estimator about a new round-trip time value */ diff --git a/src/main/kotlin/org/jitsi/nlj/rtp/bandwidthestimation/GoogleCcEstimator.kt b/src/main/kotlin/org/jitsi/nlj/rtp/bandwidthestimation/GoogleCcEstimator.kt index eb16458e7..ca0a0cf95 100644 --- a/src/main/kotlin/org/jitsi/nlj/rtp/bandwidthestimation/GoogleCcEstimator.kt +++ b/src/main/kotlin/org/jitsi/nlj/rtp/bandwidthestimation/GoogleCcEstimator.kt @@ -83,14 +83,14 @@ class GoogleCcEstimator(diagnosticContext: DiagnosticContext, parentLogger: Logg } sendSideBandwidthEstimation.updateReceiverEstimate(bitrateEstimatorAbsSendTime.latestEstimate) sendSideBandwidthEstimation.reportPacketArrived(now.toEpochMilli()) - - /* TODO: rate-limit how often we call updateEstimate? */ - sendSideBandwidthEstimation.updateEstimate(now.toEpochMilli()) - reportBandwidthEstimate(now, sendSideBandwidthEstimation.latestEstimate.bps) } override fun doProcessPacketLoss(now: Instant, sendTime: Instant?, seq: Int) { sendSideBandwidthEstimation.reportPacketLost(now.toEpochMilli()) + } + + override fun doFeedbackComplete(now: Instant) { + /* TODO: rate-limit how often we call updateEstimate? */ sendSideBandwidthEstimation.updateEstimate(now.toEpochMilli()) reportBandwidthEstimate(now, sendSideBandwidthEstimation.latestEstimate.bps) } diff --git a/src/main/kotlin/org/jitsi/nlj/transform/node/incoming/RemoteBandwidthEstimator.kt b/src/main/kotlin/org/jitsi/nlj/transform/node/incoming/RemoteBandwidthEstimator.kt index 074a112c7..49fc5fa45 100644 --- a/src/main/kotlin/org/jitsi/nlj/transform/node/incoming/RemoteBandwidthEstimator.kt +++ b/src/main/kotlin/org/jitsi/nlj/transform/node/incoming/RemoteBandwidthEstimator.kt @@ -98,12 +98,15 @@ class RemoteBandwidthEstimator( astExtId?.let { val rtpPacket = packetInfo.packetAs() rtpPacket.getHeaderExtension(it)?.let { ext -> + val now = clock.instant() bwe.processPacketArrival( - clock.instant(), + now, AbsSendTimeHeaderExtension.getTime(ext), Instant.ofEpochMilli(packetInfo.receivedTime), rtpPacket.sequenceNumber, rtpPacket.length.bytes) + /* With receiver-side bwe we need to treat each received packet as separate feedback */ + bwe.feedbackComplete(now) ssrcs.add(rtpPacket.ssrc) } } ?: numPacketsWithoutAbsSendTime++ diff --git a/src/test/kotlin/org/jitsi/nlj/rtp/bandwidthestimation/BandwidthEstimationTest.kt b/src/test/kotlin/org/jitsi/nlj/rtp/bandwidthestimation/BandwidthEstimationTest.kt index 0188a1dc4..f10da3ed6 100644 --- a/src/test/kotlin/org/jitsi/nlj/rtp/bandwidthestimation/BandwidthEstimationTest.kt +++ b/src/test/kotlin/org/jitsi/nlj/rtp/bandwidthestimation/BandwidthEstimationTest.kt @@ -177,6 +177,7 @@ class PacketReceiver( /* All delay is send -> receive in this simulation, so one-way delay is rtt. */ estimator.onRttUpdate(now, Duration.between(packet.sendTime, now)) estimator.processPacketArrival(now, packet.sendTime, now, seq, packet.packetSize) + estimator.feedbackComplete(now) seq++ val bw = estimator.getCurrentBw(now) if (timeSeriesLogger.isTraceEnabled) {