Add MID/RID configurability to RTPSender.
With the new config option |always_send_mid_and_rid|, the user
of the RTPSender can decide if MIDs and RIDs should always be sent
(when provided and negotiated), or if their sending should be disabled
after the receiver has acked the stream. Depending on the use case,
different settings might be preferable. The default setting is not
changed in this CL.
Bug: webrtc:11416
Change-Id: Ic3c71a6105fb7ee08d53cf9eb03f4e53b5799610
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/170109
Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30742}
diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h
index e897718..8cee1ba 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp.h
@@ -121,6 +121,13 @@
// Corresponds to extmap-allow-mixed in SDP negotiation.
bool extmap_allow_mixed = false;
+ // If true, the RTP sender will always annotate outgoing packets with
+ // MID and RID header extensions, if provided and negotiated.
+ // If false, the RTP sender will stop sending MID and RID header extensions,
+ // when it knows that the receiver is ready to demux based on SSRC. This is
+ // done by RTCP RR acking.
+ bool always_send_mid_and_rid = false;
+
// If set, field trials are read from |field_trials|, otherwise
// defaults to webrtc::FieldTrialBasedConfig.
const WebRtcKeyValueConfig* field_trials = nullptr;
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index c48a662..584f89c 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -112,6 +112,7 @@
rtp_header_extension_map_(config.extmap_allow_mixed),
// RTP variables
sequence_number_forced_(false),
+ always_send_mid_and_rid_(config.always_send_mid_and_rid),
ssrc_has_acked_(false),
rtx_ssrc_has_acked_(false),
last_rtp_timestamp_(0),
@@ -497,13 +498,15 @@
// in the MID and/or (R)RID header extensions if present. Therefore, the
// sender can reduce overhead by omitting these header extensions once it
// knows that the receiver has "bound" the SSRC.
+ // This optimization can be configured by setting
+ // |always_send_mid_and_rid_| appropriately.
//
// The algorithm here is fairly simple: Always attach a MID and/or RID (if
// configured) to the outgoing packets until an RTCP receiver report comes
// back for this SSRC. That feedback indicates the receiver must have
// received a packet with the SSRC and header extension(s), so the sender
// then stops attaching the MID and RID.
- if (!ssrc_has_acked_) {
+ if (always_send_mid_and_rid_ || !ssrc_has_acked_) {
// These are no-ops if the corresponding header extension is not registered.
if (!mid_.empty()) {
packet->SetExtension<RtpMid>(mid_);
@@ -686,7 +689,7 @@
// Note that RTX packets must used the RepairedRtpStreamId (RRID) header
// extension instead of the RtpStreamId (RID) header extension even though
// the payload is identical.
- if (!rtx_ssrc_has_acked_) {
+ if (always_send_mid_and_rid_ || !rtx_ssrc_has_acked_) {
// These are no-ops if the corresponding header extension is not
// registered.
if (!mid_.empty()) {
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index 8915e39..4a75509 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -179,6 +179,8 @@
std::string rid_ RTC_GUARDED_BY(send_critsect_);
// MID value to send in the MID header extension.
std::string mid_ RTC_GUARDED_BY(send_critsect_);
+ // Should we send MID/RID even when ACKed? (see below).
+ const bool always_send_mid_and_rid_;
// Track if any ACK has been received on the SSRC and RTX SSRC to indicate
// when to stop sending the MID and RID header extensions.
bool ssrc_has_acked_ RTC_GUARDED_BY(send_critsect_);
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index 3b85166..d4a7fa9 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -254,7 +254,7 @@
kMarkerBit(true),
field_trials_(ToFieldTrialString(GetParam())) {}
- void SetUp() override { SetUpRtpSender(true, false); }
+ void SetUp() override { SetUpRtpSender(true, false, false); }
RTPSender* rtp_sender() {
RTC_DCHECK(rtp_sender_context_);
@@ -266,7 +266,9 @@
return &rtp_sender_context_->packet_sender_;
}
- void SetUpRtpSender(bool pacer, bool populate_network2) {
+ void SetUpRtpSender(bool pacer,
+ bool populate_network2,
+ bool always_send_mid_and_rid) {
RtpRtcp::Configuration config;
config.clock = &fake_clock_;
config.outgoing_transport = &transport_;
@@ -279,6 +281,7 @@
config.paced_sender = pacer ? &mock_paced_sender_ : nullptr;
config.populate_network2_timestamp = populate_network2;
config.rtp_stats_callback = &rtp_stats_callback_;
+ config.always_send_mid_and_rid = always_send_mid_and_rid;
rtp_sender_context_ = std::make_unique<RtpSenderContext>(config);
rtp_sender()->SetSequenceNumber(kSeqNum);
rtp_sender()->SetTimestampOffset(0);
@@ -378,7 +381,7 @@
// default code path.
class RtpSenderTestWithoutPacer : public RtpSenderTest {
public:
- void SetUp() override { SetUpRtpSender(false, false); }
+ void SetUp() override { SetUpRtpSender(false, false, false); }
};
TEST_P(RtpSenderTestWithoutPacer, AllocatePacketSetCsrc) {
@@ -603,7 +606,7 @@
TEST_P(RtpSenderTestWithoutPacer,
SetsIncludedInFeedbackWhenTransportSequenceNumberExtensionIsRegistered) {
- SetUpRtpSender(false, false);
+ SetUpRtpSender(false, false, false);
rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionTransportSequenceNumber,
kTransportSequenceNumberExtensionId);
EXPECT_CALL(send_packet_observer_, OnSendPacket).Times(1);
@@ -614,7 +617,7 @@
TEST_P(
RtpSenderTestWithoutPacer,
SetsIncludedInAllocationWhenTransportSequenceNumberExtensionIsRegistered) {
- SetUpRtpSender(false, false);
+ SetUpRtpSender(false, false, false);
rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionTransportSequenceNumber,
kTransportSequenceNumberExtensionId);
EXPECT_CALL(send_packet_observer_, OnSendPacket).Times(1);
@@ -624,7 +627,7 @@
TEST_P(RtpSenderTestWithoutPacer,
SetsIncludedInAllocationWhenForcedAsPartOfAllocation) {
- SetUpRtpSender(false, false);
+ SetUpRtpSender(false, false, false);
rtp_egress()->ForceIncludeSendPacketsInAllocation(true);
SendGenericPacket();
EXPECT_FALSE(transport_.last_options_.included_in_feedback);
@@ -632,7 +635,7 @@
}
TEST_P(RtpSenderTestWithoutPacer, DoesnSetIncludedInAllocationByDefault) {
- SetUpRtpSender(false, false);
+ SetUpRtpSender(false, false, false);
SendGenericPacket();
EXPECT_FALSE(transport_.last_options_.included_in_feedback);
EXPECT_FALSE(transport_.last_options_.included_in_allocation);
@@ -813,7 +816,7 @@
}
TEST_P(RtpSenderTest, WritesNetwork2ToTimingExtensionWithPacer) {
- SetUpRtpSender(/*pacer=*/true, /*populate_network2=*/true);
+ SetUpRtpSender(/*pacer=*/true, /*populate_network2=*/true, false);
rtp_sender_context_->packet_history_.SetStorePacketsStatus(
RtpPacketHistory::StorageMode::kStoreAndCull, 10);
EXPECT_EQ(0, rtp_sender()->RegisterRtpHeaderExtension(
@@ -852,7 +855,7 @@
}
TEST_P(RtpSenderTest, WritesNetwork2ToTimingExtensionWithoutPacer) {
- SetUpRtpSender(/*pacer=*/false, /*populate_network2=*/true);
+ SetUpRtpSender(/*pacer=*/false, /*populate_network2=*/true, false);
EXPECT_EQ(0, rtp_sender()->RegisterRtpHeaderExtension(
kRtpExtensionVideoTiming, kVideoTimingExtensionId));
auto packet = rtp_sender()->AllocatePacket();
@@ -1440,6 +1443,27 @@
EXPECT_FALSE(second_packet.HasExtension<RtpStreamId>());
}
+TEST_P(RtpSenderTestWithoutPacer,
+ MidAndRidAlwaysIncludedOnSentPacketsWhenConfigured) {
+ SetUpRtpSender(false, false, /*always_send_mid_and_rid=*/true);
+ const char kMid[] = "mid";
+ const char kRid[] = "f";
+ EnableMidSending(kMid);
+ EnableRidSending(kRid);
+
+ // Send two media packets: one before and one after the ack.
+ auto first_packet = SendGenericPacket();
+ rtp_sender()->OnReceivedAckOnSsrc(first_packet->SequenceNumber());
+ SendGenericPacket();
+
+ // Due to the configuration, both sent packets should contain MID and RID.
+ ASSERT_EQ(2u, transport_.sent_packets_.size());
+ for (const RtpPacketReceived& packet : transport_.sent_packets_) {
+ EXPECT_EQ(packet.GetExtension<RtpMid>(), kMid);
+ EXPECT_EQ(packet.GetExtension<RtpStreamId>(), kRid);
+ }
+}
+
// Test that the first RTX packet includes both MID and RRID even if the packet
// being retransmitted did not have MID or RID. The MID and RID are needed on
// the first packets for a given SSRC, and RTX packets are sent on a separate
@@ -1517,6 +1541,45 @@
EXPECT_FALSE(third_rtx_packet.HasExtension<RepairedRtpStreamId>());
}
+TEST_P(RtpSenderTestWithoutPacer,
+ MidAndRidAlwaysIncludedOnRtxPacketsWhenConfigured) {
+ SetUpRtpSender(false, false, /*always_send_mid_and_rid=*/true);
+ const char kMid[] = "mid";
+ const char kRid[] = "f";
+ EnableRtx();
+ EnableMidSending(kMid);
+ EnableRidSending(kRid);
+
+ // Send two media packets: one before and one after the ack.
+ auto media_packet1 = SendGenericPacket();
+ rtp_sender()->OnReceivedAckOnSsrc(media_packet1->SequenceNumber());
+ auto media_packet2 = SendGenericPacket();
+
+ // Send three RTX packets with different combinations of orders w.r.t. the
+ // media and RTX acks.
+ ASSERT_LT(0, rtp_sender()->ReSendPacket(media_packet2->SequenceNumber()));
+ ASSERT_EQ(3u, transport_.sent_packets_.size());
+ rtp_sender()->OnReceivedAckOnRtxSsrc(
+ transport_.sent_packets_[2].SequenceNumber());
+ ASSERT_LT(0, rtp_sender()->ReSendPacket(media_packet1->SequenceNumber()));
+ ASSERT_LT(0, rtp_sender()->ReSendPacket(media_packet2->SequenceNumber()));
+
+ // Due to the configuration, all sent packets should contain MID
+ // and either RID (media) or RRID (RTX).
+ ASSERT_EQ(5u, transport_.sent_packets_.size());
+ for (const auto& packet : transport_.sent_packets_) {
+ EXPECT_EQ(packet.GetExtension<RtpMid>(), kMid);
+ }
+ for (size_t i = 0; i < 2; ++i) {
+ const RtpPacketReceived& packet = transport_.sent_packets_[i];
+ EXPECT_EQ(packet.GetExtension<RtpStreamId>(), kRid);
+ }
+ for (size_t i = 2; i < transport_.sent_packets_.size(); ++i) {
+ const RtpPacketReceived& packet = transport_.sent_packets_[i];
+ EXPECT_EQ(packet.GetExtension<RepairedRtpStreamId>(), kRid);
+ }
+}
+
// Test that if the RtpState indicates an ACK has been received on that SSRC
// then neither the MID nor RID header extensions will be sent.
TEST_P(RtpSenderTestWithoutPacer,
