admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert of Start probes only after network is connected. (patchset #9 id:240001 of https://codereview.webrtc.org/2458863002/ )

Browse Source 
Reason for revert:
It broke downstream test.

Original issue's description:
> 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
>
> Committed: https://crrev.com/5c99c76255ee7bface3c742c25fb5617748ac86e
> Cr-Commit-Position: refs/heads/master@{#15094}

TBR=philipel@webrtc.org,stefan@webrtc.org,sergeyu@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6332

Review-Url: https://codereview.webrtc.org/2504783002
Cr-Commit-Position: refs/heads/master@{#15098}
This commit is contained in:
honghaiz 2016-11-15 14:39:06 -08:00 committed by Commit bot
parent edec0769aa
commit 906c5dc6b7
12 changed files with 34 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
webrtc/call/bitrate_allocator.cc
View File

@ -48,7 +48,7 @@ double MediaRatio(uint32_t allocated_bitrate, uint32_t protection_bitrate) {
BitrateAllocator::BitrateAllocator(LimitObserver* limit_observer)
: limit_observer_(limit_observer),
bitrate_observer_configs_(),
last_bitrate_bps_(0),
last_bitrate_bps_(kDefaultBitrateBps),
last_non_zero_bitrate_bps_(kDefaultBitrateBps),
last_fraction_loss_(0),
last_rtt_(0),

5
webrtc/call/call.cc
View File

@ -252,8 +252,8 @@ Call::Call(const Call::Config& config)
call_stats_(new CallStats(clock_)),
bitrate_allocator_(new BitrateAllocator(this)),
config_(config),
audio_network_state_(kNetworkDown),
video_network_state_(kNetworkDown),
audio_network_state_(kNetworkUp),
video_network_state_(kNetworkUp),
receive_crit_(RWLockWrapper::CreateRWLock()),
send_crit_(RWLockWrapper::CreateRWLock()),
event_log_(config.event_log),
@ -284,7 +284,6 @@ Call::Call(const Call::Config& config)
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,

1
webrtc/media/base/videoengine_unittest.h
View File

@ -84,7 +84,6 @@ class VideoMediaChannelTest : public testing::Test,
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_);

8
webrtc/media/engine/webrtcvideoengine2_unittest.cc
View File

@ -341,7 +341,6 @@ TEST_F(WebRtcVideoEngine2Test, UseExternalFactoryForVp8WhenSupported) {
std::unique_ptr<VideoMediaChannel> channel(
SetUpForExternalEncoderFactory(&encoder_factory, parameters.codecs));
channel->OnReadyToSend(true);
EXPECT_TRUE(
channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc)));
@ -869,7 +868,6 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test {
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();
@ -1685,7 +1683,6 @@ TEST_F(WebRtcVideoChannel2Test, SetMediaConfigSuspendBelowMinBitrate) {
channel_.reset(
engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions()));
channel_->OnReadyToSend(true);
channel_->SetSendParameters(send_parameters_);
@ -1695,7 +1692,6 @@ TEST_F(WebRtcVideoChannel2Test, SetMediaConfigSuspendBelowMinBitrate) {
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_);
@ -1980,7 +1976,6 @@ TEST_F(WebRtcVideoChannel2Test, AdaptsOnOveruseAndChangeResolution) {
MediaConfig media_config = MediaConfig();
channel_.reset(
engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions()));
channel_->OnReadyToSend(true);
ASSERT_TRUE(channel_->SetSendParameters(parameters));
AddSendStream();
@ -2055,7 +2050,6 @@ TEST_F(WebRtcVideoChannel2Test, PreviousAdaptationDoesNotApplyToScreenshare) {
MediaConfig media_config = MediaConfig();
channel_.reset(
engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions()));
channel_->OnReadyToSend(true);
ASSERT_TRUE(channel_->SetSendParameters(parameters));
AddSendStream();
@ -2123,7 +2117,6 @@ void WebRtcVideoChannel2Test::TestCpuAdaptation(bool enable_overuse,
}
channel_.reset(
engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions()));
channel_->OnReadyToSend(true);
EXPECT_TRUE(channel_->SetSendParameters(parameters));
@ -3620,7 +3613,6 @@ class WebRtcVideoChannel2SimulcastTest : public testing::Test {
engine_.Init();
channel_.reset(
engine_.CreateChannel(&fake_call_, MediaConfig(), VideoOptions()));
channel_->OnReadyToSend(true);
last_ssrc_ = 123;
}

1
webrtc/modules/congestion_controller/congestion_controller.cc
View File

@ -297,7 +297,6 @@ void CongestionController::SignalNetworkState(NetworkState state) {
rtc::CritScope cs(&critsect_);
network_state_ = state;
}
probe_controller_->OnNetworkStateChanged(state);
MaybeTriggerOnNetworkChanged();
}

57
webrtc/modules/congestion_controller/probe_controller.cc
View File

@ -46,12 +46,10 @@ constexpr int kAlrProbingIntervalLimitMs = 5000;
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()) {}
@ -59,51 +57,30 @@ void ProbeController::SetBitrates(int min_bitrate_bps,
int start_bitrate_bps,
int max_bitrate_bps) {
rtc::CritScope cs(&critsect_);
start_bitrate_bps_ =
start_bitrate_bps > 0 ? start_bitrate_bps : min_bitrate_bps;
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);
}
int old_max_bitrate_bps = max_bitrate_bps_;
max_bitrate_bps_ = max_bitrate_bps;
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;
// 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);
}
}
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) {

14
webrtc/modules/congestion_controller/probe_controller.h
View File

@ -31,12 +31,12 @@ class ProbeController {
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,21 +45,13 @@ class ProbeController {
// 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_);

11
webrtc/modules/congestion_controller/probe_controller_unittest.cc
View File

@ -51,17 +51,6 @@ TEST_F(ProbeControllerTest, InitiatesProbingAtStart) {
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,

4
webrtc/test/call_test.cc
View File

@ -71,10 +71,6 @@ void CallTest::RunBaseTest(BaseTest* test) {
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());

4
webrtc/test/direct_transport.cc
View File

@ -28,10 +28,6 @@ DirectTransport::DirectTransport(const FakeNetworkPipe::Config& config,
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(); }

21
webrtc/video/end_to_end_tests.cc
View File

@ -122,11 +122,11 @@ class EndToEndTest : public test::CallTest {
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_up,
MediaType network_to_bring_down,
VideoEncoder* encoder,
Transport* transport);
void VerifyNewVideoReceiveStreamsRespectNetworkState(
MediaType network_to_bring_up,
MediaType network_to_bring_down,
Transport* transport);
};
@ -3587,11 +3587,11 @@ TEST_F(EndToEndTest, CallReportsRttForSender) {
}
void EndToEndTest::VerifyNewVideoSendStreamsRespectNetworkState(
MediaType network_to_bring_up,
MediaType network_to_bring_down,
VideoEncoder* encoder,
Transport* transport) {
CreateSenderCall(Call::Config(&event_log_));
sender_call_->SignalChannelNetworkState(network_to_bring_up, kNetworkUp);
sender_call_->SignalChannelNetworkState(network_to_bring_down, kNetworkDown);
CreateSendConfig(1, 0, 0, transport);
video_send_config_.encoder_settings.encoder = encoder;
@ -3607,11 +3607,12 @@ void EndToEndTest::VerifyNewVideoSendStreamsRespectNetworkState(
}
void EndToEndTest::VerifyNewVideoReceiveStreamsRespectNetworkState(
MediaType network_to_bring_up,
MediaType network_to_bring_down,
Transport* transport) {
Call::Config config(&event_log_);
CreateCalls(config, config);
receiver_call_->SignalChannelNetworkState(network_to_bring_up, kNetworkUp);
receiver_call_->SignalChannelNetworkState(network_to_bring_down,
kNetworkDown);
test::DirectTransport sender_transport(sender_call_.get());
sender_transport.SetReceiver(receiver_call_->Receiver());
@ -3653,7 +3654,7 @@ TEST_F(EndToEndTest, NewVideoSendStreamsRespectVideoNetworkDown) {
UnusedEncoder unused_encoder;
UnusedTransport unused_transport;
VerifyNewVideoSendStreamsRespectNetworkState(
MediaType::AUDIO, &unused_encoder, &unused_transport);
MediaType::VIDEO, &unused_encoder, &unused_transport);
}
TEST_F(EndToEndTest, NewVideoSendStreamsIgnoreAudioNetworkDown) {
@ -3681,17 +3682,17 @@ TEST_F(EndToEndTest, NewVideoSendStreamsIgnoreAudioNetworkDown) {
RequiredTransport required_transport(true /*rtp*/, false /*rtcp*/);
RequiredEncoder required_encoder;
VerifyNewVideoSendStreamsRespectNetworkState(
MediaType::VIDEO, &required_encoder, &required_transport);
MediaType::AUDIO, &required_encoder, &required_transport);
}
TEST_F(EndToEndTest, NewVideoReceiveStreamsRespectVideoNetworkDown) {
UnusedTransport transport;
VerifyNewVideoReceiveStreamsRespectNetworkState(MediaType::AUDIO, &transport);
VerifyNewVideoReceiveStreamsRespectNetworkState(MediaType::VIDEO, &transport);
}
TEST_F(EndToEndTest, NewVideoReceiveStreamsIgnoreAudioNetworkDown) {
RequiredTransport transport(false /*rtp*/, true /*rtcp*/);
VerifyNewVideoReceiveStreamsRespectNetworkState(MediaType::VIDEO, &transport);
VerifyNewVideoReceiveStreamsRespectNetworkState(MediaType::AUDIO, &transport);
}
void VerifyEmptyNackConfig(const NackConfig& config) {

2
webrtc/video/video_send_stream_tests.cc
View File

@ -1657,8 +1657,6 @@ TEST_F(VideoSendStreamTest, VideoSendStreamStopSetEncoderRateToZero) {
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;