[rtp_rtcp] rtcp::Remb cleaned and got Parse function

webrtc/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc

Index: webrtc/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc
index 487a076b1a43f69e936cd9d812d05e70e77f01fe..dccd0074dd092afdd14d21f75031d65737fb0c29 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc
@@ -12,35 +12,120 @@
 
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "webrtc/test/rtcp_packet_parser.h"
 
+using testing::ElementsAreArray;
+using testing::make_tuple;
 using webrtc::rtcp::RawPacket;
 using webrtc::rtcp::Remb;
-using webrtc::test::RtcpPacketParser;
+using webrtc::RTCPUtility::RtcpCommonHeader;
+using webrtc::RTCPUtility::RtcpParseCommonHeader;
 
 namespace webrtc {
+namespace {
 
 const uint32_t kSenderSsrc = 0x12345678;
-const uint32_t kRemoteSsrc = 0x23456789;
 
-TEST(RtcpPacketRembTest, Remb) {
+const uint32_t kRemoteSsrc[] = {0x23456789, 0x2345678a, 0x2345678b};

[åsapersson, 2016/01/20 15:33:13]

s/kRemoteSsrc/kRemoteSsrcs

[danilchap, 2016/01/20 16:13:19]

Done.

+const uint32_t kBitrate = 0x3fb93 * 2;  // 522022;
+const uint8_t kPacket[] = {0x8f, 206,  0x00, 0x07, 0x12, 0x34, 0x56, 0x78,
+                           0x00, 0x00, 0x00, 0x00, 'R',  'E',  'M',  'B',
+                           0x03, 0x07, 0xfb, 0x93, 0x23, 0x45, 0x67, 0x89,
+                           0x23, 0x45, 0x67, 0x8a, 0x23, 0x45, 0x67, 0x8b};
+const size_t kPacketLength = sizeof(kPacket);
+
+bool ParseRemb(const uint8_t* buffer, size_t length, Remb* remb) {
+  RtcpCommonHeader header;
+  EXPECT_TRUE(RtcpParseCommonHeader(buffer, length, &header));
+  EXPECT_EQ(length, header.BlockSize());
+  return remb->Parse(header, buffer + RtcpCommonHeader::kHeaderSizeBytes);
+}
+
+TEST(RtcpPacketRembTest, Create) {
   Remb remb;
   remb.From(kSenderSsrc);
-  remb.AppliesTo(kRemoteSsrc);
-  remb.AppliesTo(kRemoteSsrc + 1);
-  remb.AppliesTo(kRemoteSsrc + 2);
-  remb.WithBitrateBps(261011);
+  remb.AppliesTo(kRemoteSsrc[0]);
+  remb.AppliesTo(kRemoteSsrc[1]);
+  remb.AppliesTo(kRemoteSsrc[2]);
+  remb.WithBitrateBps(kBitrate);
 
   rtc::scoped_ptr<RawPacket> packet(remb.Build());
-  RtcpPacketParser parser;
-  parser.Parse(packet->Buffer(), packet->Length());
-  EXPECT_EQ(1, parser.psfb_app()->num_packets());
-  EXPECT_EQ(kSenderSsrc, parser.psfb_app()->Ssrc());
-  EXPECT_EQ(1, parser.remb_item()->num_packets());
-  EXPECT_EQ(261011, parser.remb_item()->last_bitrate_bps());
-  std::vector<uint32_t> ssrcs = parser.remb_item()->last_ssrc_list();
-  EXPECT_EQ(kRemoteSsrc, ssrcs[0]);
-  EXPECT_EQ(kRemoteSsrc + 1, ssrcs[1]);
-  EXPECT_EQ(kRemoteSsrc + 2, ssrcs[2]);
+
+  EXPECT_THAT(make_tuple(packet->Buffer(), packet->Length()),
+              ElementsAreArray(kPacket));
+}
+
+TEST(RtcpPacketRembTest, Parse) {
+  Remb remb;
+  EXPECT_TRUE(ParseRemb(kPacket, kPacketLength, &remb));
+  const Remb& parsed = remb;
+
+  EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc());
+  EXPECT_EQ(kBitrate, parsed.bitrate_bps());
+  EXPECT_THAT(parsed.ssrcs(), ElementsAreArray(kRemoteSsrc));
+}
+
+TEST(RtcpPacketRembTest, CreateAndParseEmpty) {

[åsapersson, 2016/01/20 15:33:13]

maybe CreateAndParseWithSsrcCountZero or similar

[danilchap, 2016/01/20 16:13:19]

Done.

+  const uint32_t kBitrate = 500000;
+  Remb remb;
+  remb.From(kSenderSsrc);
+  remb.WithBitrateBps(kBitrate);
+  rtc::scoped_ptr<RawPacket> packet(remb.Build());
+
+  Remb parsed;
+  EXPECT_TRUE(ParseRemb(packet->Buffer(), packet->Length(), &parsed));
+  EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc());
+  EXPECT_EQ(kBitrate, parsed.bitrate_bps());

[åsapersson, 2016/01/20 15:33:13]

check parsed.ssrcs() empty

[danilchap, 2016/01/20 16:13:19]

Done.

+}
+
+TEST(RtcpPacketRembTest, ParseFailsOnNonRembSmall) {

[åsapersson, 2016/01/20 15:33:13]

maybe ParseFailsOnTooSmallPacket

[danilchap, 2016/01/20 16:13:19]

Done.

+  uint8_t packet[kPacketLength];
+  memcpy(packet, kPacket, kPacketLength);
+  packet[3] = 3;  // Make it too small.
+
+  Remb remb;
+  EXPECT_FALSE(ParseRemb(packet, (1 + 3) * 4, &remb));
+}
+
+TEST(RtcpPacketRembTest, ParseFailsOnNonRembSignature) {
+  uint8_t packet[kPacketLength];
+  memcpy(packet, kPacket, kPacketLength);
+  packet[12] = 'N';  // Swap 'R' -> 'N' in the 'REMB' signature.
+
+  Remb remb;
+  EXPECT_FALSE(ParseRemb(packet, kPacketLength, &remb));
+}
+
+TEST(RtcpPacketRembTest, ParseFailsWhenSsrcCountMismatchLength) {
+  uint8_t packet[kPacketLength];
+  memcpy(packet, kPacket, kPacketLength);
+  packet[16]++;  // Swap 3 -> 4 in the ssrcs count.
+
+  Remb remb;
+  EXPECT_FALSE(ParseRemb(packet, kPacketLength, &remb));
 }
+
+TEST(RtcpPacketRembTest, TooManySsrcs) {
+  const size_t kMax = 0xff;
+  Remb remb;
+  for (size_t i = 1; i <= kMax; ++i)
+    EXPECT_TRUE(remb.AppliesTo(kRemoteSsrc[0] + i));
+  EXPECT_FALSE(remb.AppliesTo(kRemoteSsrc[0]));
+}
+
+TEST(RtcpPacketRembTest, TooManyForBatchAssign) {
+  const size_t kMax = 0xff;
+  Remb remb;

[åsapersson, 2016/01/20 15:33:13]

maybe define kRemoteSsrc and use same in test

[danilchap, 2016/01/20 16:13:19]

Done.

+
+  std::vector<uint32_t> AlmostAll(kMax - 1, kRemoteSsrc[0]);

[åsapersson, 2016/01/20 15:33:13]

s/AlmostAll/almostAll

call allButOneSsrc?

check camel case

[danilchap, 2016/01/20 16:13:19]

Done.

+  EXPECT_TRUE(remb.AppliesToMany(AlmostAll));
+  // Should be no place for 2 more.
+  std::vector<uint32_t> TwoMoreSsrcs(2, kRemoteSsrc[1]);

[åsapersson, 2016/01/20 15:33:13]

s/TwoMoreSsrcs/twoMoreSsrcs

[danilchap, 2016/01/20 16:13:19]

Done: handled same way as kAllButOneSsrc

+  EXPECT_FALSE(remb.AppliesToMany(TwoMoreSsrcs));
+  // But enough place for 1 more.
+  EXPECT_TRUE(remb.AppliesTo(kRemoteSsrc[2]));
+  // But not for another one.
+  EXPECT_FALSE(remb.AppliesTo(kRemoteSsrc[2] + 1));
+}
+
+}  // namespace
 }  // namespace webrtc