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

Renaming variables in p2ptransportchannel to be consistent.

Browse Source 
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}
This commit is contained in:
Honghai Zhang 2016-03-07 11:13:07 -08:00
parent eb648bf0e5
commit 049fbb1883
7 changed files with 73 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
webrtc/api/webrtcsession.cc
View File

@ -1123,7 +1123,7 @@ bool WebRtcSession::SetIceTransports(
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 =

2
webrtc/p2p/base/faketransportcontroller.h
View File

@ -180,7 +180,7 @@ class FakeTransportChannel : public TransportChannelImpl,
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;
}

78
webrtc/p2p/base/p2ptransportchannel.cc
View File

@ -208,21 +208,20 @@ namespace cricket {
// 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 @@ P2PTransportChannel::P2PTransportChannel(const std::string& transport_name,
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 @@ void P2PTransportChannel::AddConnection(Connection* connection) {
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 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) {
<< 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 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) {
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 @@ void P2PTransportChannel::OnCheckAndPing() {
// 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 @@ bool P2PTransportChannel::IsPingable(Connection* conn, uint32_t now) {
// 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);

16
webrtc/p2p/base/p2ptransportchannel.h
View File

@ -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 @@ class P2PTransportChannel : public TransportChannelImpl,
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 @@ class P2PTransportChannel : public TransportChannelImpl,
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 @@ class P2PTransportChannel : public TransportChannelImpl,
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_;

24
webrtc/p2p/base/p2ptransportchannel_unittest.cc
View File

@ -102,11 +102,11 @@ enum {
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 @@ TEST_F(P2PTransportChannelPingTest, TestReceivingStateChange) {
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 @@ class P2PTransportChannelMostLikelyToWorkFirstTest
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 @@ class P2PTransportChannelMostLikelyToWorkFirstTest
// 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 @@ TEST_F(P2PTransportChannelMostLikelyToWorkFirstTest,
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));

17
webrtc/p2p/base/transport.h
View File

@ -139,9 +139,8 @@ struct TransportStats {
// 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 @@ struct IceConfig {
// 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);

4
webrtc/p2p/base/transportcontroller_unittest.cc
View File

@ -134,10 +134,10 @@ class TransportControllerTest : public testing::Test,
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;
}