Allowing buffering a LNTF (loss notification) feedback message in RTCPSender
Loss notifications may either be sent immediately, or wait until another
RTCP feedback message is sent.
Bug: webrtc:10336
Change-Id: I40601d9fa1dec6c17b2ce905cb0c8cd2dcff7893
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/139242
Commit-Queue: Elad Alon <eladalon@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28142}
diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc
index f362fcb..19f1e26 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -215,7 +215,8 @@
int32_t RTCPSender::SendLossNotification(const FeedbackState& feedback_state,
uint16_t last_decoded_seq_num,
uint16_t last_received_seq_num,
- bool decodability_flag) {
+ bool decodability_flag,
+ bool buffering_allowed) {
rtc::CritScope lock(&critical_section_rtcp_sender_);
loss_notification_state_.last_decoded_seq_num = last_decoded_seq_num;
@@ -224,7 +225,11 @@
SetFlag(kRtcpLossNotification, /*is_volatile=*/true);
- // Send immediately.
+ if (buffering_allowed) {
+ // The loss notification will be batched with additional feedback messages.
+ return 0;
+ }
+
return SendCompoundRTCP(feedback_state,
{RTCPPacketType::kRtcpLossNotification});
}
diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h
index 7a9aeac..74f4cc1 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.h
+++ b/modules/rtp_rtcp/source/rtcp_sender.h
@@ -117,7 +117,8 @@
int32_t SendLossNotification(const FeedbackState& feedback_state,
uint16_t last_decoded_seq_num,
uint16_t last_received_seq_num,
- bool decodability_flag);
+ bool decodability_flag,
+ bool buffering_allowed);
void SetRemb(int64_t bitrate_bps, std::vector<uint32_t> ssrcs);
diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
index b6b3c34..0ddfb94 100644
--- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
@@ -10,6 +10,7 @@
#include <memory>
+#include "absl/base/macros.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"
@@ -366,28 +367,61 @@
TEST_F(RtcpSenderTest, SendNack) {
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
const uint16_t kList[] = {0, 1, 16};
- const int32_t kListLength = sizeof(kList) / sizeof(kList[0]);
- EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpNack, kListLength,
- kList));
+ EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpNack,
+ ABSL_ARRAYSIZE(kList), kList));
EXPECT_EQ(1, parser()->nack()->num_packets());
EXPECT_EQ(kSenderSsrc, parser()->nack()->sender_ssrc());
EXPECT_EQ(kRemoteSsrc, parser()->nack()->media_ssrc());
EXPECT_THAT(parser()->nack()->packet_ids(), ElementsAre(0, 1, 16));
}
-TEST_F(RtcpSenderTest, SendLossNotification) {
+TEST_F(RtcpSenderTest, SendLossNotificationBufferingNotAllowed) {
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
constexpr uint16_t kLastDecoded = 0x1234;
constexpr uint16_t kLastReceived = 0x4321;
constexpr bool kDecodabilityFlag = true;
- const int32_t result = rtcp_sender_->SendLossNotification(
- feedback_state(), kLastDecoded, kLastReceived, kDecodabilityFlag);
- EXPECT_EQ(result, 0);
- EXPECT_EQ(1, parser()->loss_notification()->num_packets());
+ constexpr bool kBufferingAllowed = false;
+ EXPECT_EQ(rtcp_sender_->SendLossNotification(feedback_state(), kLastDecoded,
+ kLastReceived, kDecodabilityFlag,
+ kBufferingAllowed),
+ 0);
+ EXPECT_EQ(parser()->processed_rtcp_packets(), 1u);
+ EXPECT_EQ(parser()->loss_notification()->num_packets(), 1);
EXPECT_EQ(kSenderSsrc, parser()->loss_notification()->sender_ssrc());
EXPECT_EQ(kRemoteSsrc, parser()->loss_notification()->media_ssrc());
}
+TEST_F(RtcpSenderTest, SendLossNotificationBufferingAllowed) {
+ rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
+ constexpr uint16_t kLastDecoded = 0x1234;
+ constexpr uint16_t kLastReceived = 0x4321;
+ constexpr bool kDecodabilityFlag = true;
+ constexpr bool kBufferingAllowed = true;
+ EXPECT_EQ(rtcp_sender_->SendLossNotification(feedback_state(), kLastDecoded,
+ kLastReceived, kDecodabilityFlag,
+ kBufferingAllowed),
+ 0);
+
+ // No RTCP messages sent yet.
+ ASSERT_EQ(parser()->processed_rtcp_packets(), 0u);
+
+ // Sending another messages triggers sending the LNTF messages as well.
+ const uint16_t kList[] = {0, 1, 16};
+ EXPECT_EQ(rtcp_sender_->SendRTCP(feedback_state(), kRtcpNack,
+ ABSL_ARRAYSIZE(kList), kList),
+ 0);
+
+ // Exactly one packet was produced, and it contained both the buffered LNTF
+ // as well as the message that had triggered the packet.
+ EXPECT_EQ(parser()->processed_rtcp_packets(), 1u);
+ EXPECT_EQ(parser()->loss_notification()->num_packets(), 1);
+ EXPECT_EQ(parser()->loss_notification()->sender_ssrc(), kSenderSsrc);
+ EXPECT_EQ(parser()->loss_notification()->media_ssrc(), kRemoteSsrc);
+ EXPECT_EQ(parser()->nack()->num_packets(), 1);
+ EXPECT_EQ(parser()->nack()->sender_ssrc(), kSenderSsrc);
+ EXPECT_EQ(parser()->nack()->media_ssrc(), kRemoteSsrc);
+}
+
TEST_F(RtcpSenderTest, RembNotIncludedBeforeSet) {
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index 843f616..ad83c9e 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -754,10 +754,11 @@
int32_t ModuleRtpRtcpImpl::SendLossNotification(uint16_t last_decoded_seq_num,
uint16_t last_received_seq_num,
- bool decodability_flag) {
+ bool decodability_flag,
+ bool buffering_allowed) {
return rtcp_sender_.SendLossNotification(
GetFeedbackState(), last_decoded_seq_num, last_received_seq_num,
- decodability_flag);
+ decodability_flag, buffering_allowed);
}
void ModuleRtpRtcpImpl::SetRemoteSSRC(const uint32_t ssrc) {
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
index 4cb274b..15bedc7 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
@@ -263,7 +263,8 @@
int32_t SendLossNotification(uint16_t last_decoded_seq_num,
uint16_t last_received_seq_num,
- bool decodability_flag) override;
+ bool decodability_flag,
+ bool buffering_allowed) override;
bool LastReceivedNTP(uint32_t* NTPsecs,
uint32_t* NTPfrac,