diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 23f9b91483..143e0fe677 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -396,6 +396,10 @@ void RtpTransportControllerSend::OnNetworkRouteChanged( env_.event_log().Log(std::make_unique( network_route.connected, network_route.packet_overhead)); + if (transport_maybe_support_ecn_) { + sending_packets_as_ect1_ = true; + packet_router_.ConfigureForRfc8888Feedback(sending_packets_as_ect1_); + } NetworkRouteChange msg; msg.at_time = Timestamp::Millis(env_.clock().TimeInMilliseconds()); msg.constraints = ConvertConstraints(bitrate_config, &env_.clock()); @@ -638,8 +642,9 @@ void RtpTransportControllerSend::NotifyBweOfPacedSentPacket( void RtpTransportControllerSend:: EnableCongestionControlFeedbackAccordingToRfc8888() { RTC_DCHECK_RUN_ON(&sequence_checker_); - transport_is_ecn_capable_ = true; - packet_router_.ConfigureForRfc8888Feedback(transport_is_ecn_capable_); + transport_maybe_support_ecn_ = true; + sending_packets_as_ect1_ = true; + packet_router_.ConfigureForRfc8888Feedback(sending_packets_as_ect1_); } void RtpTransportControllerSend::OnTransportFeedback( @@ -674,15 +679,15 @@ void RtpTransportControllerSend::OnCongestionControlFeedback( void RtpTransportControllerSend::HandleTransportPacketsFeedback( const TransportPacketsFeedback& feedback) { - if (transport_is_ecn_capable_) { + if (sending_packets_as_ect1_) { // If transport does not support ECN, packets should not be sent as ECT(1). // TODO: bugs.webrtc.org/42225697 - adapt to ECN feedback and continue to // send packets as ECT(1) if transport is ECN capable. - transport_is_ecn_capable_ = false; + sending_packets_as_ect1_ = false; RTC_LOG(LS_INFO) << " Transport is " << (feedback.transport_supports_ecn ? "" : " not ") << " ECN capable. Stop sending ECT(1)."; - packet_router_.ConfigureForRfc8888Feedback(transport_is_ecn_capable_); + packet_router_.ConfigureForRfc8888Feedback(sending_packets_as_ect1_); } if (controller_) PostUpdates(controller_->OnTransportPacketsFeedback(feedback)); diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index 1261706b50..20908a9f80 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -242,7 +242,9 @@ class RtpTransportControllerSend final DataSize congestion_window_size_ RTC_GUARDED_BY(sequence_checker_); bool is_congested_ RTC_GUARDED_BY(sequence_checker_); - bool transport_is_ecn_capable_ = false; + bool transport_maybe_support_ecn_ = + false; // True if RFC8888 has been negotiated. + bool sending_packets_as_ect1_ = false; // Count of feedback messages received. int feedback_count_ RTC_GUARDED_BY(sequence_checker_) = 0; int transport_cc_feedback_count_ RTC_GUARDED_BY(sequence_checker_) = 0; diff --git a/test/peer_scenario/tests/l4s_test.cc b/test/peer_scenario/tests/l4s_test.cc index 69c4978147..614b26d71d 100644 --- a/test/peer_scenario/tests/l4s_test.cc +++ b/test/peer_scenario/tests/l4s_test.cc @@ -276,9 +276,95 @@ TEST(L4STest, SendsEct1UntilFirstFeedback) { // Wait for first feedback where packets have been sent with ECT(1). Then // feedback for packets sent as not ECT since currently webrtc does not // implement adaptation to ECN. - s.WaitAndProcess(&seen_ect1_feedback, TimeDelta::Seconds(1)); + EXPECT_TRUE(s.WaitAndProcess(&seen_ect1_feedback, TimeDelta::Seconds(1))); EXPECT_FALSE(seen_not_ect_feedback); - s.WaitAndProcess(&seen_not_ect_feedback, TimeDelta::Seconds(1)); + EXPECT_TRUE(s.WaitAndProcess(&seen_not_ect_feedback, TimeDelta::Seconds(1))); +} + +TEST(L4STest, SendsEct1AfterRouteChange) { + test::ScopedFieldTrials trials( + "WebRTC-RFC8888CongestionControlFeedback/Enabled/"); + PeerScenario s(*test_info_); + + PeerScenarioClient::Config config; + config.disable_encryption = true; + config.endpoints = {{0, {.type = rtc::AdapterType::ADAPTER_TYPE_WIFI}}}; + PeerScenarioClient* caller = s.CreateClient(config); + // Callee has booth wifi and cellular adapters. + config.endpoints = {{0, {.type = rtc::AdapterType::ADAPTER_TYPE_WIFI}}, + {1, {.type = rtc::AdapterType::ADAPTER_TYPE_CELLULAR}}}; + PeerScenarioClient* callee = s.CreateClient(config); + + // Create network path from caller to callee. + auto caller_to_callee = s.net()->NodeBuilder().Build().node; + auto callee_to_caller_wifi = s.net()->NodeBuilder().Build().node; + auto callee_to_caller_cellular = s.net()->NodeBuilder().Build().node; + s.net()->CreateRoute(caller->endpoint(0), {caller_to_callee}, + callee->endpoint(0)); + s.net()->CreateRoute(caller->endpoint(0), {caller_to_callee}, + callee->endpoint(1)); + s.net()->CreateRoute(callee->endpoint(0), {callee_to_caller_wifi}, + caller->endpoint(0)); + s.net()->CreateRoute(callee->endpoint(1), {callee_to_caller_cellular}, + caller->endpoint(0)); + + RtcpFeedbackCounter wifi_feedback_counter; + std::atomic seen_ect1_on_wifi_feedback = false; + std::atomic seen_not_ect_on_wifi_feedback = false; + callee_to_caller_wifi->router()->SetWatcher( + [&](const EmulatedIpPacket& packet) { + wifi_feedback_counter.Count(packet); + if (wifi_feedback_counter.ect1() > 0) { + seen_ect1_on_wifi_feedback = true; + RTC_LOG(LS_INFO) << " ect 1 feedback on wifi: " + << wifi_feedback_counter.ect1(); + } + if (wifi_feedback_counter.not_ect() > 0) { + seen_not_ect_on_wifi_feedback = true; + RTC_LOG(LS_INFO) << " not ect feedback on wifi: " + << wifi_feedback_counter.not_ect(); + } + }); + + auto signaling = s.ConnectSignaling(caller, callee, {caller_to_callee}, + {callee_to_caller_wifi}); + PeerScenarioClient::VideoSendTrackConfig video_conf; + video_conf.generator.squares_video->framerate = 15; + + caller->CreateVideo("VIDEO_1", video_conf); + signaling.StartIceSignaling(); + + std::atomic offer_exchange_done(false); + signaling.NegotiateSdp([&](const SessionDescriptionInterface& answer) { + offer_exchange_done = true; + }); + s.WaitAndProcess(&offer_exchange_done); + + // Wait for first feedback where packets have been sent with ECT(1). Then + // feedback for packets sent as not ECT since currently webrtc does not + // implement adaptation to ECN. + EXPECT_TRUE( + s.WaitAndProcess(&seen_ect1_on_wifi_feedback, TimeDelta::Seconds(1))); + EXPECT_FALSE(seen_not_ect_on_wifi_feedback); + EXPECT_TRUE( + s.WaitAndProcess(&seen_not_ect_on_wifi_feedback, TimeDelta::Seconds(1))); + + RtcpFeedbackCounter cellular_feedback_counter; + std::atomic seen_ect1_on_cellular_feedback = false; + callee_to_caller_cellular->router()->SetWatcher( + [&](const EmulatedIpPacket& packet) { + cellular_feedback_counter.Count(packet); + if (cellular_feedback_counter.ect1() > 0) { + seen_ect1_on_cellular_feedback = true; + RTC_LOG(LS_INFO) << " ect 1 feedback on cellular: " + << cellular_feedback_counter.ect1(); + } + }); + // Disable callees wifi and expect that the connection switch to cellular and + // sends packets with ECT(1) again. + s.net()->DisableEndpoint(callee->endpoint(0)); + EXPECT_TRUE( + s.WaitAndProcess(&seen_ect1_on_cellular_feedback, TimeDelta::Seconds(5))); } } // namespace