From 59b4e3ea8c0a869ccdcd5bfcba088e4c007cf5d3 Mon Sep 17 00:00:00 2001 From: Bjorn Terelius Date: Wed, 30 May 2018 17:14:08 +0200 Subject: [PATCH] Split IceCandidatePairEventType enum. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Disjoint subsets of the enum values are used for Ice candidate config events and Ice candidate check events. This CL breaks out the config part to a separate enum and by extension changes the icelogger interface for config events. Bug: webrtc:9336, webrtc:8111 Change-Id: I405b5c3981905c3c504b45afdddb3649469ed141 Reviewed-on: https://webrtc-review.googlesource.com/79943 Reviewed-by: Qingsi Wang Commit-Queue: Björn Terelius Cr-Commit-Position: refs/heads/master@{#23464} --- .../encoder/rtc_event_log_encoder_legacy.cc | 22 ++++------------ .../events/rtc_event_ice_candidate_pair.h | 7 ----- .../rtc_event_ice_candidate_pair_config.cc | 2 +- .../rtc_event_ice_candidate_pair_config.h | 13 +++++++--- logging/rtc_event_log/icelogger.cc | 26 ++++++++----------- logging/rtc_event_log/icelogger.h | 12 ++++++--- logging/rtc_event_log/rtc_event_log_parser.cc | 12 ++++----- logging/rtc_event_log/rtc_event_log_parser.h | 2 +- .../rtc_event_log/rtc_event_log_parser_new.cc | 12 ++++----- .../rtc_event_log/rtc_event_log_parser_new.h | 2 +- p2p/base/p2ptransportchannel.cc | 13 +++++----- p2p/base/p2ptransportchannel.h | 4 +-- p2p/base/port.cc | 14 +++++++--- p2p/base/port.h | 1 + 14 files changed, 69 insertions(+), 73 deletions(-) diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc index b1497d3713..191a28542d 100644 --- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc +++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc @@ -107,18 +107,16 @@ rtclog::VideoReceiveConfig_RtcpMode ConvertRtcpMode(RtcpMode rtcp_mode) { } rtclog::IceCandidatePairConfig::IceCandidatePairConfigType -ConvertIceCandidatePairConfigType(IceCandidatePairEventType type) { +ConvertIceCandidatePairConfigType(IceCandidatePairConfigType type) { switch (type) { - case IceCandidatePairEventType::kAdded: + case IceCandidatePairConfigType::kAdded: return rtclog::IceCandidatePairConfig::ADDED; - case IceCandidatePairEventType::kUpdated: + case IceCandidatePairConfigType::kUpdated: return rtclog::IceCandidatePairConfig::UPDATED; - case IceCandidatePairEventType::kDestroyed: + case IceCandidatePairConfigType::kDestroyed: return rtclog::IceCandidatePairConfig::DESTROYED; - case IceCandidatePairEventType::kSelected: + case IceCandidatePairConfigType::kSelected: return rtclog::IceCandidatePairConfig::SELECTED; - default: - RTC_NOTREACHED(); } RTC_NOTREACHED(); return rtclog::IceCandidatePairConfig::ADDED; @@ -137,8 +135,6 @@ rtclog::IceCandidatePairConfig::IceCandidateType ConvertIceCandidateType( return rtclog::IceCandidatePairConfig::RELAY; case IceCandidateType::kUnknown: return rtclog::IceCandidatePairConfig::UNKNOWN_CANDIDATE_TYPE; - default: - RTC_NOTREACHED(); } RTC_NOTREACHED(); return rtclog::IceCandidatePairConfig::UNKNOWN_CANDIDATE_TYPE; @@ -157,8 +153,6 @@ rtclog::IceCandidatePairConfig::Protocol ConvertIceCandidatePairProtocol( return rtclog::IceCandidatePairConfig::TLS; case IceCandidatePairProtocol::kUnknown: return rtclog::IceCandidatePairConfig::UNKNOWN_PROTOCOL; - default: - RTC_NOTREACHED(); } RTC_NOTREACHED(); return rtclog::IceCandidatePairConfig::UNKNOWN_PROTOCOL; @@ -174,8 +168,6 @@ ConvertIceCandidatePairAddressFamily( return rtclog::IceCandidatePairConfig::IPV6; case IceCandidatePairAddressFamily::kUnknown: return rtclog::IceCandidatePairConfig::UNKNOWN_ADDRESS_FAMILY; - default: - RTC_NOTREACHED(); } RTC_NOTREACHED(); return rtclog::IceCandidatePairConfig::UNKNOWN_ADDRESS_FAMILY; @@ -196,8 +188,6 @@ rtclog::IceCandidatePairConfig::NetworkType ConvertIceCandidateNetworkType( return rtclog::IceCandidatePairConfig::CELLULAR; case IceCandidateNetworkType::kUnknown: return rtclog::IceCandidatePairConfig::UNKNOWN_NETWORK_TYPE; - default: - RTC_NOTREACHED(); } RTC_NOTREACHED(); return rtclog::IceCandidatePairConfig::UNKNOWN_NETWORK_TYPE; @@ -214,8 +204,6 @@ ConvertIceCandidatePairEventType(IceCandidatePairEventType type) { return rtclog::IceCandidatePairEvent::CHECK_RESPONSE_SENT; case IceCandidatePairEventType::kCheckResponseReceived: return rtclog::IceCandidatePairEvent::CHECK_RESPONSE_RECEIVED; - default: - RTC_NOTREACHED(); } RTC_NOTREACHED(); return rtclog::IceCandidatePairEvent::CHECK_SENT; diff --git a/logging/rtc_event_log/events/rtc_event_ice_candidate_pair.h b/logging/rtc_event_log/events/rtc_event_ice_candidate_pair.h index b6cd61f48b..b04ad4e19c 100644 --- a/logging/rtc_event_log/events/rtc_event_ice_candidate_pair.h +++ b/logging/rtc_event_log/events/rtc_event_ice_candidate_pair.h @@ -18,13 +18,6 @@ namespace webrtc { enum class IceCandidatePairEventType { - // Config event types for events related to the candiate pair creation and - // life-cycle management. - kAdded, - kUpdated, - kDestroyed, - kSelected, - // Non-config event types. kCheckSent, kCheckReceived, kCheckResponseSent, diff --git a/logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.cc b/logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.cc index 33fd8c7751..a825a3e44f 100644 --- a/logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.cc +++ b/logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.cc @@ -36,7 +36,7 @@ IceCandidatePairDescription::IceCandidatePairDescription( IceCandidatePairDescription::~IceCandidatePairDescription() {} RtcEventIceCandidatePairConfig::RtcEventIceCandidatePairConfig( - IceCandidatePairEventType type, + IceCandidatePairConfigType type, uint32_t candidate_pair_id, const IceCandidatePairDescription& candidate_pair_desc) : type_(type), diff --git a/logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.h b/logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.h index 70c81bf820..62d39d55eb 100644 --- a/logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.h +++ b/logging/rtc_event_log/events/rtc_event_ice_candidate_pair_config.h @@ -13,12 +13,17 @@ #include "logging/rtc_event_log/events/rtc_event.h" -#include "logging/rtc_event_log/events/rtc_event_ice_candidate_pair.h" - #include namespace webrtc { +enum class IceCandidatePairConfigType { + kAdded, + kUpdated, + kDestroyed, + kSelected, +}; + // TODO(qingsi): Change the names of candidate types to "host", "srflx", "prflx" // and "relay" after the naming is spec-compliant in the signaling part enum class IceCandidateType { @@ -72,7 +77,7 @@ class IceCandidatePairDescription { class RtcEventIceCandidatePairConfig final : public RtcEvent { public: RtcEventIceCandidatePairConfig( - IceCandidatePairEventType type, + IceCandidatePairConfigType type, uint32_t candidate_pair_id, const IceCandidatePairDescription& candidate_pair_desc); @@ -82,7 +87,7 @@ class RtcEventIceCandidatePairConfig final : public RtcEvent { bool IsConfigEvent() const override; - const IceCandidatePairEventType type_; + const IceCandidatePairConfigType type_; const uint32_t candidate_pair_id_; const IceCandidatePairDescription candidate_pair_desc_; }; diff --git a/logging/rtc_event_log/icelogger.cc b/logging/rtc_event_log/icelogger.cc index 108f966519..923c0aacd6 100644 --- a/logging/rtc_event_log/icelogger.cc +++ b/logging/rtc_event_log/icelogger.cc @@ -18,25 +18,21 @@ namespace webrtc { IceEventLog::IceEventLog() {} IceEventLog::~IceEventLog() {} -bool IceEventLog::IsIceCandidatePairConfigEvent( - IceCandidatePairEventType type) { - return (type == IceCandidatePairEventType::kAdded) || - (type == IceCandidatePairEventType::kUpdated) || - (type == IceCandidatePairEventType::kDestroyed) || - (type == IceCandidatePairEventType::kSelected); -} - -void IceEventLog::LogCandidatePairEvent( - IceCandidatePairEventType type, +void IceEventLog::LogCandidatePairConfig( + IceCandidatePairConfigType type, uint32_t candidate_pair_id, const IceCandidatePairDescription& candidate_pair_desc) { if (event_log_ == nullptr) { return; } - if (IsIceCandidatePairConfigEvent(type)) { - candidate_pair_desc_by_id_[candidate_pair_id] = candidate_pair_desc; - event_log_->Log(rtc::MakeUnique( - type, candidate_pair_id, candidate_pair_desc)); + candidate_pair_desc_by_id_[candidate_pair_id] = candidate_pair_desc; + event_log_->Log(rtc::MakeUnique( + type, candidate_pair_id, candidate_pair_desc)); +} + +void IceEventLog::LogCandidatePairEvent(IceCandidatePairEventType type, + uint32_t candidate_pair_id) { + if (event_log_ == nullptr) { return; } event_log_->Log( @@ -46,7 +42,7 @@ void IceEventLog::LogCandidatePairEvent( void IceEventLog::DumpCandidatePairDescriptionToMemoryAsConfigEvents() const { for (const auto& desc_id_pair : candidate_pair_desc_by_id_) { event_log_->Log(rtc::MakeUnique( - IceCandidatePairEventType::kUpdated, desc_id_pair.first, + IceCandidatePairConfigType::kUpdated, desc_id_pair.first, desc_id_pair.second)); } } diff --git a/logging/rtc_event_log/icelogger.h b/logging/rtc_event_log/icelogger.h index b3689c7c10..b590fd7cb3 100644 --- a/logging/rtc_event_log/icelogger.h +++ b/logging/rtc_event_log/icelogger.h @@ -27,11 +27,17 @@ class IceEventLog { public: IceEventLog(); ~IceEventLog(); + void set_event_log(RtcEventLog* event_log) { event_log_ = event_log; } - void LogCandidatePairEvent( - IceCandidatePairEventType type, + + void LogCandidatePairConfig( + IceCandidatePairConfigType type, uint32_t candidate_pair_id, const IceCandidatePairDescription& candidate_pair_desc); + + void LogCandidatePairEvent(IceCandidatePairEventType type, + uint32_t candidate_pair_id); + // This method constructs a config event for each candidate pair with their // description and logs these config events. It is intended to be called when // logging starts to ensure that we have at least one config for each @@ -39,8 +45,6 @@ class IceEventLog { void DumpCandidatePairDescriptionToMemoryAsConfigEvents() const; private: - bool IsIceCandidatePairConfigEvent(IceCandidatePairEventType type); - RtcEventLog* event_log_ = nullptr; std::unordered_map candidate_pair_desc_by_id_; diff --git a/logging/rtc_event_log/rtc_event_log_parser.cc b/logging/rtc_event_log/rtc_event_log_parser.cc index 69789287ba..030def131d 100644 --- a/logging/rtc_event_log/rtc_event_log_parser.cc +++ b/logging/rtc_event_log/rtc_event_log_parser.cc @@ -98,20 +98,20 @@ BandwidthUsage GetRuntimeDetectorState( return BandwidthUsage::kBwNormal; } -IceCandidatePairEventType GetRuntimeIceCandidatePairConfigType( +IceCandidatePairConfigType GetRuntimeIceCandidatePairConfigType( rtclog::IceCandidatePairConfig::IceCandidatePairConfigType type) { switch (type) { case rtclog::IceCandidatePairConfig::ADDED: - return IceCandidatePairEventType::kAdded; + return IceCandidatePairConfigType::kAdded; case rtclog::IceCandidatePairConfig::UPDATED: - return IceCandidatePairEventType::kUpdated; + return IceCandidatePairConfigType::kUpdated; case rtclog::IceCandidatePairConfig::DESTROYED: - return IceCandidatePairEventType::kDestroyed; + return IceCandidatePairConfigType::kDestroyed; case rtclog::IceCandidatePairConfig::SELECTED: - return IceCandidatePairEventType::kSelected; + return IceCandidatePairConfigType::kSelected; } RTC_NOTREACHED(); - return IceCandidatePairEventType::kAdded; + return IceCandidatePairConfigType::kAdded; } IceCandidateType GetRuntimeIceCandidateType( diff --git a/logging/rtc_event_log/rtc_event_log_parser.h b/logging/rtc_event_log/rtc_event_log_parser.h index d4488bc449..af4b2de39f 100644 --- a/logging/rtc_event_log/rtc_event_log_parser.h +++ b/logging/rtc_event_log/rtc_event_log_parser.h @@ -74,7 +74,7 @@ class ParsedRtcEventLog { struct IceCandidatePairConfig { uint64_t timestamp; - IceCandidatePairEventType type; + IceCandidatePairConfigType type; uint32_t candidate_pair_id; IceCandidateType local_candidate_type; IceCandidatePairProtocol local_relay_protocol; diff --git a/logging/rtc_event_log/rtc_event_log_parser_new.cc b/logging/rtc_event_log/rtc_event_log_parser_new.cc index 2d6c4cd142..6ce39c5573 100644 --- a/logging/rtc_event_log/rtc_event_log_parser_new.cc +++ b/logging/rtc_event_log/rtc_event_log_parser_new.cc @@ -108,20 +108,20 @@ BandwidthUsage GetRuntimeDetectorState( return BandwidthUsage::kBwNormal; } -IceCandidatePairEventType GetRuntimeIceCandidatePairConfigType( +IceCandidatePairConfigType GetRuntimeIceCandidatePairConfigType( rtclog::IceCandidatePairConfig::IceCandidatePairConfigType type) { switch (type) { case rtclog::IceCandidatePairConfig::ADDED: - return IceCandidatePairEventType::kAdded; + return IceCandidatePairConfigType::kAdded; case rtclog::IceCandidatePairConfig::UPDATED: - return IceCandidatePairEventType::kUpdated; + return IceCandidatePairConfigType::kUpdated; case rtclog::IceCandidatePairConfig::DESTROYED: - return IceCandidatePairEventType::kDestroyed; + return IceCandidatePairConfigType::kDestroyed; case rtclog::IceCandidatePairConfig::SELECTED: - return IceCandidatePairEventType::kSelected; + return IceCandidatePairConfigType::kSelected; } RTC_NOTREACHED(); - return IceCandidatePairEventType::kAdded; + return IceCandidatePairConfigType::kAdded; } IceCandidateType GetRuntimeIceCandidateType( diff --git a/logging/rtc_event_log/rtc_event_log_parser_new.h b/logging/rtc_event_log/rtc_event_log_parser_new.h index 23c1aee294..98d7e4554b 100644 --- a/logging/rtc_event_log/rtc_event_log_parser_new.h +++ b/logging/rtc_event_log/rtc_event_log_parser_new.h @@ -114,7 +114,7 @@ struct LoggedBweProbeFailureEvent { struct LoggedIceCandidatePairConfig { int64_t timestamp_us; - IceCandidatePairEventType type; + IceCandidatePairConfigType type; uint32_t candidate_pair_id; IceCandidateType local_candidate_type; IceCandidatePairProtocol local_relay_protocol; diff --git a/p2p/base/p2ptransportchannel.cc b/p2p/base/p2ptransportchannel.cc index 486e293c26..0b6beb9b3f 100644 --- a/p2p/base/p2ptransportchannel.cc +++ b/p2p/base/p2ptransportchannel.cc @@ -191,7 +191,8 @@ void P2PTransportChannel::AddConnection(Connection* connection) { had_connection_ = true; connection->set_ice_event_log(&ice_event_log_); - LogCandidatePairEvent(connection, webrtc::IceCandidatePairEventType::kAdded); + LogCandidatePairConfig(connection, + webrtc::IceCandidatePairConfigType::kAdded); } // Determines whether we should switch the selected connection to @@ -1658,7 +1659,7 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn) { // destroyed, so don't use it. Connection* old_selected_connection = selected_connection_; selected_connection_ = conn; - LogCandidatePairEvent(conn, webrtc::IceCandidatePairEventType::kSelected); + LogCandidatePairConfig(conn, webrtc::IceCandidatePairConfigType::kSelected); network_route_.reset(); if (old_selected_connection) { old_selected_connection->set_selected(false); @@ -2368,15 +2369,15 @@ int P2PTransportChannel::SampleRegatherAllNetworksInterval() { return rand_.Rand(interval->min(), interval->max()); } -void P2PTransportChannel::LogCandidatePairEvent( +void P2PTransportChannel::LogCandidatePairConfig( Connection* conn, - webrtc::IceCandidatePairEventType type) { + webrtc::IceCandidatePairConfigType type) { if (conn == nullptr) { return; } auto candidate_pair_id = conn->hash(); - ice_event_log_.LogCandidatePairEvent(type, candidate_pair_id, - conn->ToLogDescription()); + ice_event_log_.LogCandidatePairConfig(type, candidate_pair_id, + conn->ToLogDescription()); } } // namespace cricket diff --git a/p2p/base/p2ptransportchannel.h b/p2p/base/p2ptransportchannel.h index 5525067f39..c061431900 100644 --- a/p2p/base/p2ptransportchannel.h +++ b/p2p/base/p2ptransportchannel.h @@ -292,8 +292,8 @@ class P2PTransportChannel : public IceTransportInternal { void RegatherOnFailedNetworks(); void RegatherOnAllNetworks(); - void LogCandidatePairEvent(Connection* conn, - webrtc::IceCandidatePairEventType type); + void LogCandidatePairConfig(Connection* conn, + webrtc::IceCandidatePairConfigType type); uint32_t GetNominationAttr(Connection* conn) const; bool GetUseCandidateAttr(Connection* conn, NominationMode mode) const; diff --git a/p2p/base/port.cc b/p2p/base/port.cc index 703c1e8b80..a0bf51d756 100644 --- a/p2p/base/port.cc +++ b/p2p/base/port.cc @@ -1370,7 +1370,7 @@ void Connection::Destroy() { RTC_LOG(LS_VERBOSE) << ToString() << ": Connection destroyed"; port_->thread()->Post(RTC_FROM_HERE, this, MSG_DELETE); - LogCandidatePairEvent(webrtc::IceCandidatePairEventType::kDestroyed); + LogCandidatePairConfig(webrtc::IceCandidatePairConfigType::kDestroyed); } void Connection::FailAndDestroy() { @@ -1460,7 +1460,7 @@ void Connection::UpdateState(int64_t now) { void Connection::Ping(int64_t now) { last_ping_sent_ = now; - ConnectionRequest *req = new ConnectionRequest(this); + ConnectionRequest* req = new ConnectionRequest(this); // If not using renomination, we use "1" to mean "nominated" and "0" to mean // "not nominated". If using renomination, values greater than 1 are used for // re-nominated pairs. @@ -1634,11 +1634,19 @@ const webrtc::IceCandidatePairDescription& Connection::ToLogDescription() { return log_description_.value(); } +void Connection::LogCandidatePairConfig( + webrtc::IceCandidatePairConfigType type) { + if (ice_event_log_ == nullptr) { + return; + } + ice_event_log_->LogCandidatePairConfig(type, hash(), ToLogDescription()); +} + void Connection::LogCandidatePairEvent(webrtc::IceCandidatePairEventType type) { if (ice_event_log_ == nullptr) { return; } - ice_event_log_->LogCandidatePairEvent(type, hash(), ToLogDescription()); + ice_event_log_->LogCandidatePairEvent(type, hash()); } void Connection::OnConnectionRequestResponse(ConnectionRequest* request, diff --git a/p2p/base/port.h b/p2p/base/port.h index ac9eeaca90..81554e80ce 100644 --- a/p2p/base/port.h +++ b/p2p/base/port.h @@ -768,6 +768,7 @@ class Connection : public CandidatePairInterface, void MaybeUpdateLocalCandidate(ConnectionRequest* request, StunMessage* response); + void LogCandidatePairConfig(webrtc::IceCandidatePairConfigType type); void LogCandidatePairEvent(webrtc::IceCandidatePairEventType type); WriteState write_state_;