Handle extended route information in TransportFeedbackAdapter.
Instead of passing only the local- and remote network IDs the whole
NetworkRoute is forwarded to TransportFeedbackAdapter that can then
use more detailed information to distinguish routes.
Bug: webrtc:11434
Change-Id: I48f36aa1177822d20c2b556dcc2275f6145ed845
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/171581
Commit-Queue: Christoffer Rodbro <crodbro@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30895}
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index 42b92f9..5877291 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -63,6 +63,15 @@
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 @@
// 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 @@
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::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<BitrateConstraints> 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 @@
absl::optional<BitrateConstraints> 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 0e71cb6..9671ba7 100644
--- a/call/rtp_transport_controller_send.h
+++ b/call/rtp_transport_controller_send.h
@@ -105,7 +105,7 @@
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 @@
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 38a4bf1..b444f54 100644
--- a/modules/congestion_controller/rtp/BUILD.gn
+++ b/modules/congestion_controller/rtp/BUILD.gn
@@ -54,6 +54,7 @@
"../../../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 d2256ea..87691bf 100644
--- a/modules/congestion_controller/rtp/transport_feedback_adapter.cc
+++ b/modules/congestion_controller/rtp/transport_feedback_adapter.cc
@@ -31,12 +31,11 @@
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 @@
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 @@
}
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 @@
}
}
+// 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 @@
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 @@
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 @@
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<PacketResult>
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 @@
// 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 @@
// 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 c8ff9b9..b8148a2 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 @@
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<uint16_t, uint16_t>;
- std::map<RemoteAndLocalNetworkId, DataSize> in_flight_data_;
+ struct NetworkRouteComparator {
+ bool operator()(const rtc::NetworkRoute& a,
+ const rtc::NetworkRoute& b) const;
+ };
+ std::map<rtc::NetworkRoute, DataSize, NetworkRouteComparator> in_flight_data_;
};
class TransportFeedbackAdapter {
@@ -64,9 +67,9 @@
absl::optional<TransportPacketsFeedback> 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 @@
std::vector<PacketResult> 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 @@
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
