From c9f42ad909c07dc96e56d625cf89cdf5b701364d Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Fri, 17 Jan 2020 11:47:35 +0100 Subject: [PATCH] Simplifies transport overhead mechanism in Scenario test framework. This changes the behavior for adding virtual transport overhead so it doesn't change the size of the actual payload buffer, only the calculated packet size. Bug: webrtc:9883 Change-Id: I6e24598378c4dd6a591d36ca3b162e933ff4ef7c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/164523 Commit-Queue: Sebastian Jansson Reviewed-by: Artem Titov Cr-Commit-Position: refs/heads/master@{#30298} --- .../network_emulation_interfaces.cc | 14 +++++++++----- .../network_emulation_interfaces.h | 11 ++++------- .../bbr/bbr_network_controller_unittest.cc | 7 ++++--- .../goog_cc/goog_cc_network_control_unittest.cc | 2 +- .../pcc/pcc_network_controller_unittest.cc | 2 +- test/scenario/call_client.cc | 6 ------ test/scenario/call_client.h | 3 --- test/scenario/network_node.cc | 11 +++++------ test/scenario/scenario.cc | 1 - test/scenario/scenario_config.h | 5 +---- test/scenario/stats_collection_unittest.cc | 2 +- 11 files changed, 26 insertions(+), 38 deletions(-) diff --git a/api/test/network_emulation/network_emulation_interfaces.cc b/api/test/network_emulation/network_emulation_interfaces.cc index 50f8bed151..e023334af7 100644 --- a/api/test/network_emulation/network_emulation_interfaces.cc +++ b/api/test/network_emulation/network_emulation_interfaces.cc @@ -12,19 +12,23 @@ namespace webrtc { namespace { -constexpr size_t kIPv4HeaderSize = 20; -constexpr size_t kIPv6HeaderSize = 40; +constexpr int kIPv4HeaderSize = 20; +constexpr int kIPv6HeaderSize = 40; +constexpr int kUdpHeaderSize = 8; +int IpHeaderSize(const rtc::SocketAddress& address) { + return (address.family() == AF_INET) ? kIPv4HeaderSize : kIPv6HeaderSize; +} } // namespace EmulatedIpPacket::EmulatedIpPacket(const rtc::SocketAddress& from, const rtc::SocketAddress& to, rtc::CopyOnWriteBuffer data, - Timestamp arrival_time) + Timestamp arrival_time, + uint16_t application_overhead) : from(from), to(to), data(data), - ip_header_size((to.family() == AF_INET) ? kIPv4HeaderSize - : kIPv6HeaderSize), + headers_size(IpHeaderSize(to) + application_overhead + kUdpHeaderSize), arrival_time(arrival_time) { RTC_DCHECK(to.family() == AF_INET || to.family() == AF_INET6); } diff --git a/api/test/network_emulation/network_emulation_interfaces.h b/api/test/network_emulation/network_emulation_interfaces.h index 35ebabc005..5d75bf354a 100644 --- a/api/test/network_emulation/network_emulation_interfaces.h +++ b/api/test/network_emulation/network_emulation_interfaces.h @@ -22,12 +22,11 @@ namespace webrtc { struct EmulatedIpPacket { public: - static constexpr int kUdpHeaderSize = 8; - EmulatedIpPacket(const rtc::SocketAddress& from, const rtc::SocketAddress& to, rtc::CopyOnWriteBuffer data, - Timestamp arrival_time); + Timestamp arrival_time, + uint16_t application_overhead = 0); ~EmulatedIpPacket() = default; // This object is not copyable or assignable. EmulatedIpPacket(const EmulatedIpPacket&) = delete; @@ -39,14 +38,12 @@ struct EmulatedIpPacket { size_t size() const { return data.size(); } const uint8_t* cdata() const { return data.cdata(); } - size_t ip_packet_size() const { - return size() + kUdpHeaderSize + ip_header_size; - } + size_t ip_packet_size() const { return size() + headers_size; } rtc::SocketAddress from; rtc::SocketAddress to; // Holds the UDP payload. rtc::CopyOnWriteBuffer data; - int ip_header_size; + uint16_t headers_size; Timestamp arrival_time; }; diff --git a/modules/congestion_controller/bbr/bbr_network_controller_unittest.cc b/modules/congestion_controller/bbr/bbr_network_controller_unittest.cc index 8cf4d17a9f..3e5403a313 100644 --- a/modules/congestion_controller/bbr/bbr_network_controller_unittest.cc +++ b/modules/congestion_controller/bbr/bbr_network_controller_unittest.cc @@ -132,9 +132,10 @@ TEST_F(BbrNetworkControllerTest, UpdatesTargetSendRate) { auto ret_net = s.CreateMutableSimulationNode( [](NetworkSimulationConfig* c) { c->delay = TimeDelta::ms(100); }); auto* client = s.CreateClient("send", config); - auto routes = s.CreateRoutes(client, {send_net->node()}, + const DataSize kOverhead = DataSize::bytes(38); // IPV4 + UDP + SRTP + auto routes = s.CreateRoutes(client, {send_net->node()}, kOverhead, s.CreateClient("recv", CallClientConfig()), - {ret_net->node()}); + {ret_net->node()}, kOverhead); s.CreateVideoStream(routes->forward(), VideoStreamConfig()); s.RunFor(TimeDelta::seconds(25)); @@ -156,7 +157,7 @@ TEST_F(BbrNetworkControllerTest, UpdatesTargetSendRate) { [](NetworkSimulationConfig* c) { c->delay = TimeDelta::ms(200); }); s.RunFor(TimeDelta::seconds(35)); - EXPECT_NEAR(client->send_bandwidth().kbps(), 180, 50); + EXPECT_NEAR(client->send_bandwidth().kbps(), 170, 50); } } // namespace test diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc index 0227027954..09aec436c1 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc @@ -410,7 +410,7 @@ TEST_F(GoogCcNetworkControllerTest, LimitsToFloorIfRttIsHighInTrial) { // This will cause the RTT to be large for a while. s.TriggerPacketBurst({send_net}, kBloatPacketCount, kBloatPacketSize.bytes()); // Wait to allow the high RTT to be detected and acted upon. - s.RunFor(TimeDelta::seconds(4)); + s.RunFor(TimeDelta::seconds(6)); // By now the target rate should have dropped to the minimum configured rate. EXPECT_NEAR(client->target_rate().kbps(), kBandwidthFloor.kbps(), 5); } diff --git a/modules/congestion_controller/pcc/pcc_network_controller_unittest.cc b/modules/congestion_controller/pcc/pcc_network_controller_unittest.cc index 2383378ccc..9910a03322 100644 --- a/modules/congestion_controller/pcc/pcc_network_controller_unittest.cc +++ b/modules/congestion_controller/pcc/pcc_network_controller_unittest.cc @@ -109,7 +109,7 @@ TEST(PccNetworkControllerTest, UpdatesTargetSendRate) { ret_net->UpdateConfig( [](NetworkSimulationConfig* c) { c->delay = TimeDelta::ms(200); }); s.RunFor(TimeDelta::seconds(35)); - EXPECT_NEAR(client->target_rate().kbps(), 180, 40); + EXPECT_NEAR(client->target_rate().kbps(), 170, 50); } } // namespace test diff --git a/test/scenario/call_client.cc b/test/scenario/call_client.cc index 5823656543..61612c5a04 100644 --- a/test/scenario/call_client.cc +++ b/test/scenario/call_client.cc @@ -263,12 +263,6 @@ DataRate CallClient::padding_rate() const { } void CallClient::OnPacketReceived(EmulatedIpPacket packet) { - // Removes added overhead before delivering packet to sender. - size_t size = - packet.data.size() - route_overhead_.at(packet.to.ipaddr()).bytes(); - RTC_DCHECK_GE(size, 0); - packet.data.SetSize(size); - MediaType media_type = MediaType::ANY; if (!RtpHeaderParser::IsRtcp(packet.cdata(), packet.data.size())) { auto ssrc = RtpHeaderParser::GetSsrc(packet.cdata(), packet.data.size()); diff --git a/test/scenario/call_client.h b/test/scenario/call_client.h index 34a15c18fa..a4c04affb1 100644 --- a/test/scenario/call_client.h +++ b/test/scenario/call_client.h @@ -141,9 +141,6 @@ class CallClient : public EmulatedNetworkReceiverInterface { std::unique_ptr transport_; std::unique_ptr const header_parser_; - // Stores the configured overhead per known destination endpoint. This is used - // to subtract the overhead before processing. - std::map route_overhead_; int next_video_ssrc_index_ = 0; int next_video_local_ssrc_index_ = 0; int next_rtx_ssrc_index_ = 0; diff --git a/test/scenario/network_node.cc b/test/scenario/network_node.cc index a33df83012..c430b876de 100644 --- a/test/scenario/network_node.cc +++ b/test/scenario/network_node.cc @@ -89,11 +89,10 @@ bool NetworkNodeTransport::SendRtp(const uint8_t* packet, rtc::CritScope crit(&crit_sect_); if (!send_net_) return false; - rtc::CopyOnWriteBuffer buffer(packet, length, - length + packet_overhead_.bytes()); - buffer.SetSize(length + packet_overhead_.bytes()); + rtc::CopyOnWriteBuffer buffer(packet, length); send_net_->OnPacketReceived( - EmulatedIpPacket(local_address_, receiver_address_, buffer, send_time)); + EmulatedIpPacket(local_address_, receiver_address_, buffer, send_time, + packet_overhead_.bytes())); return true; } @@ -101,11 +100,11 @@ bool NetworkNodeTransport::SendRtcp(const uint8_t* packet, size_t length) { rtc::CopyOnWriteBuffer buffer(packet, length); Timestamp send_time = sender_clock_->CurrentTime(); rtc::CritScope crit(&crit_sect_); - buffer.SetSize(length + packet_overhead_.bytes()); if (!send_net_) return false; send_net_->OnPacketReceived( - EmulatedIpPacket(local_address_, receiver_address_, buffer, send_time)); + EmulatedIpPacket(local_address_, receiver_address_, buffer, send_time, + packet_overhead_.bytes())); return true; } diff --git a/test/scenario/scenario.cc b/test/scenario/scenario.cc index ad382bdb4c..9d27e6a4e2 100644 --- a/test/scenario/scenario.cc +++ b/test/scenario/scenario.cc @@ -166,7 +166,6 @@ void Scenario::ChangeRoute(std::pair clients, std::vector over_nodes, DataSize overhead) { rtc::IPAddress route_ip(next_route_id_++); - clients.second->route_overhead_.insert({route_ip, overhead}); EmulatedNetworkNode::CreateRoute(route_ip, over_nodes, clients.second); clients.first->transport_->Connect(over_nodes.front(), route_ip, overhead); } diff --git a/test/scenario/scenario_config.h b/test/scenario/scenario_config.h index 7b9c633e14..282d47188f 100644 --- a/test/scenario/scenario_config.h +++ b/test/scenario/scenario_config.h @@ -28,9 +28,6 @@ namespace webrtc { namespace test { struct PacketOverhead { - static constexpr size_t kIpv4 = 20; - static constexpr size_t kIpv6 = 40; - static constexpr size_t kUdp = 8; static constexpr size_t kSrtp = 10; static constexpr size_t kStun = 4; // TURN messages can be sent either with or without an establieshed channel. @@ -38,7 +35,7 @@ struct PacketOverhead { // significantly more overhead. static constexpr size_t kTurnChannelMessage = 4; static constexpr size_t kTurnIndicationMessage = 36; - static constexpr size_t kDefault = kIpv4 + kUdp + kSrtp; + static constexpr size_t kDefault = kSrtp; }; struct TransportControllerConfig { struct Rates { diff --git a/test/scenario/stats_collection_unittest.cc b/test/scenario/stats_collection_unittest.cc index 6ea03c6a5b..4159eeac7f 100644 --- a/test/scenario/stats_collection_unittest.cc +++ b/test/scenario/stats_collection_unittest.cc @@ -78,7 +78,7 @@ TEST(ScenarioAnalyzerTest, PsnrIsLowWhenNetworkIsBad) { EXPECT_NEAR(stats.call.stats().target_rate.Mean().kbps(), 75, 50); EXPECT_NEAR(stats.video_send.stats().media_bitrate.Mean().kbps(), 100, 50); EXPECT_NEAR(stats.video_receive.stats().resolution.Mean(), 180, 10); - EXPECT_NEAR(stats.audio_receive.stats().jitter_buffer.Mean().ms(), 150, 130); + EXPECT_NEAR(stats.audio_receive.stats().jitter_buffer.Mean().ms(), 200, 150); } TEST(ScenarioAnalyzerTest, CountsCapturedButNotRendered) {