Suppress REMB in bitrate ctrl if it seems lika a short network glitch.
BUG=4082
R=stefan@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/37369004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@7948 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/modules/bitrate_controller/BUILD.gn b/webrtc/modules/bitrate_controller/BUILD.gn
index 96a3782..f0c60a8 100644
--- a/webrtc/modules/bitrate_controller/BUILD.gn
+++ b/webrtc/modules/bitrate_controller/BUILD.gn
@@ -13,6 +13,8 @@
"bitrate_controller_impl.cc",
"bitrate_controller_impl.h",
"include/bitrate_controller.h",
+ "remb_suppressor.cc",
+ "remb_suppressor.h",
"send_side_bandwidth_estimation.cc",
"send_side_bandwidth_estimation.h",
]
diff --git a/webrtc/modules/bitrate_controller/bitrate_controller.gypi b/webrtc/modules/bitrate_controller/bitrate_controller.gypi
index e713282..71dbc1f 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller.gypi
+++ b/webrtc/modules/bitrate_controller/bitrate_controller.gypi
@@ -18,6 +18,8 @@
'bitrate_controller_impl.cc',
'bitrate_controller_impl.h',
'include/bitrate_controller.h',
+ 'remb_suppressor.cc',
+ 'remb_suppressor.h',
'send_side_bandwidth_estimation.cc',
'send_side_bandwidth_estimation.h',
],
diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
index 5e1fd46..605190d 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc
@@ -83,7 +83,8 @@
return new BitrateControllerImpl(clock, enforce_min_bitrate);
}
-BitrateControllerImpl::BitrateControllerImpl(Clock* clock, bool enforce_min_bitrate)
+BitrateControllerImpl::BitrateControllerImpl(Clock* clock,
+ bool enforce_min_bitrate)
: clock_(clock),
last_bitrate_update_ms_(clock_->TimeInMilliseconds()),
critsect_(CriticalSectionWrapper::CreateCriticalSection()),
@@ -96,7 +97,9 @@
last_rtt_ms_(0),
last_enforce_min_bitrate_(!enforce_min_bitrate_),
bitrate_observers_modified_(false),
- last_reserved_bitrate_bps_(0) {}
+ last_reserved_bitrate_bps_(0),
+ remb_suppressor_(new RembSuppressor(clock)) {
+}
BitrateControllerImpl::~BitrateControllerImpl() {
BitrateObserverConfList::iterator it = bitrate_observers_.begin();
@@ -219,6 +222,9 @@
void BitrateControllerImpl::OnReceivedEstimatedBitrate(uint32_t bitrate) {
CriticalSectionScoped cs(critsect_);
+ if (remb_suppressor_->SuppresNewRemb(bitrate)) {
+ return;
+ }
bandwidth_estimation_.UpdateReceiverEstimate(bitrate);
MaybeTriggerOnNetworkChanged();
}
@@ -378,4 +384,14 @@
return false;
}
+void BitrateControllerImpl::SetBitrateSent(uint32_t bitrate_sent_bps) {
+ CriticalSectionScoped cs(critsect_);
+ remb_suppressor_->SetBitrateSent(bitrate_sent_bps);
+}
+
+void BitrateControllerImpl::SetCodecMode(webrtc::VideoCodecMode mode) {
+ CriticalSectionScoped cs(critsect_);
+ remb_suppressor_->SetEnabled(mode == kScreensharing);
+}
+
} // namespace webrtc
diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h
index 3d16202..fb2622f 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h
@@ -21,6 +21,7 @@
#include <map>
#include <utility>
+#include "webrtc/modules/bitrate_controller/remb_suppressor.h"
#include "webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h"
#include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
#include "webrtc/system_wrappers/interface/scoped_ptr.h"
@@ -49,6 +50,11 @@
virtual int64_t TimeUntilNextProcess() OVERRIDE;
virtual int32_t Process() OVERRIDE;
+ // Current bitrate actually being sent.
+ virtual void SetBitrateSent(uint32_t bitrate_sent_bps) OVERRIDE;
+
+ virtual void SetCodecMode(webrtc::VideoCodecMode mode) OVERRIDE;
+
private:
class RtcpBandwidthObserverImpl;
@@ -127,6 +133,7 @@
bool last_enforce_min_bitrate_ GUARDED_BY(*critsect_);
bool bitrate_observers_modified_ GUARDED_BY(*critsect_);
uint32_t last_reserved_bitrate_bps_ GUARDED_BY(*critsect_);
+ scoped_ptr<RembSuppressor> remb_suppressor_ GUARDED_BY(*critsect_);
DISALLOW_IMPLICIT_CONSTRUCTORS(BitrateControllerImpl);
};
diff --git a/webrtc/modules/bitrate_controller/include/bitrate_controller.h b/webrtc/modules/bitrate_controller/include/bitrate_controller.h
index 8a558f5..2b84ada 100644
--- a/webrtc/modules/bitrate_controller/include/bitrate_controller.h
+++ b/webrtc/modules/bitrate_controller/include/bitrate_controller.h
@@ -77,6 +77,10 @@
virtual void EnforceMinBitrate(bool enforce_min_bitrate) = 0;
virtual void SetReservedBitrate(uint32_t reserved_bitrate_bps) = 0;
+
+ virtual void SetBitrateSent(uint32_t bitrate_sent_bps) = 0;
+
+ virtual void SetCodecMode(webrtc::VideoCodecMode mode) = 0;
};
} // namespace webrtc
#endif // WEBRTC_MODULES_BITRATE_CONTROLLER_INCLUDE_BITRATE_CONTROLLER_H_
diff --git a/webrtc/modules/bitrate_controller/remb_suppressor.cc b/webrtc/modules/bitrate_controller/remb_suppressor.cc
new file mode 100644
index 0000000..f65837a
--- /dev/null
+++ b/webrtc/modules/bitrate_controller/remb_suppressor.cc
@@ -0,0 +1,121 @@
+/*
+ * Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ *
+ */
+
+#include "webrtc/modules/bitrate_controller/remb_suppressor.h"
+
+#include <math.h>
+
+#include "webrtc/system_wrappers/interface/field_trial.h"
+
+namespace webrtc {
+
+// If BWE falls more than this fraction from one REMB to the next,
+// classify this as a glitch.
+static const double kMaxBweDropRatio = 0.45;
+
+// If we are sending less then this fraction of the last REMB when a glitch
+// is detected, start suppressing REMB.
+static const double kMinSendBitrateFraction = 0.75;
+
+// Minimum fractional BWE growth per second needed to keep suppressing.
+static const double kMinGrowth = 0.015;
+
+RembSuppressor::RembSuppressor(Clock* clock)
+ : enabled_(false),
+ clock_(clock),
+ last_remb_bps_(0),
+ bitrate_sent_bps_(0),
+ last_remb_ignored_bps_(0),
+ last_remb_ignore_time_ms_(0),
+ remb_silence_start_(0) {
+}
+
+RembSuppressor::~RembSuppressor() {
+}
+
+bool RembSuppressor::SuppresNewRemb(uint32_t bitrate_bps) {
+ if (!Enabled())
+ return false;
+
+ if (remb_silence_start_ == 0) {
+ // Not currently suppressing. Check if there is a bit rate drop
+ // significant enough to warrant suppression.
+ return StartSuppressing(bitrate_bps);
+ }
+
+ // Check if signs point to recovery, otherwise back off suppression.
+ if (!ContinueSuppressing(bitrate_bps)) {
+ remb_silence_start_ = 0;
+ last_remb_ignored_bps_ = 0;
+ last_remb_ignore_time_ms_ = 0;
+ return false;
+ }
+ return true;
+}
+
+bool RembSuppressor::StartSuppressing(uint32_t bitrate_bps) {
+ if (bitrate_bps <
+ static_cast<uint32_t>(last_remb_bps_ * kMaxBweDropRatio + 0.5)) {
+ if (bitrate_sent_bps_ <
+ static_cast<uint32_t>(last_remb_bps_ * kMinSendBitrateFraction + 0.5)) {
+ int64_t now = clock_->TimeInMilliseconds();
+ remb_silence_start_ = now;
+ last_remb_ignore_time_ms_ = now;
+ last_remb_ignored_bps_ = bitrate_bps;
+ return true;
+ }
+ }
+ last_remb_bps_ = bitrate_bps;
+ return false;
+}
+
+bool RembSuppressor::ContinueSuppressing(uint32_t bitrate_bps) {
+ int64_t now = clock_->TimeInMilliseconds();
+
+ if (bitrate_bps >= last_remb_bps_) {
+ // We have fully recovered, stop suppressing!
+ return false;
+ }
+
+ // If exactly the same REMB, we probably don't have a new estimate. Keep on
+ // suppressing. However, if REMB is going down or just not increasing fast
+ // enough (kMinGrowth = 0.015 => REMB should increase by at least 1.5% / s)
+ // it looks like the link capacity has actually deteriorated and we are
+ // currently over-utilizing; back off.
+ if (bitrate_bps != last_remb_ignored_bps_) {
+ double delta_t = (now - last_remb_ignore_time_ms_) / 1000.0;
+ double min_increase = pow(1.0 + kMinGrowth, delta_t);
+ if (bitrate_bps < last_remb_ignored_bps_ * min_increase) {
+ return false;
+ }
+ }
+
+ last_remb_ignored_bps_ = bitrate_bps;
+ last_remb_ignore_time_ms_ = now;
+
+ return true;
+}
+
+void RembSuppressor::SetBitrateSent(uint32_t bitrate_bps) {
+ bitrate_sent_bps_ = bitrate_bps;
+}
+
+bool RembSuppressor::Enabled() {
+ return enabled_;
+}
+
+void RembSuppressor::SetEnabled(bool enabled) {
+ enabled_ = enabled &&
+ webrtc::field_trial::FindFullName(
+ "WebRTC-ConditionalRembSuppression") == "Enabled";
+}
+
+} // namespace webrtc
diff --git a/webrtc/modules/bitrate_controller/remb_suppressor.h b/webrtc/modules/bitrate_controller/remb_suppressor.h
new file mode 100644
index 0000000..987f97f
--- /dev/null
+++ b/webrtc/modules/bitrate_controller/remb_suppressor.h
@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ *
+ * Usage: this class will register multiple RtcpBitrateObserver's one at each
+ * RTCP module. It will aggregate the results and run one bandwidth estimation
+ * and push the result to the encoder via VideoEncoderCallback.
+ */
+
+#ifndef THIRD_PARTY_WEBRTC_MODULES_BITRATE_CONTROLLER_REMB_SUPPRESSOR_H_
+#define THIRD_PARTY_WEBRTC_MODULES_BITRATE_CONTROLLER_REMB_SUPPRESSOR_H_
+
+#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h"
+#include "webrtc/system_wrappers/interface/clock.h"
+
+namespace webrtc {
+
+class RembSuppressor {
+ public:
+ explicit RembSuppressor(Clock* clock);
+ virtual ~RembSuppressor();
+
+ // Check whether this new REMB value should be suppressed.
+ bool SuppresNewRemb(uint32_t bitrate_bps);
+ // Update the current bitrate actually being sent.
+ void SetBitrateSent(uint32_t bitrate_bps);
+ // Turn suppression on or off.
+ void SetEnabled(bool enabled);
+
+ protected:
+ virtual bool Enabled();
+
+ private:
+ bool StartSuppressing(uint32_t bitrate_bps);
+ bool ContinueSuppressing(uint32_t bitrate_bps);
+
+ bool enabled_;
+ Clock* const clock_;
+ uint32_t last_remb_bps_;
+ uint32_t bitrate_sent_bps_;
+ uint32_t last_remb_ignored_bps_;
+ uint32_t last_remb_ignore_time_ms_;
+ int64_t remb_silence_start_;
+
+ DISALLOW_COPY_AND_ASSIGN(RembSuppressor);
+};
+
+} // namespace webrtc
+#endif // THIRD_PARTY_WEBRTC_MODULES_BITRATE_CONTROLLER_REMB_SUPPRESSOR_H_
diff --git a/webrtc/modules/bitrate_controller/remb_suppressor_unittest.cc b/webrtc/modules/bitrate_controller/remb_suppressor_unittest.cc
new file mode 100644
index 0000000..7ce99a1
--- /dev/null
+++ b/webrtc/modules/bitrate_controller/remb_suppressor_unittest.cc
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+#include <algorithm>
+#include <vector>
+
+#include "webrtc/modules/bitrate_controller/remb_suppressor.h"
+
+class TestSuppressor : public webrtc::RembSuppressor {
+ public:
+ explicit TestSuppressor(webrtc::Clock* clock) : RembSuppressor(clock) {}
+ virtual ~TestSuppressor() {}
+
+ virtual bool Enabled() OVERRIDE { return true; }
+};
+
+class RembSuppressorTest : public ::testing::Test {
+ protected:
+ RembSuppressorTest() : clock_(0), suppressor_(&clock_) {}
+ ~RembSuppressorTest() {}
+
+ virtual void SetUp() {}
+
+ virtual void TearDown() {}
+
+ bool NewRemb(uint32_t bitrate_bps) {
+ suppressor_.SetBitrateSent(bitrate_bps);
+ bool suppress = suppressor_.SuppresNewRemb(bitrate_bps);
+ // Default one REMB per second.
+ clock_.AdvanceTimeMilliseconds(1000);
+ return suppress;
+ }
+
+ webrtc::SimulatedClock clock_;
+ TestSuppressor suppressor_;
+};
+
+TEST_F(RembSuppressorTest, Basic) {
+ // Never true on first sample.
+ EXPECT_FALSE(NewRemb(50000));
+ // Some rampup.
+ EXPECT_FALSE(NewRemb(55000));
+ EXPECT_FALSE(NewRemb(60500));
+ EXPECT_FALSE(NewRemb(66550));
+ EXPECT_FALSE(NewRemb(73250));
+
+ // Reached limit, some fluctuation ok.
+ EXPECT_FALSE(NewRemb(72100));
+ EXPECT_FALSE(NewRemb(75500));
+ EXPECT_FALSE(NewRemb(69250));
+ EXPECT_FALSE(NewRemb(73250));
+}
+
+TEST_F(RembSuppressorTest, RecoveryTooSlow) {
+ // Never true on first sample.
+ EXPECT_FALSE(NewRemb(50000));
+
+ // Large drop.
+ EXPECT_TRUE(NewRemb(22499));
+
+ // No new estimate, still suppressing.
+ EXPECT_TRUE(NewRemb(22499));
+
+ // Too little increase - stop suppressing.
+ EXPECT_FALSE(NewRemb(22835));
+}
+
+TEST_F(RembSuppressorTest, RembDownDurinSupression) {
+ // Never true on first sample.
+ EXPECT_FALSE(NewRemb(50000));
+
+ // Large drop.
+ EXPECT_TRUE(NewRemb(22499));
+
+ // Remb is not allowed to fall.
+ EXPECT_FALSE(NewRemb(22498));
+}
+
+TEST_F(RembSuppressorTest, GlitchWithRecovery) {
+ const uint32_t start_bitrate = 300000;
+ uint32_t bitrate = start_bitrate;
+ // Never true on first sample.
+ EXPECT_FALSE(NewRemb(bitrate));
+
+ bitrate = static_cast<uint32_t>(bitrate * 0.44);
+ EXPECT_TRUE(NewRemb(bitrate));
+
+ while (bitrate < start_bitrate) {
+ EXPECT_TRUE(NewRemb(bitrate));
+ bitrate = static_cast<uint32_t>(bitrate * 1.10);
+ }
+
+ EXPECT_FALSE(NewRemb(bitrate));
+}
+
+TEST_F(RembSuppressorTest, BitrateSent) {
+ // Never true on first sample.
+ EXPECT_FALSE(NewRemb(50000));
+
+ // Only suppress large drop, if we are not sending at full capacity.
+ suppressor_.SetBitrateSent(37500);
+ EXPECT_FALSE(suppressor_.SuppresNewRemb(22499));
+}
diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp
index 6cd2343..84ec96a 100644
--- a/webrtc/modules/modules.gyp
+++ b/webrtc/modules/modules.gyp
@@ -191,6 +191,7 @@
'audio_processing/utility/delay_estimator_unittest.cc',
'audio_processing/utility/ring_buffer_unittest.cc',
'bitrate_controller/bitrate_controller_unittest.cc',
+ 'bitrate_controller/remb_suppressor_unittest.cc',
'bitrate_controller/send_side_bandwidth_estimation_unittest.cc',
'desktop_capture/desktop_and_cursor_composer_unittest.cc',
'desktop_capture/desktop_region_unittest.cc',
diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc
index a1ab46b..3b2ab8d 100644
--- a/webrtc/video_engine/vie_encoder.cc
+++ b/webrtc/video_engine/vie_encoder.cc
@@ -389,6 +389,7 @@
video_codec.minBitrate * 1000,
kTransmissionMaxBitrateMultiplier *
video_codec.maxBitrate * 1000);
+ bitrate_controller_->SetCodecMode(video_codec.mode);
CriticalSectionScoped crit(data_cs_.get());
int pad_up_to_bitrate_kbps = video_codec.startBitrate;
@@ -755,6 +756,7 @@
int32_t ViEEncoder::SendStatistics(const uint32_t bit_rate,
const uint32_t frame_rate) {
+ bitrate_controller_->SetBitrateSent(bit_rate);
CriticalSectionScoped cs(callback_cs_.get());
if (codec_observer_) {
codec_observer_->OutgoingRate(channel_id_, frame_rate, bit_rate);
