diff --git a/experiments/field_trials.py b/experiments/field_trials.py index 73419bf1f7..ed192a6c05 100755 --- a/experiments/field_trials.py +++ b/experiments/field_trials.py @@ -113,6 +113,9 @@ ACTIVE_FIELD_TRIALS: FrozenSet[FieldTrial] = frozenset([ FieldTrial('WebRTC-ReceiveBufferSize', 42225927, date(2024, 4, 1)), + FieldTrial('WebRTC-RFC8888CongestionControlFeedback', + 42225697, + date(2025, 1, 30)), FieldTrial('WebRTC-RtcEventLogEncodeDependencyDescriptor', 42225280, date(2024, 4, 1)), diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index c63603f7d1..3f69b02145 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -299,6 +299,7 @@ rtc_library("rtp_rtcp") { "../../api/rtc_event_log", "../../api/task_queue:pending_task_safety_flag", "../../api/task_queue:task_queue", + "../../api/transport:field_trial_based_config", "../../api/transport/rtp:dependency_descriptor", "../../api/transport/rtp:rtp_source", "../../api/units:data_rate", @@ -715,6 +716,7 @@ if (rtc_include_tests) { "../../test:mock_transport", "../../test:rtp_test_utils", "../../test:run_loop", + "../../test:scoped_key_value_config", "../../test:test_support", "../../test/time_controller:time_controller", "../video_coding:codec_globals_headers", diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 37410e2bc3..abd06c1b6e 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -29,6 +29,7 @@ #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "modules/rtp_rtcp/include/report_block_data.h" +#include "modules/rtp_rtcp/source/rtcp_packet/congestion_control_feedback.h" #include "modules/rtp_rtcp/source/rtcp_packet/remote_estimate.h" #include "system_wrappers/include/clock.h" @@ -107,7 +108,7 @@ enum RTCPPacketType : uint32_t { kRtcpXrReceiverReferenceTime = 0x40000, kRtcpXrDlrrReportBlock = 0x80000, kRtcpTransportFeedback = 0x100000, - kRtcpXrTargetBitrate = 0x200000 + kRtcpXrTargetBitrate = 0x200000, }; enum class KeyFrameReqMethod : uint8_t { @@ -164,6 +165,10 @@ class NetworkLinkRtcpObserver { virtual void OnTransportFeedback(Timestamp receive_time, const rtcp::TransportFeedback& feedback) {} + // RFC 8888 congestion control feedback. + virtual void OnCongestionControlFeedback( + Timestamp receive_time, + const rtcp::CongestionControlFeedback& feedback) {} virtual void OnReceiverEstimatedMaxBitrate(Timestamp receive_time, DataRate bitrate) {} diff --git a/modules/rtp_rtcp/mocks/mock_network_link_rtcp_observer.h b/modules/rtp_rtcp/mocks/mock_network_link_rtcp_observer.h index 16b7db7892..90dd948afe 100644 --- a/modules/rtp_rtcp/mocks/mock_network_link_rtcp_observer.h +++ b/modules/rtp_rtcp/mocks/mock_network_link_rtcp_observer.h @@ -32,6 +32,11 @@ class MockNetworkLinkRtcpObserver : public NetworkLinkRtcpObserver { OnTransportFeedback, (Timestamp receive_time, const rtcp::TransportFeedback& feedback), (override)); + MOCK_METHOD(void, + OnCongestionControlFeedback, + (Timestamp receive_time, + const rtcp::CongestionControlFeedback& feedback), + (override)); MOCK_METHOD(void, OnReceiverEstimatedMaxBitrate, (Timestamp receive_time, DataRate bitrate), diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index 63f44fb144..9df495576a 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -19,13 +19,16 @@ #include #include +#include "api/transport/field_trial_based_config.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "api/video/video_bitrate_allocation.h" #include "api/video/video_bitrate_allocator.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtcp_packet/bye.h" #include "modules/rtp_rtcp/source/rtcp_packet/common_header.h" #include "modules/rtp_rtcp/source/rtcp_packet/compound_packet.h" +#include "modules/rtp_rtcp/source/rtcp_packet/congestion_control_feedback.h" #include "modules/rtp_rtcp/source/rtcp_packet/extended_reports.h" #include "modules/rtp_rtcp/source/rtcp_packet/fir.h" #include "modules/rtp_rtcp/source/rtcp_packet/loss_notification.h" @@ -131,6 +134,7 @@ struct RTCPReceiver::PacketInformation { absl::optional rtt; uint32_t receiver_estimated_max_bitrate_bps = 0; std::unique_ptr transport_feedback; + absl::optional congestion_control_feedback; absl::optional target_bitrate_allocation; absl::optional network_state_estimate; std::unique_ptr loss_notification; @@ -140,6 +144,8 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config, ModuleRtpRtcpImpl2* owner) : clock_(config.clock), receiver_only_(config.receiver_only), + enable_congestion_controller_feedback_(FieldTrialBasedConfig().IsEnabled( + "WebRTC-RFC8888CongestionControlFeedback")), rtp_rtcp_(owner), registered_ssrcs_(false, config), network_link_rtcp_observer_(config.network_link_rtcp_observer), @@ -167,6 +173,8 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config, ModuleRtpRtcp* owner) : clock_(config.clock), receiver_only_(config.receiver_only), + enable_congestion_controller_feedback_(FieldTrialBasedConfig().IsEnabled( + "WebRTC-RFC8888CongestionControlFeedback")), rtp_rtcp_(owner), registered_ssrcs_(true, config), network_link_rtcp_observer_(config.network_link_rtcp_observer), @@ -438,6 +446,13 @@ bool RTCPReceiver::ParseCompoundPacket(rtc::ArrayView packet, case rtcp::TransportFeedback::kFeedbackMessageType: HandleTransportFeedback(rtcp_block, packet_information); break; + case rtcp::CongestionControlFeedback::kFeedbackMessageType: + if (enable_congestion_controller_feedback_) { + valid = HandleCongestionControlFeedback(rtcp_block, + packet_information); + break; + } + ABSL_FALLTHROUGH_INTENDED; default: ++num_skipped_packets_; break; @@ -1048,6 +1063,17 @@ void RTCPReceiver::HandleTransportFeedback( } } +bool RTCPReceiver::HandleCongestionControlFeedback( + const CommonHeader& rtcp_block, + PacketInformation* packet_information) { + rtcp::CongestionControlFeedback feedback; + if (!feedback.Parse(rtcp_block)) { + return false; + } + packet_information->congestion_control_feedback.emplace(std::move(feedback)); + return true; +} + void RTCPReceiver::NotifyTmmbrUpdated() { // Find bounding set. std::vector bounding = @@ -1137,6 +1163,10 @@ void RTCPReceiver::TriggerCallbacksFromRtcpPacket( network_link_rtcp_observer_->OnTransportFeedback( now, *packet_information.transport_feedback); } + if (packet_information.congestion_control_feedback) { + network_link_rtcp_observer_->OnCongestionControlFeedback( + now, *packet_information.congestion_control_feedback); + } } if ((packet_information.packet_type_flags & kRtcpSr) || diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h index 20e314b01c..6bfd1e15dc 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -342,6 +342,9 @@ class RTCPReceiver final { void HandleTransportFeedback(const rtcp::CommonHeader& rtcp_block, PacketInformation* packet_information) RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_); + bool HandleCongestionControlFeedback(const rtcp::CommonHeader& rtcp_block, + PacketInformation* packet_information) + RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_); bool RtcpRrTimeoutLocked(Timestamp now) RTC_EXCLUSIVE_LOCKS_REQUIRED(rtcp_receiver_lock_); @@ -351,6 +354,7 @@ class RTCPReceiver final { Clock* const clock_; const bool receiver_only_; + const bool enable_congestion_controller_feedback_; ModuleRtpRtcp* const rtp_rtcp_; // The set of registered local SSRCs. RegisteredSsrcs registered_ssrcs_; diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 1f5f138b83..a88e90e63f 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -26,6 +26,7 @@ #include "modules/rtp_rtcp/source/rtcp_packet/app.h" #include "modules/rtp_rtcp/source/rtcp_packet/bye.h" #include "modules/rtp_rtcp/source/rtcp_packet/compound_packet.h" +#include "modules/rtp_rtcp/source/rtcp_packet/congestion_control_feedback.h" #include "modules/rtp_rtcp/source/rtcp_packet/extended_reports.h" #include "modules/rtp_rtcp/source/rtcp_packet/fir.h" #include "modules/rtp_rtcp/source/rtcp_packet/nack.h" @@ -44,6 +45,7 @@ #include "system_wrappers/include/ntp_time.h" #include "test/gmock.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" namespace webrtc { namespace { @@ -64,6 +66,7 @@ using ::testing::SizeIs; using ::testing::StrEq; using ::testing::StrictMock; using ::testing::UnorderedElementsAre; +using ::webrtc::test::ScopedKeyValueConfig; class MockRtcpPacketTypeCounterObserver : public RtcpPacketTypeCounterObserver { public: @@ -1740,6 +1743,54 @@ TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnTransportFeedback) { receiver.IncomingPacket(packet.Build()); } +TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnCongestionControlFeedback) { + ScopedKeyValueConfig trials( + "WebRTC-RFC8888CongestionControlFeedback/Enabled/"); + ReceiverMocks mocks; + RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks); + RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl); + receiver.SetRemoteSSRC(kSenderSsrc); + + rtcp::CongestionControlFeedback packet({{ + .ssrc = 123, + .sequence_number = 1, + }}, + /*report_timestamp_compact_ntp=*/324); + packet.SetSenderSsrc(kSenderSsrc); + + EXPECT_CALL( + mocks.network_link_rtcp_observer, + OnCongestionControlFeedback( + mocks.clock.CurrentTime(), + Property(&rtcp::CongestionControlFeedback::packets, SizeIs(1)))); + receiver.IncomingPacket(packet.Build()); +} + +TEST(RtcpReceiverTest, HandlesInvalidCongestionControlFeedback) { + ScopedKeyValueConfig trials( + "WebRTC-RFC8888CongestionControlFeedback/Enabled/"); + ReceiverMocks mocks; + RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks); + RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl); + receiver.SetRemoteSSRC(kSenderSsrc); + + rtcp::CongestionControlFeedback packet({{ + .ssrc = 123, + .sequence_number = 1, + }}, + /*report_timestamp_compact_ntp=*/324); + packet.SetSenderSsrc(kSenderSsrc); + rtc::Buffer built_packet = packet.Build(); + // Modify the CongestionControlFeedback packet so that it is invalid. + const size_t kNumReportsOffset = 14; + ByteWriter::WriteBigEndian(&built_packet.data()[kNumReportsOffset], + 42); + + EXPECT_CALL(mocks.network_link_rtcp_observer, OnCongestionControlFeedback) + .Times(0); + receiver.IncomingPacket(built_packet); +} + TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnTransportFeedbackOnRtxSsrc) { ReceiverMocks mocks; diff --git a/test/fuzzers/BUILD.gn b/test/fuzzers/BUILD.gn index 623347e0b8..c667b64bed 100644 --- a/test/fuzzers/BUILD.gn +++ b/test/fuzzers/BUILD.gn @@ -222,6 +222,7 @@ webrtc_fuzzer_test("rtcp_receiver_fuzzer") { "../../modules/rtp_rtcp:rtp_rtcp_format", "../../rtc_base:checks", "../../system_wrappers", + "../../system_wrappers:field_trial", ] seed_corpus = "corpora/rtcp-corpus" } diff --git a/test/fuzzers/rtcp_receiver_fuzzer.cc b/test/fuzzers/rtcp_receiver_fuzzer.cc index e61f6c06ac..5dedf58260 100644 --- a/test/fuzzers/rtcp_receiver_fuzzer.cc +++ b/test/fuzzers/rtcp_receiver_fuzzer.cc @@ -10,8 +10,8 @@ #include "modules/rtp_rtcp/source/rtcp_packet/tmmb_item.h" #include "modules/rtp_rtcp/source/rtcp_receiver.h" #include "modules/rtp_rtcp/source/rtp_rtcp_interface.h" -#include "rtc_base/checks.h" #include "system_wrappers/include/clock.h" +#include "system_wrappers/include/field_trial.h" namespace webrtc { namespace { @@ -37,7 +37,8 @@ void FuzzOneInput(const uint8_t* data, size_t size) { if (size > kMaxInputLenBytes) { return; } - + field_trial::InitFieldTrialsFromString( + "WebRTC-RFC8888CongestionControlFeedback/Enabled/"); NullModuleRtpRtcp rtp_rtcp_module; SimulatedClock clock(1234);