diff --git a/webrtc/modules/bitrate_controller/BUILD.gn b/webrtc/modules/bitrate_controller/BUILD.gn index 96a378229a..f0c60a83db 100644 --- a/webrtc/modules/bitrate_controller/BUILD.gn +++ b/webrtc/modules/bitrate_controller/BUILD.gn @@ -13,6 +13,8 @@ source_set("bitrate_controller") { "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 e713282056..71dbc1f415 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 5e1fd46c27..605190d879 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc @@ -83,7 +83,8 @@ BitrateController* BitrateController::CreateBitrateController( 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 @@ BitrateControllerImpl::BitrateControllerImpl(Clock* clock, bool enforce_min_bitr 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::SetReservedBitrate(uint32_t reserved_bitrate_bps) { void BitrateControllerImpl::OnReceivedEstimatedBitrate(uint32_t bitrate) { CriticalSectionScoped cs(critsect_); + if (remb_suppressor_->SuppresNewRemb(bitrate)) { + return; + } bandwidth_estimation_.UpdateReceiverEstimate(bitrate); MaybeTriggerOnNetworkChanged(); } @@ -378,4 +384,14 @@ bool BitrateControllerImpl::AvailableBandwidth(uint32_t* bandwidth) const { 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 3d16202600..fb2622f921 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h @@ -21,6 +21,7 @@ #include #include +#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 @@ class BitrateControllerImpl : public BitrateController { 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 @@ class BitrateControllerImpl : public BitrateController { 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 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 8a558f50c8..2b84ada194 100644 --- a/webrtc/modules/bitrate_controller/include/bitrate_controller.h +++ b/webrtc/modules/bitrate_controller/include/bitrate_controller.h @@ -77,6 +77,10 @@ class BitrateController : public Module { 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 0000000000..f65837a046 --- /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 + +#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(last_remb_bps_ * kMaxBweDropRatio + 0.5)) { + if (bitrate_sent_bps_ < + static_cast(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 0000000000..987f97fde6 --- /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 0000000000..7ce99a1994 --- /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 +#include + +#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(bitrate * 0.44); + EXPECT_TRUE(NewRemb(bitrate)); + + while (bitrate < start_bitrate) { + EXPECT_TRUE(NewRemb(bitrate)); + bitrate = static_cast(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 6cd2343237..84ec96a027 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 a1ab46b76d..3b2ab8dd16 100644 --- a/webrtc/video_engine/vie_encoder.cc +++ b/webrtc/video_engine/vie_encoder.cc @@ -389,6 +389,7 @@ int32_t ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec) { 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::ProtectionRequest( 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);