Allow max 1 block per type in RTCP Extended Reports
Design of individual block in ExtendedReports packet suggest there is
no point to have more than one block per type.
This CL reduce complexity of having several blocks of the same type in
same report.
BUG=webrtc:5260
Review-Url: https://codereview.webrtc.org/2378113002
Cr-Commit-Position: refs/heads/master@{#14855}
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr.cc
index 97516f1..cc44886 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 @@
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 c2a4b8f..c5f5c23 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 @@
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 @@
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 @@
// 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<ReceiveTimeInfo>& 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 161974d..6144ab7 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, 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, 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 @@
uint16_t block_length = ByteReader<uint16_t>::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 0199b52..5775dd4 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 @@
}
sender_ssrc_ = ByteReader<uint32_t>::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 @@
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 @@
CreateHeader(kReserved, kPacketType, HeaderLength(), packet, index);
ByteWriter<uint32_t>::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 @@
<< " 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 @@
<< " 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 7b6c544..c433477 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 <vector>
#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 @@
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<Rrtr>& rrtrs() const { return rrtr_blocks_; }
- const std::vector<Dlrr>& dlrrs() const { return dlrr_blocks_; }
- const std::vector<VoipMetric>& voip_metrics() const {
- return voip_metric_blocks_;
+ const rtc::Optional<Rrtr>& rrtr() const { return rrtr_block_; }
+ const Dlrr& dlrr() const { return dlrr_block_; }
+ const rtc::Optional<VoipMetric>& voip_metric() const {
+ return voip_metric_block_;
}
protected:
@@ -55,9 +55,6 @@
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 @@
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 @@
void ParseVoipMetricBlock(const uint8_t* block, uint16_t block_length);
uint32_t sender_ssrc_;
- std::vector<Rrtr> rrtr_blocks_;
- std::vector<Dlrr> dlrr_blocks_;
- std::vector<VoipMetric> voip_metric_blocks_;
+ rtc::Optional<Rrtr> rrtr_block_;
+ Dlrr dlrr_block_; // Dlrr without items treated same as no dlrr block.
+ rtc::Optional<VoipMetric> 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 3c5025c..905462a 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 @@
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 @@
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<Rrtr>();
+TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithRrtrBlock) {
+ const Rrtr kRrtr = Rand<Rrtr>();
ExtendedReports xr;
xr.SetSenderSsrc(kSenderSsrc);
- EXPECT_TRUE(xr.AddRrtr(rrtr));
+ xr.SetRrtr(kRrtr);
rtc::Buffer packet = xr.Build();
ExtendedReports mparsed;
@@ -179,33 +174,14 @@
const ExtendedReports& parsed = mparsed;
EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc());
- EXPECT_THAT(parsed.rrtrs(), ElementsAre(rrtr));
-}
-
-TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithTwoRrtrBlocks) {
- Rrtr rrtr1 = Rand<Rrtr>();
- Rrtr rrtr2 = Rand<Rrtr>();
- 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<ReceiveTimeInfo>()));
+ const ReceiveTimeInfo kTimeInfo = Rand<ReceiveTimeInfo>();
ExtendedReports xr;
xr.SetSenderSsrc(kSenderSsrc);
- EXPECT_TRUE(xr.AddDlrr(dlrr));
+ xr.AddDlrrItem(kTimeInfo);
rtc::Buffer packet = xr.Build();
@@ -214,16 +190,16 @@
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<ReceiveTimeInfo>()));
- EXPECT_TRUE(dlrr.AddDlrrItem(Rand<ReceiveTimeInfo>()));
+ const ReceiveTimeInfo kTimeInfo1 = Rand<ReceiveTimeInfo>();
+ const ReceiveTimeInfo kTimeInfo2 = Rand<ReceiveTimeInfo>();
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 @@
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<ReceiveTimeInfo>()));
- Dlrr dlrr2;
- EXPECT_TRUE(dlrr2.AddDlrrItem(Rand<ReceiveTimeInfo>()));
- 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<VoipMetric>();
+ const VoipMetric kVoipMetric = Rand<VoipMetric>();
ExtendedReports xr;
xr.SetSenderSsrc(kSenderSsrc);
- EXPECT_TRUE(xr.AddVoipMetric(voip_metric));
+ xr.SetVoipMetric(kVoipMetric);
rtc::Buffer packet = xr.Build();
@@ -269,19 +225,19 @@
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<Rrtr>();
- Dlrr dlrr;
- EXPECT_TRUE(dlrr.AddDlrrItem(Rand<ReceiveTimeInfo>()));
- VoipMetric metric = Rand<VoipMetric>();
+ const Rrtr kRrtr = Rand<Rrtr>();
+ const ReceiveTimeInfo kTimeInfo = Rand<ReceiveTimeInfo>();
+ const VoipMetric kVoipMetric = Rand<VoipMetric>();
+
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 @@
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<Rrtr>();
- Dlrr dlrr;
- VoipMetric metric = Rand<VoipMetric>();
- 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<Rrtr>();
- 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<VoipMetric>();
- 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 db11d26..c78637a 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -688,13 +688,11 @@
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 e16673c..c98400f 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::StrEq;
using ::testing::StrictMock;
using ::testing::UnorderedElementsAre;
+using rtcp::ReceiveTimeInfo;
class MockRtcpPacketTypeCounterObserver : public RtcpPacketTypeCounterObserver {
public:
@@ -698,7 +699,7 @@
voip_metric.SetVoipMetric(metric);
rtcp::ExtendedReports xr;
xr.SetSenderSsrc(kSenderSsrc);
- xr.AddVoipMetric(voip_metric);
+ xr.SetVoipMetric(voip_metric);
InjectRtcpPacket(xr);
}
@@ -708,7 +709,7 @@
voip_metric.SetMediaSsrc(kNotToUsSsrc);
rtcp::ExtendedReports xr;
xr.SetSenderSsrc(kSenderSsrc);
- xr.AddVoipMetric(voip_metric);
+ xr.SetVoipMetric(voip_metric);
InjectRtcpPacket(xr);
}
@@ -719,9 +720,9 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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, LastReceivedXrReferenceTimeInfoInitiallyFalse) {
- rtcp::ReceiveTimeInfo info;
+ ReceiveTimeInfo info;
EXPECT_FALSE(rtcp_receiver_.LastReceivedXrReferenceTimeInfo(&info));
}
@@ -911,11 +898,11 @@
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 8fe3334..e6cf6695 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -701,7 +701,7 @@
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 @@
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<rtcp::RtcpPacket>(xr);
}
-// TODO(sprang): Add a unit test for this, or remove if the code isn't used.
std::unique_ptr<rtcp::RtcpPacket> RTCPSender::BuildVoIPMetric(
const RtcpContext& context) {
rtcp::ExtendedReports* xr = new rtcp::ExtendedReports();
@@ -732,7 +727,7 @@
voip.SetMediaSsrc(remote_ssrc_);
voip.SetVoipMetric(xr_voip_metric_);
- xr->AddVoipMetric(voip);
+ xr->SetVoipMetric(voip);
return std::unique_ptr<rtcp::RtcpPacket>(xr);
}
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc
index d348d3a..235c84b 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 @@
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 @@
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 @@
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 4fa1405..4a8eca0d 100644
--- a/webrtc/test/rtcp_packet_parser.h
+++ b/webrtc/test/rtcp_packet_parser.h
@@ -92,9 +92,6 @@
PacketCounter<rtcp::TransportFeedback>* transport_feedback() {
return &transport_feedback_;
}
- const RTCPVoIPMetric* voip_metric() {
- return &xr_.voip_metrics()[0].voip_metric();
- }
private:
PacketCounter<rtcp::App> app_;