From b1f063db32d2c1544ca6ec7cd610687fcf989774 Mon Sep 17 00:00:00 2001 From: Mirta Dvornicic Date: Mon, 16 Apr 2018 11:16:21 +0200 Subject: [PATCH] Handle Receiver Reference Time Report from multiple receivers. Bug: webrtc:9122 Change-Id: I996f02da26b11a4829fda740fdd452470daf4d24 Reviewed-on: https://webrtc-review.googlesource.com/66781 Commit-Queue: Mirta Dvornicic Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#22871} --- .../source/rtcp_packet/extended_reports.cc | 8 +- .../source/rtcp_packet/extended_reports.h | 3 +- .../rtcp_packet/extended_reports_unittest.cc | 22 +++- modules/rtp_rtcp/source/rtcp_receiver.cc | 72 ++++++++--- modules/rtp_rtcp/source/rtcp_receiver.h | 15 ++- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 120 ++++++++++++++---- modules/rtp_rtcp/source/rtcp_sender.cc | 13 +- modules/rtp_rtcp/source/rtcp_sender.h | 7 +- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 32 ++++- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 3 +- 10 files changed, 232 insertions(+), 63 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc b/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc index d9ee5997ca..664e02bbd3 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc @@ -18,6 +18,7 @@ namespace webrtc { namespace rtcp { constexpr uint8_t ExtendedReports::kPacketType; +constexpr size_t ExtendedReports::kMaxNumberOfDlrrItems; // From RFC 3611: RTP Control Protocol Extended Reports (RTCP XR). // // Format for XR packets: @@ -104,8 +105,13 @@ void ExtendedReports::SetRrtr(const Rrtr& rrtr) { rrtr_block_.emplace(rrtr); } -void ExtendedReports::AddDlrrItem(const ReceiveTimeInfo& time_info) { +bool ExtendedReports::AddDlrrItem(const ReceiveTimeInfo& time_info) { + if (dlrr_block_.sub_blocks().size() >= kMaxNumberOfDlrrItems) { + RTC_LOG(LS_WARNING) << "Reached maximum number of DLRR items."; + return false; + } dlrr_block_.AddDlrrItem(time_info); + return true; } void ExtendedReports::SetVoipMetric(const VoipMetric& voip_metric) { diff --git a/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h b/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h index a1da4dc01f..91794f83a9 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h +++ b/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h @@ -28,6 +28,7 @@ class CommonHeader; class ExtendedReports : public RtcpPacket { public: static constexpr uint8_t kPacketType = 207; + static constexpr size_t kMaxNumberOfDlrrItems = 50; ExtendedReports(); ~ExtendedReports() override; @@ -38,7 +39,7 @@ class ExtendedReports : public RtcpPacket { void SetSenderSsrc(uint32_t ssrc) { sender_ssrc_ = ssrc; } void SetRrtr(const Rrtr& rrtr); - void AddDlrrItem(const ReceiveTimeInfo& time_info); + bool AddDlrrItem(const ReceiveTimeInfo& time_info); void SetVoipMetric(const VoipMetric& voip_metric); void SetTargetBitrate(const TargetBitrate& target_bitrate); diff --git a/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc b/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc index 088b363c54..1e72d4a84d 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc @@ -18,6 +18,7 @@ using testing::ElementsAre; using testing::ElementsAreArray; using testing::make_tuple; +using testing::SizeIs; using webrtc::rtcp::Dlrr; using webrtc::rtcp::ExtendedReports; using webrtc::rtcp::ReceiveTimeInfo; @@ -211,6 +212,18 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithDlrrWithTwoSubBlocks) { EXPECT_THAT(parsed.dlrr().sub_blocks(), ElementsAre(kTimeInfo1, kTimeInfo2)); } +TEST_F(RtcpPacketExtendedReportsTest, CreateLimitsTheNumberOfDlrrSubBlocks) { + const ReceiveTimeInfo kTimeInfo = Rand(); + ExtendedReports xr; + + for (size_t i = 0; i < ExtendedReports::kMaxNumberOfDlrrItems; ++i) + EXPECT_TRUE(xr.AddDlrrItem(kTimeInfo)); + EXPECT_FALSE(xr.AddDlrrItem(kTimeInfo)); + + EXPECT_THAT(xr.dlrr().sub_blocks(), + SizeIs(ExtendedReports::kMaxNumberOfDlrrItems)); +} + TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithVoipMetric) { const VoipMetric kVoipMetric = Rand(); @@ -228,15 +241,15 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithVoipMetric) { EXPECT_EQ(kVoipMetric, parsed.voip_metric()); } -TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithMultipleReportBlocks) { +TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithMaximumReportBlocks) { const Rrtr kRrtr = Rand(); - const ReceiveTimeInfo kTimeInfo = Rand(); const VoipMetric kVoipMetric = Rand(); ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); xr.SetRrtr(kRrtr); - xr.AddDlrrItem(kTimeInfo); + for (size_t i = 0; i < ExtendedReports::kMaxNumberOfDlrrItems; ++i) + xr.AddDlrrItem(Rand()); xr.SetVoipMetric(kVoipMetric); rtc::Buffer packet = xr.Build(); @@ -247,7 +260,8 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithMultipleReportBlocks) { EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); EXPECT_EQ(kRrtr, parsed.rrtr()); - EXPECT_THAT(parsed.dlrr().sub_blocks(), ElementsAre(kTimeInfo)); + EXPECT_THAT(parsed.dlrr().sub_blocks(), + ElementsAreArray(xr.dlrr().sub_blocks())); EXPECT_EQ(kVoipMetric, parsed.voip_metric()); } diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index bd8e81b6b8..abe135e2ed 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -57,6 +57,9 @@ const int64_t kTmmbrTimeoutIntervalMs = 5 * 5000; const int64_t kMaxWarningLogIntervalMs = 10000; const int64_t kRtcpMinFrameLengthMs = 17; +// Maximum number of received RRTRs that will be stored. +const size_t kMaxNumberOfStoredRrtrs = 200; + } // namespace struct RTCPReceiver::PacketInformation { @@ -86,6 +89,22 @@ struct RTCPReceiver::TmmbrInformation { std::map tmmbr; }; +// Structure for storing received RRTR RTCP messages (RFC3611, section 4.4). +struct RTCPReceiver::RrtrInformation { + RrtrInformation(uint32_t ssrc, + uint32_t received_remote_mid_ntp_time, + uint32_t local_receive_mid_ntp_time) + : ssrc(ssrc), + received_remote_mid_ntp_time(received_remote_mid_ntp_time), + local_receive_mid_ntp_time(local_receive_mid_ntp_time) {} + + uint32_t ssrc; + // Received NTP timestamp in compact representation. + uint32_t received_remote_mid_ntp_time; + // NTP time when the report was received in compact representation. + uint32_t local_receive_mid_ntp_time; +}; + struct RTCPReceiver::ReportBlockWithRtt { RTCPReportBlock report_block; @@ -251,22 +270,26 @@ bool RTCPReceiver::NTP(uint32_t* received_ntp_secs, return true; } -bool RTCPReceiver::LastReceivedXrReferenceTimeInfo( - rtcp::ReceiveTimeInfo* info) const { - RTC_DCHECK(info); +std::vector +RTCPReceiver::ConsumeReceivedXrReferenceTimeInfo() { rtc::CritScope lock(&rtcp_receiver_lock_); - if (!last_received_xr_ntp_.Valid()) - return false; - info->ssrc = remote_time_info_.ssrc; - info->last_rr = remote_time_info_.last_rr; + const size_t last_xr_rtis_size = std::min( + received_rrtrs_.size(), rtcp::ExtendedReports::kMaxNumberOfDlrrItems); + std::vector last_xr_rtis; + last_xr_rtis.reserve(last_xr_rtis_size); - // Get the delay since last received report (RFC 3611). - uint32_t receive_time_ntp = CompactNtp(last_received_xr_ntp_); - uint32_t now_ntp = CompactNtp(clock_->CurrentNtpTime()); + const uint32_t now_ntp = CompactNtp(clock_->CurrentNtpTime()); - info->delay_since_last_rr = now_ntp - receive_time_ntp; - return true; + for (size_t i = 0; i < last_xr_rtis_size; ++i) { + RrtrInformation& rrtr = received_rrtrs_.front(); + last_xr_rtis.emplace_back(rrtr.ssrc, rrtr.received_remote_mid_ntp_time, + now_ntp - rrtr.local_receive_mid_ntp_time); + received_rrtrs_ssrc_it_.erase(rrtr.ssrc); + received_rrtrs_.pop_front(); + } + + return last_xr_rtis; } // We can get multiple receive reports when we receive the report from a CE. @@ -673,6 +696,11 @@ void RTCPReceiver::HandleBye(const CommonHeader& rtcp_block) { last_fir_.erase(bye.sender_ssrc()); received_cnames_.erase(bye.sender_ssrc()); + auto it = received_rrtrs_ssrc_it_.find(bye.sender_ssrc()); + if (it != received_rrtrs_ssrc_it_.end()) { + received_rrtrs_.erase(it->second); + received_rrtrs_ssrc_it_.erase(it); + } xr_rr_rtt_ms_ = 0; } @@ -698,9 +726,23 @@ void RTCPReceiver::HandleXr(const CommonHeader& rtcp_block, void RTCPReceiver::HandleXrReceiveReferenceTime(uint32_t sender_ssrc, const rtcp::Rrtr& rrtr) { - remote_time_info_.ssrc = sender_ssrc; - remote_time_info_.last_rr = CompactNtp(rrtr.ntp()); - last_received_xr_ntp_ = clock_->CurrentNtpTime(); + uint32_t received_remote_mid_ntp_time = CompactNtp(rrtr.ntp()); + uint32_t local_receive_mid_ntp_time = CompactNtp(clock_->CurrentNtpTime()); + + auto it = received_rrtrs_ssrc_it_.find(sender_ssrc); + if (it != received_rrtrs_ssrc_it_.end()) { + it->second->received_remote_mid_ntp_time = received_remote_mid_ntp_time; + it->second->local_receive_mid_ntp_time = local_receive_mid_ntp_time; + } else { + if (received_rrtrs_.size() < kMaxNumberOfStoredRrtrs) { + received_rrtrs_.emplace_back(sender_ssrc, received_remote_mid_ntp_time, + local_receive_mid_ntp_time); + received_rrtrs_ssrc_it_[sender_ssrc] = std::prev(received_rrtrs_.end()); + } else { + RTC_LOG(LS_WARNING) << "Discarding received RRTR for ssrc " << sender_ssrc + << ", reached maximum number of stored RRTRs."; + } + } } void RTCPReceiver::HandleXrDlrrReportBlock(const rtcp::ReceiveTimeInfo& rti) { diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h index c3de36c54f..c868837be6 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -11,6 +11,7 @@ #ifndef MODULES_RTP_RTCP_SOURCE_RTCP_RECEIVER_H_ #define MODULES_RTP_RTCP_SOURCE_RTCP_RECEIVER_H_ +#include #include #include #include @@ -77,7 +78,7 @@ class RTCPReceiver { uint32_t* rtcp_arrival_time_frac, uint32_t* rtcp_timestamp) const; - bool LastReceivedXrReferenceTimeInfo(rtcp::ReceiveTimeInfo* info) const; + std::vector ConsumeReceivedXrReferenceTimeInfo(); // Get rtt. int32_t RTT(uint32_t remote_ssrc, @@ -115,6 +116,7 @@ class RTCPReceiver { private: struct PacketInformation; struct TmmbrInformation; + struct RrtrInformation; struct ReportBlockWithRtt; struct LastFirStatus; // RTCP report blocks mapped by remote SSRC. @@ -226,10 +228,13 @@ class RTCPReceiver { // When did we receive the last send report. NtpTime last_received_sr_ntp_ RTC_GUARDED_BY(rtcp_receiver_lock_); - // Received XR receive time report. - rtcp::ReceiveTimeInfo remote_time_info_; - // Time when the report was received. - NtpTime last_received_xr_ntp_; + // Received RRTR information in ascending receive time order. + std::list received_rrtrs_ + RTC_GUARDED_BY(rtcp_receiver_lock_); + // Received RRTR information mapped by remote ssrc. + std::map::iterator> + received_rrtrs_ssrc_it_ RTC_GUARDED_BY(rtcp_receiver_lock_); + // Estimated rtt, zero when there is no valid estimate. bool xr_rrtr_status_ RTC_GUARDED_BY(rtcp_receiver_lock_); int64_t xr_rr_rtt_ms_; diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 5faf7ed112..3c3c4b37c7 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -620,6 +620,21 @@ TEST_F(RtcpReceiverTest, InjectByePacket_RemovesReportBlocks) { EXPECT_EQ(2u, received_blocks.size()); } +TEST_F(RtcpReceiverTest, InjectByePacketRemovesReferenceTimeInfo) { + rtcp::ExtendedReports xr; + xr.SetSenderSsrc(kSenderSsrc); + rtcp::Rrtr rrtr; + rrtr.SetNtp(NtpTime(0x10203, 0x40506)); + xr.SetRrtr(rrtr); + InjectRtcpPacket(xr); + + rtcp::Bye bye; + bye.SetSenderSsrc(kSenderSsrc); + InjectRtcpPacket(bye); + + EXPECT_THAT(rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo(), IsEmpty()); +} + TEST_F(RtcpReceiverTest, InjectPliPacket) { rtcp::Pli pli; pli.SetMediaSsrc(kReceiverMainSsrc); @@ -706,19 +721,17 @@ TEST_F(RtcpReceiverTest, InjectExtendedReportsReceiverReferenceTimePacket) { xr.SetSenderSsrc(kSenderSsrc); xr.SetRrtr(rrtr); - ReceiveTimeInfo rrtime; - EXPECT_FALSE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&rrtime)); + std::vector last_xr_rtis = + rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo(); + EXPECT_THAT(last_xr_rtis, IsEmpty()); InjectRtcpPacket(xr); - EXPECT_TRUE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&rrtime)); - EXPECT_EQ(rrtime.ssrc, kSenderSsrc); - EXPECT_EQ(rrtime.last_rr, CompactNtp(kNtp)); - EXPECT_EQ(0U, rrtime.delay_since_last_rr); - - system_clock_.AdvanceTimeMilliseconds(1500); - EXPECT_TRUE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&rrtime)); - EXPECT_NEAR(1500, CompactNtpRttToMs(rrtime.delay_since_last_rr), 1); + last_xr_rtis = rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo(); + ASSERT_THAT(last_xr_rtis, SizeIs(1)); + EXPECT_EQ(kSenderSsrc, last_xr_rtis[0].ssrc); + EXPECT_EQ(CompactNtp(kNtp), last_xr_rtis[0].last_rr); + EXPECT_EQ(0U, last_xr_rtis[0].delay_since_last_rr); } TEST_F(RtcpReceiverTest, ExtendedReportsDlrrPacketNotToUsIgnored) { @@ -788,8 +801,9 @@ TEST_F(RtcpReceiverTest, InjectExtendedReportsPacketWithMultipleReportBlocks) { InjectRtcpPacket(xr); - ReceiveTimeInfo rrtime; - EXPECT_TRUE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&rrtime)); + std::vector last_xr_rtis = + rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo(); + EXPECT_THAT(last_xr_rtis, SizeIs(1)); int64_t rtt_ms = 0; EXPECT_TRUE(rtcp_receiver_.GetAndResetXrRrRtt(&rtt_ms)); } @@ -813,8 +827,9 @@ TEST_F(RtcpReceiverTest, InjectExtendedReportsPacketWithUnknownReportBlock) { InjectRtcpPacket(packet); // Validate Rrtr was received and processed. - ReceiveTimeInfo rrtime; - EXPECT_TRUE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&rrtime)); + std::vector last_xr_rtis = + rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo(); + EXPECT_THAT(last_xr_rtis, SizeIs(1)); // Validate Dlrr report wasn't processed. int64_t rtt_ms = 0; EXPECT_FALSE(rtcp_receiver_.GetAndResetXrRrRtt(&rtt_ms)); @@ -869,12 +884,11 @@ TEST_F(RtcpReceiverTest, XrDlrrCalculatesNegativeRttAsOne) { EXPECT_EQ(1, rtt_ms); } -TEST_F(RtcpReceiverTest, LastReceivedXrReferenceTimeInfoInitiallyFalse) { - ReceiveTimeInfo info; - EXPECT_FALSE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&info)); +TEST_F(RtcpReceiverTest, ConsumeReceivedXrReferenceTimeInfoInitiallyEmpty) { + EXPECT_THAT(rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo(), IsEmpty()); } -TEST_F(RtcpReceiverTest, GetLastReceivedExtendedReportsReferenceTimeInfo) { +TEST_F(RtcpReceiverTest, ConsumeReceivedXrReferenceTimeInfo) { const NtpTime kNtp(0x10203, 0x40506); const uint32_t kNtpMid = CompactNtp(kNtp); @@ -886,15 +900,69 @@ TEST_F(RtcpReceiverTest, GetLastReceivedExtendedReportsReferenceTimeInfo) { InjectRtcpPacket(xr); - ReceiveTimeInfo info; - EXPECT_TRUE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&info)); - EXPECT_EQ(kSenderSsrc, info.ssrc); - EXPECT_EQ(kNtpMid, info.last_rr); - EXPECT_EQ(0U, info.delay_since_last_rr); - system_clock_.AdvanceTimeMilliseconds(1000); - EXPECT_TRUE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&info)); - EXPECT_EQ(65536U, info.delay_since_last_rr); + + std::vector last_xr_rtis = + rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo(); + ASSERT_THAT(last_xr_rtis, SizeIs(1)); + EXPECT_EQ(kSenderSsrc, last_xr_rtis[0].ssrc); + EXPECT_EQ(kNtpMid, last_xr_rtis[0].last_rr); + EXPECT_EQ(65536U, last_xr_rtis[0].delay_since_last_rr); +} + +TEST_F(RtcpReceiverTest, + ReceivedRrtrFromSameSsrcUpdatesReceivedReferenceTimeInfo) { + const NtpTime kNtp1(0x10203, 0x40506); + const NtpTime kNtp2(0x11223, 0x44556); + const int64_t kDelayMs = 2000; + + rtcp::ExtendedReports xr; + xr.SetSenderSsrc(kSenderSsrc); + rtcp::Rrtr rrtr1; + rrtr1.SetNtp(kNtp1); + xr.SetRrtr(rrtr1); + InjectRtcpPacket(xr); + system_clock_.AdvanceTimeMilliseconds(kDelayMs); + rtcp::Rrtr rrtr2; + rrtr2.SetNtp(kNtp2); + xr.SetRrtr(rrtr2); + InjectRtcpPacket(xr); + system_clock_.AdvanceTimeMilliseconds(kDelayMs); + + std::vector last_xr_rtis = + rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo(); + ASSERT_THAT(last_xr_rtis, SizeIs(1)); + EXPECT_EQ(kSenderSsrc, last_xr_rtis[0].ssrc); + EXPECT_EQ(CompactNtp(kNtp2), last_xr_rtis[0].last_rr); + EXPECT_EQ(kDelayMs * 65536 / 1000, last_xr_rtis[0].delay_since_last_rr); +} + +TEST_F(RtcpReceiverTest, StoresLastReceivedRrtrPerSsrc) { + const size_t kNumBufferedReports = 1; + const size_t kNumReports = + rtcp::ExtendedReports::kMaxNumberOfDlrrItems + kNumBufferedReports; + for (size_t i = 0; i < kNumReports; ++i) { + rtcp::ExtendedReports xr; + xr.SetSenderSsrc(i * 100); + rtcp::Rrtr rrtr; + rrtr.SetNtp(NtpTime(i * 200, i * 300)); + xr.SetRrtr(rrtr); + InjectRtcpPacket(xr); + system_clock_.AdvanceTimeMilliseconds(1000); + } + + std::vector last_xr_rtis = + rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo(); + ASSERT_THAT(last_xr_rtis, + SizeIs(rtcp::ExtendedReports::kMaxNumberOfDlrrItems)); + for (size_t i = 0; i < rtcp::ExtendedReports::kMaxNumberOfDlrrItems; ++i) { + EXPECT_EQ(i * 100, last_xr_rtis[i].ssrc); + EXPECT_EQ(CompactNtp(NtpTime(i * 200, i * 300)), last_xr_rtis[i].last_rr); + EXPECT_EQ(65536U * (kNumReports - i), last_xr_rtis[i].delay_since_last_rr); + } + + last_xr_rtis = rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo(); + ASSERT_THAT(last_xr_rtis, SizeIs(kNumBufferedReports)); } TEST_F(RtcpReceiverTest, ReceiveReportTimeout) { diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index 415de950cf..6b266cb66e 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -85,9 +85,14 @@ RTCPSender::FeedbackState::FeedbackState() last_rr_ntp_secs(0), last_rr_ntp_frac(0), remote_sr(0), - has_last_xr_rr(false), module(nullptr) {} +RTCPSender::FeedbackState::FeedbackState(const FeedbackState&) = default; + +RTCPSender::FeedbackState::FeedbackState(FeedbackState&&) = default; + +RTCPSender::FeedbackState::~FeedbackState() = default; + class PacketContainer : public rtcp::CompoundPacket { public: PacketContainer(Transport* transport, RtcEventLog* event_log) @@ -644,8 +649,8 @@ std::unique_ptr RTCPSender::BuildExtendedReports( xr->SetRrtr(rrtr); } - if (ctx.feedback_state_.has_last_xr_rr) { - xr->AddDlrrItem(ctx.feedback_state_.last_xr_rr); + for (const rtcp::ReceiveTimeInfo& rti : ctx.feedback_state_.last_xr_rtis) { + xr->AddDlrrItem(rti); } if (video_bitrate_allocation_) { @@ -791,7 +796,7 @@ void RTCPSender::PrepareReport(const FeedbackState& feedback_state) { if (generate_report) { if ((!sending_ && xr_send_receiver_reference_time_enabled_) || - feedback_state.has_last_xr_rr || video_bitrate_allocation_) { + !feedback_state.last_xr_rtis.empty() || video_bitrate_allocation_) { SetFlag(kRtcpAnyExtendedReports, true); } diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h index 134a6fb2e9..f6cb55e0a5 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.h +++ b/modules/rtp_rtcp/source/rtcp_sender.h @@ -59,6 +59,10 @@ class RTCPSender { public: struct FeedbackState { FeedbackState(); + FeedbackState(const FeedbackState&); + FeedbackState(FeedbackState&&); + + ~FeedbackState(); uint32_t packets_sent; size_t media_bytes_sent; @@ -68,8 +72,7 @@ class RTCPSender { uint32_t last_rr_ntp_frac; uint32_t remote_sr; - bool has_last_xr_rr; - rtcp::ReceiveTimeInfo last_xr_rr; + std::vector last_xr_rtis; // Used when generating TMMBR. ModuleRtpRtcpImpl* module; diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 77423746f1..98513325e1 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -24,6 +24,7 @@ using ::testing::_; using ::testing::ElementsAre; using ::testing::Invoke; +using ::testing::SizeIs; namespace webrtc { @@ -608,22 +609,47 @@ TEST_F(RtcpSenderTest, SendXrWithVoipMetric) { TEST_F(RtcpSenderTest, SendXrWithDlrr) { rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound); RTCPSender::FeedbackState feedback_state = rtp_rtcp_impl_->GetFeedbackState(); - feedback_state.has_last_xr_rr = true; rtcp::ReceiveTimeInfo last_xr_rr; last_xr_rr.ssrc = 0x11111111; last_xr_rr.last_rr = 0x22222222; last_xr_rr.delay_since_last_rr = 0x33333333; - feedback_state.last_xr_rr = last_xr_rr; + feedback_state.last_xr_rtis.push_back(last_xr_rr); EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state, kRtcpReport)); EXPECT_EQ(1, parser()->xr()->num_packets()); EXPECT_EQ(kSenderSsrc, parser()->xr()->sender_ssrc()); - EXPECT_EQ(1U, parser()->xr()->dlrr().sub_blocks().size()); + ASSERT_THAT(parser()->xr()->dlrr().sub_blocks(), SizeIs(1)); EXPECT_EQ(last_xr_rr.ssrc, parser()->xr()->dlrr().sub_blocks()[0].ssrc); EXPECT_EQ(last_xr_rr.last_rr, parser()->xr()->dlrr().sub_blocks()[0].last_rr); EXPECT_EQ(last_xr_rr.delay_since_last_rr, parser()->xr()->dlrr().sub_blocks()[0].delay_since_last_rr); } +TEST_F(RtcpSenderTest, SendXrWithMultipleDlrrSubBlocks) { + const size_t kNumReceivers = 2; + rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound); + RTCPSender::FeedbackState feedback_state = rtp_rtcp_impl_->GetFeedbackState(); + for (size_t i = 0; i < kNumReceivers; ++i) { + rtcp::ReceiveTimeInfo last_xr_rr; + last_xr_rr.ssrc = i; + last_xr_rr.last_rr = (i + 1) * 100; + last_xr_rr.delay_since_last_rr = (i + 2) * 200; + feedback_state.last_xr_rtis.push_back(last_xr_rr); + } + + EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state, kRtcpReport)); + EXPECT_EQ(1, parser()->xr()->num_packets()); + EXPECT_EQ(kSenderSsrc, parser()->xr()->sender_ssrc()); + ASSERT_THAT(parser()->xr()->dlrr().sub_blocks(), SizeIs(kNumReceivers)); + for (size_t i = 0; i < kNumReceivers; ++i) { + EXPECT_EQ(feedback_state.last_xr_rtis[i].ssrc, + parser()->xr()->dlrr().sub_blocks()[i].ssrc); + EXPECT_EQ(feedback_state.last_xr_rtis[i].last_rr, + parser()->xr()->dlrr().sub_blocks()[i].last_rr); + EXPECT_EQ(feedback_state.last_xr_rtis[i].delay_since_last_rr, + parser()->xr()->dlrr().sub_blocks()[i].delay_since_last_rr); + } +} + TEST_F(RtcpSenderTest, SendXrWithRrtr) { rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound); EXPECT_EQ(0, rtcp_sender_->SetSendingStatus(feedback_state(), false)); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 45983a0fd9..22e399dfd1 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -377,8 +377,7 @@ RTCPSender::FeedbackState ModuleRtpRtcpImpl::GetFeedbackState() { &state.last_rr_ntp_frac, &state.remote_sr); - state.has_last_xr_rr = - rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&state.last_xr_rr); + state.last_xr_rtis = rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo(); return state; }