diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index fb684ca014..f219c2129d 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -249,8 +249,11 @@ void RtpTransportControllerSend::OnNetworkRouteChanged( const std::string& transport_name, const rtc::NetworkRoute& network_route) { // Check if the network route is connected. + + RTC_LOG(LS_INFO) << "Network route changed on transport " << transport_name + << ": new_route = " << network_route.DebugString(); + if (!network_route.connected) { - RTC_LOG(LS_INFO) << "Transport " << transport_name << " is disconnected"; // TODO(honghaiz): Perhaps handle this in SignalChannelNetworkState and // consider merging these two methods. return; @@ -269,17 +272,23 @@ void RtpTransportControllerSend::OnNetworkRouteChanged( // No need to reset BWE if this is the first time the network connects. return; } - if (kv->second.connected != network_route.connected || - kv->second.local_network_id != network_route.local_network_id || - kv->second.remote_network_id != network_route.remote_network_id) { - kv->second = network_route; + // + auto old_route = kv->second; + kv->second = network_route; + RTC_LOG(LS_INFO) << "old_route = " << old_route.DebugString(); + + // Check if enough conditions of the new/old route has changed + // to trigger resetting of bitrates (and a probe). + // Currently we only check local/remote network id (i.e IP address) and + // connected state and do not consider if we change route due to TURN. + // + // TODO(bugs.webrtc.org/11438) : Experiment with using more information/ + // other conditions. + if (old_route.connected != network_route.connected || + old_route.local.network_id() != network_route.local.network_id() || + old_route.remote.network_id() != network_route.remote.network_id()) { BitrateConstraints bitrate_config = bitrate_configurator_.GetConfig(); - RTC_LOG(LS_INFO) << "Network route changed on transport " << transport_name - << ": new local network id " - << network_route.local_network_id - << " new remote network id " - << network_route.remote_network_id - << " Reset bitrates to min: " + RTC_LOG(LS_INFO) << "Reset bitrates to min: " << bitrate_config.min_bitrate_bps << " bps, start: " << bitrate_config.start_bitrate_bps << " bps, max: " << bitrate_config.max_bitrate_bps @@ -297,8 +306,11 @@ void RtpTransportControllerSend::OnNetworkRouteChanged( RTC_DCHECK_RUN_ON(&task_queue_); transport_overhead_bytes_per_packet_ = network_route.packet_overhead; if (reset_feedback_on_route_change_) { + // TODO(bugs.webrtc.org/11438) : Consider if transport_feedback_adapter + // should have a real "route" rather than just local/remote network_id. transport_feedback_adapter_.SetNetworkIds( - network_route.local_network_id, network_route.remote_network_id); + network_route.local.network_id(), + network_route.remote.network_id()); } if (controller_) { PostUpdates(controller_->OnNetworkRouteChange(msg)); diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 906aa598d0..d935a45303 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -57,6 +57,43 @@ uint32_t GetWeakPingIntervalInFieldTrial() { return cricket::WEAK_PING_INTERVAL; } +rtc::AdapterType GuessAdapterTypeFromNetworkCost(int network_cost) { + // The current network costs have been unchanged since they were added + // to webrtc. If they ever were to change we would need to reconsider + // this method. + switch (network_cost) { + case rtc::kNetworkCostMin: + return rtc::ADAPTER_TYPE_ETHERNET; + case rtc::kNetworkCostLow: + return rtc::ADAPTER_TYPE_WIFI; + case rtc::kNetworkCostHigh: + return rtc::ADAPTER_TYPE_CELLULAR; + case rtc::kNetworkCostUnknown: + return rtc::ADAPTER_TYPE_UNKNOWN; + case rtc::kNetworkCostMax: + return rtc::ADAPTER_TYPE_ANY; + } + return rtc::ADAPTER_TYPE_UNKNOWN; +} + +rtc::RouteEndpoint CreateRouteEndpointFromCandidate( + bool local, + const cricket::Candidate& candidate, + bool uses_turn) { + auto adapter_type = candidate.network_type(); + if (!local && adapter_type == rtc::ADAPTER_TYPE_UNKNOWN) { + adapter_type = GuessAdapterTypeFromNetworkCost(candidate.network_cost()); + } + + // TODO(bugs.webrtc.org/9446) : Rewrite if information about remote network + // adapter becomes available. The implication of this implementation is that + // we will only ever report 1 adapter per type. In practice this is probably + // fine, since the endpoint also contains network-id. + uint16_t adapter_id = static_cast(adapter_type); + return rtc::RouteEndpoint(adapter_type, adapter_id, candidate.network_id(), + uses_turn); +} + } // unnamed namespace namespace cricket { @@ -1687,10 +1724,21 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn, 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_->local = CreateRouteEndpointFromCandidate( + /* local= */ true, selected_connection_->local_candidate(), + /* uses_turn= */ selected_connection_->port()->Type() == + RELAY_PORT_TYPE); + network_route_->remote = CreateRouteEndpointFromCandidate( + /* local= */ false, selected_connection_->remote_candidate(), + /* uses_turn= */ selected_connection_->remote_candidate().type() == + RELAY_PORT_TYPE); + + // Downstream projects depend on the old representation, + // populate that until they have been migrated. + // TODO(jonaso): remove. + network_route_->local_network_id = network_route_->local.network_id(); + network_route_->remote_network_id = network_route_->remote.network_id(); + network_route_->last_sent_packet_id = last_sent_packet_id_; network_route_->packet_overhead = selected_connection_->local_candidate().address().ipaddr().overhead() + diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index ee7456a739..c66a9f7ce0 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -3221,9 +3221,9 @@ class P2PTransportChannelPingTest : public ::testing::Test, return !last_network_route_.has_value(); } else { return pair->local_candidate().network_id() == - last_network_route_->local_network_id && + last_network_route_->local.network_id() && pair->remote_candidate().network_id() == - last_network_route_->remote_network_id; + last_network_route_->remote.network_id(); } } diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index d5c51ecd8a..c1037f7193 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -843,7 +843,6 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { rtc::NetworkRoute network_route; // The transport channel becomes disconnected. fake_rtp_dtls_transport1_->ice_transport()->SignalNetworkRouteChanged( - absl::optional(network_route)); }); WaitForThreads(); @@ -854,8 +853,10 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { network_thread_->Invoke(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.local = + rtc::RouteEndpoint::CreateWithNetworkId(kLocalNetId); + network_route.remote = + rtc::RouteEndpoint::CreateWithNetworkId(kRemoteNetId); network_route.last_sent_packet_id = kLastPacketId; network_route.packet_overhead = kTransportOverheadPerPacket; // The transport channel becomes connected. @@ -867,9 +868,9 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { EXPECT_EQ(1, media_channel1->num_network_route_changes()); EXPECT_TRUE(media_channel1->last_network_route().connected); EXPECT_EQ(kLocalNetId, - media_channel1->last_network_route().local_network_id); + media_channel1->last_network_route().local.network_id()); EXPECT_EQ(kRemoteNetId, - media_channel1->last_network_route().remote_network_id); + media_channel1->last_network_route().remote.network_id()); EXPECT_EQ(kLastPacketId, media_channel1->last_network_route().last_sent_packet_id); EXPECT_EQ(kTransportOverheadPerPacket + kSrtpOverheadPerPacket, diff --git a/pc/composite_rtp_transport_test.cc b/pc/composite_rtp_transport_test.cc index 02480844a0..fee8c215b2 100644 --- a/pc/composite_rtp_transport_test.cc +++ b/pc/composite_rtp_transport_test.cc @@ -229,17 +229,17 @@ TEST_F(CompositeRtpTransportTest, NetworkRouteChange) { SetupRtpTransports(/*rtcp_mux=*/true); rtc::NetworkRoute route; - route.local_network_id = 7; + route.local = rtc::RouteEndpoint::CreateWithNetworkId(7); packet_transport_1_->SetNetworkRoute(route); EXPECT_EQ(1, network_route_count_); - EXPECT_EQ(7, last_network_route_->local_network_id); + EXPECT_EQ(7, last_network_route_->local.network_id()); - route.local_network_id = 8; + route.local = rtc::RouteEndpoint::CreateWithNetworkId(8); packet_transport_2_->SetNetworkRoute(route); EXPECT_EQ(2, network_route_count_); - EXPECT_EQ(8, last_network_route_->local_network_id); + EXPECT_EQ(8, last_network_route_->local.network_id()); } TEST_F(CompositeRtpTransportTest, RemoveTransport) { @@ -249,7 +249,7 @@ TEST_F(CompositeRtpTransportTest, RemoveTransport) { // Check that signals are disconnected. rtc::NetworkRoute route; - route.local_network_id = 7; + route.local = rtc::RouteEndpoint::CreateWithNetworkId(7); packet_transport_1_->SetNetworkRoute(route); EXPECT_EQ(0, network_route_count_); diff --git a/pc/rtp_transport_unittest.cc b/pc/rtp_transport_unittest.cc index 03e8820c30..b3bd1db2e5 100644 --- a/pc/rtp_transport_unittest.cc +++ b/pc/rtp_transport_unittest.cc @@ -155,16 +155,16 @@ TEST(RtpTransportTest, SetRtpTransportWithNetworkRouteChanged) { 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.local = rtc::RouteEndpoint::CreateWithNetworkId(kLocalNetId); + network_route.remote = rtc::RouteEndpoint::CreateWithNetworkId(kRemoteNetId); network_route.last_sent_packet_id = kLastPacketId; network_route.packet_overhead = kTransportOverheadPerPacket; fake_rtp.SetNetworkRoute(absl::optional(network_route)); transport.SetRtpPacketTransport(&fake_rtp); ASSERT_TRUE(observer.network_route()); EXPECT_TRUE(observer.network_route()->connected); - EXPECT_EQ(kLocalNetId, observer.network_route()->local_network_id); - EXPECT_EQ(kRemoteNetId, observer.network_route()->remote_network_id); + EXPECT_EQ(kLocalNetId, observer.network_route()->local.network_id()); + EXPECT_EQ(kRemoteNetId, observer.network_route()->remote.network_id()); EXPECT_EQ(kTransportOverheadPerPacket, observer.network_route()->packet_overhead); EXPECT_EQ(kLastPacketId, observer.network_route()->last_sent_packet_id); @@ -184,16 +184,16 @@ TEST(RtpTransportTest, SetRtcpTransportWithNetworkRouteChanged) { 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.local = rtc::RouteEndpoint::CreateWithNetworkId(kLocalNetId); + network_route.remote = rtc::RouteEndpoint::CreateWithNetworkId(kRemoteNetId); network_route.last_sent_packet_id = kLastPacketId; network_route.packet_overhead = kTransportOverheadPerPacket; fake_rtcp.SetNetworkRoute(absl::optional(network_route)); transport.SetRtcpPacketTransport(&fake_rtcp); ASSERT_TRUE(observer.network_route()); EXPECT_TRUE(observer.network_route()->connected); - EXPECT_EQ(kLocalNetId, observer.network_route()->local_network_id); - EXPECT_EQ(kRemoteNetId, observer.network_route()->remote_network_id); + EXPECT_EQ(kLocalNetId, observer.network_route()->local.network_id()); + EXPECT_EQ(kRemoteNetId, observer.network_route()->remote.network_id()); EXPECT_EQ(kTransportOverheadPerPacket, observer.network_route()->packet_overhead); EXPECT_EQ(kLastPacketId, observer.network_route()->last_sent_packet_id); diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 2e4138e458..2c6dd3c56b 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -765,6 +765,7 @@ rtc_library("rtc_base") { "../system_wrappers:field_trial", "network:sent_packet", "system:file_wrapper", + "system:inline", "system:rtc_export", "task_utils:to_queued_task", "third_party/base64", @@ -818,6 +819,7 @@ rtc_library("rtc_base") { "net_helpers.h", "network.cc", "network.h", + "network_constants.cc", "network_constants.h", "network_monitor.cc", "network_monitor.h", diff --git a/rtc_base/network.cc b/rtc_base/network.cc index 4906184b5d..07b121bb3a 100644 --- a/rtc_base/network.cc +++ b/rtc_base/network.cc @@ -85,28 +85,6 @@ bool SortNetworks(const Network* a, const Network* b) { return a->key() < b->key(); } -std::string AdapterTypeToString(AdapterType type) { - switch (type) { - case ADAPTER_TYPE_ANY: - return "Wildcard"; - case ADAPTER_TYPE_UNKNOWN: - return "Unknown"; - case ADAPTER_TYPE_ETHERNET: - return "Ethernet"; - case ADAPTER_TYPE_WIFI: - return "Wifi"; - case ADAPTER_TYPE_CELLULAR: - return "Cellular"; - case ADAPTER_TYPE_VPN: - return "VPN"; - case ADAPTER_TYPE_LOOPBACK: - return "Loopback"; - default: - RTC_NOTREACHED() << "Invalid type " << type; - return std::string(); - } -} - uint16_t ComputeNetworkCostByType(int type) { switch (type) { case rtc::ADAPTER_TYPE_ETHERNET: diff --git a/rtc_base/network_constants.cc b/rtc_base/network_constants.cc new file mode 100644 index 0000000000..2cb5233ad6 --- /dev/null +++ b/rtc_base/network_constants.cc @@ -0,0 +1,39 @@ +/* + * Copyright 2020 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/network_constants.h" + +#include "rtc_base/checks.h" + +namespace rtc { + +std::string AdapterTypeToString(AdapterType type) { + switch (type) { + case ADAPTER_TYPE_ANY: + return "Wildcard"; + case ADAPTER_TYPE_UNKNOWN: + return "Unknown"; + case ADAPTER_TYPE_ETHERNET: + return "Ethernet"; + case ADAPTER_TYPE_WIFI: + return "Wifi"; + case ADAPTER_TYPE_CELLULAR: + return "Cellular"; + case ADAPTER_TYPE_VPN: + return "VPN"; + case ADAPTER_TYPE_LOOPBACK: + return "Loopback"; + default: + RTC_NOTREACHED() << "Invalid type " << type; + return std::string(); + } +} + +} // namespace rtc diff --git a/rtc_base/network_constants.h b/rtc_base/network_constants.h index efb2c83455..1b43243944 100644 --- a/rtc_base/network_constants.h +++ b/rtc_base/network_constants.h @@ -13,6 +13,8 @@ #include +#include + namespace rtc { static const uint16_t kNetworkCostMax = 999; @@ -37,6 +39,8 @@ enum AdapterType { ADAPTER_TYPE_ANY = 1 << 5, }; +std::string AdapterTypeToString(AdapterType type); + } // namespace rtc #endif // RTC_BASE_NETWORK_CONSTANTS_H_ diff --git a/rtc_base/network_route.h b/rtc_base/network_route.h index 6a8f183513..c2b492ce18 100644 --- a/rtc_base/network_route.h +++ b/rtc_base/network_route.h @@ -13,21 +13,82 @@ #include +#include + +#include "rtc_base/network_constants.h" +#include "rtc_base/strings/string_builder.h" +#include "rtc_base/system/inline.h" + // TODO(honghaiz): Make a directory that describes the interfaces and structs // the media code can rely on and the network code can implement, and both can // depend on that, but not depend on each other. Then, move this file to that // directory. namespace rtc { +class RouteEndpoint { + public: + RouteEndpoint() {} // Used by tests. + RouteEndpoint(AdapterType adapter_type, + uint16_t adapter_id, + uint16_t network_id, + bool uses_turn) + : adapter_type_(adapter_type), + adapter_id_(adapter_id), + network_id_(network_id), + uses_turn_(uses_turn) {} + + RouteEndpoint(const RouteEndpoint&) = default; + RouteEndpoint& operator=(const RouteEndpoint&) = default; + + // Used by tests. + static RouteEndpoint CreateWithNetworkId(uint16_t network_id) { + return RouteEndpoint(ADAPTER_TYPE_UNKNOWN, + /* adapter_id = */ 0, network_id, + /* uses_turn = */ false); + } + + AdapterType adapter_type() const { return adapter_type_; } + uint16_t adapter_id() const { return adapter_id_; } + uint16_t network_id() const { return network_id_; } + bool uses_turn() const { return uses_turn_; } + + private: + AdapterType adapter_type_ = ADAPTER_TYPE_UNKNOWN; + uint16_t adapter_id_ = 0; + uint16_t network_id_ = 0; + bool uses_turn_ = false; +}; + struct NetworkRoute { bool connected = false; - uint16_t local_network_id = 0; - uint16_t remote_network_id = 0; + RouteEndpoint local; + RouteEndpoint remote; // Last packet id sent on the PREVIOUS route. int last_sent_packet_id = -1; // The overhead in bytes from IP layer and above. + // This is the maximum of any part of the route. int packet_overhead = 0; + + // Downstream projects depend on the old representation, + // populate that until they have been migrated. + // TODO(jonaso): remove. + uint16_t local_network_id = 0; + uint16_t remote_network_id = 0; + + RTC_NO_INLINE inline std::string DebugString() const { + rtc::StringBuilder oss; + oss << "[ connected: " << connected << " local: [ " << local.adapter_id() + << "/" << local.network_id() << " " + << AdapterTypeToString(local.adapter_type()) + << " turn: " << local.uses_turn() << " ] remote: [ " + << remote.adapter_id() << "/" << remote.network_id() << " " + << AdapterTypeToString(remote.adapter_type()) + << " turn: " << remote.uses_turn() + << " ] packet_overhead_bytes: " << packet_overhead << " ]"; + return oss.Release(); + } }; + } // namespace rtc #endif // RTC_BASE_NETWORK_ROUTE_H_ diff --git a/test/scenario/network_node.cc b/test/scenario/network_node.cc index c874add643..aa576dcf53 100644 --- a/test/scenario/network_node.cc +++ b/test/scenario/network_node.cc @@ -111,10 +111,10 @@ void NetworkNodeTransport::Connect(EmulatedEndpoint* endpoint, rtc::NetworkRoute route; route.connected = true; // We assume that the address will be unique in the lower bytes. - route.local_network_id = static_cast( - receiver_address.ipaddr().v4AddressAsHostOrderInteger()); - route.remote_network_id = static_cast( - receiver_address.ipaddr().v4AddressAsHostOrderInteger()); + route.local = rtc::RouteEndpoint::CreateWithNetworkId(static_cast( + receiver_address.ipaddr().v4AddressAsHostOrderInteger())); + route.remote = rtc::RouteEndpoint::CreateWithNetworkId(static_cast( + receiver_address.ipaddr().v4AddressAsHostOrderInteger())); route.packet_overhead = packet_overhead.bytes() + receiver_address.ipaddr().overhead() + cricket::kUdpHeaderSize; diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index 27bf0f08bf..cbc12a9f85 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -1772,8 +1772,8 @@ TEST_F(VideoSendStreamTest, ChangingNetworkRoute) { void PerformTest() override { rtc::NetworkRoute new_route; new_route.connected = true; - new_route.local_network_id = 10; - new_route.remote_network_id = 20; + new_route.local = rtc::RouteEndpoint::CreateWithNetworkId(10); + new_route.remote = rtc::RouteEndpoint::CreateWithNetworkId(20); BitrateConstraints bitrate_config; SendTask(RTC_FROM_HERE, task_queue_, @@ -1799,7 +1799,8 @@ TEST_F(VideoSendStreamTest, ChangingNetworkRoute) { // TODO(holmer): We should set the last sent packet id here and // verify that we correctly ignore any packet loss reported prior to // that id. - ++new_route.local_network_id; + new_route.local = rtc::RouteEndpoint::CreateWithNetworkId( + new_route.local.network_id() + 1); call_->GetTransportControllerSend()->OnNetworkRouteChanged( "transport", new_route); EXPECT_GE(call_->GetStats().send_bandwidth_bps, kStartBitrateBps);