From 7a3615b6b394bfe12a0ab528dee0dd33620e0e63 Mon Sep 17 00:00:00 2001 From: lliuu Date: Thu, 30 Mar 2017 10:36:53 -0700 Subject: [PATCH] Revert of Enable the bayesian bitrate estimator by default. (patchset #5 id:80001 of https://codereview.webrtc.org/2749803002/ ) Reason for revert: Looks like this has caused multiple Android webrtc perf build bot failures in RampUpTest.UpDownUpTransportSequenceNumberRtx Original issue's description: > Enable the bayesian bitrate estimator by default. > > BUG=webrtc:6566, webrtc:7415 > > Review-Url: https://codereview.webrtc.org/2749803002 > Cr-Commit-Position: refs/heads/master@{#17475} > Committed: https://chromium.googlesource.com/external/webrtc/+/c53a17f28e55c99dca7a2a47f09790af466e9334 TBR=terelius@webrtc.org,magjed@webrtc.org,stefan@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:6566, webrtc:7415 Review-Url: https://codereview.webrtc.org/2786913003 Cr-Commit-Position: refs/heads/master@{#17476} --- webrtc/call/rampup_tests.cc | 10 ++-- .../objc/AppRTCMobile/ios/ARDAppDelegate.m | 1 + .../congestion_controller/delay_based_bwe.cc | 17 +++++- .../congestion_controller/delay_based_bwe.h | 3 + .../delay_based_bwe_unittest.cc | 59 ++++++++++++++----- .../delay_based_bwe_unittest_helper.cc | 2 +- .../objc/Framework/Classes/RTCFieldTrials.mm | 1 + .../Framework/Headers/WebRTC/RTCFieldTrials.h | 1 + 8 files changed, 74 insertions(+), 20 deletions(-) diff --git a/webrtc/call/rampup_tests.cc b/webrtc/call/rampup_tests.cc index 9b7dbb828d..0a2a7384d8 100644 --- a/webrtc/call/rampup_tests.cc +++ b/webrtc/call/rampup_tests.cc @@ -25,7 +25,6 @@ static const int kExpectedHighVideoBitrateBps = 80000; static const int kExpectedHighAudioBitrateBps = 30000; static const int kLowBandwidthLimitBps = 20000; static const int kExpectedLowBitrateBps = 20000; -static const int kUnlimitedCapacityBps = 10000000; std::vector GenerateSsrcs(size_t num_streams, uint32_t ssrc_offset) { std::vector ssrcs; @@ -418,9 +417,8 @@ RampUpDownUpTester::RampUpDownUpTester(size_t num_video_streams, rtx, red, report_perf_stats), - link_rates_({GetExpectedHighBitrate() / 1000, - kLowBandwidthLimitBps / 1000, kUnlimitedCapacityBps / 1000, - 0}), + link_rates_({GetHighLinkCapacity(), kLowBandwidthLimitBps / 1000, + GetHighLinkCapacity(), 0}), test_state_(kFirstRampup), next_state_(kTransitionToNextState), state_start_ms_(clock_->TimeInMilliseconds()), @@ -490,6 +488,10 @@ int RampUpDownUpTester::GetExpectedHighBitrate() const { return expected_bitrate_bps; } +int RampUpDownUpTester::GetHighLinkCapacity() const { + return 4 * GetExpectedHighBitrate() / (3 * 1000); +} + size_t RampUpDownUpTester::GetFecBytes() const { size_t flex_fec_bytes = 0; if (num_flexfec_streams_ > 0) { diff --git a/webrtc/examples/objc/AppRTCMobile/ios/ARDAppDelegate.m b/webrtc/examples/objc/AppRTCMobile/ios/ARDAppDelegate.m index 07c83a0826..c26f0529df 100644 --- a/webrtc/examples/objc/AppRTCMobile/ios/ARDAppDelegate.m +++ b/webrtc/examples/objc/AppRTCMobile/ios/ARDAppDelegate.m @@ -26,6 +26,7 @@ - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions { NSDictionary *fieldTrials = @{ + kRTCFieldTrialImprovedBitrateEstimateKey: kRTCFieldTrialEnabledValue, kRTCFieldTrialH264HighProfileKey: kRTCFieldTrialEnabledValue, }; RTCInitFieldTrialDictionary(fieldTrials); diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index 3fa48c7a42..e8d9d21050 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -52,9 +52,14 @@ constexpr double kDefaultMedianSlopeThresholdGain = 4.0; constexpr int kMaxConsecutiveFailedLookups = 5; +const char kBitrateEstimateExperiment[] = "WebRTC-ImprovedBitrateEstimate"; const char kBweTrendlineFilterExperiment[] = "WebRTC-BweTrendlineFilter"; const char kBweMedianSlopeFilterExperiment[] = "WebRTC-BweMedianSlopeFilter"; +bool BitrateEstimateExperimentIsEnabled() { + return webrtc::field_trial::IsEnabled(kBitrateEstimateExperiment); +} + bool TrendlineFilterExperimentIsEnabled() { std::string experiment_string = webrtc::field_trial::FindFullName(kBweTrendlineFilterExperiment); @@ -148,9 +153,19 @@ DelayBasedBwe::BitrateEstimator::BitrateEstimator() current_win_ms_(0), prev_time_ms_(-1), bitrate_estimate_(-1.0f), - bitrate_estimate_var_(50.0f) {} + bitrate_estimate_var_(50.0f), + old_estimator_(kBitrateWindowMs, 8000), + in_experiment_(BitrateEstimateExperimentIsEnabled()) {} void DelayBasedBwe::BitrateEstimator::Update(int64_t now_ms, int bytes) { + if (!in_experiment_) { + old_estimator_.Update(bytes, now_ms); + rtc::Optional rate = old_estimator_.Rate(now_ms); + bitrate_estimate_ = -1.0f; + if (rate) + bitrate_estimate_ = *rate / 1000.0f; + return; + } int rate_window_ms = kRateWindowMs; // We use a larger window at the beginning to get a more stable sample that // we can use to initialize the estimate. diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.h b/webrtc/modules/congestion_controller/delay_based_bwe.h index c471310820..5ae60f7d21 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe.h @@ -17,6 +17,7 @@ #include "webrtc/base/checks.h" #include "webrtc/base/constructormagic.h" +#include "webrtc/base/rate_statistics.h" #include "webrtc/base/thread_checker.h" #include "webrtc/modules/congestion_controller/median_slope_estimator.h" #include "webrtc/modules/congestion_controller/probe_bitrate_estimator.h" @@ -76,6 +77,8 @@ class DelayBasedBwe { int64_t prev_time_ms_; float bitrate_estimate_; float bitrate_estimate_var_; + RateStatistics old_estimator_; + const bool in_experiment_; }; Result IncomingPacketFeedback(const PacketFeedback& packet_feedback); diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc index f00602fa1b..559179e518 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc @@ -119,7 +119,7 @@ TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrivalHighBitrate) { TEST_F(DelayBasedBweTest, GetProbingInterval) { int64_t default_interval_ms = bitrate_estimator_->GetProbingIntervalMs(); EXPECT_GT(default_interval_ms, 0); - CapacityDropTestHelper(1, true, 333, 0); + CapacityDropTestHelper(1, true, 567, 0); int64_t interval_ms = bitrate_estimator_->GetProbingIntervalMs(); EXPECT_GT(interval_ms, 0); EXPECT_NE(interval_ms, default_interval_ms); @@ -134,23 +134,23 @@ TEST_F(DelayBasedBweTest, RateIncreaseReordering) { } TEST_F(DelayBasedBweTest, RateIncreaseRtpTimestamps) { - RateIncreaseRtpTimestampsTestHelper(1288); + RateIncreaseRtpTimestampsTestHelper(1240); } TEST_F(DelayBasedBweTest, CapacityDropOneStream) { - CapacityDropTestHelper(1, false, 333, 0); + CapacityDropTestHelper(1, false, 567, 0); } TEST_F(DelayBasedBweTest, CapacityDropPosOffsetChange) { - CapacityDropTestHelper(1, false, 300, 30000); + CapacityDropTestHelper(1, false, 200, 30000); } TEST_F(DelayBasedBweTest, CapacityDropNegOffsetChange) { - CapacityDropTestHelper(1, false, 300, -30000); + CapacityDropTestHelper(1, false, 733, -30000); } TEST_F(DelayBasedBweTest, CapacityDropOneStreamWrap) { - CapacityDropTestHelper(1, true, 333, 0); + CapacityDropTestHelper(1, true, 567, 0); } TEST_F(DelayBasedBweTest, TestTimestampGrouping) { @@ -172,6 +172,37 @@ TEST_F(DelayBasedBweTest, TestLongTimeoutAndWrap) { TestWrappingHelper(10 * 64); } +class DelayBasedBweExperimentTest : public DelayBasedBweTest { + public: + DelayBasedBweExperimentTest() + : override_field_trials_("WebRTC-ImprovedBitrateEstimate/Enabled/") { + bitrate_estimator_.reset(new DelayBasedBwe(nullptr, &clock_)); + } + + private: + test::ScopedFieldTrials override_field_trials_; +}; + +TEST_F(DelayBasedBweExperimentTest, RateIncreaseRtpTimestamps) { + RateIncreaseRtpTimestampsTestHelper(1288); +} + +TEST_F(DelayBasedBweExperimentTest, CapacityDropOneStream) { + CapacityDropTestHelper(1, false, 333, 0); +} + +TEST_F(DelayBasedBweExperimentTest, CapacityDropPosOffsetChange) { + CapacityDropTestHelper(1, false, 300, 30000); +} + +TEST_F(DelayBasedBweExperimentTest, CapacityDropNegOffsetChange) { + CapacityDropTestHelper(1, false, 300, -30000); +} + +TEST_F(DelayBasedBweExperimentTest, CapacityDropOneStreamWrap) { + CapacityDropTestHelper(1, true, 333, 0); +} + class DelayBasedBweTrendlineExperimentTest : public DelayBasedBweTest { public: DelayBasedBweTrendlineExperimentTest() @@ -184,11 +215,11 @@ class DelayBasedBweTrendlineExperimentTest : public DelayBasedBweTest { }; TEST_F(DelayBasedBweTrendlineExperimentTest, RateIncreaseRtpTimestamps) { - RateIncreaseRtpTimestampsTestHelper(1288); + RateIncreaseRtpTimestampsTestHelper(1240); } TEST_F(DelayBasedBweTrendlineExperimentTest, CapacityDropOneStream) { - CapacityDropTestHelper(1, false, 433, 0); + CapacityDropTestHelper(1, false, 600, 0); } TEST_F(DelayBasedBweTrendlineExperimentTest, CapacityDropPosOffsetChange) { @@ -196,11 +227,11 @@ TEST_F(DelayBasedBweTrendlineExperimentTest, CapacityDropPosOffsetChange) { } TEST_F(DelayBasedBweTrendlineExperimentTest, CapacityDropNegOffsetChange) { - CapacityDropTestHelper(1, false, 767, -30000); + CapacityDropTestHelper(1, false, 1267, -30000); } TEST_F(DelayBasedBweTrendlineExperimentTest, CapacityDropOneStreamWrap) { - CapacityDropTestHelper(1, true, 433, 0); + CapacityDropTestHelper(1, true, 600, 0); } class DelayBasedBweMedianSlopeExperimentTest : public DelayBasedBweTest { @@ -215,11 +246,11 @@ class DelayBasedBweMedianSlopeExperimentTest : public DelayBasedBweTest { }; TEST_F(DelayBasedBweMedianSlopeExperimentTest, RateIncreaseRtpTimestamps) { - RateIncreaseRtpTimestampsTestHelper(1288); + RateIncreaseRtpTimestampsTestHelper(1240); } TEST_F(DelayBasedBweMedianSlopeExperimentTest, CapacityDropOneStream) { - CapacityDropTestHelper(1, false, 367, 0); + CapacityDropTestHelper(1, false, 600, 0); } TEST_F(DelayBasedBweMedianSlopeExperimentTest, CapacityDropPosOffsetChange) { @@ -227,11 +258,11 @@ TEST_F(DelayBasedBweMedianSlopeExperimentTest, CapacityDropPosOffsetChange) { } TEST_F(DelayBasedBweMedianSlopeExperimentTest, CapacityDropNegOffsetChange) { - CapacityDropTestHelper(1, false, 767, -30000); + CapacityDropTestHelper(1, false, 1267, -30000); } TEST_F(DelayBasedBweMedianSlopeExperimentTest, CapacityDropOneStreamWrap) { - CapacityDropTestHelper(1, true, 367, 0); + CapacityDropTestHelper(1, true, 600, 0); } } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc index cceb38a7b5..c87439e5c7 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc @@ -411,7 +411,7 @@ void DelayBasedBweTest::CapacityDropTestHelper( uint32_t bitrate_bps = SteadyStateRun( kDefaultSsrc, steady_state_time * kFramerate, kStartBitrate, kMinExpectedBitrate, kMaxExpectedBitrate, kInitialCapacityBps); - EXPECT_NEAR(kInitialCapacityBps, bitrate_bps, 150000u); + EXPECT_NEAR(kInitialCapacityBps, bitrate_bps, 130000u); bitrate_observer_.Reset(); // Add an offset to make sure the BWE can handle it. diff --git a/webrtc/sdk/objc/Framework/Classes/RTCFieldTrials.mm b/webrtc/sdk/objc/Framework/Classes/RTCFieldTrials.mm index 4f28674e5d..5e1ee0afc1 100644 --- a/webrtc/sdk/objc/Framework/Classes/RTCFieldTrials.mm +++ b/webrtc/sdk/objc/Framework/Classes/RTCFieldTrials.mm @@ -20,6 +20,7 @@ NSString * const kRTCFieldTrialAudioSendSideBweKey = @"WebRTC-Audio-SendSideBwe" NSString * const kRTCFieldTrialSendSideBweWithOverheadKey = @"WebRTC-SendSideBwe-WithOverhead"; NSString * const kRTCFieldTrialFlexFec03AdvertisedKey = @"WebRTC-FlexFEC-03-Advertised"; NSString * const kRTCFieldTrialFlexFec03Key = @"WebRTC-FlexFEC-03"; +NSString * const kRTCFieldTrialImprovedBitrateEstimateKey = @"WebRTC-ImprovedBitrateEstimate"; NSString * const kRTCFieldTrialMedianSlopeFilterKey = @"WebRTC-BweMedianSlopeFilter"; NSString * const kRTCFieldTrialTrendlineFilterKey = @"WebRTC-BweTrendlineFilter"; NSString * const kRTCFieldTrialH264HighProfileKey = @"WebRTC-H264HighProfile"; diff --git a/webrtc/sdk/objc/Framework/Headers/WebRTC/RTCFieldTrials.h b/webrtc/sdk/objc/Framework/Headers/WebRTC/RTCFieldTrials.h index 7eee2550aa..a9d68066b5 100644 --- a/webrtc/sdk/objc/Framework/Headers/WebRTC/RTCFieldTrials.h +++ b/webrtc/sdk/objc/Framework/Headers/WebRTC/RTCFieldTrials.h @@ -17,6 +17,7 @@ RTC_EXTERN NSString * const kRTCFieldTrialAudioSendSideBweKey; RTC_EXTERN NSString * const kRTCFieldTrialSendSideBweWithOverheadKey; RTC_EXTERN NSString * const kRTCFieldTrialFlexFec03AdvertisedKey; RTC_EXTERN NSString * const kRTCFieldTrialFlexFec03Key; +RTC_EXTERN NSString * const kRTCFieldTrialImprovedBitrateEstimateKey; RTC_EXTERN NSString * const kRTCFieldTrialH264HighProfileKey; /** The valid value for field trials above. */