admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reland "Refactor IceControllerEvent" 2/n

Browse Source 
This is a partial reland of commit 794e68cf3dbc3b16ee8b12b5615d8a1622154dbd
which uses the new enum in P2PTransportChannel and BasicIceController.

Original change's description:
> Refactor IceControllerEvent
>
> This change is the first step in decoupling IceControllerEvent from
> the ICE switch reason. Further cleanup is earmarked, and will be
> landed after some internal cleanup.
>
> This change
> - adds a new enum - IceSwitchReason
> - adds a member for the new enum in IceControllerEvent
> - uses the new enum within P2PTransportChannel
> - adds methods to IceControllerInterface accepting the new enum
> - deprecates usages of the old enum in IceControllerInterface
> - adds conversion between the old and new enums for compatibility
>
> Bug: webrtc:14125, webrtc:14131
> Change-Id: I5b7201c3f631eb40db334dfeec842841a7e58174
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264140
> Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
> Commit-Queue: Sameer Vijaykar <samvi@google.com>
> Cr-Commit-Position: refs/heads/main@{#37051}

Bug: webrtc:14125, webrtc:14131
Change-Id: I81b0338ae2f0560cd3df7ad9bd9af37e7c3499df
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264554
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Commit-Queue: Sameer Vijaykar <samvi@google.com>
Cr-Commit-Position: refs/heads/main@{#37105}
This commit is contained in:
Sameer Vijaykar 2022-06-02 16:01:12 +02:00 committed by WebRTC LUCI CQ
parent 35fc144e01
commit 781c12e7fb
6 changed files with 52 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
p2p/base/basic_ice_controller.cc
View File

@ -415,7 +415,7 @@ BasicIceController::GetBestWritableConnectionPerNetwork() const {
IceControllerInterface::SwitchResult
BasicIceController::HandleInitialSelectDampening(
IceControllerEvent reason,
IceSwitchReason reason,
const Connection* new_connection) {
if (!field_trials_->initial_select_dampening.has_value() &&
!field_trials_->initial_select_dampening_ping_received.has_value()) {
@ -464,13 +464,13 @@ BasicIceController::HandleInitialSelectDampening(
}
RTC_LOG(LS_INFO) << "delay initial selection up to " << min_delay << "ms";
reason.type = IceControllerEvent::ICE_CONTROLLER_RECHECK;
reason.recheck_delay_ms = min_delay;
return {absl::nullopt, reason};
return {.connection = absl::nullopt,
.recheck_event = IceControllerEvent(
IceSwitchReason::ICE_CONTROLLER_RECHECK, min_delay)};
}
IceControllerInterface::SwitchResult BasicIceController::ShouldSwitchConnection(
IceControllerEvent reason,
IceSwitchReason reason,
const Connection* new_connection) {
if (!ReadyToSend(new_connection) || selected_connection_ == new_connection) {
return {absl::nullopt, absl::nullopt};
@ -503,9 +503,8 @@ IceControllerInterface::SwitchResult BasicIceController::ShouldSwitchConnection(
// threshold, the new connection is in a better receiving state than the
// currently selected connection. So we need to re-check whether it needs
// to be switched at a later time.
recheck_event = reason;
recheck_event->recheck_delay_ms =
config_.receiving_switching_delay_or_default();
recheck_event.emplace(reason,
config_.receiving_switching_delay_or_default());
}
if (cmp < 0) {
@ -524,7 +523,7 @@ IceControllerInterface::SwitchResult BasicIceController::ShouldSwitchConnection(
}
IceControllerInterface::SwitchResult
BasicIceController::SortAndSwitchConnection(IceControllerEvent reason) {
BasicIceController::SortAndSwitchConnection(IceSwitchReason reason) {
// Find the best alternative connection by sorting. It is important to note
// that amongst equal preference, writable connections, this will choose the
// one whose estimated latency is lowest. So it is the only one that we

6
p2p/base/basic_ice_controller.h
View File

@ -46,9 +46,9 @@ class BasicIceController : public IceControllerInterface {
NominationMode mode,
IceMode remote_ice_mode) const override;
SwitchResult ShouldSwitchConnection(IceControllerEvent reason,
SwitchResult ShouldSwitchConnection(IceSwitchReason reason,
const Connection* connection) override;
SwitchResult SortAndSwitchConnection(IceControllerEvent reason) override;
SwitchResult SortAndSwitchConnection(IceSwitchReason reason) override;
std::vector<const Connection*> PruneConnections() override;
@ -136,7 +136,7 @@ class BasicIceController : public IceControllerInterface {
absl::optional<int64_t> receiving_unchanged_threshold,
bool* missed_receiving_unchanged_threshold) const;
SwitchResult HandleInitialSelectDampening(IceControllerEvent reason,
SwitchResult HandleInitialSelectDampening(IceSwitchReason reason,
const Connection* new_connection);
std::function<IceTransportState()> ice_transport_state_func_;

25
p2p/base/ice_controller_interface.h
View File

@ -42,8 +42,8 @@ struct IceControllerEvent {
ICE_CONTROLLER_RECHECK,
};
// TODO(bugs.webrtc.org/14125) remove.
IceControllerEvent(const Type& _type) // NOLINT: runtime/explicit
[[deprecated("bugs.webrtc.org/14125")]] IceControllerEvent(
const Type& _type) // NOLINT: runtime/explicit
: type(_type), reason(FromType(_type)) {}
IceControllerEvent(IceSwitchReason _reason, int _recheck_delay_ms)
@ -151,20 +151,21 @@ class IceControllerInterface {
// Check if we should switch to `connection`.
// This method is called for IceSwitchReasons that can switch directly
// i.e without resorting.
virtual SwitchResult ShouldSwitchConnection(IceControllerEvent reason,
const Connection* connection) = 0;
virtual SwitchResult ShouldSwitchConnection(IceSwitchReason reason,
const Connection* connection) {
return ShouldSwitchConnection(
IceControllerEvent::FromIceSwitchReason(reason), connection);
[[deprecated("bugs.webrtc.org/14125")]] virtual SwitchResult
ShouldSwitchConnection(IceControllerEvent reason,
const Connection* connection) {
return ShouldSwitchConnection(IceControllerEvent::FromType(reason.type),
connection);
}
virtual SwitchResult ShouldSwitchConnection(IceSwitchReason reason,
const Connection* connection) = 0;
// Sort connections and check if we should switch.
virtual SwitchResult SortAndSwitchConnection(IceControllerEvent reason) = 0;
virtual SwitchResult SortAndSwitchConnection(IceSwitchReason reason) {
return SortAndSwitchConnection(
IceControllerEvent::FromIceSwitchReason(reason));
[[deprecated("bugs.webrtc.org/14125")]] virtual SwitchResult
SortAndSwitchConnection(IceControllerEvent reason) {
return SortAndSwitchConnection(IceControllerEvent::FromType(reason.type));
}
virtual SwitchResult SortAndSwitchConnection(IceSwitchReason reason) = 0;
// Prune connections.
virtual std::vector<const Connection*> PruneConnections() = 0;

42
p2p/base/p2p_transport_channel.cc
View File

@ -284,7 +284,7 @@ void P2PTransportChannel::AddConnection(Connection* connection) {
bool P2PTransportChannel::MaybeSwitchSelectedConnection(
Connection* new_connection,
IceControllerEvent reason) {
IceSwitchReason reason) {
RTC_DCHECK_RUN_ON(network_thread_);
return MaybeSwitchSelectedConnection(
@ -292,12 +292,12 @@ bool P2PTransportChannel::MaybeSwitchSelectedConnection(
}
bool P2PTransportChannel::MaybeSwitchSelectedConnection(
IceControllerEvent reason,
IceSwitchReason reason,
IceControllerInterface::SwitchResult result) {
RTC_DCHECK_RUN_ON(network_thread_);
if (result.connection.has_value()) {
RTC_LOG(LS_INFO) << "Switching selected connection due to: "
<< reason.ToString();
<< IceSwitchReasonToString(reason);
SwitchSelectedConnection(FromIceController(*result.connection), reason);
}
@ -308,8 +308,8 @@ bool P2PTransportChannel::MaybeSwitchSelectedConnection(
// to be switched at a later time.
network_thread_->PostDelayedTask(
ToQueuedTask(task_safety_,
[this, recheck = *result.recheck_event]() {
SortConnectionsAndUpdateState(recheck);
[this, reason = result.recheck_event->reason]() {
SortConnectionsAndUpdateState(reason);
}),
result.recheck_event->recheck_delay_ms);
}
@ -521,7 +521,7 @@ void P2PTransportChannel::SetRemoteIceParameters(
}
// Updating the remote ICE candidate generation could change the sort order.
RequestSortAndStateUpdate(
IceControllerEvent::REMOTE_CANDIDATE_GENERATION_CHANGE);
IceSwitchReason::REMOTE_CANDIDATE_GENERATION_CHANGE);
}
void P2PTransportChannel::SetRemoteIceMode(IceMode mode) {
@ -675,7 +675,7 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) {
if (config_.network_preference != config.network_preference) {
config_.network_preference = config.network_preference;
RequestSortAndStateUpdate(IceControllerEvent::NETWORK_PREFERENCE_CHANGE);
RequestSortAndStateUpdate(IceSwitchReason::NETWORK_PREFERENCE_CHANGE);
RTC_LOG(LS_INFO) << "Set network preference to "
<< (config_.network_preference.has_value()
? config_.network_preference.value()
@ -979,7 +979,7 @@ void P2PTransportChannel::OnPortReady(PortAllocatorSession* session,
}
SortConnectionsAndUpdateState(
IceControllerEvent::NEW_CONNECTION_FROM_LOCAL_CANDIDATE);
IceSwitchReason::NEW_CONNECTION_FROM_LOCAL_CANDIDATE);
}
// A new candidate is available, let listeners know
@ -1159,7 +1159,7 @@ void P2PTransportChannel::OnUnknownAddress(PortInterface* port,
// after sending the response since it could (in principle) delete the
// connection in question.
SortConnectionsAndUpdateState(
IceControllerEvent::NEW_CONNECTION_FROM_UNKNOWN_REMOTE_ADDRESS);
IceSwitchReason::NEW_CONNECTION_FROM_UNKNOWN_REMOTE_ADDRESS);
}
void P2PTransportChannel::OnCandidateFilterChanged(uint32_t prev_filter,
@ -1211,11 +1211,10 @@ void P2PTransportChannel::OnNominated(Connection* conn) {
// TODO(qingsi): RequestSortAndStateUpdate will eventually call
// MaybeSwitchSelectedConnection again. Rewrite this logic.
if (MaybeSwitchSelectedConnection(
conn, IceControllerEvent::NOMINATION_ON_CONTROLLED_SIDE)) {
conn, IceSwitchReason::NOMINATION_ON_CONTROLLED_SIDE)) {
// Now that we have selected a connection, it is time to prune other
// connections and update the read/write state of the channel.
RequestSortAndStateUpdate(
IceControllerEvent::NOMINATION_ON_CONTROLLED_SIDE);
RequestSortAndStateUpdate(IceSwitchReason::NOMINATION_ON_CONTROLLED_SIDE);
} else {
RTC_LOG(LS_INFO)
<< "Not switching the selected connection on controlled side yet: "
@ -1365,7 +1364,7 @@ void P2PTransportChannel::FinishAddingRemoteCandidate(
// Resort the connections list, which may have new elements.
SortConnectionsAndUpdateState(
IceControllerEvent::NEW_CONNECTION_FROM_REMOTE_CANDIDATE);
IceSwitchReason::NEW_CONNECTION_FROM_REMOTE_CANDIDATE);
}
void P2PTransportChannel::RemoveRemoteCandidate(
@ -1718,7 +1717,7 @@ void P2PTransportChannel::UpdateConnectionStates() {
// Prepare for best candidate sorting.
void P2PTransportChannel::RequestSortAndStateUpdate(
IceControllerEvent reason_to_sort) {
IceSwitchReason reason_to_sort) {
RTC_DCHECK_RUN_ON(network_thread_);
if (!sort_dirty_) {
network_thread_->PostTask(
@ -1768,7 +1767,7 @@ bool P2PTransportChannel::PresumedWritable(const Connection* conn) const {
// Sort the available connections to find the best one. We also monitor
// the number of available connections and the current state.
void P2PTransportChannel::SortConnectionsAndUpdateState(
IceControllerEvent reason_to_sort) {
IceSwitchReason reason_to_sort) {
RTC_DCHECK_RUN_ON(network_thread_);
// Make sure the connection states are up-to-date since this affects how they
@ -1850,7 +1849,7 @@ rtc::NetworkRoute P2PTransportChannel::ConfigureNetworkRoute(
// Change the selected connection, and let listeners know.
void P2PTransportChannel::SwitchSelectedConnection(Connection* conn,
IceControllerEvent reason) {
IceSwitchReason reason) {
RTC_DCHECK_RUN_ON(network_thread_);
// Note: if conn is NULL, the previous `selected_connection_` has been
// destroyed, so don't use it.
@ -1899,7 +1898,7 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn,
// Create event for candidate pair change.
if (selected_connection_) {
CandidatePairChangeEvent pair_change;
pair_change.reason = reason.ToString();
pair_change.reason = IceSwitchReasonToString(reason);
pair_change.selected_candidate_pair = *GetSelectedCandidatePair();
pair_change.last_data_received_ms =
selected_connection_->last_data_received();
@ -2029,7 +2028,7 @@ void P2PTransportChannel::MaybeStopPortAllocatorSessions() {
// RTC_RUN_ON(network_thread_)
void P2PTransportChannel::OnSelectedConnectionDestroyed() {
RTC_LOG(LS_INFO) << "Selected connection destroyed. Will choose a new one.";
IceControllerEvent reason = IceControllerEvent::SELECTED_CONNECTION_DESTROYED;
IceSwitchReason reason = IceSwitchReason::SELECTED_CONNECTION_DESTROYED;
SwitchSelectedConnection(nullptr, reason);
RequestSortAndStateUpdate(reason);
}
@ -2159,8 +2158,8 @@ void P2PTransportChannel::OnConnectionStateChange(Connection* connection) {
// We have to unroll the stack before doing this because we may be changing
// the state of connections while sorting.
RequestSortAndStateUpdate(
IceControllerEvent::CONNECT_STATE_CHANGE); // "candidate pair state
// changed");
IceSwitchReason::CONNECT_STATE_CHANGE); // "candidate pair state
// changed");
}
// When a connection is removed, edit it out, and then update our best
@ -2289,8 +2288,7 @@ void P2PTransportChannel::OnReadPacket(Connection* connection,
// May need to switch the sending connection based on the receiving media path
// if this is the controlled side.
if (ice_role_ == ICEROLE_CONTROLLED) {
MaybeSwitchSelectedConnection(connection,
IceControllerEvent::DATA_RECEIVED);
MaybeSwitchSelectedConnection(connection, IceSwitchReason::DATA_RECEIVED);
}
}

11
p2p/base/p2p_transport_channel.h
View File

@ -49,6 +49,7 @@
#include "p2p/base/connection.h"
#include "p2p/base/ice_controller_factory_interface.h"
#include "p2p/base/ice_controller_interface.h"
#include "p2p/base/ice_switch_reason.h"
#include "p2p/base/ice_transport_internal.h"
#include "p2p/base/p2p_constants.h"
#include "p2p/base/p2p_transport_channel_ice_field_trials.h"
@ -257,16 +258,16 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal {
bool ReadyToSend(const Connection* connection) const;
bool PresumedWritable(const Connection* conn) const;
void UpdateConnectionStates();
void RequestSortAndStateUpdate(IceControllerEvent reason_to_sort);
void RequestSortAndStateUpdate(IceSwitchReason reason_to_sort);
// Start pinging if we haven't already started, and we now have a connection
// that's pingable.
void MaybeStartPinging();
void SortConnectionsAndUpdateState(IceControllerEvent reason_to_sort);
void SortConnectionsAndUpdateState(IceSwitchReason reason_to_sort);
void SortConnections();
void SortConnectionsIfNeeded();
rtc::NetworkRoute ConfigureNetworkRoute(const Connection* conn);
void SwitchSelectedConnection(Connection* conn, IceControllerEvent reason);
void SwitchSelectedConnection(Connection* conn, IceSwitchReason reason);
void UpdateState();
void HandleAllTimedOut();
void MaybeStopPortAllocatorSessions();
@ -341,9 +342,9 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal {
// Returns true if the new_connection is selected for transmission.
bool MaybeSwitchSelectedConnection(Connection* new_connection,
IceControllerEvent reason);
IceSwitchReason reason);
bool MaybeSwitchSelectedConnection(
IceControllerEvent reason,
IceSwitchReason reason,
IceControllerInterface::SwitchResult result);
void PruneConnections();

6
p2p/base/p2p_transport_channel_unittest.cc
View File

@ -6097,15 +6097,13 @@ class ForgetLearnedStateController : public cricket::BasicIceController {
const cricket::IceControllerFactoryArgs& args)
: cricket::BasicIceController(args) {}
SwitchResult SortAndSwitchConnection(IceControllerEvent reason) override {
SwitchResult SortAndSwitchConnection(IceSwitchReason reason) override {
auto result = cricket::BasicIceController::SortAndSwitchConnection(reason);
if (forget_connnection_) {
result.connections_to_forget_state_on.push_back(forget_connnection_);
forget_connnection_ = nullptr;
}
result.recheck_event =
IceControllerEvent(IceControllerEvent::ICE_CONTROLLER_RECHECK);
result.recheck_event->recheck_delay_ms = 100;
result.recheck_event.emplace(IceSwitchReason::ICE_CONTROLLER_RECHECK, 100);
return result;
}