Renaming variables in p2ptransportchannel to be consistent.
Also change the type of "time interval" to int from uint32.
Fixed a few TODO therein. I think we should have the following convention:
1. All time delay/intervals should have type int although the time instant should have time uint32_t.
2. "interval" is preferred to "delay" if the delay will be repeated (like rescheduling).
BUG=
R=pthatcher@webrtc.org
Review URL: https://codereview.webrtc.org/1762863002 .
Cr-Commit-Position: refs/heads/master@{#11888}
diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc
index 57f80c7..8cbcb97 100644
--- a/webrtc/api/webrtcsession.cc
+++ b/webrtc/api/webrtcsession.cc
@@ -1123,7 +1123,7 @@
cricket::IceConfig WebRtcSession::ParseIceConfig(
const PeerConnectionInterface::RTCConfiguration& config) const {
cricket::IceConfig ice_config;
- ice_config.receiving_timeout_ms = config.ice_connection_receiving_timeout;
+ ice_config.receiving_timeout = config.ice_connection_receiving_timeout;
ice_config.prioritize_most_likely_candidate_pairs =
config.prioritize_most_likely_ice_candidate_pairs;
ice_config.backup_connection_ping_interval =
diff --git a/webrtc/p2p/base/faketransportcontroller.h b/webrtc/p2p/base/faketransportcontroller.h
index 63e8fd3..c5e2afea 100644
--- a/webrtc/p2p/base/faketransportcontroller.h
+++ b/webrtc/p2p/base/faketransportcontroller.h
@@ -180,7 +180,7 @@
void SetReceiving(bool receiving) { set_receiving(receiving); }
void SetIceConfig(const IceConfig& config) override {
- receiving_timeout_ = config.receiving_timeout_ms;
+ receiving_timeout_ = config.receiving_timeout;
gather_continually_ = config.gather_continually;
}
diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc
index d6f3cbc..b21d584 100644
--- a/webrtc/p2p/base/p2ptransportchannel.cc
+++ b/webrtc/p2p/base/p2ptransportchannel.cc
@@ -208,21 +208,20 @@
// we don't want to degrade the quality on a modem. These numbers should work
// well on a 28.8K modem, which is the slowest connection on which the voice
// quality is reasonable at all.
-static const uint32_t PING_PACKET_SIZE = 60 * 8;
-// TODO(honghaiz): Change the word DELAY to INTERVAL whenever appropriate.
-// STRONG_PING_DELAY (480ms) is applied when the best connection is both
+static const int PING_PACKET_SIZE = 60 * 8;
+// STRONG_PING_INTERVAL (480ms) is applied when the best connection is both
// writable and receiving.
-static const uint32_t STRONG_PING_DELAY = 1000 * PING_PACKET_SIZE / 1000;
-// WEAK_PING_DELAY (48ms) is applied when the best connection is either not
+static const int STRONG_PING_INTERVAL = 1000 * PING_PACKET_SIZE / 1000;
+// WEAK_PING_INTERVAL (48ms) is applied when the best connection is either not
// writable or not receiving.
-const uint32_t WEAK_PING_DELAY = 1000 * PING_PACKET_SIZE / 10000;
+const int WEAK_PING_INTERVAL = 1000 * PING_PACKET_SIZE / 10000;
// If the current best connection is both writable and receiving, then we will
// also try hard to make sure it is pinged at this rate (a little less than
-// 2 * STRONG_PING_DELAY).
-static const uint32_t MAX_CURRENT_STRONG_DELAY = 900;
+// 2 * STRONG_PING_INTERVAL).
+static const int MAX_CURRENT_STRONG_INTERVAL = 900; // ms
-static const int MIN_CHECK_RECEIVING_DELAY = 50; // ms
+static const int MIN_CHECK_RECEIVING_INTERVAL = 50; // ms
P2PTransportChannel::P2PTransportChannel(const std::string& transport_name,
int component,
@@ -245,17 +244,17 @@
ice_role_(ICEROLE_UNKNOWN),
tiebreaker_(0),
gathering_state_(kIceGatheringNew),
- check_receiving_delay_(MIN_CHECK_RECEIVING_DELAY * 5),
- config_(MIN_CHECK_RECEIVING_DELAY * 50 /* receiving_timeout */,
+ check_receiving_interval_(MIN_CHECK_RECEIVING_INTERVAL * 5),
+ config_(MIN_CHECK_RECEIVING_INTERVAL * 50 /* receiving_timeout */,
0 /* backup_connection_ping_interval */,
false /* gather_continually */,
false /* prioritize_most_likely_candidate_pairs */,
- MAX_CURRENT_STRONG_DELAY /* most_strong_delay */) {
- uint32_t weak_ping_delay = ::strtoul(
+ MAX_CURRENT_STRONG_INTERVAL /* max_strong_interval */) {
+ uint32_t weak_ping_interval = ::strtoul(
webrtc::field_trial::FindFullName("WebRTC-StunInterPacketDelay").c_str(),
nullptr, 10);
- if (weak_ping_delay) {
- weak_ping_delay_ = weak_ping_delay;
+ if (weak_ping_interval) {
+ weak_ping_interval_ = static_cast<int>(weak_ping_interval);
}
}
@@ -291,7 +290,7 @@
connections_.push_back(connection);
unpinged_connections_.insert(connection);
connection->set_remote_ice_mode(remote_ice_mode_);
- connection->set_receiving_timeout(config_.receiving_timeout_ms);
+ connection->set_receiving_timeout(config_.receiving_timeout);
connection->SignalReadPacket.connect(
this, &P2PTransportChannel::OnReadPacket);
connection->SignalReadyToSend.connect(
@@ -413,17 +412,17 @@
<< config_.backup_connection_ping_interval << " milliseconds.";
}
- if (config.receiving_timeout_ms >= 0 &&
- config_.receiving_timeout_ms != config.receiving_timeout_ms) {
- config_.receiving_timeout_ms = config.receiving_timeout_ms;
- check_receiving_delay_ =
- std::max(MIN_CHECK_RECEIVING_DELAY, config_.receiving_timeout_ms / 10);
+ if (config.receiving_timeout >= 0 &&
+ config_.receiving_timeout != config.receiving_timeout) {
+ config_.receiving_timeout = config.receiving_timeout;
+ check_receiving_interval_ =
+ std::max(MIN_CHECK_RECEIVING_INTERVAL, config_.receiving_timeout / 10);
for (Connection* connection : connections_) {
- connection->set_receiving_timeout(config_.receiving_timeout_ms);
+ connection->set_receiving_timeout(config_.receiving_timeout);
}
- LOG(LS_INFO) << "Set ICE receiving timeout to "
- << config_.receiving_timeout_ms << " milliseconds";
+ LOG(LS_INFO) << "Set ICE receiving timeout to " << config_.receiving_timeout
+ << " milliseconds";
}
config_.prioritize_most_likely_candidate_pairs =
@@ -431,10 +430,11 @@
LOG(LS_INFO) << "Set ping most likely connection to "
<< config_.prioritize_most_likely_candidate_pairs;
- if (config.max_strong_delay >= 0 &&
- config_.max_strong_delay != config.max_strong_delay) {
- config_.max_strong_delay = config.max_strong_delay;
- LOG(LS_INFO) << "Set max strong delay to " << config_.max_strong_delay;
+ if (config.max_strong_interval >= 0 &&
+ config_.max_strong_interval != config.max_strong_interval) {
+ config_.max_strong_interval = config.max_strong_interval;
+ LOG(LS_INFO) << "Set max strong interval to "
+ << config_.max_strong_interval;
}
}
@@ -1245,17 +1245,17 @@
// which ones are pingable).
UpdateConnectionStates();
// When the best connection is either not receiving or not writable,
- // switch to weak ping delay.
- int ping_delay = weak() ? weak_ping_delay_ : STRONG_PING_DELAY;
- if (rtc::Time() >= last_ping_sent_ms_ + ping_delay) {
+ // switch to weak ping interval.
+ int ping_interval = weak() ? weak_ping_interval_ : STRONG_PING_INTERVAL;
+ if (rtc::Time() >= last_ping_sent_ms_ + ping_interval) {
Connection* conn = FindNextPingableConnection();
if (conn) {
PingConnection(conn);
MarkConnectionPinged(conn);
}
}
- int check_delay = std::min(ping_delay, check_receiving_delay_);
- thread()->PostDelayed(check_delay, this, MSG_CHECK_AND_PING);
+ int delay = std::min(ping_interval, check_receiving_interval_);
+ thread()->PostDelayed(delay, this, MSG_CHECK_AND_PING);
}
// A connection is considered a backup connection if the channel state
@@ -1300,16 +1300,18 @@
// Returns the next pingable connection to ping. This will be the oldest
// pingable connection unless we have a connected, writable connection that is
-// past the maximum acceptable ping delay. When reconnecting a TCP connection,
-// the best connection is disconnected, although still WRITABLE while
-// reconnecting. The newly created connection should be selected as the ping
-// target to become writable instead. See the big comment in CompareConnections.
+// past the maximum acceptable ping interval. When reconnecting a TCP
+// connection, the best connection is disconnected, although still WRITABLE
+// while reconnecting. The newly created connection should be selected as the
+// ping target to become writable instead. See the big comment in
+// CompareConnections.
Connection* P2PTransportChannel::FindNextPingableConnection() {
uint32_t now = rtc::Time();
Connection* conn_to_ping = nullptr;
if (best_connection_ && best_connection_->connected() &&
best_connection_->writable() &&
- (best_connection_->last_ping_sent() + config_.max_strong_delay <= now)) {
+ (best_connection_->last_ping_sent() + config_.max_strong_interval <=
+ now)) {
conn_to_ping = best_connection_;
} else {
conn_to_ping = FindConnectionToPing(now);
diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h
index eac405d..3ca25d4 100644
--- a/webrtc/p2p/base/p2ptransportchannel.h
+++ b/webrtc/p2p/base/p2ptransportchannel.h
@@ -34,7 +34,7 @@
namespace cricket {
-extern const uint32_t WEAK_PING_DELAY;
+extern const int WEAK_PING_INTERVAL;
struct IceParameters {
std::string ufrag;
@@ -92,8 +92,10 @@
return gathering_state_;
}
void AddRemoteCandidate(const Candidate& candidate) override;
- // Sets the receiving timeout and gather_continually.
- // This also sets the check_receiving_delay proportionally.
+ // Sets the parameters in IceConfig. We do not set them blindly. Instead, we
+ // only update the parameter if it is considered set in |config|. For example,
+ // a negative value of receiving_timeout will be considered "not set" and we
+ // will not use it to update the respective parameter in |config_|.
void SetIceConfig(const IceConfig& config) override;
const IceConfig& config() const;
@@ -166,8 +168,8 @@
return false;
}
- int receiving_timeout() const { return config_.receiving_timeout_ms; }
- int check_receiving_delay() const { return check_receiving_delay_; }
+ int receiving_timeout() const { return config_.receiving_timeout; }
+ int check_receiving_interval() const { return check_receiving_interval_; }
// Helper method used only in unittest.
rtc::DiffServCodePoint DefaultDscpValue() const;
@@ -317,9 +319,9 @@
uint64_t tiebreaker_;
IceGatheringState gathering_state_;
- int check_receiving_delay_;
+ int check_receiving_interval_;
uint32_t last_ping_sent_ms_ = 0;
- int weak_ping_delay_ = WEAK_PING_DELAY;
+ int weak_ping_interval_ = WEAK_PING_INTERVAL;
TransportChannelState state_ = TransportChannelState::STATE_INIT;
IceConfig config_;
diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
index 49446dd..c54f054 100644
--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc
+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
@@ -102,11 +102,11 @@
MSG_CANDIDATE
};
-static cricket::IceConfig CreateIceConfig(int receiving_timeout_ms,
+static cricket::IceConfig CreateIceConfig(int receiving_timeout,
bool gather_continually,
int backup_ping_interval = -1) {
cricket::IceConfig config;
- config.receiving_timeout_ms = receiving_timeout_ms;
+ config.receiving_timeout = receiving_timeout;
config.gather_continually = gather_continually;
config.backup_connection_ping_interval = backup_ping_interval;
return config;
@@ -2084,13 +2084,13 @@
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("receiving state change", 1, &pa);
PrepareChannel(&ch);
- // Default receiving timeout and checking receiving delay should not be too
+ // Default receiving timeout and checking receiving interval should not be too
// small.
EXPECT_LE(1000, ch.receiving_timeout());
- EXPECT_LE(200, ch.check_receiving_delay());
+ EXPECT_LE(200, ch.check_receiving_interval());
ch.SetIceConfig(CreateIceConfig(500, false));
EXPECT_EQ(500, ch.receiving_timeout());
- EXPECT_EQ(50, ch.check_receiving_delay());
+ EXPECT_EQ(50, ch.check_receiving_interval());
ch.Connect();
ch.MaybeStartGathering();
ch.AddRemoteCandidate(CreateHostCandidate("1.1.1.1", 1, 1));
@@ -2492,13 +2492,13 @@
cricket::P2PTransportChannel& StartTransportChannel(
bool prioritize_most_likely_to_work,
- uint32_t max_strong_delay) {
+ int max_strong_interval) {
channel_.reset(
new cricket::P2PTransportChannel("checks", 1, nullptr, allocator()));
cricket::IceConfig config = channel_->config();
config.prioritize_most_likely_candidate_pairs =
prioritize_most_likely_to_work;
- config.max_strong_delay = max_strong_delay;
+ config.max_strong_interval = max_strong_interval;
channel_->SetIceConfig(config);
PrepareChannel(channel_.get());
channel_->Connect();
@@ -2546,9 +2546,9 @@
// we have a best connection.
TEST_F(P2PTransportChannelMostLikelyToWorkFirstTest,
TestRelayRelayFirstWhenNothingPingedYet) {
- const uint32_t max_strong_delay = 100;
+ const int max_strong_interval = 100;
cricket::P2PTransportChannel& ch =
- StartTransportChannel(true, max_strong_delay);
+ StartTransportChannel(true, max_strong_interval);
EXPECT_TRUE_WAIT(ch.ports().size() == 2, 5000);
EXPECT_EQ(ch.ports()[0]->Type(), cricket::LOCAL_PORT_TYPE);
EXPECT_EQ(ch.ports()[1]->Type(), cricket::RELAY_PORT_TYPE);
@@ -2579,10 +2579,10 @@
conn3->ReceivedPing();
// Verify that conn3 will be the "best connection" since it is readable and
- // writable. After |MAX_CURRENT_STRONG_DELAY|, it should be the next pingable
- // connection.
+ // writable. After |MAX_CURRENT_STRONG_INTERVAL|, it should be the next
+ // pingable connection.
EXPECT_TRUE_WAIT(conn3 == ch.best_connection(), 5000);
- WAIT(false, max_strong_delay + 100);
+ WAIT(false, max_strong_interval + 100);
conn3->ReceivedPingResponse();
ASSERT_TRUE(conn3->writable());
EXPECT_EQ(conn3, FindNextPingableConnectionAndPingIt(&ch));
diff --git a/webrtc/p2p/base/transport.h b/webrtc/p2p/base/transport.h
index 13c8431..84581c8 100644
--- a/webrtc/p2p/base/transport.h
+++ b/webrtc/p2p/base/transport.h
@@ -139,9 +139,8 @@
// Information about ICE configuration.
struct IceConfig {
- // The ICE connection receiving timeout value.
- // TODO(honghaiz): Remove suffix _ms to be consistent.
- int receiving_timeout_ms = -1;
+ // The ICE connection receiving timeout value in milliseconds.
+ int receiving_timeout = -1;
// Time interval in milliseconds to ping a backup connection when the ICE
// channel is strongly connected.
int backup_connection_ping_interval = -1;
@@ -154,21 +153,21 @@
// If the current best connection is both writable and receiving,
// then we will also try hard to make sure it is pinged at this rate
- // (Default value is a little less than 2 * STRONG_PING_DELAY).
- int max_strong_delay = -1;
+ // (Default value is a little less than 2 * STRONG_PING_INTERVAL).
+ int max_strong_interval = -1;
IceConfig() {}
- IceConfig(int receiving_timeout,
+ IceConfig(int receiving_timeout_ms,
int backup_connection_ping_interval,
bool gather_continually,
bool prioritize_most_likely_candidate_pairs,
- int max_strong_delay)
- : receiving_timeout_ms(receiving_timeout),
+ int max_strong_interval_ms)
+ : receiving_timeout(receiving_timeout_ms),
backup_connection_ping_interval(backup_connection_ping_interval),
gather_continually(gather_continually),
prioritize_most_likely_candidate_pairs(
prioritize_most_likely_candidate_pairs),
- max_strong_delay(max_strong_delay) {}
+ max_strong_interval(max_strong_interval_ms) {}
};
bool BadTransportDescription(const std::string& desc, std::string* err_desc);
diff --git a/webrtc/p2p/base/transportcontroller_unittest.cc b/webrtc/p2p/base/transportcontroller_unittest.cc
index 96d0e98..4016273 100644
--- a/webrtc/p2p/base/transportcontroller_unittest.cc
+++ b/webrtc/p2p/base/transportcontroller_unittest.cc
@@ -134,10 +134,10 @@
channel2->SetConnectionCount(1);
}
- cricket::IceConfig CreateIceConfig(int receiving_timeout_ms,
+ cricket::IceConfig CreateIceConfig(int receiving_timeout,
bool gather_continually) {
cricket::IceConfig config;
- config.receiving_timeout_ms = receiving_timeout_ms;
+ config.receiving_timeout = receiving_timeout;
config.gather_continually = gather_continually;
return config;
}
