rtcp::ReportBlock refactored to contain parsing
Review URL: https://codereview.webrtc.org/1420283022
Cr-Commit-Position: refs/heads/master@{#10633}
diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp
index 9db7cfd..eaedf60 100644
--- a/webrtc/modules/modules.gyp
+++ b/webrtc/modules/modules.gyp
@@ -247,6 +247,7 @@
'rtp_rtcp/source/remote_ntp_time_estimator_unittest.cc',
'rtp_rtcp/source/rtcp_format_remb_unittest.cc',
'rtp_rtcp/source/rtcp_packet_unittest.cc',
+ 'rtp_rtcp/source/rtcp_packet/report_block_unittest.cc',
'rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc',
'rtp_rtcp/source/rtcp_receiver_unittest.cc',
'rtp_rtcp/source/rtcp_sender_unittest.cc',
diff --git a/webrtc/modules/rtp_rtcp/BUILD.gn b/webrtc/modules/rtp_rtcp/BUILD.gn
index 6239c04..fb6a1e8 100644
--- a/webrtc/modules/rtp_rtcp/BUILD.gn
+++ b/webrtc/modules/rtp_rtcp/BUILD.gn
@@ -46,6 +46,8 @@
"source/remote_ntp_time_estimator.cc",
"source/rtcp_packet.cc",
"source/rtcp_packet.h",
+ "source/rtcp_packet/report_block.cc",
+ "source/rtcp_packet/report_block.h",
"source/rtcp_packet/transport_feedback.cc",
"source/rtcp_packet/transport_feedback.h",
"source/rtcp_receiver.cc",
diff --git a/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi b/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi
index 57bea0b..025623d 100644
--- a/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi
+++ b/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi
@@ -41,6 +41,8 @@
'source/rtp_rtcp_impl.h',
'source/rtcp_packet.cc',
'source/rtcp_packet.h',
+ 'source/rtcp_packet/report_block.cc',
+ 'source/rtcp_packet/report_block.h',
'source/rtcp_packet/transport_feedback.cc',
'source/rtcp_packet/transport_feedback.h',
'source/rtcp_receiver.cc',
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet.cc
index 792caa7..4af3292 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_packet.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet.cc
@@ -154,18 +154,12 @@
// | delay since last SR (DLSR) |
// +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
-void CreateReportBlocks(const std::vector<RTCPPacketReportBlockItem>& blocks,
+void CreateReportBlocks(const std::vector<ReportBlock>& blocks,
uint8_t* buffer,
size_t* pos) {
- for (std::vector<RTCPPacketReportBlockItem>::const_iterator
- it = blocks.begin(); it != blocks.end(); ++it) {
- AssignUWord32(buffer, pos, (*it).SSRC);
- AssignUWord8(buffer, pos, (*it).FractionLost);
- AssignUWord24(buffer, pos, (*it).CumulativeNumOfPacketsLost);
- AssignUWord32(buffer, pos, (*it).ExtendedHighestSequenceNumber);
- AssignUWord32(buffer, pos, (*it).Jitter);
- AssignUWord32(buffer, pos, (*it).LastSR);
- AssignUWord32(buffer, pos, (*it).DelayLastSR);
+ for (const ReportBlock& block : blocks) {
+ block.Create(buffer + *pos);
+ *pos += ReportBlock::kLength;
}
}
@@ -781,7 +775,7 @@
LOG(LS_WARNING) << "Max report blocks reached.";
return false;
}
- report_blocks_.push_back(block.report_block_);
+ report_blocks_.push_back(block);
sr_.NumberOfReportBlocks = report_blocks_.size();
return true;
}
@@ -805,7 +799,7 @@
LOG(LS_WARNING) << "Max report blocks reached.";
return false;
}
- report_blocks_.push_back(block.report_block_);
+ report_blocks_.push_back(block);
rr_.NumberOfReportBlocks = report_blocks_.size();
return true;
}
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet.h
index 2956bc7..85aa438 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_packet.h
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet.h
@@ -17,6 +17,7 @@
#include <vector>
#include "webrtc/base/scoped_ptr.h"
+#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h"
#include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "webrtc/typedefs.h"
@@ -145,63 +146,6 @@
RTC_DISALLOW_COPY_AND_ASSIGN(Empty);
};
-// From RFC 3550, RTP: A Transport Protocol for Real-Time Applications.
-//
-// RTCP report block (RFC 3550).
-//
-// 0 1 2 3
-// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
-// +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
-// | SSRC_1 (SSRC of first source) |
-// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-// | fraction lost | cumulative number of packets lost |
-// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-// | extended highest sequence number received |
-// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-// | interarrival jitter |
-// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-// | last SR (LSR) |
-// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-// | delay since last SR (DLSR) |
-// +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
-
-class ReportBlock {
- public:
- ReportBlock() {
- // TODO(asapersson): Consider adding a constructor to struct.
- memset(&report_block_, 0, sizeof(report_block_));
- }
-
- ~ReportBlock() {}
-
- void To(uint32_t ssrc) {
- report_block_.SSRC = ssrc;
- }
- void WithFractionLost(uint8_t fraction_lost) {
- report_block_.FractionLost = fraction_lost;
- }
- void WithCumulativeLost(uint32_t cumulative_lost) {
- report_block_.CumulativeNumOfPacketsLost = cumulative_lost;
- }
- void WithExtHighestSeqNum(uint32_t ext_highest_seq_num) {
- report_block_.ExtendedHighestSequenceNumber = ext_highest_seq_num;
- }
- void WithJitter(uint32_t jitter) {
- report_block_.Jitter = jitter;
- }
- void WithLastSr(uint32_t last_sr) {
- report_block_.LastSR = last_sr;
- }
- void WithDelayLastSr(uint32_t delay_last_sr) {
- report_block_.DelayLastSR = delay_last_sr;
- }
-
- private:
- friend class SenderReport;
- friend class ReceiverReport;
- RTCPUtility::RTCPPacketReportBlockItem report_block_;
-};
-
// RTCP sender report (RFC 3550).
//
// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
@@ -268,7 +212,7 @@
}
RTCPUtility::RTCPPacketSR sr_;
- std::vector<RTCPUtility::RTCPPacketReportBlockItem> report_blocks_;
+ std::vector<ReportBlock> report_blocks_;
RTC_DISALLOW_COPY_AND_ASSIGN(SenderReport);
};
@@ -314,7 +258,7 @@
}
RTCPUtility::RTCPPacketRR rr_;
- std::vector<RTCPUtility::RTCPPacketReportBlockItem> report_blocks_;
+ std::vector<ReportBlock> report_blocks_;
RTC_DISALLOW_COPY_AND_ASSIGN(ReceiverReport);
};
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.cc
new file mode 100644
index 0000000..4911dbf
--- /dev/null
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.cc
@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h"
+
+#include "webrtc/base/checks.h"
+#include "webrtc/base/logging.h"
+#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
+
+namespace webrtc {
+namespace rtcp {
+
+// From RFC 3550, RTP: A Transport Protocol for Real-Time Applications.
+//
+// RTCP report block (RFC 3550).
+//
+// 0 1 2 3
+// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+// +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
+// 0 | SSRC_1 (SSRC of first source) |
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// 4 | fraction lost | cumulative number of packets lost |
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// 8 | extended highest sequence number received |
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// 12 | interarrival jitter |
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// 16 | last SR (LSR) |
+// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+// 20 | delay since last SR (DLSR) |
+// 24 +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
+ReportBlock::ReportBlock()
+ : source_ssrc_(0),
+ fraction_lost_(0),
+ cumulative_lost_(0),
+ extended_high_seq_num_(0),
+ jitter_(0),
+ last_sr_(0),
+ delay_since_last_sr_(0) {}
+
+bool ReportBlock::Parse(const uint8_t* buffer, size_t length) {
+ RTC_DCHECK(buffer != nullptr);
+ if (length < ReportBlock::kLength) {
+ LOG(LS_ERROR) << "Report Block should be 24 bytes long";
+ return false;
+ }
+
+ source_ssrc_ = ByteReader<uint32_t>::ReadBigEndian(&buffer[0]);
+ fraction_lost_ = buffer[4];
+ cumulative_lost_ = ByteReader<uint32_t, 3>::ReadBigEndian(&buffer[5]);
+ extended_high_seq_num_ = ByteReader<uint32_t>::ReadBigEndian(&buffer[8]);
+ jitter_ = ByteReader<uint32_t>::ReadBigEndian(&buffer[12]);
+ last_sr_ = ByteReader<uint32_t>::ReadBigEndian(&buffer[16]);
+ delay_since_last_sr_ = ByteReader<uint32_t>::ReadBigEndian(&buffer[20]);
+
+ return true;
+}
+
+void ReportBlock::Create(uint8_t* buffer) const {
+ // Runtime check should be done while setting cumulative_lost.
+ RTC_DCHECK_LT(cumulative_lost(), (1u << 24)); // Have only 3 bytes for it.
+
+ ByteWriter<uint32_t>::WriteBigEndian(&buffer[0], source_ssrc());
+ ByteWriter<uint8_t>::WriteBigEndian(&buffer[4], fraction_lost());
+ ByteWriter<uint32_t, 3>::WriteBigEndian(&buffer[5], cumulative_lost());
+ ByteWriter<uint32_t>::WriteBigEndian(&buffer[8], extended_high_seq_num());
+ ByteWriter<uint32_t>::WriteBigEndian(&buffer[12], jitter());
+ ByteWriter<uint32_t>::WriteBigEndian(&buffer[16], last_sr());
+ ByteWriter<uint32_t>::WriteBigEndian(&buffer[20], delay_since_last_sr());
+}
+
+bool ReportBlock::WithCumulativeLost(uint32_t cumulative_lost) {
+ if (cumulative_lost >= (1u << 24)) { // Have only 3 bytes to store it.
+ LOG(LS_WARNING) << "Cumulative lost is too big to fit into Report Block";
+ return false;
+ }
+ cumulative_lost_ = cumulative_lost;
+ return true;
+}
+
+} // namespace rtcp
+} // namespace webrtc
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h
new file mode 100644
index 0000000..ef99e17
--- /dev/null
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h
@@ -0,0 +1,67 @@
+/*
+ * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ *
+ */
+
+#ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_REPORT_BLOCK_H_
+#define WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_REPORT_BLOCK_H_
+
+#include "webrtc/base/basictypes.h"
+
+namespace webrtc {
+namespace rtcp {
+
+class ReportBlock {
+ public:
+ static const size_t kLength = 24;
+
+ ReportBlock();
+ ~ReportBlock() {}
+
+ bool Parse(const uint8_t* buffer, size_t length);
+
+ // Fills buffer with the ReportBlock.
+ // Consumes ReportBlock::kLength bytes.
+ void Create(uint8_t* buffer) const;
+
+ void To(uint32_t ssrc) { source_ssrc_ = ssrc; }
+ void WithFractionLost(uint8_t fraction_lost) {
+ fraction_lost_ = fraction_lost;
+ }
+ bool WithCumulativeLost(uint32_t cumulative_lost);
+ void WithExtHighestSeqNum(uint32_t ext_highest_seq_num) {
+ extended_high_seq_num_ = ext_highest_seq_num;
+ }
+ void WithJitter(uint32_t jitter) { jitter_ = jitter; }
+ void WithLastSr(uint32_t last_sr) { last_sr_ = last_sr; }
+ void WithDelayLastSr(uint32_t delay_last_sr) {
+ delay_since_last_sr_ = delay_last_sr;
+ }
+
+ uint32_t source_ssrc() const { return source_ssrc_; }
+ uint8_t fraction_lost() const { return fraction_lost_; }
+ uint32_t cumulative_lost() const { return cumulative_lost_; }
+ uint32_t extended_high_seq_num() const { return extended_high_seq_num_; }
+ uint32_t jitter() const { return jitter_; }
+ uint32_t last_sr() const { return last_sr_; }
+ uint32_t delay_since_last_sr() const { return delay_since_last_sr_; }
+
+ private:
+ uint32_t source_ssrc_;
+ uint8_t fraction_lost_;
+ uint32_t cumulative_lost_;
+ uint32_t extended_high_seq_num_;
+ uint32_t jitter_;
+ uint32_t last_sr_;
+ uint32_t delay_since_last_sr_;
+};
+
+} // namespace rtcp
+} // namespace webrtc
+#endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_REPORT_BLOCK_H_
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc
new file mode 100644
index 0000000..fcde0e4
--- /dev/null
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc
@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h"
+
+#include <limits>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/test/random.h"
+
+using webrtc::rtcp::ReportBlock;
+
+namespace webrtc {
+namespace {
+
+const uint32_t kRemoteSsrc = 0x23456789;
+const uint8_t kFractionLost = 55;
+// Use values that are streamed differently LE and BE.
+const uint32_t kCumulativeLost = 0x111213;
+const uint32_t kExtHighestSeqNum = 0x22232425;
+const uint32_t kJitter = 0x33343536;
+const uint32_t kLastSr = 0x44454647;
+const uint32_t kDelayLastSr = 0x55565758;
+const size_t kBufferLength = ReportBlock::kLength;
+
+TEST(RtcpPacketReportBlockTest, ParseChecksLength) {
+ uint8_t buffer[kBufferLength];
+ memset(buffer, 0, sizeof(buffer));
+
+ ReportBlock rb;
+ EXPECT_FALSE(rb.Parse(buffer, kBufferLength - 1));
+ EXPECT_TRUE(rb.Parse(buffer, kBufferLength));
+}
+
+TEST(RtcpPacketReportBlockTest, ParseAnyData) {
+ uint8_t buffer[kBufferLength];
+ // Fill buffer with semi-random data.
+ test::Random generator(testing::FLAGS_gtest_random_seed);
+ for (size_t i = 0; i < kBufferLength; ++i)
+ buffer[i] = static_cast<uint8_t>(generator.Rand(0, 0xff));
+
+ ReportBlock rb;
+ EXPECT_TRUE(rb.Parse(buffer, kBufferLength));
+}
+
+TEST(RtcpPacketReportBlockTest, ParseMatchCreate) {
+ ReportBlock rb;
+ rb.To(kRemoteSsrc);
+ rb.WithFractionLost(kFractionLost);
+ rb.WithCumulativeLost(kCumulativeLost);
+ rb.WithExtHighestSeqNum(kExtHighestSeqNum);
+ rb.WithJitter(kJitter);
+ rb.WithLastSr(kLastSr);
+ rb.WithDelayLastSr(kDelayLastSr);
+
+ uint8_t buffer[kBufferLength];
+ rb.Create(buffer);
+
+ ReportBlock parsed;
+ EXPECT_TRUE(parsed.Parse(buffer, kBufferLength));
+
+ EXPECT_EQ(kRemoteSsrc, parsed.source_ssrc());
+ EXPECT_EQ(kFractionLost, parsed.fraction_lost());
+ EXPECT_EQ(kCumulativeLost, parsed.cumulative_lost());
+ EXPECT_EQ(kExtHighestSeqNum, parsed.extended_high_seq_num());
+ EXPECT_EQ(kJitter, parsed.jitter());
+ EXPECT_EQ(kLastSr, parsed.last_sr());
+ EXPECT_EQ(kDelayLastSr, parsed.delay_since_last_sr());
+}
+
+TEST(RtcpPacketReportBlockTest, ValidateCumulativeLost) {
+ const uint32_t kMaxCumulativeLost = 0xffffff;
+ ReportBlock rb;
+ EXPECT_FALSE(rb.WithCumulativeLost(kMaxCumulativeLost + 1));
+ EXPECT_TRUE(rb.WithCumulativeLost(kMaxCumulativeLost));
+}
+
+} // namespace
+} // namespace webrtc
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
index 22b9477..1b8c62f 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -459,7 +459,10 @@
rtcp::ReportBlock* block = &report_blocks_[report_block.remoteSSRC];
block->To(report_block.remoteSSRC);
block->WithFractionLost(report_block.fractionLost);
- block->WithCumulativeLost(report_block.cumulativeLost);
+ if (!block->WithCumulativeLost(report_block.cumulativeLost)) {
+ LOG(LS_WARNING) << "Cumulative lost is oversized.";
+ return -1;
+ }
block->WithExtHighestSeqNum(report_block.extendedHighSeqNum);
block->WithJitter(report_block.jitter);
block->WithLastSr(report_block.lastSR);
@@ -1024,6 +1027,8 @@
RTCPReportBlock report_block;
if (PrepareReport(feedback_state, it->first, it->second,
&report_block)) {
+ // TODO(danilchap) AddReportBlock may fail (for 2 different reasons).
+ // Probably it shouldn't be ignored.
AddReportBlock(report_block);
}
}
