Wait for a longer time (5 seconds) before establishing the first bandwidth estimate.
This reduces the risk of getting a small initial estimate when doing combined a/v BWE, and the audio stream is received earlier than the video stream.
In addition a check is added to make sure a probe can't reduce the BWE.
R=pbos@webrtc.org
Review URL: https://codereview.webrtc.org/1219303002 .
Cr-Commit-Position: refs/heads/master@{#9560}
diff --git a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc
index bcb71d6..9bac153 100644
--- a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc
+++ b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc
@@ -14,6 +14,7 @@
#include <cassert>
#include <cmath>
+#include "webrtc/base/checks.h"
#include "webrtc/modules/remote_bitrate_estimator/overuse_detector.h"
#include "webrtc/modules/remote_bitrate_estimator/test/bwe_test_logging.h"
@@ -102,11 +103,13 @@
// Set the initial bit rate value to what we're receiving the first half
// second.
if (!bitrate_is_initialized_) {
+ const int64_t kInitializationTimeMs = 5000;
+ DCHECK_LE(kBitrateWindowMs, kInitializationTimeMs);
if (time_first_incoming_estimate_ < 0) {
if (input->_incomingBitRate > 0) {
time_first_incoming_estimate_ = now_ms;
}
- } else if (now_ms - time_first_incoming_estimate_ > 500 &&
+ } else if (now_ms - time_first_incoming_estimate_ > kInitializationTimeMs &&
input->_incomingBitRate > 0) {
current_bitrate_bps_ = input->_incomingBitRate;
bitrate_is_initialized_ = true;
@@ -136,6 +139,11 @@
if (!updated_) {
return current_bitrate_bps_;
}
+ // An over-use should always trigger us to reduce the bitrate, even though
+ // we have not yet established our first estimate. By acting on the over-use,
+ // we will end up with a valid estimate.
+ if (!bitrate_is_initialized_ && current_input_._bwState != kBwOverusing)
+ return current_bitrate_bps_;
updated_ = false;
ChangeState(current_input_, now_ms);
// Calculated here because it's used in multiple places.
@@ -172,6 +180,7 @@
break;
case kRcDecrease:
+ bitrate_is_initialized_ = true;
if (incoming_bitrate_bps < min_configured_bitrate_bps_) {
current_bitrate_bps = min_configured_bitrate_bps_;
} else {
diff --git a/webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h b/webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h
index f915a0f..844fde5 100644
--- a/webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h
+++ b/webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h
@@ -17,6 +17,9 @@
#define BWE_MIN(a,b) ((a)<(b)?(a):(b))
namespace webrtc {
+
+static const int64_t kBitrateWindowMs = 1000;
+
enum BandwidthUsage
{
kBwNormal = 0,
diff --git a/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h b/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
index 52060e2..057dfb8 100644
--- a/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
+++ b/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
@@ -116,7 +116,7 @@
virtual bool GetStats(ReceiveBandwidthEstimatorStats* output) const = 0;
protected:
- static const int64_t kProcessIntervalMs = 1000;
+ static const int64_t kProcessIntervalMs = 500;
static const int64_t kStreamTimeOutMs = 2000;
};
diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
index 7033c66..e83ac81 100644
--- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
+++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
@@ -97,7 +97,7 @@
inter_arrival_(),
estimator_(OverUseDetectorOptions()),
detector_(OverUseDetectorOptions()),
- incoming_bitrate_(1000, 8000),
+ incoming_bitrate_(kBitrateWindowMs, 8000),
remote_rate_(min_bitrate_bps),
last_process_time_(-1),
process_interval_ms_(kProcessIntervalMs),
@@ -188,7 +188,10 @@
if (best_it != clusters.end()) {
int probe_bitrate_bps =
std::min(best_it->GetSendBitrateBps(), best_it->GetRecvBitrateBps());
- if (IsBitrateImproving(probe_bitrate_bps)) {
+ // Make sure that a probe sent on a lower bitrate than our estimate can't
+ // reduce the estimate.
+ if (IsBitrateImproving(probe_bitrate_bps) &&
+ probe_bitrate_bps > static_cast<int>(incoming_bitrate_.Rate(now_ms))) {
LOG(LS_INFO) << "Probe successful, sent at "
<< best_it->GetSendBitrateBps() << " bps, received at "
<< best_it->GetRecvBitrateBps()
diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc
index 91e8e1d..6f8d6cb 100644
--- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc
+++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc
@@ -38,39 +38,39 @@
}
TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, RateIncreaseRtpTimestamps) {
- RateIncreaseRtpTimestampsTestHelper(1090);
+ RateIncreaseRtpTimestampsTestHelper(1089);
}
TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropOneStream) {
- CapacityDropTestHelper(1, false, 567);
+ CapacityDropTestHelper(1, false, 600);
}
TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropOneStreamWrap) {
- CapacityDropTestHelper(1, true, 567);
+ CapacityDropTestHelper(1, true, 600);
}
TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropTwoStreamsWrap) {
- CapacityDropTestHelper(2, true, 667);
+ CapacityDropTestHelper(2, true, 533);
}
TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropThreeStreamsWrap) {
- CapacityDropTestHelper(3, true, 633);
+ CapacityDropTestHelper(3, true, 700);
}
TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropThirteenStreamsWrap) {
- CapacityDropTestHelper(13, true, 633);
+ CapacityDropTestHelper(13, true, 700);
}
TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropNineteenStreamsWrap) {
- CapacityDropTestHelper(19, true, 633);
+ CapacityDropTestHelper(19, true, 700);
}
TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropThirtyStreamsWrap) {
- CapacityDropTestHelper(30, true, 633);
+ CapacityDropTestHelper(30, true, 700);
}
TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, TestTimestampGrouping) {
- TestTimestampGroupingTestHelper(361080u);
+ TestTimestampGroupingTestHelper();
}
TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, TestGetStats) {
diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
index bca8ba6..762a317 100644
--- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
+++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
@@ -47,7 +47,7 @@
Clock* clock,
uint32_t min_bitrate_bps)
: clock_(clock),
- incoming_bitrate_(1000, 8000),
+ incoming_bitrate_(kBitrateWindowMs, 8000),
remote_rate_(new AimdRateControl(min_bitrate_bps)),
observer_(observer),
crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc
index 7cd7704..ea4e268 100644
--- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc
+++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc
@@ -38,38 +38,38 @@
}
TEST_F(RemoteBitrateEstimatorSingleTest, RateIncreaseRtpTimestamps) {
- RateIncreaseRtpTimestampsTestHelper(1089);
+ RateIncreaseRtpTimestampsTestHelper(1240);
}
TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropOneStream) {
- CapacityDropTestHelper(1, false, 567);
+ CapacityDropTestHelper(1, false, 600);
}
TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropOneStreamWrap) {
- CapacityDropTestHelper(1, true, 567);
+ CapacityDropTestHelper(1, true, 600);
}
TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropTwoStreamsWrap) {
- CapacityDropTestHelper(2, true, 667);
+ CapacityDropTestHelper(2, true, 700);
}
TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropThreeStreamsWrap) {
- CapacityDropTestHelper(3, true, 633);
+ CapacityDropTestHelper(3, true, 734);
}
TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropThirteenStreamsWrap) {
- CapacityDropTestHelper(13, true, 633);
+ CapacityDropTestHelper(13, true, 700);
}
TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropNineteenStreamsWrap) {
- CapacityDropTestHelper(19, true, 633);
+ CapacityDropTestHelper(19, true, 700);
}
TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropThirtyStreamsWrap) {
- CapacityDropTestHelper(30, true, 600);
+ CapacityDropTestHelper(30, true, 700);
}
TEST_F(RemoteBitrateEstimatorSingleTest, TestTimestampGrouping) {
- TestTimestampGroupingTestHelper(361080u);
+ TestTimestampGroupingTestHelper();
}
} // namespace webrtc
diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc
index 0d8bd19..de9873a 100644
--- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc
+++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc
@@ -320,7 +320,7 @@
EXPECT_FALSE(bitrate_observer_->updated());
bitrate_observer_->Reset();
clock_.AdvanceTimeMilliseconds(1000);
- // Inserting a packet. Still no valid estimate. We need to wait 1 second.
+ // Inserting a packet. Still no valid estimate. We need to wait 5 seconds.
IncomingPacket(kDefaultSsrc, kMtu, clock_.TimeInMilliseconds(), timestamp,
absolute_send_time, true);
bitrate_estimator_->Process();
@@ -328,8 +328,8 @@
EXPECT_EQ(0u, ssrcs.size());
EXPECT_FALSE(bitrate_observer_->updated());
bitrate_observer_->Reset();
- // Inserting packets for one second to get a valid estimate.
- for (int i = 0; i < kFramerate; ++i) {
+ // Inserting packets for 5 seconds to get a valid estimate.
+ for (int i = 0; i < 5 * kFramerate + 1; ++i) {
IncomingPacket(kDefaultSsrc, kMtu, clock_.TimeInMilliseconds(), timestamp,
absolute_send_time, true);
clock_.AdvanceTimeMilliseconds(1000 / kFramerate);
@@ -363,7 +363,7 @@
bitrate_estimator_->Process();
EXPECT_FALSE(bitrate_observer_->updated()); // No valid estimate.
// Inserting packets for one second to get a valid estimate.
- for (int i = 0; i < kFramerate; ++i) {
+ for (int i = 0; i < 5 * kFramerate + 1; ++i) {
IncomingPacket(kDefaultSsrc, kMtu, clock_.TimeInMilliseconds(), timestamp,
absolute_send_time, true);
clock_.AdvanceTimeMilliseconds(kFrameIntervalMs);
@@ -439,7 +439,7 @@
steady_state_time = 10;
AddDefaultStream();
} else {
- steady_state_time = 8 * number_of_streams;
+ steady_state_time = 10 * number_of_streams;
int bitrate_sum = 0;
int kBitrateDenom = number_of_streams * (number_of_streams - 1);
for (int i = 0; i < number_of_streams; i++) {
@@ -493,8 +493,8 @@
}
}
- EXPECT_EQ(expected_bitrate_drop_delta,
- bitrate_drop_time - overuse_start_time);
+ EXPECT_NEAR(expected_bitrate_drop_delta,
+ bitrate_drop_time - overuse_start_time, 33);
// Remove stream one by one.
unsigned int latest_bps = 0;
@@ -513,8 +513,7 @@
EXPECT_EQ(0u, latest_bps);
}
-void RemoteBitrateEstimatorTest::TestTimestampGroupingTestHelper(
- uint32_t bitrate_bps) {
+void RemoteBitrateEstimatorTest::TestTimestampGroupingTestHelper() {
const int kFramerate = 50; // 50 fps to avoid rounding errors.
const int kFrameIntervalMs = 1000 / kFramerate;
const uint32_t kFrameIntervalAbsSendTime = AbsSendTime(1, kFramerate);
@@ -523,8 +522,9 @@
// during the test.
uint32_t absolute_send_time =
AddAbsSendTime((1 << 24), -int(50 * kFrameIntervalAbsSendTime));
- // Initial set of frames to increase the bitrate.
- for (int i = 0; i <= 100; ++i) {
+ // Initial set of frames to increase the bitrate. 6 seconds to have enough
+ // time for the first estimate to be generated and for Process() to be called.
+ for (int i = 0; i <= 6 * kFramerate; ++i) {
IncomingPacket(kDefaultSsrc, 1000, clock_.TimeInMilliseconds(), timestamp,
absolute_send_time, true);
bitrate_estimator_->Process();
@@ -534,7 +534,7 @@
kFrameIntervalAbsSendTime);
}
EXPECT_TRUE(bitrate_observer_->updated());
- EXPECT_NEAR(470000u, bitrate_observer_->latest_bitrate(), 10000u);
+ EXPECT_GE(bitrate_observer_->latest_bitrate(), 400000u);
// Insert batches of frames which were sent very close in time. Also simulate
// capacity over-use to see that we back off correctly.
@@ -562,7 +562,7 @@
}
EXPECT_TRUE(bitrate_observer_->updated());
// Should have reduced the estimate.
- EXPECT_EQ(bitrate_bps, bitrate_observer_->latest_bitrate());
+ EXPECT_LT(bitrate_observer_->latest_bitrate(), 400000u);
}
void RemoteBitrateEstimatorTest::TestGetStatsHelper() {
diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h
index fa4cc54..0658ec5 100644
--- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h
+++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h
@@ -190,7 +190,7 @@
unsigned int max_bitrate,
unsigned int target_bitrate);
- void TestTimestampGroupingTestHelper(uint32_t bitrate_bps);
+ void TestTimestampGroupingTestHelper();
void TestGetStatsHelper();
diff --git a/webrtc/video/rampup_tests.cc b/webrtc/video/rampup_tests.cc
index 1c4bee1..c35991f 100644
--- a/webrtc/video/rampup_tests.cc
+++ b/webrtc/video/rampup_tests.cc
@@ -120,8 +120,8 @@
receive_stats_->IncomingPacket(header, length, false);
payload_registry_->SetIncomingPayloadType(header);
DCHECK(remote_bitrate_estimator_ != nullptr);
- remote_bitrate_estimator_->IncomingPacket(clock_->TimeInMilliseconds(),
- length - 12, header, true);
+ remote_bitrate_estimator_->IncomingPacket(
+ clock_->TimeInMilliseconds(), length - header.headerLength, header, true);
if (remote_bitrate_estimator_->TimeUntilNextProcess() <= 0) {
remote_bitrate_estimator_->Process();
}
@@ -269,8 +269,8 @@
RTPHeader header;
EXPECT_TRUE(rtp_parser_->Parse(packet, length, &header));
receive_stats_->IncomingPacket(header, length, false);
- remote_bitrate_estimator_->IncomingPacket(clock_->TimeInMilliseconds(),
- length - 12, header, true);
+ remote_bitrate_estimator_->IncomingPacket(
+ clock_->TimeInMilliseconds(), length - header.headerLength, header, true);
if (remote_bitrate_estimator_->TimeUntilNextProcess() <= 0) {
remote_bitrate_estimator_->Process();
}
@@ -462,6 +462,7 @@
&receiver_transport, Clock::GetRealTimeClock(), number_of_streams, rtx);
Call::Config call_config(&stream_observer);
+ call_config.bitrate_config.start_bitrate_bps = 60000;
CreateSenderCall(call_config);
receiver_transport.SetReceiver(sender_call_->Receiver());
@@ -471,6 +472,7 @@
send_config_.rtp.extensions.push_back(RtpExtension(
RtpExtension::kAbsSendTime, kAbsSendTimeExtensionId));
send_config_.suspend_below_min_bitrate = true;
+
if (rtx) {
send_config_.rtp.rtx.payload_type = kSendRtxPayloadType;
send_config_.rtp.rtx.ssrcs = GenerateSsrcs(number_of_streams, 200);
diff --git a/webrtc/video/rampup_tests.h b/webrtc/video/rampup_tests.h
index 2d9187d..ae9e9d9 100644
--- a/webrtc/video/rampup_tests.h
+++ b/webrtc/video/rampup_tests.h
@@ -128,6 +128,7 @@
const bool rtx_used_;
const rtc::scoped_ptr<EventWrapper> test_done_;
const rtc::scoped_ptr<RtpHeaderParser> rtp_parser_;
+ const rtc::scoped_ptr<RTPPayloadRegistry> payload_registry_;
rtc::scoped_ptr<RtpRtcp> rtp_rtcp_;
internal::TransportAdapter feedback_transport_;
const rtc::scoped_ptr<ReceiveStatistics> receive_stats_;
