Test RtcpParser rewritten to use rtcp packet classes

webrtc/test/rtcp_packet_parser.cc

Index: webrtc/test/rtcp_packet_parser.cc
diff --git a/webrtc/test/rtcp_packet_parser.cc b/webrtc/test/rtcp_packet_parser.cc
index 65f450d2e3dfdfa91fcf4d9d5a9f34b4cc450bc1..d8cac5b0b07c2e06177b8d46e0abacc9705eacf5 100644
--- a/webrtc/test/rtcp_packet_parser.cc
+++ b/webrtc/test/rtcp_packet_parser.cc
@@ -8,134 +8,89 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
+#include "webrtc/base/checks.h"
 #include "webrtc/test/rtcp_packet_parser.h"
 
-#include "testing/gtest/include/gtest/gtest.h"
-
 namespace webrtc {
 namespace test {
 
-using namespace RTCPUtility;
-
-RtcpPacketParser::RtcpPacketParser() {}
-
-RtcpPacketParser::~RtcpPacketParser() {}
+RtcpPacketParser::RtcpPacketParser() = default;
+RtcpPacketParser::~RtcpPacketParser() = default;
 
-void RtcpPacketParser::Parse(const void *data, size_t len) {
-  const uint8_t* packet = static_cast<const uint8_t*>(data);
-  RTCPUtility::RTCPParserV2 parser(packet, len, true);
-  EXPECT_TRUE(parser.IsValid());
-  for (RTCPUtility::RTCPPacketTypes type = parser.Begin();
-       type != RTCPPacketTypes::kInvalid; type = parser.Iterate()) {
-    switch (type) {
-      case RTCPPacketTypes::kSr:
-        sender_report_.Set(parser.Packet().SR);
-        break;
-      case RTCPPacketTypes::kRr:
-        receiver_report_.Set(parser.Packet().RR);
-        break;
-      case RTCPPacketTypes::kReportBlockItem:
-        report_block_.Set(parser.Packet().ReportBlockItem);
-        ++report_blocks_per_ssrc_[parser.Packet().ReportBlockItem.SSRC];
-        break;
-      case RTCPPacketTypes::kSdes:
-        sdes_.Set();
-        break;
-      case RTCPPacketTypes::kSdesChunk:
-        sdes_chunk_.Set(parser.Packet().CName);
-        break;
-      case RTCPPacketTypes::kBye:
-        bye_.Set(parser.Packet().BYE);
-        break;
-      case RTCPPacketTypes::kApp:
-        app_.Set(parser.Packet().APP);
-        break;
-      case RTCPPacketTypes::kAppItem:
-        app_item_.Set(parser.Packet().APP);
-        break;
-      case RTCPPacketTypes::kExtendedIj:
-        ij_.Set();
-        break;
-      case RTCPPacketTypes::kExtendedIjItem:
-        ij_item_.Set(parser.Packet().ExtendedJitterReportItem);
-        break;
-      case RTCPPacketTypes::kPsfbPli:
-        pli_.Set(parser.Packet().PLI);
-        break;
-      case RTCPPacketTypes::kPsfbSli:
-        sli_.Set(parser.Packet().SLI);
-        break;
-      case RTCPPacketTypes::kPsfbSliItem:
-        sli_item_.Set(parser.Packet().SLIItem);
-        break;
-      case RTCPPacketTypes::kPsfbRpsiItem:
-        rpsi_.Set(parser.Packet().RPSI);
-        break;
-      case RTCPPacketTypes::kPsfbFir:
-        fir_.Set(parser.Packet().FIR);
-        break;
-      case RTCPPacketTypes::kPsfbFirItem:
-        fir_item_.Set(parser.Packet().FIRItem);
-        break;
-      case RTCPPacketTypes::kRtpfbNack:
-        nack_.Set(parser.Packet().NACK);
-        nack_item_.Clear();
-        break;
-      case RTCPPacketTypes::kRtpfbNackItem:
-        nack_item_.Set(parser.Packet().NACKItem);
-        break;
-      case RTCPPacketTypes::kPsfbApp:
-        psfb_app_.Set(parser.Packet().PSFBAPP);
-        break;
-      case RTCPPacketTypes::kPsfbRembItem:
-        remb_item_.Set(parser.Packet().REMBItem);
-        break;
-      case RTCPPacketTypes::kRtpfbTmmbr:
-        tmmbr_.Set(parser.Packet().TMMBR);
-        break;
-      case RTCPPacketTypes::kRtpfbTmmbrItem:
-        tmmbr_item_.Set(parser.Packet().TMMBRItem);
-        break;
-      case RTCPPacketTypes::kRtpfbTmmbn:
-        tmmbn_.Set(parser.Packet().TMMBN);
-        tmmbn_items_.Clear();
-        break;
-      case RTCPPacketTypes::kRtpfbTmmbnItem:
-        tmmbn_items_.Set(parser.Packet().TMMBNItem);
-        break;
-      case RTCPPacketTypes::kXrHeader:
-        xr_header_.Set(parser.Packet().XR);
-        dlrr_items_.Clear();
-        break;
-      case RTCPPacketTypes::kXrReceiverReferenceTime:
-        rrtr_.Set(parser.Packet().XRReceiverReferenceTimeItem);
-        break;
-      case RTCPPacketTypes::kXrDlrrReportBlock:
-        dlrr_.Set();
-        break;
-      case RTCPPacketTypes::kXrDlrrReportBlockItem:
-        dlrr_items_.Set(parser.Packet().XRDLRRReportBlockItem);
-        break;
-      case RTCPPacketTypes::kXrVoipMetric:
-        voip_metric_.Set(parser.Packet().XRVOIPMetricItem);
-        break;
-      default:
+bool RtcpPacketParser::Parse(const void* data, size_t length) {
+  const uint8_t* const buffer = static_cast<const uint8_t*>(data);
+  const uint8_t* const buffer_end = buffer + length;
+  rtcp::CommonHeader header;
+  for (const uint8_t *next_packet = buffer;
+       RTC_DCHECK(next_packet <= buffer_end), next_packet != buffer_end;
+       next_packet = header.NextPacket()) {

[sprang_webrtc, 2016/09/02 07:43:21]

The dcheck inside the test statement of the for-loop looks kinda weird? Why not
just RTC_DCHECK_LE first thing in the body?

[danilchap, 2016/09/02 08:15:53]

Done.
Failed to find reasons I had for putting DCHECK into that unnatural place.

+    if (!header.Parse(next_packet, buffer_end - next_packet)) {
+      // Invalid rtcp header or unaligned rtcp packet.
+      return false;
+    }
+    switch (header.type()) {

[sprang_webrtc, 2016/09/02 07:43:21]

Header type is just an uin8_t, right? Can we add a default clause for
unrecognized values? With a warning or an ADD_FAILURE() if this just for test
and there should be no unknowns.

[danilchap, 2016/09/02 08:15:53]

Done.
ADD_FAILURE would crash fuzzers fast, so added logging.

+      case rtcp::App::kPacketType:
+        app_.Parse(header);
+        break;
+      case rtcp::Bye::kPacketType:
+        bye_.Parse(header);
+        break;
+      case rtcp::ExtendedReports::kPacketType:
+        xr_.Parse(header);
+        break;
+      case rtcp::ExtendedJitterReport::kPacketType:
+        ij_.Parse(header);
+        break;
+      case rtcp::Psfb::kPacketType:
+        switch (header.fmt()) {
+          case rtcp::Fir::kFeedbackMessageType:
+            fir_.Parse(header);
+            break;
+          case rtcp::Pli::kFeedbackMessageType:
+            pli_.Parse(header);
+            break;
+          case rtcp::Remb::kFeedbackMessageType:
+            remb_.Parse(header);
+            break;
+          case rtcp::Rpsi::kFeedbackMessageType:
+            rpsi_.Parse(header);
+            break;
+          case rtcp::Sli::kFeedbackMessageType:
+            sli_.Parse(header);
+            break;
+        }
+        break;
+      case rtcp::ReceiverReport::kPacketType:
+        receiver_report_.Parse(header);
+        break;
+      case rtcp::Rtpfb::kPacketType:
+        switch (header.fmt()) {
+          case rtcp::Nack::kFeedbackMessageType:
+            nack_.Parse(header);
+            break;
+          case rtcp::RapidResyncRequest::kFeedbackMessageType:
+            rrr_.Parse(header);
+            break;
+          case rtcp::Tmmbn::kFeedbackMessageType:
+            tmmbn_.Parse(header);
+            break;
+          case rtcp::Tmmbr::kFeedbackMessageType:
+            tmmbr_.Parse(header);
+            break;
+          case rtcp::TransportFeedback::kFeedbackMessageType:
+            transport_feedback_.Parse(header);
+            break;
+        }
+        break;
+      case rtcp::Sdes::kPacketType:
+        sdes_.Parse(header);
+        break;
+      case rtcp::SenderReport::kPacketType:
+        sender_report_.Parse(header);
         break;
     }
   }
-}
-
-uint64_t Rpsi::PictureId() const {
-  assert(num_packets_ > 0);
-  uint16_t num_bytes = rpsi_.NumberOfValidBits / 8;
-  assert(num_bytes > 0);
-  uint64_t picture_id = 0;
-  for (uint16_t i = 0; i < num_bytes - 1; ++i) {
-    picture_id += (rpsi_.NativeBitString[i] & 0x7f);
-    picture_id <<= 7;
-  }
-  picture_id += (rpsi_.NativeBitString[num_bytes - 1] & 0x7f);
-  return picture_id;
+  return true;
 }
 
 }  // namespace test