Add support for receiving CongestionControlFeedback to RTCPReceiver
Support for parsing the packet is gated behind field trial
WebRTC-RFC8888CongestionControlFeedback/Enabled/.
Bug: webrtc:15368
Change-Id: Ib4478e821fe5a43510af5131543e7861cf54d901
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/348664
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42215}
diff --git a/experiments/field_trials.py b/experiments/field_trials.py
index 73419bf..ed192a6 100755
--- a/experiments/field_trials.py
+++ b/experiments/field_trials.py
@@ -113,6 +113,9 @@
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 c63603f..3f69b02 100644
--- a/modules/rtp_rtcp/BUILD.gn
+++ b/modules/rtp_rtcp/BUILD.gn
@@ -299,6 +299,7 @@
"../../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 @@
"../../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 37410e2..abd06c1 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 @@
kRtcpXrReceiverReferenceTime = 0x40000,
kRtcpXrDlrrReportBlock = 0x80000,
kRtcpTransportFeedback = 0x100000,
- kRtcpXrTargetBitrate = 0x200000
+ kRtcpXrTargetBitrate = 0x200000,
};
enum class KeyFrameReqMethod : uint8_t {
@@ -164,6 +165,10 @@
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 16b7db7..90dd948 100644
--- a/modules/rtp_rtcp/mocks/mock_network_link_rtcp_observer.h
+++ b/modules/rtp_rtcp/mocks/mock_network_link_rtcp_observer.h
@@ -33,6 +33,11 @@
(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),
(override));
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc
index 63f44fb..9df4955 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -19,13 +19,16 @@
#include <utility>
#include <vector>
+#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 @@
absl::optional<TimeDelta> rtt;
uint32_t receiver_estimated_max_bitrate_bps = 0;
std::unique_ptr<rtcp::TransportFeedback> transport_feedback;
+ absl::optional<rtcp::CongestionControlFeedback> congestion_control_feedback;
absl::optional<VideoBitrateAllocation> target_bitrate_allocation;
absl::optional<NetworkStateEstimate> network_state_estimate;
std::unique_ptr<rtcp::LossNotification> loss_notification;
@@ -140,6 +144,8 @@
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 @@
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 @@
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 @@
}
}
+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<rtcp::TmmbItem> bounding =
@@ -1137,6 +1163,10 @@
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 20e314b..6bfd1e1 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.h
+++ b/modules/rtp_rtcp/source/rtcp_receiver.h
@@ -342,6 +342,9 @@
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 @@
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 1f5f138..a88e90e 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::StrEq;
using ::testing::StrictMock;
using ::testing::UnorderedElementsAre;
+using ::webrtc::test::ScopedKeyValueConfig;
class MockRtcpPacketTypeCounterObserver : public RtcpPacketTypeCounterObserver {
public:
@@ -1740,6 +1743,54 @@
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<uint16_t>::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 623347e..c667b64 100644
--- a/test/fuzzers/BUILD.gn
+++ b/test/fuzzers/BUILD.gn
@@ -222,6 +222,7 @@
"../../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 e61f6c0..5dedf58 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 @@
if (size > kMaxInputLenBytes) {
return;
}
-
+ field_trial::InitFieldTrialsFromString(
+ "WebRTC-RFC8888CongestionControlFeedback/Enabled/");
NullModuleRtpRtcp rtp_rtcp_module;
SimulatedClock clock(1234);
