From 6d815bdd9b6028bac504ff94cbdfe4269d5b6348 Mon Sep 17 00:00:00 2001 From: Sun Shin Date: Fri, 18 Oct 2024 02:13:04 -0700 Subject: [PATCH] Let the existing TransportFeedback work with RFC8888 congesting control In the `MaybeProcess` method, a new variable `time_until_cc_rep` is introduced to track the time until congestion control feedback generation is processed. The minimum of this value and the times until RBE and transport sequence number feedback processing are calculated. Co-authored-by: Shridhar Majali Bug: webrtc:42225697 Change-Id: I44173062d8f8f84bf7e7791e05578c0ffc4fd017 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365273 Reviewed-by: Per Kjellander Reviewed-by: Harald Alvestrand Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#43271} --- .../receive_side_congestion_controller.cc | 25 +++++++++++---- ...ive_side_congestion_controller_unittest.cc | 31 ++++++++++++++----- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/modules/congestion_controller/receive_side_congestion_controller.cc b/modules/congestion_controller/receive_side_congestion_controller.cc index abf4fac305..a3d0e38867 100644 --- a/modules/congestion_controller/receive_side_congestion_controller.cc +++ b/modules/congestion_controller/receive_side_congestion_controller.cc @@ -109,14 +109,22 @@ void ReceiveSideCongestionController:: void ReceiveSideCongestionController::OnReceivedPacket( const RtpPacketReceived& packet, MediaType media_type) { - if (send_rfc8888_congestion_feedback_) { - RTC_DCHECK_RUN_ON(&sequence_checker_); - congestion_control_feedback_generator_.OnReceivedPacket(packet); - return; - } bool has_transport_sequence_number = packet.HasExtension() || packet.HasExtension(); + if (send_rfc8888_congestion_feedback_) { + RTC_DCHECK_RUN_ON(&sequence_checker_); + congestion_control_feedback_generator_.OnReceivedPacket(packet); + // TODO(https://bugs.webrtc.org/374197376): Utilize RFC 8888 feedback, which + // provides comprehensive details similar to transport-cc. To ensure a + // smooth transition, we will continue using transport sequence number + // feedback temporarily. Once validation is complete, we will fully + // transition to using RFC 8888 feedback exclusively. + if (has_transport_sequence_number) { + transport_sequence_number_feedback_generator_.OnReceivedPacket(packet); + } + return; + } if (media_type == MediaType::AUDIO && !has_transport_sequence_number) { // For audio, we only support send side BWE. return; @@ -146,7 +154,12 @@ TimeDelta ReceiveSideCongestionController::MaybeProcess() { Timestamp now = env_.clock().CurrentTime(); if (send_rfc8888_congestion_feedback_) { RTC_DCHECK_RUN_ON(&sequence_checker_); - return congestion_control_feedback_generator_.Process(now); + TimeDelta time_until_cc_rep = + congestion_control_feedback_generator_.Process(now); + TimeDelta time_until_rep = + transport_sequence_number_feedback_generator_.Process(now); + TimeDelta time_until = std::min(time_until_cc_rep, time_until_rep); + return std::max(time_until, TimeDelta::Zero()); } mutex_.Lock(); TimeDelta time_until_rbe = rbe_->Process(); diff --git a/modules/congestion_controller/receive_side_congestion_controller_unittest.cc b/modules/congestion_controller/receive_side_congestion_controller_unittest.cc index 50141cee4e..385a6b446c 100644 --- a/modules/congestion_controller/receive_side_congestion_controller_unittest.cc +++ b/modules/congestion_controller/receive_side_congestion_controller_unittest.cc @@ -86,6 +86,17 @@ TEST(ReceiveSideCongestionControllerTest, controller.SetMaxDesiredReceiveBitrate(DataRate::BitsPerSec(123)); } +void CheckRfc8888Feedback( + const std::vector>& rtcp_packets) { + ASSERT_THAT(rtcp_packets, SizeIs(1)); + rtc::Buffer buffer = rtcp_packets[0]->Build(); + rtcp::CommonHeader header; + EXPECT_TRUE(header.Parse(buffer.data(), buffer.size())); + // Check for RFC 8888 format message type 11(CCFB) + EXPECT_EQ(header.fmt(), + rtcp::CongestionControlFeedback::kFeedbackMessageType); +} + TEST(ReceiveSideCongestionControllerTest, SendsRfc8888FeedbackIfForced) { test::ExplicitKeyValueConfig field_trials( "WebRTC-RFC8888CongestionControlFeedback/force_send:true/"); @@ -97,7 +108,15 @@ TEST(ReceiveSideCongestionControllerTest, SendsRfc8888FeedbackIfForced) { CreateEnvironment(&clock, &field_trials), rtcp_sender.AsStdFunction(), remb_sender.AsStdFunction(), nullptr); - EXPECT_CALL(rtcp_sender, Call); + // Expect that RTCP feedback is sent. + EXPECT_CALL(rtcp_sender, Call) + .WillOnce( + [&](std::vector> rtcp_packets) { + CheckRfc8888Feedback(rtcp_packets); + }); + // Expect that REMB is not sent. + EXPECT_CALL(remb_sender, Call).Times(0); + RtpPacketReceived packet; packet.set_arrival_time(clock.CurrentTime()); controller.OnReceivedPacket(packet, MediaType::VIDEO); @@ -116,16 +135,14 @@ TEST(ReceiveSideCongestionControllerTest, SendsRfc8888FeedbackIfEnabled) { remb_sender.AsStdFunction(), nullptr); controller.EnablSendCongestionControlFeedbackAccordingToRfc8888(); + // Expect that RTCP feedback is sent. EXPECT_CALL(rtcp_sender, Call) .WillOnce( [&](std::vector> rtcp_packets) { - ASSERT_THAT(rtcp_packets, SizeIs(1)); - rtc::Buffer buffer = rtcp_packets[0]->Build(); - rtcp::CommonHeader header; - EXPECT_TRUE(header.Parse(buffer.data(), buffer.size())); - EXPECT_EQ(header.fmt(), - rtcp::CongestionControlFeedback::kFeedbackMessageType); + CheckRfc8888Feedback(rtcp_packets); }); + // Expect that REMB is not sent. + EXPECT_CALL(remb_sender, Call).Times(0); RtpPacketReceived packet; packet.set_arrival_time(clock.CurrentTime());