Start probes only after network is connected.
Previously ProbeController was starting probing as soon as SetBitrates()
is called. As result these probes would often timeout while connection
is being established. Now ProbeController receives notifications about
network route changes. This allows to start probing only when transport
is connected. This also makes it possible to restart probing whenever
transport route changes (will be done in a separate change).
BUG=webrtc:6332
R=honghaiz@webrtc.org, philipel@webrtc.org, stefan@webrtc.org
Review URL: https://codereview.webrtc.org/2458863002 .
Committed: https://crrev.com/5c99c76255ee7bface3c742c25fb5617748ac86e
Cr-Original-Commit-Position: refs/heads/master@{#15094}
Cr-Commit-Position: refs/heads/master@{#15204}
diff --git a/webrtc/call/bitrate_allocator.cc b/webrtc/call/bitrate_allocator.cc
index 645ee3c..2c5ba6d 100644
--- a/webrtc/call/bitrate_allocator.cc
+++ b/webrtc/call/bitrate_allocator.cc
@@ -48,7 +48,7 @@
BitrateAllocator::BitrateAllocator(LimitObserver* limit_observer)
: limit_observer_(limit_observer),
bitrate_observer_configs_(),
- last_bitrate_bps_(kDefaultBitrateBps),
+ last_bitrate_bps_(0),
last_non_zero_bitrate_bps_(kDefaultBitrateBps),
last_fraction_loss_(0),
last_rtt_(0),
diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc
index baa8936..d2bc153 100644
--- a/webrtc/call/call.cc
+++ b/webrtc/call/call.cc
@@ -252,8 +252,8 @@
call_stats_(new CallStats(clock_)),
bitrate_allocator_(new BitrateAllocator(this)),
config_(config),
- audio_network_state_(kNetworkUp),
- video_network_state_(kNetworkUp),
+ audio_network_state_(kNetworkDown),
+ video_network_state_(kNetworkDown),
receive_crit_(RWLockWrapper::CreateRWLock()),
send_crit_(RWLockWrapper::CreateRWLock()),
event_log_(config.event_log),
@@ -284,6 +284,7 @@
Trace::CreateTrace();
call_stats_->RegisterStatsObserver(congestion_controller_.get());
+ congestion_controller_->SignalNetworkState(kNetworkDown);
congestion_controller_->SetBweBitrates(
config_.bitrate_config.min_bitrate_bps,
config_.bitrate_config.start_bitrate_bps,
diff --git a/webrtc/media/base/videoengine_unittest.h b/webrtc/media/base/videoengine_unittest.h
index 024ae5d..e307d9f 100644
--- a/webrtc/media/base/videoengine_unittest.h
+++ b/webrtc/media/base/videoengine_unittest.h
@@ -84,6 +84,7 @@
engine_.Init();
channel_.reset(engine_.CreateChannel(call_.get(), cricket::MediaConfig(),
cricket::VideoOptions()));
+ channel_->OnReadyToSend(true);
EXPECT_TRUE(channel_.get() != NULL);
network_interface_.SetDestination(channel_.get());
channel_->SetInterface(&network_interface_);
diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
index c473a40..ea384e0 100644
--- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc
+++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
@@ -353,6 +353,7 @@
std::unique_ptr<VideoMediaChannel> channel(
SetUpForExternalEncoderFactory(&encoder_factory));
+ channel->OnReadyToSend(true);
EXPECT_TRUE(
channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc)));
@@ -916,6 +917,7 @@
engine_.Init();
channel_.reset(
engine_.CreateChannel(fake_call_.get(), MediaConfig(), VideoOptions()));
+ channel_->OnReadyToSend(true);
last_ssrc_ = 123;
send_parameters_.codecs = engine_.codecs();
recv_parameters_.codecs = engine_.codecs();
@@ -1731,6 +1733,7 @@
channel_.reset(
engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions()));
+ channel_->OnReadyToSend(true);
channel_->SetSendParameters(send_parameters_);
@@ -1740,6 +1743,7 @@
media_config.video.suspend_below_min_bitrate = false;
channel_.reset(
engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions()));
+ channel_->OnReadyToSend(true);
channel_->SetSendParameters(send_parameters_);
@@ -2024,6 +2028,7 @@
MediaConfig media_config = MediaConfig();
channel_.reset(
engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions()));
+ channel_->OnReadyToSend(true);
ASSERT_TRUE(channel_->SetSendParameters(parameters));
AddSendStream();
@@ -2098,6 +2103,7 @@
MediaConfig media_config = MediaConfig();
channel_.reset(
engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions()));
+ channel_->OnReadyToSend(true);
ASSERT_TRUE(channel_->SetSendParameters(parameters));
AddSendStream();
@@ -2165,6 +2171,7 @@
}
channel_.reset(
engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions()));
+ channel_->OnReadyToSend(true);
EXPECT_TRUE(channel_->SetSendParameters(parameters));
@@ -3865,6 +3872,7 @@
engine_.Init();
channel_.reset(
engine_.CreateChannel(&fake_call_, MediaConfig(), VideoOptions()));
+ channel_->OnReadyToSend(true);
last_ssrc_ = 123;
}
diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc
index 0dafdf6..accc141 100644
--- a/webrtc/modules/congestion_controller/congestion_controller.cc
+++ b/webrtc/modules/congestion_controller/congestion_controller.cc
@@ -297,6 +297,7 @@
rtc::CritScope cs(&critsect_);
network_state_ = state;
}
+ probe_controller_->OnNetworkStateChanged(state);
MaybeTriggerOnNetworkChanged();
}
diff --git a/webrtc/modules/congestion_controller/probe_controller.cc b/webrtc/modules/congestion_controller/probe_controller.cc
index 7e1d936..1749e29 100644
--- a/webrtc/modules/congestion_controller/probe_controller.cc
+++ b/webrtc/modules/congestion_controller/probe_controller.cc
@@ -46,10 +46,12 @@
ProbeController::ProbeController(PacedSender* pacer, Clock* clock)
: pacer_(pacer),
clock_(clock),
+ network_state_(kNetworkUp),
state_(State::kInit),
min_bitrate_to_probe_further_bps_(kExponentialProbingDisabled),
time_last_probing_initiated_ms_(0),
estimated_bitrate_bps_(0),
+ start_bitrate_bps_(0),
max_bitrate_bps_(0),
last_alr_probing_time_(clock_->TimeInMilliseconds()) {}
@@ -57,30 +59,54 @@
int start_bitrate_bps,
int max_bitrate_bps) {
rtc::CritScope cs(&critsect_);
- if (state_ == State::kInit) {
- // When probing at 1.8 Mbps ( 6x 300), this represents a threshold of
- // 1.2 Mbps to continue probing.
- InitiateProbing({3 * start_bitrate_bps, 6 * start_bitrate_bps},
- 4 * start_bitrate_bps);
+
+ if (start_bitrate_bps > 0) {
+ start_bitrate_bps_ = start_bitrate_bps;
+ } else if (start_bitrate_bps_ == 0) {
+ start_bitrate_bps_ = min_bitrate_bps;
}
int old_max_bitrate_bps = max_bitrate_bps_;
max_bitrate_bps_ = max_bitrate_bps;
- // Only do probing if:
- // we are mid-call, which we consider to be if
- // exponential probing is not active and
- // |estimated_bitrate_bps_| is valid (> 0) and
- // the current bitrate is lower than the new |max_bitrate_bps|, and
- // |max_bitrate_bps_| was increased.
- if (state_ != State::kWaitingForProbingResult &&
- estimated_bitrate_bps_ != 0 &&
- estimated_bitrate_bps_ < old_max_bitrate_bps &&
- max_bitrate_bps_ > old_max_bitrate_bps) {
- InitiateProbing({max_bitrate_bps}, kExponentialProbingDisabled);
+ switch (state_) {
+ case State::kInit:
+ if (network_state_ == kNetworkUp)
+ InitiateExponentialProbing();
+ break;
+
+ case State::kWaitingForProbingResult:
+ break;
+
+ case State::kProbingComplete:
+ // Initiate probing when |max_bitrate_| was increased mid-call.
+ if (estimated_bitrate_bps_ != 0 &&
+ estimated_bitrate_bps_ < old_max_bitrate_bps &&
+ max_bitrate_bps_ > old_max_bitrate_bps) {
+ InitiateProbing({max_bitrate_bps}, kExponentialProbingDisabled);
+ }
+ break;
}
}
+void ProbeController::OnNetworkStateChanged(NetworkState network_state) {
+ rtc::CritScope cs(&critsect_);
+ network_state_ = network_state;
+ if (network_state_ == kNetworkUp && state_ == State::kInit)
+ InitiateExponentialProbing();
+}
+
+void ProbeController::InitiateExponentialProbing() {
+ RTC_DCHECK(network_state_ == kNetworkUp);
+ RTC_DCHECK(state_ == State::kInit);
+ RTC_DCHECK_GT(start_bitrate_bps_, 0);
+
+ // When probing at 1.8 Mbps ( 6x 300), this represents a threshold of
+ // 1.2 Mbps to continue probing.
+ InitiateProbing({3 * start_bitrate_bps_, 6 * start_bitrate_bps_},
+ 4 * start_bitrate_bps_);
+}
+
void ProbeController::SetEstimatedBitrate(int bitrate_bps) {
rtc::CritScope cs(&critsect_);
if (state_ == State::kWaitingForProbingResult) {
diff --git a/webrtc/modules/congestion_controller/probe_controller.h b/webrtc/modules/congestion_controller/probe_controller.h
index efcc2a1..e60a007 100644
--- a/webrtc/modules/congestion_controller/probe_controller.h
+++ b/webrtc/modules/congestion_controller/probe_controller.h
@@ -31,12 +31,12 @@
void SetBitrates(int min_bitrate_bps,
int start_bitrate_bps,
int max_bitrate_bps);
+
+ void OnNetworkStateChanged(NetworkState state);
+
void SetEstimatedBitrate(int bitrate_bps);
private:
- void InitiateProbing(std::initializer_list<int> bitrates_to_probe,
- int min_bitrate_to_probe_further_bps)
- EXCLUSIVE_LOCKS_REQUIRED(critsect_);
enum class State {
// Initial state where no probing has been triggered yet.
kInit,
@@ -45,13 +45,21 @@
// Probing is complete.
kProbingComplete,
};
+
+ void InitiateExponentialProbing() EXCLUSIVE_LOCKS_REQUIRED(critsect_);
+ void InitiateProbing(std::initializer_list<int> bitrates_to_probe,
+ int min_bitrate_to_probe_further_bps)
+ EXCLUSIVE_LOCKS_REQUIRED(critsect_);
+
rtc::CriticalSection critsect_;
PacedSender* const pacer_;
Clock* const clock_;
+ NetworkState network_state_ GUARDED_BY(critsect_);
State state_ GUARDED_BY(critsect_);
int min_bitrate_to_probe_further_bps_ GUARDED_BY(critsect_);
int64_t time_last_probing_initiated_ms_ GUARDED_BY(critsect_);
int estimated_bitrate_bps_ GUARDED_BY(critsect_);
+ int start_bitrate_bps_ GUARDED_BY(critsect_);
int max_bitrate_bps_ GUARDED_BY(critsect_);
int64_t last_alr_probing_time_ GUARDED_BY(critsect_);
diff --git a/webrtc/modules/congestion_controller/probe_controller_unittest.cc b/webrtc/modules/congestion_controller/probe_controller_unittest.cc
index 98e1166..3842ded 100644
--- a/webrtc/modules/congestion_controller/probe_controller_unittest.cc
+++ b/webrtc/modules/congestion_controller/probe_controller_unittest.cc
@@ -51,6 +51,17 @@
kMaxBitrateBps);
}
+TEST_F(ProbeControllerTest, ProbeOnlyWhenNetworkIsUp) {
+ probe_controller_->OnNetworkStateChanged(kNetworkDown);
+ EXPECT_CALL(pacer_, CreateProbeCluster(_, _)).Times(0);
+ probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps,
+ kMaxBitrateBps);
+
+ testing::Mock::VerifyAndClearExpectations(&pacer_);
+ EXPECT_CALL(pacer_, CreateProbeCluster(_, _)).Times(AtLeast(2));
+ probe_controller_->OnNetworkStateChanged(kNetworkUp);
+}
+
TEST_F(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncrease) {
EXPECT_CALL(pacer_, CreateProbeCluster(_, _)).Times(AtLeast(2));
probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps,
diff --git a/webrtc/test/call_test.cc b/webrtc/test/call_test.cc
index 28971af..aaafdb3 100644
--- a/webrtc/test/call_test.cc
+++ b/webrtc/test/call_test.cc
@@ -75,6 +75,10 @@
if (test->ShouldCreateReceivers()) {
send_transport_->SetReceiver(receiver_call_->Receiver());
receive_transport_->SetReceiver(sender_call_->Receiver());
+ if (num_video_streams_ > 0)
+ receiver_call_->SignalChannelNetworkState(MediaType::VIDEO, kNetworkUp);
+ if (num_audio_streams_ > 0)
+ receiver_call_->SignalChannelNetworkState(MediaType::AUDIO, kNetworkUp);
} else {
// Sender-only call delivers to itself.
send_transport_->SetReceiver(sender_call_->Receiver());
diff --git a/webrtc/test/direct_transport.cc b/webrtc/test/direct_transport.cc
index 18d3ee6..b26aadd 100644
--- a/webrtc/test/direct_transport.cc
+++ b/webrtc/test/direct_transport.cc
@@ -28,6 +28,10 @@
shutting_down_(false),
fake_network_(clock_, config) {
thread_.Start();
+ if (send_call_) {
+ send_call_->SignalChannelNetworkState(MediaType::AUDIO, kNetworkUp);
+ send_call_->SignalChannelNetworkState(MediaType::VIDEO, kNetworkUp);
+ }
}
DirectTransport::~DirectTransport() { StopSending(); }
diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc
index d93dd24..e406d0f 100644
--- a/webrtc/video/end_to_end_tests.cc
+++ b/webrtc/video/end_to_end_tests.cc
@@ -122,11 +122,11 @@
void TestRtpStatePreservation(bool use_rtx, bool provoke_rtcpsr_before_rtp);
void VerifyHistogramStats(bool use_rtx, bool use_red, bool screenshare);
void VerifyNewVideoSendStreamsRespectNetworkState(
- MediaType network_to_bring_down,
+ MediaType network_to_bring_up,
VideoEncoder* encoder,
Transport* transport);
void VerifyNewVideoReceiveStreamsRespectNetworkState(
- MediaType network_to_bring_down,
+ MediaType network_to_bring_up,
Transport* transport);
};
@@ -3671,11 +3671,11 @@
}
void EndToEndTest::VerifyNewVideoSendStreamsRespectNetworkState(
- MediaType network_to_bring_down,
+ MediaType network_to_bring_up,
VideoEncoder* encoder,
Transport* transport) {
CreateSenderCall(Call::Config(&event_log_));
- sender_call_->SignalChannelNetworkState(network_to_bring_down, kNetworkDown);
+ sender_call_->SignalChannelNetworkState(network_to_bring_up, kNetworkUp);
CreateSendConfig(1, 0, 0, transport);
video_send_config_.encoder_settings.encoder = encoder;
@@ -3691,12 +3691,11 @@
}
void EndToEndTest::VerifyNewVideoReceiveStreamsRespectNetworkState(
- MediaType network_to_bring_down,
+ MediaType network_to_bring_up,
Transport* transport) {
Call::Config config(&event_log_);
CreateCalls(config, config);
- receiver_call_->SignalChannelNetworkState(network_to_bring_down,
- kNetworkDown);
+ receiver_call_->SignalChannelNetworkState(network_to_bring_up, kNetworkUp);
test::DirectTransport sender_transport(sender_call_.get());
sender_transport.SetReceiver(receiver_call_->Receiver());
@@ -3738,7 +3737,7 @@
UnusedEncoder unused_encoder;
UnusedTransport unused_transport;
VerifyNewVideoSendStreamsRespectNetworkState(
- MediaType::VIDEO, &unused_encoder, &unused_transport);
+ MediaType::AUDIO, &unused_encoder, &unused_transport);
}
TEST_F(EndToEndTest, NewVideoSendStreamsIgnoreAudioNetworkDown) {
@@ -3766,17 +3765,17 @@
RequiredTransport required_transport(true /*rtp*/, false /*rtcp*/);
RequiredEncoder required_encoder;
VerifyNewVideoSendStreamsRespectNetworkState(
- MediaType::AUDIO, &required_encoder, &required_transport);
+ MediaType::VIDEO, &required_encoder, &required_transport);
}
TEST_F(EndToEndTest, NewVideoReceiveStreamsRespectVideoNetworkDown) {
UnusedTransport transport;
- VerifyNewVideoReceiveStreamsRespectNetworkState(MediaType::VIDEO, &transport);
+ VerifyNewVideoReceiveStreamsRespectNetworkState(MediaType::AUDIO, &transport);
}
TEST_F(EndToEndTest, NewVideoReceiveStreamsIgnoreAudioNetworkDown) {
RequiredTransport transport(false /*rtp*/, true /*rtcp*/);
- VerifyNewVideoReceiveStreamsRespectNetworkState(MediaType::AUDIO, &transport);
+ VerifyNewVideoReceiveStreamsRespectNetworkState(MediaType::VIDEO, &transport);
}
void VerifyEmptyNackConfig(const NackConfig& config) {
diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc
index cc76b13..b4df558 100644
--- a/webrtc/video/video_send_stream_tests.cc
+++ b/webrtc/video/video_send_stream_tests.cc
@@ -1836,6 +1836,8 @@
test::NullTransport transport;
CreateSendConfig(1, 0, 0, &transport);
+ sender_call_->SignalChannelNetworkState(MediaType::VIDEO, kNetworkUp);
+
StartStopBitrateObserver encoder;
video_send_config_.encoder_settings.encoder = &encoder;
video_send_config_.encoder_settings.internal_source = true;