Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(312)

Unified Diff: webrtc/p2p/base/port.cc

Issue 2722933002: Measure packet loss so we can use it to select ICE candidate pairs. (Closed)
Patch Set: better comments and test names Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: webrtc/p2p/base/port.cc
diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc
index 7410b20bf4c20bf53694175742c0c4dab4de96d2..fb417ee359bf4e8716927c6555ed405207a16e44 100644
--- a/webrtc/p2p/base/port.cc
+++ b/webrtc/p2p/base/port.cc
@@ -132,6 +132,57 @@ static std::string ComputeFoundation(const std::string& type,
return rtc::ToString<uint32_t>(rtc::ComputeCrc32(ost.str()));
}
+PacketLossEstimator::PacketLossEstimator(int64_t expire_after)
+ : expire_after_(expire_after) {}
+
+void PacketLossEstimator::ExpectResponse(std::string id, int64_t sent_time) {
+ outstanding_packets_.emplace_back(id, sent_time);
+}
+
+void PacketLossEstimator::ReceivedResponse(std::string id,
+ int64_t received_time) {
+ auto iter =
+ std::find_if(outstanding_packets_.begin(), outstanding_packets_.end(),
+ [id](const PacketInfo& packet) { return packet.id == id; });
+
+ // There are several reasons we might not find id:
+ // 1) We already expired this packet (and responses_expected_ was already
+ // incremented).
+ // 2) We already received (and counted) a response with this id.
+ // 3) we never expected to see this id (something else went wrong).
Taylor Brandstetter 2017/03/01 02:26:12 nit: Capitalization of "we"
Zach Stein 2017/03/02 00:02:23 Done.
+
+ if (iter != outstanding_packets_.end()) {
+ responses_expected_ += 1;
+ if (!IsExpired(*iter, received_time)) {
+ responses_received_ += 1;
+ }
+ outstanding_packets_.erase(iter);
+ }
+
+ RemoveExpiredPackets(received_time);
+}
+
+void PacketLossEstimator::RemoveExpiredPackets(int64_t now) {
+ auto iter = outstanding_packets_.begin();
+ while (iter != outstanding_packets_.end() && IsExpired(*iter, now)) {
+ responses_expected_ += 1;
+ ++iter;
+ }
+ outstanding_packets_.erase(outstanding_packets_.begin(), iter);
+
+ UpdateResponseRate();
+}
+
+void PacketLossEstimator::UpdateResponseRate() {
+ response_rate_ =
+ static_cast<double>(responses_received_) / responses_expected_;
Taylor Brandstetter 2017/03/01 02:26:12 Divide by zero?
Zach Stein 2017/03/02 00:02:23 Done.
+}
+
+bool PacketLossEstimator::IsExpired(const PacketInfo& packet,
+ int64_t received_time) const {
+ return packet.sent_time < received_time - expire_after_;
+}
+
Port::Port(rtc::Thread* thread,
const std::string& type,
rtc::PacketSocketFactory* factory,
@@ -885,6 +936,7 @@ Connection::Connection(Port* port,
last_ping_received_(0),
last_data_received_(0),
last_ping_response_received_(0),
+ packet_loss_estimator_(kPortTimeoutDelay),
Taylor Brandstetter 2017/03/01 02:26:12 This value recently became very large (~45 seconds
Zach Stein 2017/03/02 00:02:23 I like this idea. As is, we probably won't get a u
reported_(false),
state_(IceCandidatePairState::WAITING),
receiving_timeout_(WEAK_CONNECTION_RECEIVE_TIMEOUT),
@@ -1237,6 +1289,7 @@ void Connection::Ping(int64_t now) {
last_ping_sent_ = now;
ConnectionRequest *req = new ConnectionRequest(this);
pings_since_last_response_.push_back(SentPing(req->id(), now, nomination_));
+ packet_loss_estimator_.ExpectResponse(req->id(), now);
LOG_J(LS_VERBOSE, this) << "Sending STUN ping "
<< ", id=" << rtc::hex_encode(req->id())
<< ", nomination=" << nomination_;
@@ -1391,6 +1444,9 @@ void Connection::OnConnectionRequestResponse(ConnectionRequest* request,
}
ReceivedPingResponse(rtt, request->id());
+ int64_t time_received = rtc::TimeMillis();
+ packet_loss_estimator_.ReceivedResponse(request->id(), time_received);
+
stats_.recv_ping_responses++;
MaybeUpdateLocalCandidate(request, response);

Powered by Google App Engine
This is Rietveld 408576698