From 5b6a4d8908f87b22132ddee4ed2bd114b524ff03 Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Tue, 24 Mar 2020 07:36:52 +0100 Subject: [PATCH] Only print route if it has changed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a follow up change to https://webrtc-review.googlesource.com/c/src/+/170628 and modifies code to only LOG if the route really has changed. Existing code will LOG like this, which is slightly annoying. Notice that the same route change is LOG:ed twice. 03-23 13:28:49.281 17986 18850 I rtp_transport_controller_send.cc: [1183:590] [18850] (line 253): Network route changed on transport audio: new_route = [ connected: 1 local: [ 2/4 Wifi turn: 0 ] remote: [ 2/3 Wifi turn: 0 ] packet_overhead_bytes: 32 ] 03-23 13:28:49.281 17986 18850 I rtp_transport_controller_send.cc: [1183:590] [18850] (line 278): old_route = [ connected: 1 local: [ 2/4 Wifi turn: 1 ] remote: [ 2/3 Wifi turn: 0 ] packet_overhead_bytes: 28 ] 03-23 13:28:49.281 17986 18850 I rtp_transport_controller_send.cc: [1183:591] [18850] (line 253): Network route changed on transport audio: new_route = [ connected: 1 local: [ 2/4 Wifi turn: 0 ] remote: [ 2/3 Wifi turn: 0 ] packet_overhead_bytes: 32 ] 03-23 13:28:49.282 17986 18850 I rtp_transport_controller_send.cc: [1183:591] [18850] (line 278): old_route = [ connected: 1 local: [ 2/4 Wifi turn: 0 ] remote: [ 2/3 Wifi turn: 0 ] packet_overhead_bytes: 32 ] The way this method is called twice with same argument is out of scope for this change. BUG: webrtc:11434 Change-Id: I052d089c59714513a09cbaed49f24c8f1300af58 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/171460 Reviewed-by: Harald Alvestrand Reviewed-by: Björn Terelius Commit-Queue: Jonas Oreland Cr-Commit-Position: refs/heads/master@{#30865} --- call/rtp_transport_controller_send.cc | 16 ++++++++++------ rtc_base/BUILD.gn | 2 ++ rtc_base/network_route.cc | 27 +++++++++++++++++++++++++++ rtc_base/network_route.h | 4 ++++ rtc_base/network_route_unittest.cc | 24 ++++++++++++++++++++++++ 5 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 rtc_base/network_route.cc create mode 100644 rtc_base/network_route_unittest.cc diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index f219c2129d..42b92f9b40 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -250,9 +250,6 @@ void RtpTransportControllerSend::OnNetworkRouteChanged( 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) { // TODO(honghaiz): Perhaps handle this in SignalChannelNetworkState and // consider merging these two methods. @@ -264,6 +261,14 @@ void RtpTransportControllerSend::OnNetworkRouteChanged( network_routes_.insert(std::make_pair(transport_name, network_route)); auto kv = result.first; bool inserted = result.second; + if (inserted || !(kv->second == network_route)) { + RTC_LOG(LS_INFO) << "Network route changed on transport " << transport_name + << ": new_route = " << network_route.DebugString(); + if (!inserted) { + RTC_LOG(LS_INFO) << "old_route = " << kv->second.DebugString(); + } + } + if (inserted) { task_queue_.PostTask([this, network_route] { RTC_DCHECK_RUN_ON(&task_queue_); @@ -272,10 +277,9 @@ void RtpTransportControllerSend::OnNetworkRouteChanged( // No need to reset BWE if this is the first time the network connects. return; } - // - auto old_route = kv->second; + + const rtc::NetworkRoute 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). diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 5167e5a4a5..60dda76f6d 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -819,6 +819,7 @@ rtc_library("rtc_base") { "net_helpers.cc", "net_helpers.h", "network.cc", + "network_route.cc", "network.h", "network_constants.cc", "network_constants.h", @@ -1307,6 +1308,7 @@ if (rtc_include_tests) { "message_digest_unittest.cc", "nat_unittest.cc", "network_unittest.cc", + "network_route_unittest.cc", "proxy_unittest.cc", "rolling_accumulator_unittest.cc", "rtc_certificate_generator_unittest.cc", diff --git a/rtc_base/network_route.cc b/rtc_base/network_route.cc new file mode 100644 index 0000000000..80d135a92c --- /dev/null +++ b/rtc_base/network_route.cc @@ -0,0 +1,27 @@ +/* + * 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_route.h" + +namespace rtc { + +bool RouteEndpoint::operator==(const RouteEndpoint& other) const { + return adapter_type_ == other.adapter_type_ && + adapter_id_ == other.adapter_id_ && network_id_ == other.network_id_ && + uses_turn_ == other.uses_turn_; +} + +bool NetworkRoute::operator==(const NetworkRoute& other) const { + return connected == other.connected && local == other.local && + remote == other.remote && packet_overhead == other.packet_overhead && + last_sent_packet_id == other.last_sent_packet_id; +} + +} // namespace rtc diff --git a/rtc_base/network_route.h b/rtc_base/network_route.h index c2b492ce18..c97c6ea8eb 100644 --- a/rtc_base/network_route.h +++ b/rtc_base/network_route.h @@ -52,6 +52,8 @@ class RouteEndpoint { uint16_t network_id() const { return network_id_; } bool uses_turn() const { return uses_turn_; } + bool operator==(const RouteEndpoint& other) const; + private: AdapterType adapter_type_ = ADAPTER_TYPE_UNKNOWN; uint16_t adapter_id_ = 0; @@ -87,6 +89,8 @@ struct NetworkRoute { << " ] packet_overhead_bytes: " << packet_overhead << " ]"; return oss.Release(); } + + bool operator==(const NetworkRoute& other) const; }; } // namespace rtc diff --git a/rtc_base/network_route_unittest.cc b/rtc_base/network_route_unittest.cc new file mode 100644 index 0000000000..485683b71f --- /dev/null +++ b/rtc_base/network_route_unittest.cc @@ -0,0 +1,24 @@ +/* + * Copyright 2004 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_route.h" + +#include "rtc_base/gunit.h" +#include "test/gmock.h" + +namespace rtc { + +TEST(NetworkRoute, Equals) { + NetworkRoute r1; + NetworkRoute r2 = r1; + EXPECT_TRUE(r1 == r2); +} + +} // namespace rtc