From b0bb77fd615c5ce413b16f6157b2ffc25898631f Mon Sep 17 00:00:00 2001 From: guoweis Date: Mon, 26 Oct 2015 15:10:01 -0700 Subject: [PATCH] Reland of Add experiment on weak ping delay during call set up time (patchset #1 id:1 of https://codereview.webrtc.org/1416773003/ ) Reason for revert: https://codereview.chromium.org/1419253002 is landed to address this linker issue. Keep my fingers crossed. Original issue's description: > Revert of Add experiment on weak ping delay during call set up time (patchset #4 id:60001 of https://codereview.webrtc.org/1411883002/ ) > > Reason for revert: > This CL breaks Chromium, undefined reference link error to webrtc::field_trial::FindFullName. Adding the dependency system_wrappers to the rtc_p2p target is not enough to fix this. > > Looking at field_trial.h (in system_wrappers/interface/, not to be confused with the one in test/) the documentation says "WebRTC clients MUST provide an implementation of: ...FindFullName... Or link with a default one provided in: ...system_wrappers.gyp:field_trial_default). > > So maybe just depend on field_trial_default? Not sure what should be done in case there are implications to adding this dependency, I'm reverting this. Sorry :) > > Original issue's description: > > Add experiment on weak ping delay during call set up time > > > > BUG= > > R=pthatcher@webrtc.org > > > > Committed: https://crrev.com/3bf69b15f4c0c0ca4ab17c237084684a37bb8279 > > Cr-Commit-Position: refs/heads/master@{#10343} > > TBR=pthatcher@webrtc.org,juberti@webrtc.org,guoweis@webrtc.org > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= > TBR=pthatcher@webrtc.org,juberti@webrtc.org,hbos@webrtc.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= Review URL: https://codereview.webrtc.org/1413603005 Cr-Commit-Position: refs/heads/master@{#10418} --- webrtc/p2p/base/p2ptransportchannel.cc | 53 +++++++++++++++----------- webrtc/p2p/base/p2ptransportchannel.h | 3 ++ 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index 9d598f57f3..17d224f982 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -19,32 +19,13 @@ #include "webrtc/base/crc32.h" #include "webrtc/base/logging.h" #include "webrtc/base/stringencode.h" +#include "webrtc/system_wrappers/interface/field_trial.h" namespace { // messages for queuing up work for ourselves enum { MSG_SORT = 1, MSG_CHECK_AND_PING }; -// When the socket is unwritable, we will use 10 Kbps (ignoring IP+UDP headers) -// for pinging. When the socket is writable, we will use only 1 Kbps because -// 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; -// STRONG_PING_DELAY (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 -// writable or not receiving. -static const uint32_t WEAK_PING_DELAY = 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; - -static const int MIN_CHECK_RECEIVING_DELAY = 50; // ms - // The minimum improvement in RTT that justifies a switch. static const double kMinImprovement = 10; @@ -200,6 +181,27 @@ bool ShouldSwitch(cricket::Connection* a_conn, namespace cricket { +// When the socket is unwritable, we will use 10 Kbps (ignoring IP+UDP headers) +// for pinging. When the socket is writable, we will use only 1 Kbps because +// 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; +// STRONG_PING_DELAY (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 +// writable or not receiving. +const uint32_t WEAK_PING_DELAY = 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; + +static const int MIN_CHECK_RECEIVING_DELAY = 50; // ms + + P2PTransportChannel::P2PTransportChannel(const std::string& transport_name, int component, P2PTransport* transport, @@ -220,7 +222,14 @@ P2PTransportChannel::P2PTransportChannel(const std::string& transport_name, remote_candidate_generation_(0), gathering_state_(kIceGatheringNew), check_receiving_delay_(MIN_CHECK_RECEIVING_DELAY * 5), - receiving_timeout_(MIN_CHECK_RECEIVING_DELAY * 50) {} + receiving_timeout_(MIN_CHECK_RECEIVING_DELAY * 50) { + uint32_t weak_ping_delay = ::strtoul( + webrtc::field_trial::FindFullName("WebRTC-StunInterPacketDelay").c_str(), + nullptr, 10); + if (weak_ping_delay) { + weak_ping_delay_ = weak_ping_delay; + } +} P2PTransportChannel::~P2PTransportChannel() { ASSERT(worker_thread_ == rtc::Thread::Current()); @@ -1157,7 +1166,7 @@ void P2PTransportChannel::OnCheckAndPing() { 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; + int ping_delay = weak() ? weak_ping_delay_ : STRONG_PING_DELAY; if (rtc::Time() >= last_ping_sent_ms_ + ping_delay) { Connection* conn = FindNextPingableConnection(); if (conn) { diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 51979df8d8..e49fc27323 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -34,6 +34,8 @@ namespace cricket { +extern const uint32_t WEAK_PING_DELAY; + // Adds the port on which the candidate originated. class RemoteCandidate : public Candidate { public: @@ -252,6 +254,7 @@ class P2PTransportChannel : public TransportChannelImpl, int receiving_timeout_; uint32_t last_ping_sent_ms_ = 0; bool gather_continually_ = false; + int weak_ping_delay_ = WEAK_PING_DELAY; RTC_DISALLOW_COPY_AND_ASSIGN(P2PTransportChannel); };