Revert "Revert of Update the BWE when the network route changes. (patchset #5 id:180001 of https://codereview.webrtc.org/2000063003/ )"
This reverts commit 72d41aa6da94dacb8a8464d1abd4ca7d1afffc65.
New change made:
Do not reset the BWE when the new network route is not ready to send media.
BUG=
R=pthatcher@webrtc.org, stefan@webrtc.org
Review URL: https://codereview.webrtc.org/2041593002 .
Cr-Original-Commit-Position: refs/heads/master@{#13280}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 5b5d2cdad7018993272525a723ef34f7da5c45f2
diff --git a/base/networkroute.h b/base/networkroute.h
index 52fa6d6..a34e6d3 100644
--- a/base/networkroute.h
+++ b/base/networkroute.h
@@ -30,10 +30,11 @@
last_sent_packet_id(-1) {}
// The route is connected if the local and remote network ids are provided.
- NetworkRoute(uint16_t local_net_id,
+ NetworkRoute(bool connected,
+ uint16_t local_net_id,
uint16_t remote_net_id,
int last_packet_id)
- : connected(true),
+ : connected(connected),
local_network_id(local_net_id),
remote_network_id(remote_net_id),
last_sent_packet_id(last_packet_id) {}
diff --git a/call/call.cc b/call/call.cc
index 93b638a..a6a7978 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -636,9 +636,13 @@
kv->second = network_route;
LOG(LS_INFO) << "Network route changed on transport " << transport_name
<< ": new local network id " << network_route.local_network_id
- << " new remote network id "
- << network_route.remote_network_id;
- // TODO(holmer): Update the BWE bitrates.
+ << " new remote network id " << network_route.remote_network_id
+ << " Reset bitrate to "
+ << config_.bitrate_config.start_bitrate_bps << "bps";
+ congestion_controller_->ResetBweAndBitrates(
+ config_.bitrate_config.start_bitrate_bps,
+ config_.bitrate_config.min_bitrate_bps,
+ config_.bitrate_config.max_bitrate_bps);
}
}
diff --git a/modules/bitrate_controller/bitrate_controller_impl.cc b/modules/bitrate_controller/bitrate_controller_impl.cc
index 09652d8..c99bd53 100644
--- a/modules/bitrate_controller/bitrate_controller_impl.cc
+++ b/modules/bitrate_controller/bitrate_controller_impl.cc
@@ -138,6 +138,18 @@
MaybeTriggerOnNetworkChanged();
}
+void BitrateControllerImpl::ResetBitrates(int bitrate_bps,
+ int min_bitrate_bps,
+ int max_bitrate_bps) {
+ {
+ rtc::CritScope cs(&critsect_);
+ bandwidth_estimation_ = SendSideBandwidthEstimation();
+ bandwidth_estimation_.SetBitrates(bitrate_bps, min_bitrate_bps,
+ max_bitrate_bps);
+ }
+ MaybeTriggerOnNetworkChanged();
+}
+
void BitrateControllerImpl::SetReservedBitrate(uint32_t reserved_bitrate_bps) {
{
rtc::CritScope cs(&critsect_);
diff --git a/modules/bitrate_controller/bitrate_controller_impl.h b/modules/bitrate_controller/bitrate_controller_impl.h
index 5a61379..91f0902 100644
--- a/modules/bitrate_controller/bitrate_controller_impl.h
+++ b/modules/bitrate_controller/bitrate_controller_impl.h
@@ -46,6 +46,10 @@
int min_bitrate_bps,
int max_bitrate_bps) override;
+ void ResetBitrates(int bitrate_bps,
+ int min_bitrate_bps,
+ int max_bitrate_bps) override;
+
void UpdateDelayBasedEstimate(uint32_t bitrate_bps) override;
void SetReservedBitrate(uint32_t reserved_bitrate_bps) override;
diff --git a/modules/bitrate_controller/include/bitrate_controller.h b/modules/bitrate_controller/include/bitrate_controller.h
index a61cf6a..4165f06 100644
--- a/modules/bitrate_controller/include/bitrate_controller.h
+++ b/modules/bitrate_controller/include/bitrate_controller.h
@@ -70,6 +70,10 @@
int min_bitrate_bps,
int max_bitrate_bps) = 0;
+ virtual void ResetBitrates(int bitrate_bps,
+ int min_bitrate_bps,
+ int max_bitrate_bps) = 0;
+
virtual void UpdateDelayBasedEstimate(uint32_t bitrate_bps) = 0;
virtual void SetEventLog(RtcEventLog* event_log) = 0;
diff --git a/modules/bitrate_controller/include/mock/mock_bitrate_controller.h b/modules/bitrate_controller/include/mock/mock_bitrate_controller.h
index da6169e..7e9b4ec 100644
--- a/modules/bitrate_controller/include/mock/mock_bitrate_controller.h
+++ b/modules/bitrate_controller/include/mock/mock_bitrate_controller.h
@@ -35,6 +35,8 @@
void(int start_bitrate_bps,
int min_bitrate_bps,
int max_bitrate_bps));
+ MOCK_METHOD3(ResetBitrates,
+ void(int bitrate_bps, int min_bitrate_bps, int max_bitrate_bps));
MOCK_METHOD1(UpdateDelayBasedEstimate, void(uint32_t bitrate_bps));
MOCK_METHOD1(SetEventLog, void(RtcEventLog* event_log));
MOCK_CONST_METHOD1(AvailableBandwidth, bool(uint32_t* bandwidth));
diff --git a/modules/congestion_controller/congestion_controller.cc b/modules/congestion_controller/congestion_controller.cc
index a585e33..c836f40 100644
--- a/modules/congestion_controller/congestion_controller.cc
+++ b/modules/congestion_controller/congestion_controller.cc
@@ -33,6 +33,22 @@
static const uint32_t kTimeOffsetSwitchThreshold = 30;
+// Makes sure that the bitrate and the min, max values are in valid range.
+static void ClampBitrates(int* bitrate_bps,
+ int* min_bitrate_bps,
+ int* max_bitrate_bps) {
+ // TODO(holmer): We should make sure the default bitrates are set to 10 kbps,
+ // and that we don't try to set the min bitrate to 0 from any applications.
+ // The congestion controller should allow a min bitrate of 0.
+ const int kMinBitrateBps = 10000;
+ if (*min_bitrate_bps < kMinBitrateBps)
+ *min_bitrate_bps = kMinBitrateBps;
+ if (*max_bitrate_bps > 0)
+ *max_bitrate_bps = std::max(*min_bitrate_bps, *max_bitrate_bps);
+ if (*bitrate_bps > 0)
+ *bitrate_bps = std::max(*min_bitrate_bps, *bitrate_bps);
+}
+
class WrappingBitrateEstimator : public RemoteBitrateEstimator {
public:
WrappingBitrateEstimator(RemoteBitrateObserver* observer, Clock* clock)
@@ -212,21 +228,10 @@
min_bitrate_bps_);
}
-
void CongestionController::SetBweBitrates(int min_bitrate_bps,
int start_bitrate_bps,
int max_bitrate_bps) {
- // TODO(holmer): We should make sure the default bitrates are set to 10 kbps,
- // and that we don't try to set the min bitrate to 0 from any applications.
- // The congestion controller should allow a min bitrate of 0.
- const int kMinBitrateBps = 10000;
- if (min_bitrate_bps < kMinBitrateBps)
- min_bitrate_bps = kMinBitrateBps;
- if (max_bitrate_bps > 0)
- max_bitrate_bps = std::max(min_bitrate_bps, max_bitrate_bps);
- if (start_bitrate_bps > 0)
- start_bitrate_bps = std::max(min_bitrate_bps, start_bitrate_bps);
-
+ ClampBitrates(&start_bitrate_bps, &min_bitrate_bps, &max_bitrate_bps);
bitrate_controller_->SetBitrates(start_bitrate_bps,
min_bitrate_bps,
max_bitrate_bps);
@@ -239,6 +244,28 @@
MaybeTriggerOnNetworkChanged();
}
+void CongestionController::ResetBweAndBitrates(int bitrate_bps,
+ int min_bitrate_bps,
+ int max_bitrate_bps) {
+ ClampBitrates(&bitrate_bps, &min_bitrate_bps, &max_bitrate_bps);
+ // TODO(honghaiz): Recreate this object once the bitrate controller is
+ // no longer exposed outside CongestionController.
+ bitrate_controller_->ResetBitrates(bitrate_bps, min_bitrate_bps,
+ max_bitrate_bps);
+ min_bitrate_bps_ = min_bitrate_bps;
+ // TODO(honghaiz): Recreate this object once the remote bitrate estimator is
+ // no longer exposed outside CongestionController.
+ if (remote_bitrate_estimator_)
+ remote_bitrate_estimator_->SetMinBitrate(min_bitrate_bps);
+
+ RemoteBitrateEstimator* rbe =
+ new RemoteBitrateEstimatorAbsSendTime(&transport_feedback_adapter_);
+ transport_feedback_adapter_.SetBitrateEstimator(rbe);
+ rbe->SetMinBitrate(min_bitrate_bps);
+ // TODO(holmer): Trigger a new probe once mid-call probing is implemented.
+ MaybeTriggerOnNetworkChanged();
+}
+
BitrateController* CongestionController::GetBitrateController() const {
return bitrate_controller_.get();
}
diff --git a/modules/congestion_controller/congestion_controller_unittest.cc b/modules/congestion_controller/congestion_controller_unittest.cc
index c82c75d..1a26582 100644
--- a/modules/congestion_controller/congestion_controller_unittest.cc
+++ b/modules/congestion_controller/congestion_controller_unittest.cc
@@ -124,6 +124,20 @@
controller_->SignalNetworkState(kNetworkDown);
}
+TEST_F(CongestionControllerTest, ResetBweAndBitrates) {
+ int new_bitrate = 200000;
+ EXPECT_CALL(observer_, OnNetworkChanged(new_bitrate, _, _));
+ EXPECT_CALL(*pacer_, SetEstimatedBitrate(new_bitrate));
+ controller_->ResetBweAndBitrates(new_bitrate, -1, -1);
+
+ // If the bitrate is reset to -1, the new starting bitrate will be
+ // the minimum default bitrate 10000bps.
+ int min_default_bitrate = 10000;
+ EXPECT_CALL(observer_, OnNetworkChanged(min_default_bitrate, _, _));
+ EXPECT_CALL(*pacer_, SetEstimatedBitrate(min_default_bitrate));
+ controller_->ResetBweAndBitrates(-1, -1, -1);
+}
+
TEST_F(CongestionControllerTest,
SignalNetworkStateAndQueueIsFullAndEstimateChange) {
// Send queue is full
diff --git a/modules/congestion_controller/include/congestion_controller.h b/modules/congestion_controller/include/congestion_controller.h
index 1c04c11..82906ef 100644
--- a/modules/congestion_controller/include/congestion_controller.h
+++ b/modules/congestion_controller/include/congestion_controller.h
@@ -70,6 +70,11 @@
virtual void SetBweBitrates(int min_bitrate_bps,
int start_bitrate_bps,
int max_bitrate_bps);
+ // Resets both the BWE state and the bitrate estimator. Note the first
+ // argument is the bitrate_bps.
+ virtual void ResetBweAndBitrates(int bitrate_bps,
+ int min_bitrate_bps,
+ int max_bitrate_bps);
virtual void SignalNetworkState(NetworkState state);
virtual BitrateController* GetBitrateController() const;
virtual RemoteBitrateEstimator* GetRemoteBitrateEstimator(
diff --git a/p2p/base/candidatepairinterface.h b/p2p/base/candidatepairinterface.h
index 1d11c22..91b47f4 100644
--- a/p2p/base/candidatepairinterface.h
+++ b/p2p/base/candidatepairinterface.h
@@ -21,6 +21,8 @@
virtual const Candidate& local_candidate() const = 0;
virtual const Candidate& remote_candidate() const = 0;
+
+ virtual bool ReadyToSendMedia() const = 0;
};
} // namespace cricket
diff --git a/p2p/base/faketransportcontroller.h b/p2p/base/faketransportcontroller.h
index 7b7aec5..59b618a 100644
--- a/p2p/base/faketransportcontroller.h
+++ b/p2p/base/faketransportcontroller.h
@@ -476,6 +476,8 @@
return remote_candidate_;
}
+ bool ReadyToSendMedia() const override { return true; }
+
private:
Candidate local_candidate_;
Candidate remote_candidate_;
diff --git a/p2p/base/port.cc b/p2p/base/port.cc
index d4dfd91..ab7ba09 100644
--- a/p2p/base/port.cc
+++ b/p2p/base/port.cc
@@ -1525,7 +1525,7 @@
int ProxyConnection::Send(const void* data, size_t size,
const rtc::PacketOptions& options) {
- if (write_state_ == STATE_WRITE_INIT || write_state_ == STATE_WRITE_TIMEOUT) {
+ if (!ReadyToSendMedia()) {
error_ = EWOULDBLOCK;
return SOCKET_ERROR;
}
diff --git a/p2p/base/port.h b/p2p/base/port.h
index 06231ec..5532239 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -461,6 +461,11 @@
bool active() const {
return write_state_ != STATE_WRITE_TIMEOUT;
}
+ virtual bool ReadyToSendMedia() const {
+ return write_state_ == STATE_WRITABLE ||
+ write_state_ == STATE_WRITE_UNRELIABLE;
+ }
+
// A connection is dead if it can be safely deleted.
bool dead(int64_t now) const;
diff --git a/pc/channel.cc b/pc/channel.cc
index 6f980e1..9fe8aef 100644
--- a/pc/channel.cc
+++ b/pc/channel.cc
@@ -608,6 +608,7 @@
rtc::NetworkRoute network_route;
if (selected_candidate_pair) {
network_route = rtc::NetworkRoute(
+ selected_candidate_pair->ReadyToSendMedia(),
selected_candidate_pair->local_candidate().network_id(),
selected_candidate_pair->remote_candidate().network_id(),
last_sent_packet_id);
diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc
index 0e56652..c57ac76 100644
--- a/pc/channel_unittest.cc
+++ b/pc/channel_unittest.cc
@@ -937,7 +937,7 @@
});
WaitForThreads();
EXPECT_EQ(1, media_channel1->num_network_route_changes());
- rtc::NetworkRoute expected_network_route(kLocalNetId, kRemoteNetId,
+ rtc::NetworkRoute expected_network_route(true, kLocalNetId, kRemoteNetId,
kLastPacketId);
EXPECT_EQ(expected_network_route, media_channel1->last_network_route());
EXPECT_EQ(kLastPacketId,
