From e5f1a3992e3bbfa0445b90f317576c8229524d74 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 14 Jun 2021 16:56:02 +0200 Subject: [PATCH] Avoid sending empty receiver reports with RtcpTransceiver Bug: None Change-Id: Ia017c2df285febefb72ba88ba43366455bde5a78 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/222402 Reviewed-by: Per Kjellander Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#34281} --- .../rtp_rtcp/source/rtcp_transceiver_impl.cc | 7 +++- .../source/rtcp_transceiver_impl_unittest.cc | 42 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc index df67c07327..db065e2cd0 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc @@ -355,9 +355,12 @@ void RtcpTransceiverImpl::CreateCompoundPacket(PacketSender* sender) { rtcp::ReceiverReport receiver_report; receiver_report.SetSenderSsrc(sender_ssrc); receiver_report.SetReportBlocks(CreateReportBlocks(now)); - sender->AppendPacket(receiver_report); + if (config_.rtcp_mode == RtcpMode::kCompound || + !receiver_report.report_blocks().empty()) { + sender->AppendPacket(receiver_report); + } - if (!config_.cname.empty()) { + if (!config_.cname.empty() && !sender->IsEmpty()) { rtcp::Sdes sdes; bool added = sdes.AddCName(config_.feedback_ssrc, config_.cname); RTC_DCHECK(added) << "Failed to add cname " << config_.cname diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc index 6af34a9428..71b457b122 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc @@ -38,6 +38,7 @@ namespace { using ::testing::_; using ::testing::ElementsAre; +using ::testing::NiceMock; using ::testing::Return; using ::testing::SizeIs; using ::testing::StrictMock; @@ -392,6 +393,47 @@ TEST(RtcpTransceiverImplTest, SendsMinimalCompoundPacket) { EXPECT_EQ(rtcp_parser.sdes()->chunks()[0].cname, config.cname); } +TEST(RtcpTransceiverImplTest, AvoidsEmptyPacketsInReducedMode) { + MockTransport transport; + EXPECT_CALL(transport, SendRtcp).Times(0); + NiceMock receive_statistics; + SimulatedClock clock(0); + + RtcpTransceiverConfig config = DefaultTestConfig(); + config.clock = &clock; + config.outgoing_transport = &transport; + config.rtcp_mode = webrtc::RtcpMode::kReducedSize; + config.schedule_periodic_compound_packets = false; + config.receive_statistics = &receive_statistics; + RtcpTransceiverImpl rtcp_transceiver(config); + + rtcp_transceiver.SendCompoundPacket(); +} + +TEST(RtcpTransceiverImplTest, AvoidsEmptyReceiverReportsInReducedMode) { + RtcpPacketParser rtcp_parser; + RtcpParserTransport transport(&rtcp_parser); + NiceMock receive_statistics; + SimulatedClock clock(0); + + RtcpTransceiverConfig config = DefaultTestConfig(); + config.clock = &clock; + config.outgoing_transport = &transport; + config.rtcp_mode = webrtc::RtcpMode::kReducedSize; + config.schedule_periodic_compound_packets = false; + config.receive_statistics = &receive_statistics; + // Set it to produce something (RRTR) in the "periodic" rtcp packets. + config.non_sender_rtt_measurement = true; + RtcpTransceiverImpl rtcp_transceiver(config); + + // Rather than waiting for the right time to produce the periodic packet, + // trigger it manually. + rtcp_transceiver.SendCompoundPacket(); + + EXPECT_EQ(rtcp_parser.receiver_report()->num_packets(), 0); + EXPECT_GT(rtcp_parser.xr()->num_packets(), 0); +} + TEST(RtcpTransceiverImplTest, SendsNoRembInitially) { const uint32_t kSenderSsrc = 12345; SimulatedClock clock(0);