JitterEstimator: add field trial for not estimating noise for congested frames
The reason for rejecting the congested frames in the first place, is
that they do not obey a Gaussian distribution around the line from the
Kalman filter. It therefore also does not make sense to include them
in the noise (*) estimation.
(*) noise = variation around the line from the Kalman filter.
Bug: webrtc:14151
Change-Id: Id8a44ba5f13bf9787ab54848109430ef7657f67a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275762
Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38100}
diff --git a/modules/video_coding/timing/jitter_estimator.cc b/modules/video_coding/timing/jitter_estimator.cc
index 22fdcac..2416ed0 100644
--- a/modules/video_coding/timing/jitter_estimator.cc
+++ b/modules/video_coding/timing/jitter_estimator.cc
@@ -263,9 +263,6 @@
// the frame size also is large the deviation is probably due to an incorrect
// line slope.
if (abs_delay_is_not_outlier || size_is_positive_outlier) {
- // Update the variance of the deviation from the line given by the Kalman
- // filter.
- EstimateRandomJitter(delay_deviation_ms);
// Prevent updating with frames which have been congested by a large frame,
// and therefore arrives almost at the same time as that frame.
// This can occur when we receive a large frame (key frame) which has been
@@ -276,14 +273,25 @@
config_.MaxFrameSizePercentileEnabled()
? max_frame_size_bytes_percentile_.GetFilteredValue()
: max_frame_size_bytes_;
- if (delta_frame_bytes >
- GetCongestionRejectionFactor() * filtered_max_frame_size_bytes) {
- // Update the Kalman filter with the new data
+ bool is_not_congested =
+ delta_frame_bytes >
+ GetCongestionRejectionFactor() * filtered_max_frame_size_bytes;
+
+ if (is_not_congested || config_.estimate_noise_when_congested) {
+ // Update the variance of the deviation from the line given by the Kalman
+ // filter.
+ EstimateRandomJitter(delay_deviation_ms);
+ }
+ if (is_not_congested) {
+ // Neither a delay outlier nor a congested frame, so we can safely update
+ // the Kalman filter with the sample.
kalman_filter_.PredictAndUpdate(frame_delay.ms(), delta_frame_bytes,
filtered_max_frame_size_bytes,
var_noise_ms2_);
}
} else {
+ // Delay outliers affect the noise estimate through a value equal to the
+ // outlier rejection threshold.
double num_stddev = (delay_deviation_ms >= 0) ? num_stddev_delay_outlier
: -num_stddev_delay_outlier;
EstimateRandomJitter(num_stddev * sqrt(var_noise_ms2_));
diff --git a/modules/video_coding/timing/jitter_estimator.h b/modules/video_coding/timing/jitter_estimator.h
index c615be1..769bb12 100644
--- a/modules/video_coding/timing/jitter_estimator.h
+++ b/modules/video_coding/timing/jitter_estimator.h
@@ -51,7 +51,8 @@
"num_stddev_delay_clamp", &num_stddev_delay_clamp,
"num_stddev_delay_outlier", &num_stddev_delay_outlier,
"num_stddev_size_outlier", &num_stddev_size_outlier,
- "congestion_rejection_factor", &congestion_rejection_factor);
+ "congestion_rejection_factor", &congestion_rejection_factor,
+ "estimate_noise_when_congested", &estimate_noise_when_congested);
// clang-format on
}
@@ -59,7 +60,7 @@
return max_frame_size_percentile.has_value();
}
- // If set, the "avg" frame size is calculated as the median over a window
+ // If true, the "avg" frame size is calculated as the median over a window
// of recent frame sizes.
bool avg_frame_size_median = false;
@@ -96,6 +97,12 @@
//
// Decreasing this value rejects fewer samples.
absl::optional<double> congestion_rejection_factor = absl::nullopt;
+
+ // If true, the noise estimate will be updated for congestion rejected
+ // frames. This is currently enabled by default, but that may not be optimal
+ // since congested frames typically are not spread around the line with
+ // Gaussian noise. (This is the whole reason for the congestion rejection!)
+ bool estimate_noise_when_congested = true;
};
JitterEstimator(Clock* clock, const FieldTrialsView& field_trials);
diff --git a/modules/video_coding/timing/jitter_estimator_unittest.cc b/modules/video_coding/timing/jitter_estimator_unittest.cc
index 2602b0d..8e0c015 100644
--- a/modules/video_coding/timing/jitter_estimator_unittest.cc
+++ b/modules/video_coding/timing/jitter_estimator_unittest.cc
@@ -161,6 +161,24 @@
EXPECT_GT(outlier_jitter.ms(), steady_state_jitter.ms());
}
+// Under the default config, congested frames are used when calculating the
+// noise variance, meaning that they will impact the final jitter estimate.
+TEST_F(JitterEstimatorTest, CongestedFrameImpactsJitterEstimate) {
+ ValueGenerator gen(10);
+
+ // Steady state.
+ Run(/*duration_s=*/10, /*framerate_fps=*/30, gen);
+ TimeDelta steady_state_jitter =
+ estimator_.GetJitterEstimate(0, absl::nullopt);
+
+ // Congested frame...
+ estimator_.UpdateEstimate(-10 * gen.Delay(), 0.1 * gen.FrameSize());
+ TimeDelta outlier_jitter = estimator_.GetJitterEstimate(0, absl::nullopt);
+
+ // ...impacts the estimate.
+ EXPECT_GT(outlier_jitter.ms(), steady_state_jitter.ms());
+}
+
TEST_F(JitterEstimatorTest, EmptyFieldTrialsParsesToUnsetConfig) {
JitterEstimator::Config config = estimator_.GetConfigForTest();
EXPECT_FALSE(config.avg_frame_size_median);
@@ -170,6 +188,7 @@
EXPECT_FALSE(config.num_stddev_delay_outlier.has_value());
EXPECT_FALSE(config.num_stddev_size_outlier.has_value());
EXPECT_FALSE(config.congestion_rejection_factor.has_value());
+ EXPECT_TRUE(config.estimate_noise_when_congested);
}
class FieldTrialsOverriddenJitterEstimatorTest : public JitterEstimatorTest {
@@ -183,7 +202,8 @@
"num_stddev_delay_clamp:1.1,"
"num_stddev_delay_outlier:2,"
"num_stddev_size_outlier:3.1,"
- "congestion_rejection_factor:-1.55/") {}
+ "congestion_rejection_factor:-1.55,"
+ "estimate_noise_when_congested:false/") {}
~FieldTrialsOverriddenJitterEstimatorTest() {}
};
@@ -196,6 +216,7 @@
EXPECT_EQ(*config.num_stddev_delay_outlier, 2.0);
EXPECT_EQ(*config.num_stddev_size_outlier, 3.1);
EXPECT_EQ(*config.congestion_rejection_factor, -1.55);
+ EXPECT_FALSE(config.estimate_noise_when_congested);
}
TEST_F(FieldTrialsOverriddenJitterEstimatorTest,
@@ -239,6 +260,25 @@
EXPECT_GT(outlier_jitter_4x.ms(), outlier_jitter_3x.ms());
}
+// When so configured, congested frames are NOT used when calculating the
+// noise variance, meaning that they will NOT impact the final jitter estimate.
+TEST_F(FieldTrialsOverriddenJitterEstimatorTest,
+ CongestedFrameDoesNotImpactJitterEstimate) {
+ ValueGenerator gen(10);
+
+ // Steady state.
+ Run(/*duration_s=*/10, /*framerate_fps=*/30, gen);
+ TimeDelta steady_state_jitter =
+ estimator_.GetJitterEstimate(0, absl::nullopt);
+
+ // Congested frame...
+ estimator_.UpdateEstimate(-10 * gen.Delay(), 0.1 * gen.FrameSize());
+ TimeDelta outlier_jitter = estimator_.GetJitterEstimate(0, absl::nullopt);
+
+ // ...does not impact the estimate.
+ EXPECT_EQ(outlier_jitter.ms(), steady_state_jitter.ms());
+}
+
class MisconfiguredFieldTrialsJitterEstimatorTest : public JitterEstimatorTest {
protected:
MisconfiguredFieldTrialsJitterEstimatorTest()
