diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr.cc index 97516f1468..cc44886966 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr.cc @@ -81,23 +81,5 @@ void Dlrr::Create(uint8_t* buffer) const { RTC_DCHECK_EQ(buffer + BlockLength(), write_at); } -bool Dlrr::AddDlrrItem(const ReceiveTimeInfo& block) { - if (sub_blocks_.size() >= kMaxNumberOfDlrrItems) { - LOG(LS_WARNING) << "Max DLRR items reached."; - return false; - } - sub_blocks_.push_back(block); - return true; -} - -bool Dlrr::AddDlrrItem(uint32_t ssrc, - uint32_t last_rr, - uint32_t delay_last_rr) { - ReceiveTimeInfo block; - block.ssrc = ssrc; - block.last_rr = last_rr; - block.delay_since_last_rr = delay_last_rr; - return AddDlrrItem(block); -} } // namespace rtcp } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr.h index c2a4b8fffc..c5f5c23bf1 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr.h @@ -32,7 +32,6 @@ struct ReceiveTimeInfo { class Dlrr { public: static const uint8_t kBlockType = 5; - static const size_t kMaxNumberOfDlrrItems = 100; Dlrr() {} Dlrr(const Dlrr& other) = default; @@ -40,6 +39,9 @@ class Dlrr { Dlrr& operator=(const Dlrr& other) = default; + // Dlrr without items treated same as no dlrr block. + explicit operator bool() const { return !sub_blocks_.empty(); } + // Second parameter is value read from block header, // i.e. size of block in 32bits excluding block header itself. bool Parse(const uint8_t* buffer, uint16_t block_length_32bits); @@ -49,9 +51,10 @@ class Dlrr { // Consumes BlockLength() bytes. void Create(uint8_t* buffer) const; - // Max 100 DLRR Items can be added per DLRR report block. - bool AddDlrrItem(const ReceiveTimeInfo& time_info); - bool AddDlrrItem(uint32_t ssrc, uint32_t last_rr, uint32_t delay_last_rr); + void ClearItems() { sub_blocks_.clear(); } + void AddDlrrItem(const ReceiveTimeInfo& time_info) { + sub_blocks_.push_back(time_info); + } const std::vector& sub_blocks() const { return sub_blocks_; } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr_unittest.cc index 161974da9e..6144ab7f36 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr_unittest.cc @@ -34,7 +34,7 @@ TEST(RtcpPacketDlrrTest, Empty) { TEST(RtcpPacketDlrrTest, Create) { Dlrr dlrr; - EXPECT_TRUE(dlrr.AddDlrrItem(kSsrc, kLastRR, kDelay)); + dlrr.AddDlrrItem(ReceiveTimeInfo(kSsrc, kLastRR, kDelay)); ASSERT_EQ(kBlockSizeBytes, dlrr.BlockLength()); uint8_t buffer[kBlockSizeBytes]; @@ -69,23 +69,15 @@ TEST(RtcpPacketDlrrTest, ParseFailsOnBadSize) { } } -TEST(RtcpPacketDlrrTest, FailsOnTooManySubBlocks) { - Dlrr dlrr; - for (size_t i = 1; i <= Dlrr::kMaxNumberOfDlrrItems; ++i) { - EXPECT_TRUE(dlrr.AddDlrrItem(kSsrc + i, kLastRR + i, kDelay + i)); - } - EXPECT_FALSE(dlrr.AddDlrrItem(kSsrc, kLastRR, kDelay)); -} - -TEST(RtcpPacketDlrrTest, CreateAndParseMaxSubBlocks) { +TEST(RtcpPacketDlrrTest, CreateAndParseManySubBlocks) { const size_t kBufferSize = 0x1000; // More than enough. + const size_t kManyDlrrItems = 50; uint8_t buffer[kBufferSize]; // Create. Dlrr dlrr; - for (size_t i = 1; i <= Dlrr::kMaxNumberOfDlrrItems; ++i) { - EXPECT_TRUE(dlrr.AddDlrrItem(kSsrc + i, kLastRR + i, kDelay + i)); - } + for (size_t i = 1; i <= kManyDlrrItems; ++i) + dlrr.AddDlrrItem(ReceiveTimeInfo(kSsrc + i, kLastRR + i, kDelay + i)); size_t used_buffer_size = dlrr.BlockLength(); ASSERT_LE(used_buffer_size, kBufferSize); dlrr.Create(buffer); @@ -95,6 +87,6 @@ TEST(RtcpPacketDlrrTest, CreateAndParseMaxSubBlocks) { uint16_t block_length = ByteReader::ReadBigEndian(&buffer[2]); EXPECT_EQ(used_buffer_size, (block_length + 1) * 4u); EXPECT_TRUE(parsed.Parse(buffer, block_length)); - EXPECT_TRUE(parsed.sub_blocks().size() == Dlrr::kMaxNumberOfDlrrItems); + EXPECT_EQ(kManyDlrrItems, parsed.sub_blocks().size()); } } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc index 0199b5276a..5775dd4ffc 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc @@ -52,9 +52,9 @@ bool ExtendedReports::Parse(const CommonHeader& packet) { } sender_ssrc_ = ByteReader::ReadBigEndian(packet.payload()); - rrtr_blocks_.clear(); - dlrr_blocks_.clear(); - voip_metric_blocks_.clear(); + rrtr_block_.reset(); + dlrr_block_.ClearItems(); + voip_metric_block_.reset(); const uint8_t* current_block = packet.payload() + kXrBaseLength; const uint8_t* const packet_end = @@ -91,31 +91,20 @@ bool ExtendedReports::Parse(const CommonHeader& packet) { return true; } -bool ExtendedReports::AddRrtr(const Rrtr& rrtr) { - if (rrtr_blocks_.size() >= kMaxNumberOfRrtrBlocks) { - LOG(LS_WARNING) << "Max RRTR blocks reached."; - return false; - } - rrtr_blocks_.push_back(rrtr); - return true; +void ExtendedReports::SetRrtr(const Rrtr& rrtr) { + if (rrtr_block_) + LOG(LS_WARNING) << "Rrtr already set, overwriting."; + rrtr_block_.emplace(rrtr); } -bool ExtendedReports::AddDlrr(const Dlrr& dlrr) { - if (dlrr_blocks_.size() >= kMaxNumberOfDlrrBlocks) { - LOG(LS_WARNING) << "Max DLRR blocks reached."; - return false; - } - dlrr_blocks_.push_back(dlrr); - return true; +void ExtendedReports::AddDlrrItem(const ReceiveTimeInfo& time_info) { + dlrr_block_.AddDlrrItem(time_info); } -bool ExtendedReports::AddVoipMetric(const VoipMetric& voip_metric) { - if (voip_metric_blocks_.size() >= kMaxNumberOfVoipMetricBlocks) { - LOG(LS_WARNING) << "Max Voip Metric blocks reached."; - return false; - } - voip_metric_blocks_.push_back(voip_metric); - return true; +void ExtendedReports::SetVoipMetric(const VoipMetric& voip_metric) { + if (voip_metric_block_) + LOG(LS_WARNING) << "Voip metric already set, overwriting."; + voip_metric_block_.emplace(voip_metric); } bool ExtendedReports::Create(uint8_t* packet, @@ -131,30 +120,22 @@ bool ExtendedReports::Create(uint8_t* packet, CreateHeader(kReserved, kPacketType, HeaderLength(), packet, index); ByteWriter::WriteBigEndian(packet + *index, sender_ssrc_); *index += sizeof(uint32_t); - for (const Rrtr& block : rrtr_blocks_) { - block.Create(packet + *index); + if (rrtr_block_) { + rrtr_block_->Create(packet + *index); *index += Rrtr::kLength; } - for (const Dlrr& block : dlrr_blocks_) { - block.Create(packet + *index); - *index += block.BlockLength(); + if (dlrr_block_) { + dlrr_block_.Create(packet + *index); + *index += dlrr_block_.BlockLength(); } - for (const VoipMetric& block : voip_metric_blocks_) { - block.Create(packet + *index); + if (voip_metric_block_) { + voip_metric_block_->Create(packet + *index); *index += VoipMetric::kLength; } RTC_CHECK_EQ(*index, index_end); return true; } -size_t ExtendedReports::DlrrLength() const { - size_t length = 0; - for (const Dlrr& block : dlrr_blocks_) { - length += block.BlockLength(); - } - return length; -} - void ExtendedReports::ParseRrtrBlock(const uint8_t* block, uint16_t block_length) { if (block_length != Rrtr::kBlockLength) { @@ -162,16 +143,21 @@ void ExtendedReports::ParseRrtrBlock(const uint8_t* block, << " Should be " << Rrtr::kBlockLength; return; } - rrtr_blocks_.push_back(Rrtr()); - rrtr_blocks_.back().Parse(block); + if (rrtr_block_) { + LOG(LS_WARNING) << "Two rrtr blocks found in same Extended Report packet"; + return; + } + rrtr_block_.emplace(); + rrtr_block_->Parse(block); } void ExtendedReports::ParseDlrrBlock(const uint8_t* block, uint16_t block_length) { - dlrr_blocks_.push_back(Dlrr()); - if (!dlrr_blocks_.back().Parse(block, block_length)) { - dlrr_blocks_.pop_back(); + if (dlrr_block_) { + LOG(LS_WARNING) << "Two Dlrr blocks found in same Extended Report packet"; + return; } + dlrr_block_.Parse(block, block_length); } void ExtendedReports::ParseVoipMetricBlock(const uint8_t* block, @@ -181,8 +167,13 @@ void ExtendedReports::ParseVoipMetricBlock(const uint8_t* block, << " Should be " << VoipMetric::kBlockLength; return; } - voip_metric_blocks_.push_back(VoipMetric()); - voip_metric_blocks_.back().Parse(block); + if (voip_metric_block_) { + LOG(LS_WARNING) + << "Two Voip Metric blocks found in same Extended Report packet"; + return; + } + voip_metric_block_.emplace(); + voip_metric_block_->Parse(block); } } // namespace rtcp } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h index 7b6c54452f..c433477707 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h @@ -14,6 +14,7 @@ #include #include "webrtc/base/constructormagic.h" +#include "webrtc/base/optional.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/rrtr.h" @@ -36,16 +37,15 @@ class ExtendedReports : public RtcpPacket { void SetSenderSsrc(uint32_t ssrc) { sender_ssrc_ = ssrc; } - // Max 50 items of each of {Rrtr, Dlrr, VoipMetric} allowed per Xr. - bool AddRrtr(const Rrtr& rrtr); - bool AddDlrr(const Dlrr& dlrr); - bool AddVoipMetric(const VoipMetric& voip_metric); + void SetRrtr(const Rrtr& rrtr); + void AddDlrrItem(const ReceiveTimeInfo& time_info); + void SetVoipMetric(const VoipMetric& voip_metric); uint32_t sender_ssrc() const { return sender_ssrc_; } - const std::vector& rrtrs() const { return rrtr_blocks_; } - const std::vector& dlrrs() const { return dlrr_blocks_; } - const std::vector& voip_metrics() const { - return voip_metric_blocks_; + const rtc::Optional& rrtr() const { return rrtr_block_; } + const Dlrr& dlrr() const { return dlrr_block_; } + const rtc::Optional& voip_metric() const { + return voip_metric_block_; } protected: @@ -55,9 +55,6 @@ class ExtendedReports : public RtcpPacket { RtcpPacket::PacketReadyCallback* callback) const override; private: - static const size_t kMaxNumberOfRrtrBlocks = 50; - static const size_t kMaxNumberOfDlrrBlocks = 50; - static const size_t kMaxNumberOfVoipMetricBlocks = 50; static constexpr size_t kXrBaseLength = 4; size_t BlockLength() const override { @@ -65,10 +62,10 @@ class ExtendedReports : public RtcpPacket { VoipMetricLength(); } - size_t RrtrLength() const { return Rrtr::kLength * rrtr_blocks_.size(); } - size_t DlrrLength() const; + size_t RrtrLength() const { return rrtr_block_ ? Rrtr::kLength : 0; } + size_t DlrrLength() const { return dlrr_block_.BlockLength(); } size_t VoipMetricLength() const { - return VoipMetric::kLength * voip_metric_blocks_.size(); + return voip_metric_block_ ? VoipMetric::kLength : 0; } void ParseRrtrBlock(const uint8_t* block, uint16_t block_length); @@ -76,9 +73,9 @@ class ExtendedReports : public RtcpPacket { void ParseVoipMetricBlock(const uint8_t* block, uint16_t block_length); uint32_t sender_ssrc_; - std::vector rrtr_blocks_; - std::vector dlrr_blocks_; - std::vector voip_metric_blocks_; + rtc::Optional rrtr_block_; + Dlrr dlrr_block_; // Dlrr without items treated same as no dlrr block. + rtc::Optional voip_metric_block_; RTC_DISALLOW_COPY_AND_ASSIGN(ExtendedReports); }; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc index 3c5025caaa..905462ace9 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc @@ -17,7 +17,6 @@ using testing::ElementsAre; using testing::ElementsAreArray; -using testing::IsEmpty; using testing::make_tuple; using webrtc::rtcp::Dlrr; using webrtc::rtcp::ExtendedReports; @@ -62,10 +61,6 @@ bool operator==(const ReceiveTimeInfo& time1, const ReceiveTimeInfo& time2) { time1.delay_since_last_rr == time2.delay_since_last_rr; } -bool operator==(const Dlrr& dlrr1, const Dlrr& dlrr2) { - return dlrr1.sub_blocks() == dlrr2.sub_blocks(); -} - bool operator==(const VoipMetric& metric1, const VoipMetric& metric2) { return metric1.ssrc() == metric2.ssrc() && metric1.voip_metric() == metric2.voip_metric(); @@ -162,16 +157,16 @@ TEST_F(RtcpPacketExtendedReportsTest, ParseWithoutReportBlocks) { ExtendedReports parsed; EXPECT_TRUE(test::ParseSinglePacket(kEmptyPacket, &parsed)); EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); - EXPECT_THAT(parsed.rrtrs(), IsEmpty()); - EXPECT_THAT(parsed.dlrrs(), IsEmpty()); - EXPECT_THAT(parsed.voip_metrics(), IsEmpty()); + EXPECT_FALSE(parsed.rrtr()); + EXPECT_FALSE(parsed.dlrr()); + EXPECT_FALSE(parsed.voip_metric()); } -TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithOneRrtrBlock) { - Rrtr rrtr = Rand(); +TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithRrtrBlock) { + const Rrtr kRrtr = Rand(); ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - EXPECT_TRUE(xr.AddRrtr(rrtr)); + xr.SetRrtr(kRrtr); rtc::Buffer packet = xr.Build(); ExtendedReports mparsed; @@ -179,33 +174,14 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithOneRrtrBlock) { const ExtendedReports& parsed = mparsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); - EXPECT_THAT(parsed.rrtrs(), ElementsAre(rrtr)); -} - -TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithTwoRrtrBlocks) { - Rrtr rrtr1 = Rand(); - Rrtr rrtr2 = Rand(); - ExtendedReports xr; - xr.SetSenderSsrc(kSenderSsrc); - EXPECT_TRUE(xr.AddRrtr(rrtr1)); - EXPECT_TRUE(xr.AddRrtr(rrtr2)); - - rtc::Buffer packet = xr.Build(); - - ExtendedReports mparsed; - EXPECT_TRUE(test::ParseSinglePacket(packet, &mparsed)); - const ExtendedReports& parsed = mparsed; - - EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); - EXPECT_THAT(parsed.rrtrs(), ElementsAre(rrtr1, rrtr2)); + EXPECT_EQ(kRrtr, parsed.rrtr()); } TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithDlrrWithOneSubBlock) { - Dlrr dlrr; - EXPECT_TRUE(dlrr.AddDlrrItem(Rand())); + const ReceiveTimeInfo kTimeInfo = Rand(); ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - EXPECT_TRUE(xr.AddDlrr(dlrr)); + xr.AddDlrrItem(kTimeInfo); rtc::Buffer packet = xr.Build(); @@ -214,16 +190,16 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithDlrrWithOneSubBlock) { const ExtendedReports& parsed = mparsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); - EXPECT_THAT(parsed.dlrrs(), ElementsAre(dlrr)); + EXPECT_THAT(parsed.dlrr().sub_blocks(), ElementsAre(kTimeInfo)); } TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithDlrrWithTwoSubBlocks) { - Dlrr dlrr; - EXPECT_TRUE(dlrr.AddDlrrItem(Rand())); - EXPECT_TRUE(dlrr.AddDlrrItem(Rand())); + const ReceiveTimeInfo kTimeInfo1 = Rand(); + const ReceiveTimeInfo kTimeInfo2 = Rand(); ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - EXPECT_TRUE(xr.AddDlrr(dlrr)); + xr.AddDlrrItem(kTimeInfo1); + xr.AddDlrrItem(kTimeInfo2); rtc::Buffer packet = xr.Build(); @@ -232,35 +208,15 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithDlrrWithTwoSubBlocks) { const ExtendedReports& parsed = mparsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); - EXPECT_THAT(parsed.dlrrs(), ElementsAre(dlrr)); -} - -TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithTwoDlrrBlocks) { - Dlrr dlrr1; - EXPECT_TRUE(dlrr1.AddDlrrItem(Rand())); - Dlrr dlrr2; - EXPECT_TRUE(dlrr2.AddDlrrItem(Rand())); - ExtendedReports xr; - xr.SetSenderSsrc(kSenderSsrc); - EXPECT_TRUE(xr.AddDlrr(dlrr1)); - EXPECT_TRUE(xr.AddDlrr(dlrr2)); - - rtc::Buffer packet = xr.Build(); - - ExtendedReports mparsed; - EXPECT_TRUE(test::ParseSinglePacket(packet, &mparsed)); - const ExtendedReports& parsed = mparsed; - - EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); - EXPECT_THAT(parsed.dlrrs(), ElementsAre(dlrr1, dlrr2)); + EXPECT_THAT(parsed.dlrr().sub_blocks(), ElementsAre(kTimeInfo1, kTimeInfo2)); } TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithVoipMetric) { - VoipMetric voip_metric = Rand(); + const VoipMetric kVoipMetric = Rand(); ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - EXPECT_TRUE(xr.AddVoipMetric(voip_metric)); + xr.SetVoipMetric(kVoipMetric); rtc::Buffer packet = xr.Build(); @@ -269,19 +225,19 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithVoipMetric) { const ExtendedReports& parsed = mparsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); - EXPECT_THAT(parsed.voip_metrics(), ElementsAre(voip_metric)); + EXPECT_EQ(kVoipMetric, parsed.voip_metric()); } TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithMultipleReportBlocks) { - Rrtr rrtr = Rand(); - Dlrr dlrr; - EXPECT_TRUE(dlrr.AddDlrrItem(Rand())); - VoipMetric metric = Rand(); + const Rrtr kRrtr = Rand(); + const ReceiveTimeInfo kTimeInfo = Rand(); + const VoipMetric kVoipMetric = Rand(); + ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - EXPECT_TRUE(xr.AddRrtr(rrtr)); - EXPECT_TRUE(xr.AddDlrr(dlrr)); - EXPECT_TRUE(xr.AddVoipMetric(metric)); + xr.SetRrtr(kRrtr); + xr.AddDlrrItem(kTimeInfo); + xr.SetVoipMetric(kVoipMetric); rtc::Buffer packet = xr.Build(); @@ -290,49 +246,9 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithMultipleReportBlocks) { const ExtendedReports& parsed = mparsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); - EXPECT_THAT(parsed.rrtrs(), ElementsAre(rrtr)); - EXPECT_THAT(parsed.dlrrs(), ElementsAre(dlrr)); - EXPECT_THAT(parsed.voip_metrics(), ElementsAre(metric)); + EXPECT_EQ(kRrtr, parsed.rrtr()); + EXPECT_THAT(parsed.dlrr().sub_blocks(), ElementsAre(kTimeInfo)); + EXPECT_EQ(kVoipMetric, parsed.voip_metric()); } -TEST_F(RtcpPacketExtendedReportsTest, DlrrWithoutItemNotIncludedInPacket) { - Rrtr rrtr = Rand(); - Dlrr dlrr; - VoipMetric metric = Rand(); - ExtendedReports xr; - xr.SetSenderSsrc(kSenderSsrc); - EXPECT_TRUE(xr.AddRrtr(rrtr)); - EXPECT_TRUE(xr.AddDlrr(dlrr)); - EXPECT_TRUE(xr.AddVoipMetric(metric)); - - rtc::Buffer packet = xr.Build(); - - ExtendedReports mparsed; - EXPECT_TRUE(test::ParseSinglePacket(packet, &mparsed)); - const ExtendedReports& parsed = mparsed; - - EXPECT_THAT(parsed.rrtrs(), ElementsAre(rrtr)); - EXPECT_THAT(parsed.dlrrs(), IsEmpty()); - EXPECT_THAT(parsed.voip_metrics(), ElementsAre(metric)); -} - -TEST_F(RtcpPacketExtendedReportsTest, WithTooManyBlocks) { - const size_t kMaxBlocks = 50; - ExtendedReports xr; - - Rrtr rrtr = Rand(); - for (size_t i = 0; i < kMaxBlocks; ++i) - EXPECT_TRUE(xr.AddRrtr(rrtr)); - EXPECT_FALSE(xr.AddRrtr(rrtr)); - - Dlrr dlrr; - for (size_t i = 0; i < kMaxBlocks; ++i) - EXPECT_TRUE(xr.AddDlrr(dlrr)); - EXPECT_FALSE(xr.AddDlrr(dlrr)); - - VoipMetric voip_metric = Rand(); - for (size_t i = 0; i < kMaxBlocks; ++i) - EXPECT_TRUE(xr.AddVoipMetric(voip_metric)); - EXPECT_FALSE(xr.AddVoipMetric(voip_metric)); -} } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index db11d26bc6..c78637abf3 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -688,13 +688,11 @@ void RTCPReceiver::HandleXr(const CommonHeader& rtcp_block, return; } - for (const rtcp::Rrtr& rrtr : xr.rrtrs()) - HandleXrReceiveReferenceTime(xr.sender_ssrc(), rrtr); + if (xr.rrtr()) + HandleXrReceiveReferenceTime(xr.sender_ssrc(), *xr.rrtr()); - for (const rtcp::Dlrr& dlrr : xr.dlrrs()) { - for (const rtcp::ReceiveTimeInfo& time_info : dlrr.sub_blocks()) - HandleXrDlrrReportBlock(time_info); - } + for (const rtcp::ReceiveTimeInfo& time_info : xr.dlrr().sub_blocks()) + HandleXrDlrrReportBlock(time_info); } void RTCPReceiver::HandleXrReceiveReferenceTime( diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index e16673c18e..c98400f3fb 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -53,6 +53,7 @@ using ::testing::SizeIs; using ::testing::StrEq; using ::testing::StrictMock; using ::testing::UnorderedElementsAre; +using rtcp::ReceiveTimeInfo; class MockRtcpPacketTypeCounterObserver : public RtcpPacketTypeCounterObserver { public: @@ -698,7 +699,7 @@ TEST_F(RtcpReceiverTest, InjectExtendedReportsVoipPacket) { voip_metric.SetVoipMetric(metric); rtcp::ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - xr.AddVoipMetric(voip_metric); + xr.SetVoipMetric(voip_metric); InjectRtcpPacket(xr); } @@ -708,7 +709,7 @@ TEST_F(RtcpReceiverTest, ExtendedReportsVoipPacketNotToUsIgnored) { voip_metric.SetMediaSsrc(kNotToUsSsrc); rtcp::ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - xr.AddVoipMetric(voip_metric); + xr.SetVoipMetric(voip_metric); InjectRtcpPacket(xr); } @@ -719,9 +720,9 @@ TEST_F(RtcpReceiverTest, InjectExtendedReportsReceiverReferenceTimePacket) { rrtr.SetNtp(kNtp); rtcp::ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - xr.AddRrtr(rrtr); + xr.SetRrtr(rrtr); - rtcp::ReceiveTimeInfo rrtime; + ReceiveTimeInfo rrtime; EXPECT_FALSE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&rrtime)); InjectRtcpPacket(xr); @@ -740,11 +741,9 @@ TEST_F(RtcpReceiverTest, ExtendedReportsDlrrPacketNotToUsIgnored) { // Allow calculate rtt using dlrr/rrtr, simulating media receiver side. rtcp_receiver_.SetRtcpXrRrtrStatus(true); - rtcp::Dlrr dlrr; - dlrr.AddDlrrItem(kNotToUsSsrc, 0x12345, 0x67890); rtcp::ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - xr.AddDlrr(dlrr); + xr.AddDlrrItem(ReceiveTimeInfo(kNotToUsSsrc, 0x12345, 0x67890)); InjectRtcpPacket(xr); @@ -759,11 +758,9 @@ TEST_F(RtcpReceiverTest, InjectExtendedReportsDlrrPacketWithSubBlock) { int64_t rtt_ms = 0; EXPECT_FALSE(rtcp_receiver_.GetAndResetXrRrRtt(&rtt_ms)); - rtcp::Dlrr dlrr; - dlrr.AddDlrrItem(kReceiverMainSsrc, kLastRR, kDelay); rtcp::ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - xr.AddDlrr(dlrr); + xr.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, kLastRR, kDelay)); InjectRtcpPacket(xr); @@ -780,11 +777,9 @@ TEST_F(RtcpReceiverTest, InjectExtendedReportsDlrrPacketWithMultipleSubBlocks) { rtcp::ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - rtcp::Dlrr dlrr; - dlrr.AddDlrrItem(kReceiverMainSsrc, kLastRR, kDelay); - dlrr.AddDlrrItem(kReceiverMainSsrc + 1, 0x12345, 0x67890); - dlrr.AddDlrrItem(kReceiverMainSsrc + 2, 0x12345, 0x67890); - xr.AddDlrr(dlrr); + xr.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, kLastRR, kDelay)); + xr.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc + 1, 0x12345, 0x67890)); + xr.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc + 2, 0x12345, 0x67890)); InjectRtcpPacket(xr); @@ -799,19 +794,17 @@ TEST_F(RtcpReceiverTest, InjectExtendedReportsPacketWithMultipleReportBlocks) { rtcp_receiver_.SetRtcpXrRrtrStatus(true); rtcp::Rrtr rrtr; - rtcp::Dlrr dlrr; - dlrr.AddDlrrItem(kReceiverMainSsrc, 0x12345, 0x67890); rtcp::VoipMetric metric; metric.SetMediaSsrc(kReceiverMainSsrc); rtcp::ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - xr.AddRrtr(rrtr); - xr.AddDlrr(dlrr); - xr.AddVoipMetric(metric); + xr.SetRrtr(rrtr); + xr.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, 0x12345, 0x67890)); + xr.SetVoipMetric(metric); InjectRtcpPacket(xr); - rtcp::ReceiveTimeInfo rrtime; + ReceiveTimeInfo rrtime; EXPECT_TRUE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&rrtime)); int64_t rtt_ms = 0; EXPECT_TRUE(rtcp_receiver_.GetAndResetXrRrRtt(&rtt_ms)); @@ -821,15 +814,13 @@ TEST_F(RtcpReceiverTest, InjectExtendedReportsPacketWithUnknownReportBlock) { rtcp_receiver_.SetRtcpXrRrtrStatus(true); rtcp::Rrtr rrtr; - rtcp::Dlrr dlrr; - dlrr.AddDlrrItem(kReceiverMainSsrc, 0x12345, 0x67890); rtcp::VoipMetric metric; metric.SetMediaSsrc(kReceiverMainSsrc); rtcp::ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - xr.AddRrtr(rrtr); - xr.AddDlrr(dlrr); - xr.AddVoipMetric(metric); + xr.SetRrtr(rrtr); + xr.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, 0x12345, 0x67890)); + xr.SetVoipMetric(metric); rtc::Buffer packet = xr.Build(); // Modify the DLRR block to have an unsupported block type, from 5 to 6. @@ -838,7 +829,7 @@ TEST_F(RtcpReceiverTest, InjectExtendedReportsPacketWithUnknownReportBlock) { InjectRtcpPacket(packet); // Validate Rrtr was received and processed. - rtcp::ReceiveTimeInfo rrtime; + ReceiveTimeInfo rrtime; EXPECT_TRUE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&rrtime)); // Validate Dlrr report wasn't processed. int64_t rtt_ms = 0; @@ -862,11 +853,9 @@ TEST_F(RtcpReceiverTest, RttCalculatedAfterExtendedReportsDlrr) { uint32_t sent_ntp = CompactNtp(now); system_clock_.AdvanceTimeMilliseconds(kRttMs + kDelayMs); - rtcp::Dlrr dlrr; - dlrr.AddDlrrItem(kReceiverMainSsrc, sent_ntp, kDelayNtp); rtcp::ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - xr.AddDlrr(dlrr); + xr.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, sent_ntp, kDelayNtp)); InjectRtcpPacket(xr); @@ -885,11 +874,9 @@ TEST_F(RtcpReceiverTest, XrDlrrCalculatesNegativeRttAsOne) { system_clock_.AdvanceTimeMilliseconds(kRttMs + kDelayMs); rtcp_receiver_.SetRtcpXrRrtrStatus(true); - rtcp::Dlrr dlrr; - dlrr.AddDlrrItem(kReceiverMainSsrc, sent_ntp, kDelayNtp); rtcp::ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - xr.AddDlrr(dlrr); + xr.AddDlrrItem(ReceiveTimeInfo(kReceiverMainSsrc, sent_ntp, kDelayNtp)); InjectRtcpPacket(xr); @@ -899,7 +886,7 @@ TEST_F(RtcpReceiverTest, XrDlrrCalculatesNegativeRttAsOne) { } TEST_F(RtcpReceiverTest, LastReceivedXrReferenceTimeInfoInitiallyFalse) { - rtcp::ReceiveTimeInfo info; + ReceiveTimeInfo info; EXPECT_FALSE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&info)); } @@ -911,11 +898,11 @@ TEST_F(RtcpReceiverTest, GetLastReceivedExtendedReportsReferenceTimeInfo) { rrtr.SetNtp(kNtp); rtcp::ExtendedReports xr; xr.SetSenderSsrc(kSenderSsrc); - xr.AddRrtr(rrtr); + xr.SetRrtr(rrtr); InjectRtcpPacket(xr); - rtcp::ReceiveTimeInfo info; + ReceiveTimeInfo info; EXPECT_TRUE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&info)); EXPECT_EQ(kSenderSsrc, info.ssrc); EXPECT_EQ(kNtpMid, info.last_rr); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc index 8fe3334789..e6cf669522 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc @@ -701,7 +701,7 @@ std::unique_ptr RTCPSender::BuildReceiverReferenceTime( rtcp::Rrtr rrtr; rrtr.SetNtp(NtpTime(ctx.ntp_sec_, ctx.ntp_frac_)); - xr->AddRrtr(rrtr); + xr->SetRrtr(rrtr); // TODO(sprang): Merge XR report sending to contain all of RRTR, DLRR, VOIP? @@ -712,17 +712,12 @@ std::unique_ptr RTCPSender::BuildDlrr( const RtcpContext& ctx) { rtcp::ExtendedReports* xr = new rtcp::ExtendedReports(); xr->SetSenderSsrc(ssrc_); - - rtcp::Dlrr dlrr; RTC_DCHECK(ctx.feedback_state_.has_last_xr_rr); - dlrr.AddDlrrItem(ctx.feedback_state_.last_xr_rr); - - xr->AddDlrr(dlrr); + xr->AddDlrrItem(ctx.feedback_state_.last_xr_rr); return std::unique_ptr(xr); } -// TODO(sprang): Add a unit test for this, or remove if the code isn't used. std::unique_ptr RTCPSender::BuildVoIPMetric( const RtcpContext& context) { rtcp::ExtendedReports* xr = new rtcp::ExtendedReports(); @@ -732,7 +727,7 @@ std::unique_ptr RTCPSender::BuildVoIPMetric( voip.SetMediaSsrc(remote_ssrc_); voip.SetVoipMetric(xr_voip_metric_); - xr->AddVoipMetric(voip); + xr->SetVoipMetric(voip); return std::unique_ptr(xr); } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index d348d3a94a..235c84b09e 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -606,28 +606,29 @@ TEST_F(RtcpSenderTest, SendXrWithVoipMetric) { EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpXrVoipMetric)); EXPECT_EQ(1, parser()->xr()->num_packets()); EXPECT_EQ(kSenderSsrc, parser()->xr()->sender_ssrc()); - EXPECT_EQ(1U, parser()->xr()->voip_metrics().size()); - EXPECT_EQ(kRemoteSsrc, parser()->xr()->voip_metrics()[0].ssrc()); - EXPECT_EQ(metric.lossRate, parser()->voip_metric()->lossRate); - EXPECT_EQ(metric.discardRate, parser()->voip_metric()->discardRate); - EXPECT_EQ(metric.burstDensity, parser()->voip_metric()->burstDensity); - EXPECT_EQ(metric.gapDensity, parser()->voip_metric()->gapDensity); - EXPECT_EQ(metric.burstDuration, parser()->voip_metric()->burstDuration); - EXPECT_EQ(metric.gapDuration, parser()->voip_metric()->gapDuration); - EXPECT_EQ(metric.roundTripDelay, parser()->voip_metric()->roundTripDelay); - EXPECT_EQ(metric.endSystemDelay, parser()->voip_metric()->endSystemDelay); - EXPECT_EQ(metric.signalLevel, parser()->voip_metric()->signalLevel); - EXPECT_EQ(metric.noiseLevel, parser()->voip_metric()->noiseLevel); - EXPECT_EQ(metric.RERL, parser()->voip_metric()->RERL); - EXPECT_EQ(metric.Gmin, parser()->voip_metric()->Gmin); - EXPECT_EQ(metric.Rfactor, parser()->voip_metric()->Rfactor); - EXPECT_EQ(metric.extRfactor, parser()->voip_metric()->extRfactor); - EXPECT_EQ(metric.MOSLQ, parser()->voip_metric()->MOSLQ); - EXPECT_EQ(metric.MOSCQ, parser()->voip_metric()->MOSCQ); - EXPECT_EQ(metric.RXconfig, parser()->voip_metric()->RXconfig); - EXPECT_EQ(metric.JBnominal, parser()->voip_metric()->JBnominal); - EXPECT_EQ(metric.JBmax, parser()->voip_metric()->JBmax); - EXPECT_EQ(metric.JBabsMax, parser()->voip_metric()->JBabsMax); + ASSERT_TRUE(parser()->xr()->voip_metric()); + EXPECT_EQ(kRemoteSsrc, parser()->xr()->voip_metric()->ssrc()); + const auto& parsed_metric = parser()->xr()->voip_metric()->voip_metric(); + EXPECT_EQ(metric.lossRate, parsed_metric.lossRate); + EXPECT_EQ(metric.discardRate, parsed_metric.discardRate); + EXPECT_EQ(metric.burstDensity, parsed_metric.burstDensity); + EXPECT_EQ(metric.gapDensity, parsed_metric.gapDensity); + EXPECT_EQ(metric.burstDuration, parsed_metric.burstDuration); + EXPECT_EQ(metric.gapDuration, parsed_metric.gapDuration); + EXPECT_EQ(metric.roundTripDelay, parsed_metric.roundTripDelay); + EXPECT_EQ(metric.endSystemDelay, parsed_metric.endSystemDelay); + EXPECT_EQ(metric.signalLevel, parsed_metric.signalLevel); + EXPECT_EQ(metric.noiseLevel, parsed_metric.noiseLevel); + EXPECT_EQ(metric.RERL, parsed_metric.RERL); + EXPECT_EQ(metric.Gmin, parsed_metric.Gmin); + EXPECT_EQ(metric.Rfactor, parsed_metric.Rfactor); + EXPECT_EQ(metric.extRfactor, parsed_metric.extRfactor); + EXPECT_EQ(metric.MOSLQ, parsed_metric.MOSLQ); + EXPECT_EQ(metric.MOSCQ, parsed_metric.MOSCQ); + EXPECT_EQ(metric.RXconfig, parsed_metric.RXconfig); + EXPECT_EQ(metric.JBnominal, parsed_metric.JBnominal); + EXPECT_EQ(metric.JBmax, parsed_metric.JBmax); + EXPECT_EQ(metric.JBabsMax, parsed_metric.JBabsMax); } TEST_F(RtcpSenderTest, SendXrWithDlrr) { @@ -642,13 +643,11 @@ TEST_F(RtcpSenderTest, SendXrWithDlrr) { 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()->dlrrs().size()); - EXPECT_EQ(1U, parser()->xr()->dlrrs()[0].sub_blocks().size()); - EXPECT_EQ(last_xr_rr.ssrc, parser()->xr()->dlrrs()[0].sub_blocks()[0].ssrc); - EXPECT_EQ(last_xr_rr.last_rr, - parser()->xr()->dlrrs()[0].sub_blocks()[0].last_rr); + EXPECT_EQ(1U, parser()->xr()->dlrr().sub_blocks().size()); + 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()->dlrrs()[0].sub_blocks()[0].delay_since_last_rr); + parser()->xr()->dlrr().sub_blocks()[0].delay_since_last_rr); } TEST_F(RtcpSenderTest, SendXrWithRrtr) { @@ -661,10 +660,11 @@ TEST_F(RtcpSenderTest, SendXrWithRrtr) { 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(0U, parser()->xr()->dlrrs().size()); - EXPECT_EQ(1U, parser()->xr()->rrtrs().size()); - EXPECT_EQ(ntp_secs, parser()->xr()->rrtrs()[0].ntp().seconds()); - EXPECT_EQ(ntp_frac, parser()->xr()->rrtrs()[0].ntp().fractions()); + EXPECT_FALSE(parser()->xr()->dlrr()); + EXPECT_FALSE(parser()->xr()->voip_metric()); + ASSERT_TRUE(parser()->xr()->rrtr()); + EXPECT_EQ(ntp_secs, parser()->xr()->rrtr()->ntp().seconds()); + EXPECT_EQ(ntp_frac, parser()->xr()->rrtr()->ntp().fractions()); } TEST_F(RtcpSenderTest, TestNoXrRrtrSentIfSending) { diff --git a/webrtc/test/rtcp_packet_parser.h b/webrtc/test/rtcp_packet_parser.h index 4fa1405e7b..4a8eca0d67 100644 --- a/webrtc/test/rtcp_packet_parser.h +++ b/webrtc/test/rtcp_packet_parser.h @@ -92,9 +92,6 @@ class RtcpPacketParser { PacketCounter* transport_feedback() { return &transport_feedback_; } - const RTCPVoIPMetric* voip_metric() { - return &xr_.voip_metrics()[0].voip_metric(); - } private: PacketCounter app_;