diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 42b92f9b40..58772910b9 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -63,6 +63,15 @@ bool IsEnabled(const WebRtcKeyValueConfig* trials, absl::string_view key) { return trials->Lookup(key).find("Enabled") == 0; } +bool IsRelevantRouteChange(const rtc::NetworkRoute& old_route, + const rtc::NetworkRoute& new_route) { + // TODO(bugs.webrtc.org/11438): Experiment with using more information/ + // other conditions. + return old_route.connected != new_route.connected || + old_route.local.network_id() != new_route.local.network_id() || + old_route.remote.network_id() != new_route.remote.network_id(); +} + } // namespace RtpTransportControllerSend::RtpTransportControllerSend( @@ -283,14 +292,7 @@ void RtpTransportControllerSend::OnNetworkRouteChanged( // 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()) { + if (IsRelevantRouteChange(old_route, network_route)) { BitrateConstraints bitrate_config = bitrate_configurator_.GetConfig(); RTC_LOG(LS_INFO) << "Reset bitrates to min: " << bitrate_config.min_bitrate_bps @@ -310,11 +312,7 @@ 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()); + transport_feedback_adapter_.SetNetworkRoute(network_route); } if (controller_) { PostUpdates(controller_->OnNetworkRouteChange(msg)); @@ -395,20 +393,25 @@ void RtpTransportControllerSend::OnReceivedPacket( }); } +void RtpTransportControllerSend::UpdateBitrateConstraints( + const BitrateConstraints& updated) { + TargetRateConstraints msg = ConvertConstraints(updated, clock_); + task_queue_.PostTask([this, msg]() { + RTC_DCHECK_RUN_ON(&task_queue_); + if (controller_) { + PostUpdates(controller_->OnTargetRateConstraints(msg)); + } else { + UpdateInitialConstraints(msg); + } + }); +} + void RtpTransportControllerSend::SetSdpBitrateParameters( const BitrateConstraints& constraints) { absl::optional updated = bitrate_configurator_.UpdateWithSdpParameters(constraints); if (updated.has_value()) { - TargetRateConstraints msg = ConvertConstraints(*updated, clock_); - task_queue_.PostTask([this, msg]() { - RTC_DCHECK_RUN_ON(&task_queue_); - if (controller_) { - PostUpdates(controller_->OnTargetRateConstraints(msg)); - } else { - UpdateInitialConstraints(msg); - } - }); + UpdateBitrateConstraints(*updated); } else { RTC_LOG(LS_VERBOSE) << "WebRTC.RtpTransportControllerSend.SetSdpBitrateParameters: " @@ -421,15 +424,7 @@ void RtpTransportControllerSend::SetClientBitratePreferences( absl::optional updated = bitrate_configurator_.UpdateWithClientPreferences(preferences); if (updated.has_value()) { - TargetRateConstraints msg = ConvertConstraints(*updated, clock_); - task_queue_.PostTask([this, msg]() { - RTC_DCHECK_RUN_ON(&task_queue_); - if (controller_) { - PostUpdates(controller_->OnTargetRateConstraints(msg)); - } else { - UpdateInitialConstraints(msg); - } - }); + UpdateBitrateConstraints(*updated); } else { RTC_LOG(LS_VERBOSE) << "WebRTC.RtpTransportControllerSend.SetClientBitratePreferences: " diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index 0e71cb652f..9671ba7522 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -105,7 +105,7 @@ class RtpTransportControllerSend final void SetClientBitratePreferences(const BitrateSettings& preferences) override; void OnTransportOverheadChanged( - size_t transport_overhead_per_packet) override; + size_t transport_overhead_bytes_per_packet) override; void AccountForAudioPacketsInPacedSender(bool account_for_audio) override; void IncludeOverheadInPacedSender() override; @@ -131,6 +131,7 @@ class RtpTransportControllerSend final void StartProcessPeriodicTasks() RTC_RUN_ON(task_queue_); void UpdateControllerWithTimeInterval() RTC_RUN_ON(task_queue_); + void UpdateBitrateConstraints(const BitrateConstraints& updated); void UpdateStreamsConfig() RTC_RUN_ON(task_queue_); void OnReceivedRtcpReceiverReportBlocks(const ReportBlockList& report_blocks, int64_t now_ms) diff --git a/modules/congestion_controller/rtp/BUILD.gn b/modules/congestion_controller/rtp/BUILD.gn index 38a4bf19df..b444f5495b 100644 --- a/modules/congestion_controller/rtp/BUILD.gn +++ b/modules/congestion_controller/rtp/BUILD.gn @@ -54,6 +54,7 @@ rtc_library("transport_feedback") { "../../../api/transport:network_control", "../../../api/units:data_size", "../../../api/units:timestamp", + "../../../rtc_base", "../../../rtc_base:checks", "../../../rtc_base:rtc_base_approved", "../../../rtc_base/network:sent_packet", diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter.cc b/modules/congestion_controller/rtp/transport_feedback_adapter.cc index d2256eae97..87691bf263 100644 --- a/modules/congestion_controller/rtp/transport_feedback_adapter.cc +++ b/modules/congestion_controller/rtp/transport_feedback_adapter.cc @@ -31,12 +31,11 @@ constexpr TimeDelta kSendTimeHistoryWindow = TimeDelta::Seconds(60); void InFlightBytesTracker::AddInFlightPacketBytes( const PacketFeedback& packet) { RTC_DCHECK(packet.sent.send_time.IsFinite()); - auto it = in_flight_data_.find({packet.local_net_id, packet.remote_net_id}); + auto it = in_flight_data_.find(packet.network_route); if (it != in_flight_data_.end()) { it->second += packet.sent.size; } else { - in_flight_data_.insert( - {{packet.local_net_id, packet.remote_net_id}, packet.sent.size}); + in_flight_data_.insert({packet.network_route, packet.sent.size}); } } @@ -44,7 +43,7 @@ void InFlightBytesTracker::RemoveInFlightPacketBytes( const PacketFeedback& packet) { if (packet.sent.send_time.IsInfinite()) return; - auto it = in_flight_data_.find({packet.local_net_id, packet.remote_net_id}); + auto it = in_flight_data_.find(packet.network_route); if (it != in_flight_data_.end()) { RTC_DCHECK_GE(it->second, packet.sent.size); it->second -= packet.sent.size; @@ -54,9 +53,8 @@ void InFlightBytesTracker::RemoveInFlightPacketBytes( } DataSize InFlightBytesTracker::GetOutstandingData( - uint16_t local_net_id, - uint16_t remote_net_id) const { - auto it = in_flight_data_.find({local_net_id, remote_net_id}); + const rtc::NetworkRoute& network_route) const { + auto it = in_flight_data_.find(network_route); if (it != in_flight_data_.end()) { return it->second; } else { @@ -64,6 +62,28 @@ DataSize InFlightBytesTracker::GetOutstandingData( } } +// Comparator for consistent map with NetworkRoute as key. +bool InFlightBytesTracker::NetworkRouteComparator::operator()( + const rtc::NetworkRoute& a, + const rtc::NetworkRoute& b) const { + if (a.local.network_id() != b.local.network_id()) + return a.local.network_id() < b.local.network_id(); + if (a.remote.network_id() != b.remote.network_id()) + return a.remote.network_id() < b.remote.network_id(); + + if (a.local.adapter_id() != b.local.adapter_id()) + return a.local.adapter_id() < b.local.adapter_id(); + if (a.remote.adapter_id() != b.remote.adapter_id()) + return a.remote.adapter_id() < b.remote.adapter_id(); + + if (a.local.uses_turn() != b.local.uses_turn()) + return a.local.uses_turn() < b.local.uses_turn(); + if (a.remote.uses_turn() != b.remote.uses_turn()) + return a.remote.uses_turn() < b.remote.uses_turn(); + + return a.connected < b.connected; +} + TransportFeedbackAdapter::TransportFeedbackAdapter() = default; @@ -76,8 +96,7 @@ void TransportFeedbackAdapter::AddPacket(const RtpPacketSendInfo& packet_info, seq_num_unwrapper_.Unwrap(packet_info.transport_sequence_number); packet.sent.size = DataSize::Bytes(packet_info.length + overhead_bytes); packet.sent.audio = packet_info.packet_type == RtpPacketMediaType::kAudio; - packet.local_net_id = local_net_id_; - packet.remote_net_id = remote_net_id_; + packet.network_route = network_route_; packet.sent.pacing_info = packet_info.pacing_info; while (!history_.empty() && @@ -142,8 +161,7 @@ TransportFeedbackAdapter::ProcessTransportFeedback( TransportPacketsFeedback msg; msg.feedback_time = feedback_receive_time; - msg.prior_in_flight = - in_flight_.GetOutstandingData(local_net_id_, remote_net_id_); + msg.prior_in_flight = in_flight_.GetOutstandingData(network_route_); msg.packet_feedbacks = ProcessTransportFeedbackInner(feedback, feedback_receive_time); if (msg.packet_feedbacks.empty()) @@ -153,31 +171,29 @@ TransportFeedbackAdapter::ProcessTransportFeedback( if (it != history_.end()) { msg.first_unacked_send_time = it->second.sent.send_time; } - msg.data_in_flight = - in_flight_.GetOutstandingData(local_net_id_, remote_net_id_); + msg.data_in_flight = in_flight_.GetOutstandingData(network_route_); return msg; } -void TransportFeedbackAdapter::SetNetworkIds(uint16_t local_id, - uint16_t remote_id) { - local_net_id_ = local_id; - remote_net_id_ = remote_id; +void TransportFeedbackAdapter::SetNetworkRoute( + const rtc::NetworkRoute& network_route) { + network_route_ = network_route; } DataSize TransportFeedbackAdapter::GetOutstandingData() const { - return in_flight_.GetOutstandingData(local_net_id_, remote_net_id_); + return in_flight_.GetOutstandingData(network_route_); } std::vector TransportFeedbackAdapter::ProcessTransportFeedbackInner( const rtcp::TransportFeedback& feedback, - Timestamp feedback_time) { + Timestamp feedback_receive_time) { // Add timestamp deltas to a local time base selected on first packet arrival. // This won't be the true time base, but makes it easier to manually inspect // time stamps. if (last_timestamp_.IsInfinite()) { - current_offset_ = feedback_time; + current_offset_ = feedback_receive_time; } else { // TODO(srte): We shouldn't need to do rounding here. const TimeDelta delta = feedback.GetBaseDelta(last_timestamp_) @@ -185,7 +201,7 @@ TransportFeedbackAdapter::ProcessTransportFeedbackInner( // Protect against assigning current_offset_ negative value. if (delta < Timestamp::Zero() - current_offset_) { RTC_LOG(LS_WARNING) << "Unexpected feedback timestamp received."; - current_offset_ = feedback_time; + current_offset_ = feedback_receive_time; } else { current_offset_ += delta; } @@ -234,8 +250,7 @@ TransportFeedbackAdapter::ProcessTransportFeedbackInner( // reported as received by a later feedback. history_.erase(it); } - if (packet_feedback.local_net_id == local_net_id_ && - packet_feedback.remote_net_id == remote_net_id_) { + if (packet_feedback.network_route == network_route_) { PacketResult result; result.sent_packet = packet_feedback.sent; result.receive_time = packet_feedback.receive_time; diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter.h b/modules/congestion_controller/rtp/transport_feedback_adapter.h index c8ff9b9db5..b8148a252f 100644 --- a/modules/congestion_controller/rtp/transport_feedback_adapter.h +++ b/modules/congestion_controller/rtp/transport_feedback_adapter.h @@ -21,6 +21,7 @@ #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/critical_section.h" #include "rtc_base/network/sent_packet.h" +#include "rtc_base/network_route.h" #include "rtc_base/thread_annotations.h" #include "rtc_base/thread_checker.h" @@ -32,24 +33,26 @@ struct PacketFeedback { Timestamp creation_time = Timestamp::MinusInfinity(); SentPacket sent; // Time corresponding to when the packet was received. Timestamped with the - // receiver's clock. For unreceived packet, Timestamp::PlusInfinity() is used. + // receiver's clock. For unreceived packet, Timestamp::PlusInfinity() is + // used. Timestamp receive_time = Timestamp::PlusInfinity(); - // The network route ids that this packet is associated with. - uint16_t local_net_id = 0; - uint16_t remote_net_id = 0; + // The network route that this packet is associated with. + rtc::NetworkRoute network_route; }; class InFlightBytesTracker { public: void AddInFlightPacketBytes(const PacketFeedback& packet); void RemoveInFlightPacketBytes(const PacketFeedback& packet); - DataSize GetOutstandingData(uint16_t local_net_id, - uint16_t remote_net_id) const; + DataSize GetOutstandingData(const rtc::NetworkRoute& network_route) const; private: - using RemoteAndLocalNetworkId = std::pair; - std::map in_flight_data_; + struct NetworkRouteComparator { + bool operator()(const rtc::NetworkRoute& a, + const rtc::NetworkRoute& b) const; + }; + std::map in_flight_data_; }; class TransportFeedbackAdapter { @@ -64,9 +67,9 @@ class TransportFeedbackAdapter { absl::optional ProcessTransportFeedback( const rtcp::TransportFeedback& feedback, - Timestamp feedback_time); + Timestamp feedback_receive_time); - void SetNetworkIds(uint16_t local_id, uint16_t remote_id); + void SetNetworkRoute(const rtc::NetworkRoute& network_route); DataSize GetOutstandingData() const; @@ -75,7 +78,7 @@ class TransportFeedbackAdapter { std::vector ProcessTransportFeedbackInner( const rtcp::TransportFeedback& feedback, - Timestamp feedback_time); + Timestamp feedback_receive_time); DataSize pending_untracked_size_ = DataSize::Zero(); Timestamp last_send_time_ = Timestamp::MinusInfinity(); @@ -91,8 +94,7 @@ class TransportFeedbackAdapter { Timestamp current_offset_ = Timestamp::MinusInfinity(); TimeDelta last_timestamp_ = TimeDelta::MinusInfinity(); - uint16_t local_net_id_ = 0; - uint16_t remote_net_id_ = 0; + rtc::NetworkRoute network_route_; }; } // namespace webrtc