Fix issue with send-side bandwidth estimation over TURN TCP connections.
AsyncStunTCPSocket wasn't firing SignalSentPacket, which the bandwidth
estimator requires for every packet in order to look up send times when
feedback arrives. If the signal isn't fired, it always assumes feedback
is arriving extremely late, and decreases the bandwidth by a factor of
2 until it reaches the minimum of 10kbps.
BUG=webrtc:7717
TBR=pthatcher@webrtc.org
Review-Url: https://codereview.webrtc.org/2912523003
Cr-Commit-Position: refs/heads/master@{#18279}
diff --git a/webrtc/p2p/base/asyncstuntcpsocket.cc b/webrtc/p2p/base/asyncstuntcpsocket.cc
index 296a9d4..3369bc8 100644
--- a/webrtc/p2p/base/asyncstuntcpsocket.cc
+++ b/webrtc/p2p/base/asyncstuntcpsocket.cc
@@ -79,6 +79,9 @@
return res;
}
+ rtc::SentPacket sent_packet(options.packet_id, rtc::TimeMillis());
+ SignalSentPacket(this, sent_packet);
+
// We claim to have sent the whole thing, even if we only sent partial
return static_cast<int>(cb);
}
diff --git a/webrtc/p2p/base/asyncstuntcpsocket_unittest.cc b/webrtc/p2p/base/asyncstuntcpsocket_unittest.cc
index 6c026f6..61e1411 100644
--- a/webrtc/p2p/base/asyncstuntcpsocket_unittest.cc
+++ b/webrtc/p2p/base/asyncstuntcpsocket_unittest.cc
@@ -86,6 +86,8 @@
kClientAddr.family(), SOCK_STREAM);
send_socket_.reset(AsyncStunTCPSocket::Create(
client, kClientAddr, recv_socket_->GetLocalAddress()));
+ send_socket_->SignalSentPacket.connect(
+ this, &AsyncStunTCPSocketTest::OnSentPacket);
ASSERT_TRUE(send_socket_.get() != NULL);
vss_->ProcessMessagesUntilIdle();
}
@@ -96,6 +98,11 @@
recv_packets_.push_back(std::string(data, len));
}
+ void OnSentPacket(rtc::AsyncPacketSocket* socket,
+ const rtc::SentPacket& packet) {
+ ++sent_packets_;
+ }
+
void OnNewConnection(rtc::AsyncPacketSocket* server,
rtc::AsyncPacketSocket* new_socket) {
listen_socket_.reset(new_socket);
@@ -127,6 +134,7 @@
std::unique_ptr<AsyncStunTCPSocket> recv_socket_;
std::unique_ptr<rtc::AsyncPacketSocket> listen_socket_;
std::list<std::string> recv_packets_;
+ int sent_packets_ = 0;
};
// Testing a stun packet sent/recv properly.
@@ -259,4 +267,25 @@
sizeof(kTurnChannelDataMessageWithOddLength)));
}
+// Test that SignalSentPacket is fired when a packet is sent.
+TEST_F(AsyncStunTCPSocketTest, SignalSentPacketFiredWhenPacketSent) {
+ ASSERT_TRUE(
+ Send(kStunMessageWithZeroLength, sizeof(kStunMessageWithZeroLength)));
+ EXPECT_EQ(1, sent_packets_);
+ // Send another packet for good measure.
+ ASSERT_TRUE(
+ Send(kStunMessageWithZeroLength, sizeof(kStunMessageWithZeroLength)));
+ EXPECT_EQ(2, sent_packets_);
+}
+
+// Test that SignalSentPacket isn't fired when a packet isn't sent (for
+// example, because it's invalid).
+TEST_F(AsyncStunTCPSocketTest, SignalSentPacketNotFiredWhenPacketNotSent) {
+ // Attempt to send a packet that's too small; since it isn't sent,
+ // SignalSentPacket shouldn't fire.
+ char data[1];
+ ASSERT_FALSE(Send(data, sizeof(data)));
+ EXPECT_EQ(0, sent_packets_);
+}
+
} // namespace cricket
