Revert "Replaced the SignalSelectedCandidatePairChanged with a new signal."
This reverts commit 71677452f9cf210aa98162c6f4bd8d339e625337.
Reason for revert: Broke Chromium.
Original change's description:
> Replaced the SignalSelectedCandidatePairChanged with a new signal.
>
> |transport overhead| field is added to rtc::NetworkRoute structure.
>
> In PackTransportInternal:
> 1. network_route() is added which returns the current network route.
> 2. debug_name() is removed.
> 3. transport_name() is moved from DtlsTransportInternal and
> IceTransportInternal to PacketTransportInternal.
>
> When the selected candidate pair is changed, the P2PTransportChannel
> will fire the SignalNetworkRouteChanged instead of
> SignalSelectedCandidatePairChanged to upper layers.
>
> The Rtp/SrtpTransport takes the responsibility of calculating the
> transport overhead from the BaseChannel so that the BaseChannel
> doesn't need to depend on P2P layer transports.
>
> Bug: webrtc:7013
> Change-Id: I60d30d785666a50a95052d00bf08f829d8f57e9c
> Reviewed-on: https://webrtc-review.googlesource.com/13520
> Commit-Queue: Zhi Huang <zhihuang@webrtc.org>
> Reviewed-by: Peter Thatcher <pthatcher@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#20661}
TBR=steveanton@webrtc.org,zhihuang@webrtc.org,pthatcher@webrtc.org
Change-Id: Ie0c76786855b65bb8caba7065593c961e4bf9de7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:7013
Reviewed-on: https://webrtc-review.googlesource.com/22764
Reviewed-by: Zhi Huang <zhihuang@webrtc.org>
Commit-Queue: Zhi Huang <zhihuang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20662}
diff --git a/p2p/base/dtlstransport.cc b/p2p/base/dtlstransport.cc
index 0520a85..c1b65ee 100644
--- a/p2p/base/dtlstransport.cc
+++ b/p2p/base/dtlstransport.cc
@@ -124,7 +124,6 @@
ssl_role_(rtc::SSL_CLIENT),
ssl_max_version_(rtc::SSL_PROTOCOL_DTLS_12),
crypto_options_(crypto_options) {
- RTC_DCHECK(ice_transport_);
ice_transport_->SignalWritableState.connect(this,
&DtlsTransport::OnWritableState);
ice_transport_->SignalReadPacket.connect(this, &DtlsTransport::OnReadPacket);
@@ -133,8 +132,6 @@
&DtlsTransport::OnReadyToSend);
ice_transport_->SignalReceivingState.connect(
this, &DtlsTransport::OnReceivingState);
- ice_transport_->SignalNetworkRouteChanged.connect(
- this, &DtlsTransport::OnNetworkRouteChanged);
}
DtlsTransport::~DtlsTransport() = default;
@@ -431,10 +428,6 @@
return ice_transport_->GetError();
}
-rtc::Optional<rtc::NetworkRoute> DtlsTransport::network_route() const {
- return ice_transport_->network_route();
-}
-
bool DtlsTransport::GetOption(rtc::Socket::Option opt, int* value) {
return ice_transport_->GetOption(opt, value);
}
@@ -638,11 +631,6 @@
}
}
-void DtlsTransport::OnNetworkRouteChanged(
- rtc::Optional<rtc::NetworkRoute> network_route) {
- SignalNetworkRouteChanged(network_route);
-}
-
void DtlsTransport::MaybeStartDtls() {
if (dtls_ && ice_transport_->writable()) {
ConfigureHandshakeTimeout();
diff --git a/p2p/base/dtlstransport.h b/p2p/base/dtlstransport.h
index 4dd9bc2..eb86deb 100644
--- a/p2p/base/dtlstransport.h
+++ b/p2p/base/dtlstransport.h
@@ -156,8 +156,6 @@
int GetError() override;
- rtc::Optional<rtc::NetworkRoute> network_route() const override;
-
int SetOption(rtc::Socket::Option opt, int value) override;
std::string ToString() const {
@@ -181,7 +179,6 @@
void OnReadyToSend(rtc::PacketTransportInternal* transport);
void OnReceivingState(rtc::PacketTransportInternal* transport);
void OnDtlsEvent(rtc::StreamInterface* stream_, int sig, int err);
- void OnNetworkRouteChanged(rtc::Optional<rtc::NetworkRoute> network_route);
bool SetupDtls();
void MaybeStartDtls();
bool HandleDtlsPacket(const char* data, size_t size);
diff --git a/p2p/base/dtlstransport_unittest.cc b/p2p/base/dtlstransport_unittest.cc
index ed010e0..eaa0cec 100644
--- a/p2p/base/dtlstransport_unittest.cc
+++ b/p2p/base/dtlstransport_unittest.cc
@@ -101,11 +101,11 @@
(role == cricket::ICEROLE_CONTROLLING) ? 1 : 2);
dtls->SetSslMaxProtocolVersion(ssl_max_version_);
dtls->SignalWritableState.connect(
- this, &DtlsTestClient::OnTransportWritableState);
- dtls->SignalReadPacket.connect(this,
- &DtlsTestClient::OnTransportReadPacket);
- dtls->SignalSentPacket.connect(this,
- &DtlsTestClient::OnTransportSentPacket);
+ this, &DtlsTestClient::OnTransportChannelWritableState);
+ dtls->SignalReadPacket.connect(
+ this, &DtlsTestClient::OnTransportChannelReadPacket);
+ dtls->SignalSentPacket.connect(
+ this, &DtlsTestClient::OnTransportChannelSentPacket);
dtls_transports_.push_back(std::unique_ptr<cricket::DtlsTransport>(dtls));
fake_ice_transports_.push_back(
std::unique_ptr<cricket::FakeIceTransport>(fake_ice_channel));
@@ -356,17 +356,18 @@
return (num_matches < ((static_cast<int>(size) - 5) / 10));
}
- // Transport callbacks
- void OnTransportWritableState(rtc::PacketTransportInternal* transport) {
- RTC_LOG(LS_INFO) << name_ << ": Transport '" << transport->transport_name()
+ // Transport channel callbacks
+ void OnTransportChannelWritableState(
+ rtc::PacketTransportInternal* transport) {
+ RTC_LOG(LS_INFO) << name_ << ": Channel '" << transport->debug_name()
<< "' is writable";
}
- void OnTransportReadPacket(rtc::PacketTransportInternal* transport,
- const char* data,
- size_t size,
- const rtc::PacketTime& packet_time,
- int flags) {
+ void OnTransportChannelReadPacket(rtc::PacketTransportInternal* transport,
+ const char* data,
+ size_t size,
+ const rtc::PacketTime& packet_time,
+ int flags) {
uint32_t packet_num = 0;
ASSERT_TRUE(VerifyPacket(data, size, &packet_num));
received_.insert(packet_num);
@@ -376,8 +377,8 @@
ASSERT_EQ(expected_flags, flags);
}
- void OnTransportSentPacket(rtc::PacketTransportInternal* transport,
- const rtc::SentPacket& sent_packet) {
+ void OnTransportChannelSentPacket(rtc::PacketTransportInternal* transport,
+ const rtc::SentPacket& sent_packet) {
sent_packet_ = sent_packet;
}
diff --git a/p2p/base/dtlstransportinternal.cc b/p2p/base/dtlstransportinternal.cc
index a294ccb..5e28cb0 100644
--- a/p2p/base/dtlstransportinternal.cc
+++ b/p2p/base/dtlstransportinternal.cc
@@ -16,4 +16,8 @@
DtlsTransportInternal::~DtlsTransportInternal() = default;
+std::string DtlsTransportInternal::debug_name() const {
+ return transport_name() + " " + rtc::ToString(component());
+}
+
} // namespace cricket
diff --git a/p2p/base/dtlstransportinternal.h b/p2p/base/dtlstransportinternal.h
index b08868f..be3db6b 100644
--- a/p2p/base/dtlstransportinternal.h
+++ b/p2p/base/dtlstransportinternal.h
@@ -43,6 +43,8 @@
virtual DtlsTransportState dtls_state() const = 0;
+ virtual const std::string& transport_name() const = 0;
+
virtual int component() const = 0;
virtual bool IsDtlsActive() const = 0;
@@ -91,6 +93,9 @@
// Emitted whenever the Dtls handshake failed on some transport channel.
sigslot::signal1<rtc::SSLHandshakeError> SignalDtlsHandshakeError;
+ // Debugging description of this transport.
+ std::string debug_name() const override;
+
protected:
DtlsTransportInternal();
diff --git a/p2p/base/fakedtlstransport.h b/p2p/base/fakedtlstransport.h
index 508158f..01a4ed8 100644
--- a/p2p/base/fakedtlstransport.h
+++ b/p2p/base/fakedtlstransport.h
@@ -31,11 +31,8 @@
transport_name_(ice_transport->transport_name()),
component_(ice_transport->component()),
dtls_fingerprint_("", nullptr, 0) {
- RTC_DCHECK(ice_transport_);
ice_transport_->SignalReadPacket.connect(
this, &FakeDtlsTransport::OnIceTransportReadPacket);
- ice_transport_->SignalNetworkRouteChanged.connect(
- this, &FakeDtlsTransport::OnNetworkRouteChanged);
}
// If this constructor is called, a new fake ICE transport will be created,
@@ -48,8 +45,6 @@
ice_transport_ = owned_ice_transport_.get();
ice_transport_->SignalReadPacket.connect(
this, &FakeDtlsTransport::OnIceTransportReadPacket);
- ice_transport_->SignalNetworkRouteChanged.connect(
- this, &FakeDtlsTransport::OnNetworkRouteChanged);
}
~FakeDtlsTransport() override {
@@ -206,10 +201,6 @@
}
int GetError() override { return ice_transport_->GetError(); }
- rtc::Optional<rtc::NetworkRoute> network_route() const override {
- return ice_transport_->network_route();
- }
-
private:
void OnIceTransportReadPacket(PacketTransportInternal* ice_,
const char* data,
@@ -238,10 +229,6 @@
SignalWritableState(this);
}
- void OnNetworkRouteChanged(rtc::Optional<rtc::NetworkRoute> network_route) {
- SignalNetworkRouteChanged(network_route);
- }
-
FakeIceTransport* ice_transport_;
std::unique_ptr<FakeIceTransport> owned_ice_transport_;
std::string transport_name_;
diff --git a/p2p/base/fakeicetransport.h b/p2p/base/fakeicetransport.h
index cc6cec9..7b6f78a 100644
--- a/p2p/base/fakeicetransport.h
+++ b/p2p/base/fakeicetransport.h
@@ -190,7 +190,6 @@
SignalSentPacket(this, sent_packet);
return static_cast<int>(len);
}
-
int SetOption(rtc::Socket::Option opt, int value) override {
socket_options_[opt] = value;
return true;
@@ -204,16 +203,8 @@
return false;
}
}
-
int GetError() override { return 0; }
- rtc::Optional<rtc::NetworkRoute> network_route() const override {
- return network_route_;
- }
- void SetNetworkRoute(rtc::Optional<rtc::NetworkRoute> network_route) {
- network_route_ = network_route;
- }
-
private:
void set_writable(bool writable) {
if (writable_ == writable) {
@@ -264,7 +255,6 @@
bool receiving_ = false;
bool combine_outgoing_packets_ = false;
rtc::CopyOnWriteBuffer send_packet_;
- rtc::Optional<rtc::NetworkRoute> network_route_;
std::map<rtc::Socket::Option, int> socket_options_;
};
diff --git a/p2p/base/fakepackettransport.h b/p2p/base/fakepackettransport.h
index cbff511..2af14c1 100644
--- a/p2p/base/fakepackettransport.h
+++ b/p2p/base/fakepackettransport.h
@@ -23,8 +23,8 @@
// Used to simulate a packet-based transport.
class FakePacketTransport : public PacketTransportInternal {
public:
- explicit FakePacketTransport(const std::string& transport_name)
- : transport_name_(transport_name) {}
+ explicit FakePacketTransport(const std::string& debug_name)
+ : debug_name_(debug_name) {}
~FakePacketTransport() override {
if (dest_ && dest_->dest_ == this) {
dest_->dest_ = nullptr;
@@ -59,7 +59,7 @@
}
// Fake PacketTransportInternal implementation.
- const std::string& transport_name() const override { return transport_name_; }
+ std::string debug_name() const override { return debug_name_; }
bool writable() const override { return writable_; }
bool receiving() const override { return receiving_; }
int SendPacket(const char* data,
@@ -88,13 +88,6 @@
const CopyOnWriteBuffer* last_sent_packet() { return &last_sent_packet_; }
- Optional<NetworkRoute> network_route() const override {
- return network_route_;
- }
- void SetNetworkRoute(Optional<NetworkRoute> network_route) {
- network_route_ = network_route;
- }
-
private:
void set_writable(bool writable) {
if (writable_ == writable) {
@@ -125,14 +118,12 @@
CopyOnWriteBuffer last_sent_packet_;
AsyncInvoker invoker_;
- std::string transport_name_;
+ std::string debug_name_;
FakePacketTransport* dest_ = nullptr;
bool async_ = false;
int async_delay_ms_ = 0;
bool writable_ = false;
bool receiving_ = false;
-
- Optional<NetworkRoute> network_route_;
};
} // namespace rtc
diff --git a/p2p/base/icetransportinternal.cc b/p2p/base/icetransportinternal.cc
index d5fa989..a741205 100644
--- a/p2p/base/icetransportinternal.cc
+++ b/p2p/base/icetransportinternal.cc
@@ -26,4 +26,8 @@
SetRemoteIceParameters(IceParameters(ice_ufrag, ice_pwd, false));
}
+std::string IceTransportInternal::debug_name() const {
+ return transport_name() + " " + rtc::ToString(component());
+}
+
} // namespace cricket
diff --git a/p2p/base/icetransportinternal.h b/p2p/base/icetransportinternal.h
index 0a71162..b7dfc42 100644
--- a/p2p/base/icetransportinternal.h
+++ b/p2p/base/icetransportinternal.h
@@ -52,6 +52,8 @@
virtual IceTransportState GetState() const = 0;
+ virtual const std::string& transport_name() const = 0;
+
virtual int component() const = 0;
virtual IceRole GetIceRole() const = 0;
@@ -109,14 +111,21 @@
sigslot::signal2<IceTransportInternal*, const Candidates&>
SignalCandidatesRemoved;
- // Deprecated by PacketTransportInternal::SignalNetworkRouteChanged.
+ // Deprecated by SignalSelectedCandidatePairChanged
// This signal occurs when there is a change in the way that packets are
// being routed, i.e. to a different remote location. The candidate
// indicates where and how we are currently sending media.
- // TODO(zhihuang): Update the Chrome remoting to use the new
- // SignalNetworkRouteChanged.
sigslot::signal2<IceTransportInternal*, const Candidate&> SignalRouteChange;
+ // Signalled when the current selected candidate pair has changed.
+ // The first parameter is the transport that signals the event.
+ // The second parameter is the new selected candidate pair. The third
+ // parameter is the last packet id sent on the previous candidate pair.
+ // The fourth parameter is a boolean which is true if the Transport
+ // is ready to send with this candidate pair.
+ sigslot::signal4<IceTransportInternal*, CandidatePairInterface*, int, bool>
+ SignalSelectedCandidatePairChanged;
+
// Invoked when there is conflict in the ICE role between local and remote
// agents.
sigslot::signal1<IceTransportInternal*> SignalRoleConflict;
@@ -126,6 +135,9 @@
// Invoked when the transport is being destroyed.
sigslot::signal1<IceTransportInternal*> SignalDestroyed;
+
+ // Debugging description of this transport.
+ std::string debug_name() const override;
};
} // namespace cricket
diff --git a/p2p/base/jseptransport_unittest.cc b/p2p/base/jseptransport_unittest.cc
index 8d09e0f..644bcd9 100644
--- a/p2p/base/jseptransport_unittest.cc
+++ b/p2p/base/jseptransport_unittest.cc
@@ -17,6 +17,7 @@
#include "rtc_base/network.h"
using cricket::JsepTransport;
+using cricket::TransportChannel;
using cricket::FakeDtlsTransport;
using cricket::FakeIceTransport;
using cricket::IceRole;
diff --git a/p2p/base/p2ptransportchannel.cc b/p2p/base/p2ptransportchannel.cc
index 19abf38..71af75c 100644
--- a/p2p/base/p2ptransportchannel.cc
+++ b/p2p/base/p2ptransportchannel.cc
@@ -23,7 +23,6 @@
#include "rtc_base/checks.h"
#include "rtc_base/crc32.h"
#include "rtc_base/logging.h"
-#include "rtc_base/nethelper.h"
#include "rtc_base/stringencode.h"
#include "rtc_base/timeutils.h"
#include "system_wrappers/include/field_trial.h"
@@ -1083,10 +1082,6 @@
return true;
}
-rtc::Optional<rtc::NetworkRoute> P2PTransportChannel::network_route() const {
- return network_route_;
-}
-
rtc::DiffServCodePoint P2PTransportChannel::DefaultDscpValue() const {
OptionMap::const_iterator it = options_.find(rtc::Socket::OPT_DSCP);
if (it == options_.end()) {
@@ -1472,7 +1467,6 @@
// destroyed, so don't use it.
Connection* old_selected_connection = selected_connection_;
selected_connection_ = conn;
- network_route_.reset();
if (selected_connection_) {
++nomination_;
if (old_selected_connection) {
@@ -1491,23 +1485,12 @@
PresumedWritable(selected_connection_)) {
SignalReadyToSend(this);
}
-
- network_route_.emplace(rtc::NetworkRoute());
- network_route_->connected = ReadyToSend(selected_connection_);
- network_route_->local_network_id =
- selected_connection_->local_candidate().network_id();
- network_route_->remote_network_id =
- selected_connection_->remote_candidate().network_id();
- network_route_->last_sent_packet_id = last_sent_packet_id_;
- network_route_->packet_overhead =
- GetIpOverhead(
- selected_connection_->local_candidate().address().family()) +
- GetProtocolOverhead(selected_connection_->local_candidate().protocol());
} else {
LOG_J(LS_INFO, this) << "No selected connection";
}
-
- SignalNetworkRouteChanged(network_route_);
+ SignalSelectedCandidatePairChanged(this, selected_connection_,
+ last_sent_packet_id_,
+ ReadyToSend(selected_connection_));
}
// Warning: UpdateState should eventually be called whenever a connection
diff --git a/p2p/base/p2ptransportchannel.h b/p2p/base/p2ptransportchannel.h
index 188f60c..f9e9a53 100644
--- a/p2p/base/p2ptransportchannel.h
+++ b/p2p/base/p2ptransportchannel.h
@@ -127,8 +127,6 @@
int receiving_timeout() const { return config_.receiving_timeout; }
int check_receiving_interval() const { return check_receiving_interval_; }
- rtc::Optional<rtc::NetworkRoute> network_route() const override;
-
// Helper method used only in unittest.
rtc::DiffServCodePoint DefaultDscpValue() const;
@@ -402,8 +400,6 @@
webrtc::MetricsObserverInterface* metrics_observer_ = nullptr;
- rtc::Optional<rtc::NetworkRoute> network_route_;
-
RTC_DISALLOW_COPY_AND_ASSIGN(P2PTransportChannel);
};
diff --git a/p2p/base/p2ptransportchannel_unittest.cc b/p2p/base/p2ptransportchannel_unittest.cc
index 37f6807..326cafa 100644
--- a/p2p/base/p2ptransportchannel_unittest.cc
+++ b/p2p/base/p2ptransportchannel_unittest.cc
@@ -379,8 +379,8 @@
this, &P2PTransportChannelTestBase::OnReadPacket);
channel->SignalRoleConflict.connect(
this, &P2PTransportChannelTestBase::OnRoleConflict);
- channel->SignalNetworkRouteChanged.connect(
- this, &P2PTransportChannelTestBase::OnNetworkRouteChanged);
+ channel->SignalSelectedCandidatePairChanged.connect(
+ this, &P2PTransportChannelTestBase::OnSelectedCandidatePairChanged);
channel->SetIceParameters(local_ice);
if (remote_ice_parameter_source_ == FROM_SETICEPARAMETERS) {
channel->SetRemoteIceParameters(remote_ice);
@@ -697,12 +697,14 @@
new CandidatesData(ch, c));
}
}
-
- void OnNetworkRouteChanged(rtc::Optional<rtc::NetworkRoute> network_route) {
- // If the |network_route| is unset, don't count. This is used in the case
- // when the network on remote side is down, the signal will be fired with an
- // unset network route and it shouldn't trigger a connection switch.
- if (network_route) {
+ void OnSelectedCandidatePairChanged(
+ IceTransportInternal* transport_channel,
+ CandidatePairInterface* selected_candidate_pair,
+ int last_sent_packet_id,
+ bool ready_to_send) {
+ // Do not count if it switches to nullptr. This may happen if all
+ // connections timed out.
+ if (selected_candidate_pair != nullptr) {
++selected_candidate_pair_switches_;
}
}
@@ -3053,8 +3055,8 @@
ch->SetIceRole(ICEROLE_CONTROLLING);
ch->SetIceParameters(kIceParams[0]);
ch->SetRemoteIceParameters(kIceParams[1]);
- ch->SignalNetworkRouteChanged.connect(
- this, &P2PTransportChannelPingTest::OnNetworkRouteChanged);
+ ch->SignalSelectedCandidatePairChanged.connect(
+ this, &P2PTransportChannelPingTest::OnSelectedCandidatePairChanged);
ch->SignalReadyToSend.connect(this,
&P2PTransportChannelPingTest::OnReadyToSend);
ch->SignalStateChanged.connect(
@@ -3140,11 +3142,13 @@
conn->SignalNominated(conn);
}
- void OnNetworkRouteChanged(rtc::Optional<rtc::NetworkRoute> network_route) {
- last_network_route_ = network_route;
- if (last_network_route_) {
- last_sent_packet_id_ = last_network_route_->last_sent_packet_id;
- }
+ void OnSelectedCandidatePairChanged(
+ IceTransportInternal* transport_channel,
+ CandidatePairInterface* selected_candidate_pair,
+ int last_sent_packet_id,
+ bool ready_to_send) {
+ last_selected_candidate_pair_ = selected_candidate_pair;
+ last_sent_packet_id_ = last_sent_packet_id;
++selected_candidate_pair_switches_;
}
@@ -3178,6 +3182,9 @@
channel_state_ = channel->GetState();
}
+ CandidatePairInterface* last_selected_candidate_pair() {
+ return last_selected_candidate_pair_;
+ }
int last_sent_packet_id() { return last_sent_packet_id_; }
bool channel_ready_to_send() { return channel_ready_to_send_; }
void reset_channel_ready_to_send() { channel_ready_to_send_ = false; }
@@ -3188,26 +3195,14 @@
return switches;
}
- // Return true if the |pair| matches the last network route.
- bool CandidatePairMatchesNetworkRoute(CandidatePairInterface* pair) {
- if (!pair) {
- return !last_network_route_.has_value();
- } else {
- return pair->local_candidate().network_id() ==
- last_network_route_->local_network_id &&
- pair->remote_candidate().network_id() ==
- last_network_route_->remote_network_id;
- }
- }
-
private:
std::unique_ptr<rtc::VirtualSocketServer> vss_;
rtc::AutoSocketServerThread thread_;
+ CandidatePairInterface* last_selected_candidate_pair_ = nullptr;
int selected_candidate_pair_switches_ = 0;
int last_sent_packet_id_ = -1;
bool channel_ready_to_send_ = false;
IceTransportState channel_state_ = IceTransportState::STATE_INIT;
- rtc::Optional<rtc::NetworkRoute> last_network_route_;
};
TEST_F(P2PTransportChannelPingTest, TestTriggeredChecks) {
@@ -3585,7 +3580,7 @@
// The controlled side will select a connection as the "selected connection"
// based on priority until the controlling side nominates a connection, at which
// point the controlled side will select that connection as the
-// "selected connection". Plus, SignalNetworkRouteChanged will be fired if the
+// "selected connection". Plus, SignalSelectedCandidatePair will be fired if the
// selected connection changes and SignalReadyToSend will be fired if the new
// selected connection is writable.
TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
@@ -3608,7 +3603,7 @@
// A connection needs to be writable before it is selected for transmission.
conn1->ReceivedPingResponse(LOW_RTT, "id");
EXPECT_EQ_WAIT(conn1, ch.selected_connection(), kDefaultTimeout);
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1));
+ EXPECT_EQ(conn1, last_selected_candidate_pair());
EXPECT_EQ(len, SendData(ch, data, len, ++last_packet_id));
// When a higher priority candidate comes in, the new connection is chosen
@@ -3618,7 +3613,7 @@
ASSERT_TRUE(conn2 != nullptr);
conn2->ReceivedPingResponse(LOW_RTT, "id");
EXPECT_EQ_WAIT(conn2, ch.selected_connection(), kDefaultTimeout);
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2));
+ EXPECT_EQ(conn2, last_selected_candidate_pair());
EXPECT_TRUE(channel_ready_to_send());
EXPECT_EQ(last_packet_id, last_sent_packet_id());
@@ -3636,7 +3631,7 @@
// connection.
NominateConnection(conn3);
EXPECT_EQ(conn3, ch.selected_connection());
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn3));
+ EXPECT_EQ(conn3, last_selected_candidate_pair());
EXPECT_EQ(last_packet_id, last_sent_packet_id());
EXPECT_TRUE(channel_ready_to_send());
@@ -3657,7 +3652,7 @@
// The selected connection switches after conn4 becomes writable.
conn4->ReceivedPingResponse(LOW_RTT, "id");
EXPECT_EQ_WAIT(conn4, ch.selected_connection(), kDefaultTimeout);
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn4));
+ EXPECT_EQ(conn4, last_selected_candidate_pair());
EXPECT_EQ(last_packet_id, last_sent_packet_id());
// SignalReadyToSend is fired again because conn4 is writable.
EXPECT_TRUE(channel_ready_to_send());
@@ -3819,7 +3814,7 @@
// Initially, connections are selected based on priority.
EXPECT_EQ(1, reset_selected_candidate_pair_switches());
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1));
+ EXPECT_EQ(conn1, last_selected_candidate_pair());
// conn2 receives data; it becomes selected.
// Advance the clock by 1ms so that the last data receiving timestamp of
@@ -3827,12 +3822,12 @@
SIMULATED_WAIT(false, 1, clock);
conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
EXPECT_EQ(1, reset_selected_candidate_pair_switches());
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2));
+ EXPECT_EQ(conn2, last_selected_candidate_pair());
// conn1 also receives data; it becomes selected due to priority again.
conn1->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
EXPECT_EQ(1, reset_selected_candidate_pair_switches());
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2));
+ EXPECT_EQ(conn1, last_selected_candidate_pair());
// conn2 received data more recently; it is selected now because it
// received data more recently.
@@ -3841,7 +3836,7 @@
conn2->ReceivedPingResponse(LOW_RTT, "id");
conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
EXPECT_EQ(1, reset_selected_candidate_pair_switches());
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2));
+ EXPECT_EQ(conn2, last_selected_candidate_pair());
// Make sure sorting won't reselect candidate pair.
SIMULATED_WAIT(false, 10, clock);
@@ -3871,17 +3866,17 @@
SIMULATED_WAIT(false, 1, clock);
conn1->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
EXPECT_EQ(1, reset_selected_candidate_pair_switches());
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1));
+ EXPECT_EQ(conn1, last_selected_candidate_pair());
// conn2 is nominated; it becomes the selected connection.
NominateConnection(conn2);
EXPECT_EQ(1, reset_selected_candidate_pair_switches());
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2));
+ EXPECT_EQ(conn2, last_selected_candidate_pair());
// conn1 is selected because it has higher priority and also nominated.
NominateConnection(conn1);
EXPECT_EQ(1, reset_selected_candidate_pair_switches());
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2));
+ EXPECT_EQ(conn1, last_selected_candidate_pair());
// Make sure sorting won't reselect candidate pair.
SIMULATED_WAIT(false, 10, clock);
@@ -3907,28 +3902,24 @@
ASSERT_TRUE(conn2 != nullptr);
// conn1 is the selected connection because it has a higher priority,
- EXPECT_EQ_SIMULATED_WAIT(conn1, ch.selected_connection(), kDefaultTimeout,
- clock);
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1));
+ EXPECT_EQ_SIMULATED_WAIT(conn1, last_selected_candidate_pair(),
+ kDefaultTimeout, clock);
reset_selected_candidate_pair_switches();
// conn2 is nominated; it becomes selected.
NominateConnection(conn2);
EXPECT_EQ(1, reset_selected_candidate_pair_switches());
- EXPECT_EQ(conn2, ch.selected_connection());
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2));
+ EXPECT_EQ(conn2, last_selected_candidate_pair());
// conn1 is selected because of its priority.
NominateConnection(conn1);
EXPECT_EQ(1, reset_selected_candidate_pair_switches());
- EXPECT_EQ(conn1, ch.selected_connection());
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1));
+ EXPECT_EQ(conn1, last_selected_candidate_pair());
// conn2 gets higher remote nomination; it is selected again.
NominateConnection(conn2, 2U);
EXPECT_EQ(1, reset_selected_candidate_pair_switches());
- EXPECT_EQ(conn2, ch.selected_connection());
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2));
+ EXPECT_EQ(conn2, last_selected_candidate_pair());
// Make sure sorting won't reselect candidate pair.
SIMULATED_WAIT(false, 100, clock);
@@ -3979,17 +3970,15 @@
conn2->ReceivedPingResponse(LOW_RTT, "id");
EXPECT_EQ_SIMULATED_WAIT(1, reset_selected_candidate_pair_switches(),
kDefaultTimeout, clock);
- EXPECT_EQ_SIMULATED_WAIT(conn2, ch.selected_connection(), kDefaultTimeout,
- clock);
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2));
+ EXPECT_EQ_SIMULATED_WAIT(conn2, last_selected_candidate_pair(),
+ kDefaultTimeout, clock);
// If conn1 is also writable, it will become selected.
conn1->ReceivedPingResponse(LOW_RTT, "id");
EXPECT_EQ_SIMULATED_WAIT(1, reset_selected_candidate_pair_switches(),
kDefaultTimeout, clock);
- EXPECT_EQ_SIMULATED_WAIT(conn1, ch.selected_connection(), kDefaultTimeout,
- clock);
- EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1));
+ EXPECT_EQ_SIMULATED_WAIT(conn1, last_selected_candidate_pair(),
+ kDefaultTimeout, clock);
// Make sure sorting won't reselect candidate pair.
SIMULATED_WAIT(false, 10, clock);
diff --git a/p2p/base/packettransportinternal.h b/p2p/base/packettransportinternal.h
index fa9a22c..41e9fbc 100644
--- a/p2p/base/packettransportinternal.h
+++ b/p2p/base/packettransportinternal.h
@@ -14,15 +14,16 @@
#include <string>
#include <vector>
-#include "api/optional.h"
// This is included for PacketOptions.
#include "api/ortc/packettransportinterface.h"
-#include "p2p/base/port.h"
#include "rtc_base/asyncpacketsocket.h"
-#include "rtc_base/networkroute.h"
#include "rtc_base/sigslot.h"
#include "rtc_base/socket.h"
+namespace cricket {
+class TransportChannel;
+}
+
namespace rtc {
struct PacketOptions;
struct PacketTime;
@@ -31,7 +32,8 @@
class PacketTransportInternal : public virtual webrtc::PacketTransportInterface,
public sigslot::has_slots<> {
public:
- virtual const std::string& transport_name() const = 0;
+ // Identify the object for logging and debug purpose.
+ virtual std::string debug_name() const = 0;
// The transport has been established.
virtual bool writable() const = 0;
@@ -64,9 +66,6 @@
// Returns the most recent error that occurred on this channel.
virtual int GetError() = 0;
- // Returns the current network route with transport overhead.
- virtual rtc::Optional<NetworkRoute> network_route() const = 0;
-
// Emitted when the writable state, represented by |writable()|, changes.
sigslot::signal1<PacketTransportInternal*> SignalWritableState;
@@ -92,9 +91,6 @@
sigslot::signal2<PacketTransportInternal*, const rtc::SentPacket&>
SignalSentPacket;
- // Signalled when the current network route has changed.
- sigslot::signal1<rtc::Optional<rtc::NetworkRoute>> SignalNetworkRouteChanged;
-
protected:
PacketTransportInternal();
~PacketTransportInternal() override;
diff --git a/p2p/base/port.cc b/p2p/base/port.cc
index 1c316d2..8ffc688 100644
--- a/p2p/base/port.cc
+++ b/p2p/base/port.cc
@@ -99,6 +99,11 @@
const char PRFLX_PORT_TYPE[] = "prflx";
const char RELAY_PORT_TYPE[] = "relay";
+const char UDP_PROTOCOL_NAME[] = "udp";
+const char TCP_PROTOCOL_NAME[] = "tcp";
+const char SSLTCP_PROTOCOL_NAME[] = "ssltcp";
+const char TLS_PROTOCOL_NAME[] = "tls";
+
static const char* const PROTO_NAMES[] = {UDP_PROTOCOL_NAME, TCP_PROTOCOL_NAME,
SSLTCP_PROTOCOL_NAME,
TLS_PROTOCOL_NAME};
diff --git a/p2p/base/port.h b/p2p/base/port.h
index 2957fd8..3bcfae2 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -28,7 +28,6 @@
#include "p2p/base/stunrequest.h"
#include "rtc_base/asyncpacketsocket.h"
#include "rtc_base/checks.h"
-#include "rtc_base/nethelper.h"
#include "rtc_base/network.h"
#include "rtc_base/proxyinfo.h"
#include "rtc_base/ratetracker.h"
@@ -46,6 +45,11 @@
extern const char PRFLX_PORT_TYPE[];
extern const char RELAY_PORT_TYPE[];
+extern const char UDP_PROTOCOL_NAME[];
+extern const char TCP_PROTOCOL_NAME[];
+extern const char SSLTCP_PROTOCOL_NAME[];
+extern const char TLS_PROTOCOL_NAME[];
+
// RFC 6544, TCP candidate encoding rules.
extern const int DISCARD_PORT;
extern const char TCPTYPE_ACTIVE_STR[];
diff --git a/p2p/base/udptransport.cc b/p2p/base/udptransport.cc
index 8b4ce00..9f3d7d0 100644
--- a/p2p/base/udptransport.cc
+++ b/p2p/base/udptransport.cc
@@ -16,7 +16,6 @@
#include "rtc_base/asyncpacketsocket.h"
#include "rtc_base/asyncudpsocket.h"
#include "rtc_base/logging.h"
-#include "rtc_base/nethelper.h"
#include "rtc_base/socketaddress.h"
#include "rtc_base/thread.h"
#include "rtc_base/thread_checker.h"
@@ -64,7 +63,7 @@
return remote_address_;
}
-const std::string& UdpTransport::transport_name() const {
+std::string UdpTransport::debug_name() const {
return transport_name_;
}
@@ -96,13 +95,6 @@
return result;
}
-rtc::Optional<rtc::NetworkRoute> UdpTransport::network_route() const {
- rtc::NetworkRoute network_route;
- network_route.packet_overhead =
- /*kUdpOverhead=*/8 + GetIpOverhead(GetLocalAddress().family());
- return rtc::Optional<rtc::NetworkRoute>(network_route);
-}
-
int UdpTransport::SetOption(rtc::Socket::Option opt, int value) {
return 0;
}
diff --git a/p2p/base/udptransport.h b/p2p/base/udptransport.h
index 118b596..6a9d211 100644
--- a/p2p/base/udptransport.h
+++ b/p2p/base/udptransport.h
@@ -46,7 +46,7 @@
rtc::SocketAddress GetRemoteAddress() const override;
// Overrides of PacketTransportInternal, used by webrtc internally.
- const std::string& transport_name() const override;
+ std::string debug_name() const override;
bool receiving() const override;
@@ -61,8 +61,6 @@
int GetError() override;
- rtc::Optional<rtc::NetworkRoute> network_route() const override;
-
protected:
PacketTransportInternal* GetInternal() override;
@@ -75,7 +73,6 @@
void OnSocketSentPacket(rtc::AsyncPacketSocket* socket,
const rtc::SentPacket& packet);
bool IsLocalConsistent();
-
std::string transport_name_;
int send_error_ = 0;
std::unique_ptr<rtc::AsyncPacketSocket> socket_;
diff --git a/pc/channel.cc b/pc/channel.cc
index 4318aae..3950c03 100644
--- a/pc/channel.cc
+++ b/pc/channel.cc
@@ -174,8 +174,6 @@
// channel to signal.
rtp_transport_->SignalPacketReceived.connect(this,
&BaseChannel::OnPacketReceived);
- rtp_transport_->SignalNetworkRouteChanged.connect(
- this, &BaseChannel::OnNetworkRouteChanged);
RTC_LOG(LS_INFO) << "Created channel for " << content_name;
}
@@ -303,7 +301,7 @@
transport_name_ = rtp_dtls_transport->transport_name();
debug_name = transport_name_;
} else {
- debug_name = rtp_packet_transport->transport_name();
+ debug_name = rtp_packet_transport->debug_name();
}
if (rtp_packet_transport == rtp_transport_->rtp_packet_transport()) {
// Nothing to do if transport isn't changing.
@@ -346,9 +344,6 @@
DtlsTransportInternal* new_dtls_transport,
rtc::PacketTransportInternal* new_packet_transport) {
RTC_DCHECK(network_thread_->IsCurrent());
- if (new_dtls_transport) {
- RTC_DCHECK(new_dtls_transport == new_packet_transport);
- }
DtlsTransportInternal*& old_dtls_transport =
rtcp ? rtcp_dtls_transport_ : rtp_dtls_transport_;
rtc::PacketTransportInternal* old_packet_transport =
@@ -403,14 +398,21 @@
transport->SignalWritableState.connect(this, &BaseChannel::OnWritableState);
transport->SignalDtlsState.connect(this, &BaseChannel::OnDtlsState);
transport->SignalSentPacket.connect(this, &BaseChannel::SignalSentPacket_n);
+ transport->ice_transport()->SignalSelectedCandidatePairChanged.connect(
+ this, &BaseChannel::OnSelectedCandidatePairChanged);
}
void BaseChannel::DisconnectFromDtlsTransport(
DtlsTransportInternal* transport) {
RTC_DCHECK(network_thread_->IsCurrent());
+ OnSelectedCandidatePairChanged(transport->ice_transport(), nullptr, -1,
+ false);
+
transport->SignalWritableState.disconnect(this);
transport->SignalDtlsState.disconnect(this);
transport->SignalSentPacket.disconnect(this);
+ transport->ice_transport()->SignalSelectedCandidatePairChanged.disconnect(
+ this);
}
void BaseChannel::ConnectToPacketTransport(
@@ -590,24 +592,29 @@
}
}
-void BaseChannel::OnNetworkRouteChanged(
- rtc::Optional<rtc::NetworkRoute> network_route) {
+void BaseChannel::OnSelectedCandidatePairChanged(
+ IceTransportInternal* ice_transport,
+ CandidatePairInterface* selected_candidate_pair,
+ int last_sent_packet_id,
+ bool ready_to_send) {
+ RTC_DCHECK((rtp_dtls_transport_ &&
+ ice_transport == rtp_dtls_transport_->ice_transport()) ||
+ (rtcp_dtls_transport_ &&
+ ice_transport == rtcp_dtls_transport_->ice_transport()));
RTC_DCHECK(network_thread_->IsCurrent());
- rtc::NetworkRoute new_route;
- if (network_route) {
- invoker_.AsyncInvoke<void>(RTC_FROM_HERE, worker_thread_, [=] {
- media_channel_->OnTransportOverheadChanged(
- network_route->packet_overhead);
- });
- new_route = *(network_route);
- }
+ selected_candidate_pair_ = selected_candidate_pair;
+ std::string transport_name = ice_transport->transport_name();
+ rtc::NetworkRoute network_route;
+ if (selected_candidate_pair) {
+ network_route = rtc::NetworkRoute(
+ ready_to_send, selected_candidate_pair->local_candidate().network_id(),
+ selected_candidate_pair->remote_candidate().network_id(),
+ last_sent_packet_id);
- // Note: When the RTCP-muxing is not enabled, RTCP transport and RTP transport
- // use the same transport name and MediaChannel::OnNetworkRouteChanged cannot
- // work correctly. Intentionally leave it broken to simplify the code and
- // encourage the users to stop using non-muxing RTCP.
+ UpdateTransportOverhead();
+ }
invoker_.AsyncInvoke<void>(RTC_FROM_HERE, worker_thread_, [=] {
- media_channel_->OnNetworkRouteChanged(transport_name_, new_route);
+ media_channel_->OnNetworkRouteChanged(transport_name, network_route);
});
}
@@ -908,8 +915,9 @@
if (!ret) {
RTC_LOG(LS_WARNING) << "DTLS-SRTP key installation failed";
+ } else {
+ UpdateTransportOverhead();
}
-
return ret;
}
@@ -1013,7 +1021,6 @@
if (srtp_transport_ == nullptr) {
rtp_transport_->SignalReadyToSend.disconnect(this);
rtp_transport_->SignalPacketReceived.disconnect(this);
- rtp_transport_->SignalNetworkRouteChanged.disconnect(this);
auto transport = rtc::MakeUnique<webrtc::SrtpTransport>(
std::move(rtp_transport_), content_name_);
@@ -1024,8 +1031,6 @@
this, &BaseChannel::OnTransportReadyToSend);
rtp_transport_->SignalPacketReceived.connect(
this, &BaseChannel::OnPacketReceived);
- rtp_transport_->SignalNetworkRouteChanged.connect(
- this, &BaseChannel::OnNetworkRouteChanged);
RTC_LOG(LS_INFO) << "Wrapping RtpTransport in SrtpTransport.";
}
}
@@ -1155,7 +1160,7 @@
// the RTCP transport.
std::string debug_name =
transport_name_.empty()
- ? rtp_transport_->rtp_packet_transport()->transport_name()
+ ? rtp_transport_->rtp_packet_transport()->debug_name()
: transport_name_;
RTC_LOG(LS_INFO) << "Enabling rtcp-mux for " << content_name()
<< "; no longer need RTCP transport for "
@@ -1674,6 +1679,47 @@
Bind(&BaseChannel::UpdateMediaSendRecvState_w, this));
}
+int BaseChannel::GetTransportOverheadPerPacket() const {
+ RTC_DCHECK(network_thread_->IsCurrent());
+
+ if (!selected_candidate_pair_)
+ return 0;
+
+ int transport_overhead_per_packet = 0;
+
+ constexpr int kIpv4Overhaed = 20;
+ constexpr int kIpv6Overhaed = 40;
+ transport_overhead_per_packet +=
+ selected_candidate_pair_->local_candidate().address().family() == AF_INET
+ ? kIpv4Overhaed
+ : kIpv6Overhaed;
+
+ constexpr int kUdpOverhaed = 8;
+ constexpr int kTcpOverhaed = 20;
+ transport_overhead_per_packet +=
+ selected_candidate_pair_->local_candidate().protocol() ==
+ TCP_PROTOCOL_NAME
+ ? kTcpOverhaed
+ : kUdpOverhaed;
+
+ if (srtp_active()) {
+ int srtp_overhead = 0;
+ if (srtp_transport_->GetSrtpOverhead(&srtp_overhead))
+ transport_overhead_per_packet += srtp_overhead;
+ }
+
+ return transport_overhead_per_packet;
+}
+
+void BaseChannel::UpdateTransportOverhead() {
+ int transport_overhead_per_packet = GetTransportOverheadPerPacket();
+ if (transport_overhead_per_packet)
+ invoker_.AsyncInvoke<void>(
+ RTC_FROM_HERE, worker_thread_,
+ Bind(&MediaChannel::OnTransportOverheadChanged, media_channel_.get(),
+ transport_overhead_per_packet));
+}
+
void VoiceChannel::UpdateMediaSendRecvState_w() {
// Render incoming data if we're the active call, and we have the local
// content. We receive data on the default channel and multiplexed streams.
diff --git a/pc/channel.h b/pc/channel.h
index 352fda4..ec13f07 100644
--- a/pc/channel.h
+++ b/pc/channel.h
@@ -243,7 +243,11 @@
void OnDtlsState(DtlsTransportInternal* transport, DtlsTransportState state);
- void OnNetworkRouteChanged(rtc::Optional<rtc::NetworkRoute> network_route);
+ void OnSelectedCandidatePairChanged(
+ IceTransportInternal* ice_transport,
+ CandidatePairInterface* selected_candidate_pair,
+ int last_sent_packet_id,
+ bool ready_to_send);
bool PacketIsRtcp(const rtc::PacketTransportInternal* transport,
const char* data,
@@ -359,6 +363,8 @@
void SignalSentPacket_w(const rtc::SentPacket& sent_packet);
bool IsReadyToSendMedia_n() const;
void CacheRtpAbsSendTimeHeaderExtension_n(int rtp_abs_sendtime_extn_id);
+ int GetTransportOverheadPerPacket() const;
+ void UpdateTransportOverhead();
// Wraps the existing RtpTransport in an SrtpTransport.
void EnableSrtpTransport_n();
diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc
index 36d7b00..5736bb6 100644
--- a/pc/channel_unittest.cc
+++ b/pc/channel_unittest.cc
@@ -1202,8 +1202,6 @@
static constexpr uint16_t kLocalNetId = 1;
static constexpr uint16_t kRemoteNetId = 2;
static constexpr int kLastPacketId = 100;
- // Ipv4(20) + UDP(8).
- static constexpr int kTransportOverheadPerPacket = 28;
CreateChannels(0, 0);
@@ -1211,17 +1209,12 @@
static_cast<typename T::MediaChannel*>(channel1_->media_channel());
ASSERT_TRUE(media_channel1);
- // Need to wait for the threads before calling
- // |set_num_network_route_changes| because the network route would be set
- // when creating the channel.
- WaitForThreads();
media_channel1->set_num_network_route_changes(0);
network_thread_->Invoke<void>(RTC_FROM_HERE, [this] {
- rtc::NetworkRoute network_route;
// The transport channel becomes disconnected.
- fake_rtp_dtls_transport1_->ice_transport()->SignalNetworkRouteChanged(
-
- rtc::Optional<rtc::NetworkRoute>(network_route));
+ fake_rtp_dtls_transport1_->ice_transport()
+ ->SignalSelectedCandidatePairChanged(
+ fake_rtp_dtls_transport1_->ice_transport(), nullptr, -1, false);
});
WaitForThreads();
EXPECT_EQ(1, media_channel1->num_network_route_changes());
@@ -1229,16 +1222,15 @@
media_channel1->set_num_network_route_changes(0);
network_thread_->Invoke<void>(RTC_FROM_HERE, [this] {
- rtc::NetworkRoute network_route;
- network_route.connected = true;
- network_route.local_network_id = kLocalNetId;
- network_route.remote_network_id = kRemoteNetId;
- network_route.last_sent_packet_id = kLastPacketId;
- network_route.packet_overhead = kTransportOverheadPerPacket;
// The transport channel becomes connected.
- fake_rtp_dtls_transport1_->ice_transport()->SignalNetworkRouteChanged(
-
- rtc::Optional<rtc::NetworkRoute>(network_route));
+ rtc::SocketAddress local_address("192.168.1.1", 1000 /* port number */);
+ rtc::SocketAddress remote_address("192.168.1.2", 2000 /* port number */);
+ auto candidate_pair = cricket::FakeCandidatePair::Create(
+ local_address, kLocalNetId, remote_address, kRemoteNetId);
+ fake_rtp_dtls_transport1_->ice_transport()
+ ->SignalSelectedCandidatePairChanged(
+ fake_rtp_dtls_transport1_->ice_transport(), candidate_pair.get(),
+ kLastPacketId, true);
});
WaitForThreads();
EXPECT_EQ(1, media_channel1->num_network_route_changes());
@@ -1247,6 +1239,7 @@
EXPECT_EQ(expected_network_route, media_channel1->last_network_route());
EXPECT_EQ(kLastPacketId,
media_channel1->last_network_route().last_sent_packet_id);
+ constexpr int kTransportOverheadPerPacket = 28; // Ipv4(20) + UDP(8).
EXPECT_EQ(kTransportOverheadPerPacket,
media_channel1->transport_overhead_per_packet());
}
diff --git a/pc/rtptransport.cc b/pc/rtptransport.cc
index d2604f9..1701c4d 100644
--- a/pc/rtptransport.cc
+++ b/pc/rtptransport.cc
@@ -11,7 +11,6 @@
#include "pc/rtptransport.h"
#include "media/base/rtputils.h"
-#include "p2p/base/p2pconstants.h"
#include "p2p/base/packettransportinterface.h"
#include "rtc_base/checks.h"
#include "rtc_base/copyonwritebuffer.h"
@@ -32,22 +31,15 @@
if (rtp_packet_transport_) {
rtp_packet_transport_->SignalReadyToSend.disconnect(this);
rtp_packet_transport_->SignalReadPacket.disconnect(this);
- rtp_packet_transport_->SignalNetworkRouteChanged.disconnect(this);
- // Reset the network route of the old transport.
- SignalNetworkRouteChanged(rtc::Optional<rtc::NetworkRoute>());
}
if (new_packet_transport) {
new_packet_transport->SignalReadyToSend.connect(
this, &RtpTransport::OnReadyToSend);
new_packet_transport->SignalReadPacket.connect(this,
&RtpTransport::OnReadPacket);
- new_packet_transport->SignalNetworkRouteChanged.connect(
- this, &RtpTransport::OnNetworkRouteChange);
- // Set the network route for the new transport.
- SignalNetworkRouteChanged(new_packet_transport->network_route());
}
-
rtp_packet_transport_ = new_packet_transport;
+
// Assumes the transport is ready to send if it is writable. If we are wrong,
// ready to send will be updated the next time we try to send.
SetReadyToSend(false,
@@ -62,19 +54,12 @@
if (rtcp_packet_transport_) {
rtcp_packet_transport_->SignalReadyToSend.disconnect(this);
rtcp_packet_transport_->SignalReadPacket.disconnect(this);
- rtcp_packet_transport_->SignalNetworkRouteChanged.disconnect(this);
- // Reset the network route of the old transport.
- SignalNetworkRouteChanged(rtc::Optional<rtc::NetworkRoute>());
}
if (new_packet_transport) {
new_packet_transport->SignalReadyToSend.connect(
this, &RtpTransport::OnReadyToSend);
new_packet_transport->SignalReadPacket.connect(this,
&RtpTransport::OnReadPacket);
- new_packet_transport->SignalNetworkRouteChanged.connect(
- this, &RtpTransport::OnNetworkRouteChange);
- // Set the network route for the new transport.
- SignalNetworkRouteChanged(new_packet_transport->network_route());
}
rtcp_packet_transport_ = new_packet_transport;
@@ -176,11 +161,6 @@
SetReadyToSend(transport == rtcp_packet_transport_, true);
}
-void RtpTransport::OnNetworkRouteChange(
- rtc::Optional<rtc::NetworkRoute> network_route) {
- SignalNetworkRouteChanged(network_route);
-}
-
void RtpTransport::SetReadyToSend(bool rtcp, bool ready) {
if (rtcp) {
rtcp_ready_to_send_ = ready;
diff --git a/pc/rtptransport.h b/pc/rtptransport.h
index 34ec1b4..5e1aa20 100644
--- a/pc/rtptransport.h
+++ b/pc/rtptransport.h
@@ -11,8 +11,6 @@
#ifndef PC_RTPTRANSPORT_H_
#define PC_RTPTRANSPORT_H_
-#include <string>
-
#include "pc/bundlefilter.h"
#include "pc/rtptransportinternal.h"
#include "rtc_base/sigslot.h"
@@ -78,7 +76,6 @@
bool HandlesPacket(const uint8_t* data, size_t len);
void OnReadyToSend(rtc::PacketTransportInternal* transport);
- void OnNetworkRouteChange(rtc::Optional<rtc::NetworkRoute> network_route);
// Updates "ready to send" for an individual channel and fires
// SignalReadyToSend.
diff --git a/pc/rtptransport_unittest.cc b/pc/rtptransport_unittest.cc
index d6eb336..1e1657d 100644
--- a/pc/rtptransport_unittest.cc
+++ b/pc/rtptransport_unittest.cc
@@ -19,10 +19,6 @@
constexpr bool kMuxDisabled = false;
constexpr bool kMuxEnabled = true;
-constexpr uint16_t kLocalNetId = 1;
-constexpr uint16_t kRemoteNetId = 2;
-constexpr int kLastPacketId = 100;
-constexpr int kTransportOverheadPerPacket = 28; // Ipv4(20) + UDP(8).
TEST(RtpTransportTest, SetRtcpParametersCantDisableRtcpMux) {
RtpTransport transport(kMuxDisabled);
@@ -60,21 +56,12 @@
public:
explicit SignalObserver(RtpTransport* transport) {
transport->SignalReadyToSend.connect(this, &SignalObserver::OnReadyToSend);
- transport->SignalNetworkRouteChanged.connect(
- this, &SignalObserver::OnNetworkRouteChanged);
}
-
bool ready() const { return ready_; }
void OnReadyToSend(bool ready) { ready_ = ready; }
- rtc::Optional<rtc::NetworkRoute> network_route() { return network_route_; }
- void OnNetworkRouteChanged(rtc::Optional<rtc::NetworkRoute> network_route) {
- network_route_ = network_route;
- }
-
private:
bool ready_ = false;
- rtc::Optional<rtc::NetworkRoute> network_route_;
};
TEST(RtpTransportTest, SettingRtcpAndRtpSignalsReady) {
@@ -141,61 +128,6 @@
EXPECT_TRUE(observer.ready());
}
-// Tests the SignalNetworkRoute is fired when setting a packet transport.
-TEST(RtpTransportTest, SetRtpTransportWithNetworkRouteChanged) {
- RtpTransport transport(kMuxDisabled);
- SignalObserver observer(&transport);
- rtc::FakePacketTransport fake_rtp("fake_rtp");
-
- EXPECT_FALSE(observer.network_route());
-
- rtc::NetworkRoute network_route;
- // Set a non-null RTP transport with a new network route.
- network_route.connected = true;
- network_route.local_network_id = kLocalNetId;
- network_route.remote_network_id = kRemoteNetId;
- network_route.last_sent_packet_id = kLastPacketId;
- network_route.packet_overhead = kTransportOverheadPerPacket;
- fake_rtp.SetNetworkRoute(rtc::Optional<rtc::NetworkRoute>(network_route));
- transport.SetRtpPacketTransport(&fake_rtp);
- ASSERT_TRUE(observer.network_route());
- EXPECT_EQ(network_route, *(observer.network_route()));
- EXPECT_EQ(kTransportOverheadPerPacket,
- observer.network_route()->packet_overhead);
- EXPECT_EQ(kLastPacketId, observer.network_route()->last_sent_packet_id);
-
- // Set a null RTP transport.
- transport.SetRtpPacketTransport(nullptr);
- EXPECT_FALSE(observer.network_route());
-}
-
-TEST(RtpTransportTest, SetRtcpTransportWithNetworkRouteChanged) {
- RtpTransport transport(kMuxDisabled);
- SignalObserver observer(&transport);
- rtc::FakePacketTransport fake_rtcp("fake_rtcp");
-
- EXPECT_FALSE(observer.network_route());
-
- rtc::NetworkRoute network_route;
- // Set a non-null RTCP transport with a new network route.
- network_route.connected = true;
- network_route.local_network_id = kLocalNetId;
- network_route.remote_network_id = kRemoteNetId;
- network_route.last_sent_packet_id = kLastPacketId;
- network_route.packet_overhead = kTransportOverheadPerPacket;
- fake_rtcp.SetNetworkRoute(rtc::Optional<rtc::NetworkRoute>(network_route));
- transport.SetRtcpPacketTransport(&fake_rtcp);
- ASSERT_TRUE(observer.network_route());
- EXPECT_EQ(network_route, *(observer.network_route()));
- EXPECT_EQ(kTransportOverheadPerPacket,
- observer.network_route()->packet_overhead);
- EXPECT_EQ(kLastPacketId, observer.network_route()->last_sent_packet_id);
-
- // Set a null RTCP transport.
- transport.SetRtcpPacketTransport(nullptr);
- EXPECT_FALSE(observer.network_route());
-}
-
class SignalCounter : public sigslot::has_slots<> {
public:
explicit SignalCounter(RtpTransport* transport) {
diff --git a/pc/rtptransportinternal.h b/pc/rtptransportinternal.h
index 04d2ef3..e589619 100644
--- a/pc/rtptransportinternal.h
+++ b/pc/rtptransportinternal.h
@@ -11,11 +11,7 @@
#ifndef PC_RTPTRANSPORTINTERNAL_H_
#define PC_RTPTRANSPORTINTERNAL_H_
-#include <string>
-
#include "api/ortc/rtptransportinterface.h"
-#include "p2p/base/icetransportinternal.h"
-#include "rtc_base/networkroute.h"
#include "rtc_base/sigslot.h"
namespace rtc {
@@ -56,10 +52,6 @@
sigslot::signal3<bool, rtc::CopyOnWriteBuffer*, const rtc::PacketTime&>
SignalPacketReceived;
- // Called whenever the network route of the P2P layer transport changes.
- // The argument is an optional network route.
- sigslot::signal1<rtc::Optional<rtc::NetworkRoute>> SignalNetworkRouteChanged;
-
virtual bool IsWritable(bool rtcp) const = 0;
virtual bool SendRtpPacket(rtc::CopyOnWriteBuffer* packet,
diff --git a/pc/srtptransport.cc b/pc/srtptransport.cc
index bb42ad4..1343fd0 100644
--- a/pc/srtptransport.cc
+++ b/pc/srtptransport.cc
@@ -42,8 +42,6 @@
this, &SrtpTransport::OnPacketReceived);
rtp_transport_->SignalReadyToSend.connect(this,
&SrtpTransport::OnReadyToSend);
- rtp_transport_->SignalNetworkRouteChanged.connect(
- this, &SrtpTransport::OnNetworkRouteChanged);
}
bool SrtpTransport::SendRtpPacket(rtc::CopyOnWriteBuffer* packet,
@@ -172,20 +170,6 @@
SignalPacketReceived(rtcp, packet, packet_time);
}
-void SrtpTransport::OnNetworkRouteChanged(
-
- rtc::Optional<rtc::NetworkRoute> network_route) {
- // Only append the SRTP overhead when there is a selected network route.
- if (network_route) {
- int srtp_overhead = 0;
- if (IsActive()) {
- GetSrtpOverhead(&srtp_overhead);
- }
- network_route->packet_overhead += srtp_overhead;
- }
- SignalNetworkRouteChanged(network_route);
-}
-
bool SrtpTransport::SetRtpParams(int send_cs,
const uint8_t* send_key,
int send_key_len,
diff --git a/pc/srtptransport.h b/pc/srtptransport.h
index 919d4b9..13abd6b 100644
--- a/pc/srtptransport.h
+++ b/pc/srtptransport.h
@@ -16,7 +16,6 @@
#include <utility>
#include <vector>
-#include "p2p/base/icetransportinternal.h"
#include "pc/rtptransportinternal.h"
#include "pc/srtpfilter.h"
#include "pc/srtpsession.h"
@@ -159,8 +158,8 @@
void OnPacketReceived(bool rtcp,
rtc::CopyOnWriteBuffer* packet,
const rtc::PacketTime& packet_time);
+
void OnReadyToSend(bool ready) { SignalReadyToSend(ready); }
- void OnNetworkRouteChanged(rtc::Optional<rtc::NetworkRoute> network_route);
bool ProtectRtp(void* data, int in_len, int max_len, int* out_len);
diff --git a/pc/transportcontroller.cc b/pc/transportcontroller.cc
index 7cf9bbd..57f90b7 100644
--- a/pc/transportcontroller.cc
+++ b/pc/transportcontroller.cc
@@ -761,7 +761,7 @@
void TransportController::OnChannelWritableState_n(
rtc::PacketTransportInternal* transport) {
RTC_DCHECK(network_thread_->IsCurrent());
- RTC_LOG(LS_INFO) << " Transport " << transport->transport_name()
+ RTC_LOG(LS_INFO) << " TransportChannel " << transport->debug_name()
<< " writability changed to " << transport->writable()
<< ".";
UpdateAggregateStates_n();
diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn
index 20797cc..971d32d 100644
--- a/rtc_base/BUILD.gn
+++ b/rtc_base/BUILD.gn
@@ -525,8 +525,6 @@
"messagehandler.h",
"messagequeue.cc",
"messagequeue.h",
- "nethelper.cc",
- "nethelper.h",
"nethelpers.cc",
"nethelpers.h",
"network.cc",
diff --git a/rtc_base/nethelper.cc b/rtc_base/nethelper.cc
deleted file mode 100644
index e654fe3..0000000
--- a/rtc_base/nethelper.cc
+++ /dev/null
@@ -1,42 +0,0 @@
-/*
- * Copyright 2017 The WebRTC Project Authors. All rights reserved.
- *
- * Use of this source code is governed by a BSD-style license
- * that can be found in the LICENSE file in the root of the source
- * tree. An additional intellectual property rights grant can be found
- * in the file PATENTS. All contributing project authors may
- * be found in the AUTHORS file in the root of the source tree.
- */
-
-#include "rtc_base/nethelper.h"
-
-#include "rtc_base/checks.h"
-#include "rtc_base/ipaddress.h"
-
-namespace cricket {
-
-const char UDP_PROTOCOL_NAME[] = "udp";
-const char TCP_PROTOCOL_NAME[] = "tcp";
-const char SSLTCP_PROTOCOL_NAME[] = "ssltcp";
-const char TLS_PROTOCOL_NAME[] = "tls";
-
-int GetIpOverhead(int addr_family) {
- switch (addr_family) {
- case AF_INET: // IPv4
- return 20;
- case AF_INET6: // IPv6
- return 40;
- default:
- RTC_NOTREACHED() << "Invaild address family.";
- return 0;
- }
-}
-
-int GetProtocolOverhead(const std::string& protocol) {
- if (protocol == TCP_PROTOCOL_NAME || protocol == SSLTCP_PROTOCOL_NAME) {
- return 20;
- }
- return 8;
-}
-
-} // namespace cricket
diff --git a/rtc_base/nethelper.h b/rtc_base/nethelper.h
deleted file mode 100644
index e86d126..0000000
--- a/rtc_base/nethelper.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * Copyright 2017 The WebRTC Project Authors. All rights reserved.
- *
- * Use of this source code is governed by a BSD-style license
- * that can be found in the LICENSE file in the root of the source
- * tree. An additional intellectual property rights grant can be found
- * in the file PATENTS. All contributing project authors may
- * be found in the AUTHORS file in the root of the source tree.
- */
-#ifndef RTC_BASE_NETHELPER_H_
-#define RTC_BASE_NETHELPER_H_
-
-#include <cstdlib>
-#include <string>
-
-// This header contains helper functions and constants used by different types
-// of transports.
-namespace cricket {
-
-extern const char UDP_PROTOCOL_NAME[];
-extern const char TCP_PROTOCOL_NAME[];
-extern const char SSLTCP_PROTOCOL_NAME[];
-extern const char TLS_PROTOCOL_NAME[];
-
-// Get the network layer overhead per packet based on the IP address family.
-int GetIpOverhead(int addr_family);
-
-// Get the transport layer overhead per packet based on the protocol.
-int GetProtocolOverhead(const std::string& protocol);
-
-} // namespace cricket
-
-#endif // RTC_BASE_NETHELPER_H_
diff --git a/rtc_base/networkroute.h b/rtc_base/networkroute.h
index 07cba63..f245cb9 100644
--- a/rtc_base/networkroute.h
+++ b/rtc_base/networkroute.h
@@ -22,17 +22,14 @@
uint16_t local_network_id;
uint16_t remote_network_id;
int last_sent_packet_id; // Last packet id sent on the PREVIOUS route.
- int packet_overhead; // The overhead in bytes from IP layer and above.
NetworkRoute()
: connected(false),
local_network_id(0),
remote_network_id(0),
- last_sent_packet_id(-1),
- packet_overhead(0) {}
+ last_sent_packet_id(-1) {}
// The route is connected if the local and remote network ids are provided.
- // TODO(zhihuang): Remove this and let the caller set the fields explicitly.
NetworkRoute(bool connected,
uint16_t local_net_id,
uint16_t remote_net_id,
@@ -40,11 +37,9 @@
: connected(connected),
local_network_id(local_net_id),
remote_network_id(remote_net_id),
- last_sent_packet_id(last_packet_id),
- packet_overhead(0) {}
+ last_sent_packet_id(last_packet_id) {}
- // |last_sent_packet_id| and |packet_overhead| do not affect the NetworkRoute
- // comparison.
+ // |last_sent_packet_id| does not affect the NetworkRoute comparison.
bool operator==(const NetworkRoute& nr) const {
return connected == nr.connected &&
local_network_id == nr.local_network_id &&