New class RtxReceiveStream.

webrtc/call/rtx_receive_stream_unittest.cc

Index: webrtc/call/rtx_receive_stream_unittest.cc
diff --git a/webrtc/call/rtx_receive_stream_unittest.cc b/webrtc/call/rtx_receive_stream_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b4deb2e6f3502837b2f660b8e4d810029381ce9f
--- /dev/null
+++ b/webrtc/call/rtx_receive_stream_unittest.cc
@@ -0,0 +1,92 @@
+/*
+ *  Copyright (c) 2017 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/call/rtx_receive_stream.h"
+#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h"
+#include "webrtc/test/gmock.h"
+#include "webrtc/test/gtest.h"
+
+namespace webrtc {
+
+namespace {
+
+class MockRtpPacketSink : public RtpPacketSinkInterface {
+ public:
+  MOCK_METHOD1(OnRtpPacket, void(const RtpPacketReceived&));
+};
+
+constexpr int kMediaPayloadType = 100;
+constexpr int kRtxPayloadType = 98;
+constexpr int kMediaSSRC = 0x3333333;

[danilchap, 2017/05/19 08:39:02]

may be uint32_t for SSRC

[nisse-webrtc, 2017/05/19 10:59:14]

Done.

+

[danilchap, 2017/05/19 08:39:03]

might be helpful to add constants for the original sequence number and payload
too.
(to avoid browsing up and bottom to ensure correct value are expected in the
individual tests)
Not a problem now, but might be an issue when more tests will be added.

[nisse-webrtc, 2017/05/19 10:59:14]

Added constant for the seqno.

+constexpr uint8_t kRtxPacket[] = {
+    0x80,                    // Version 2,
+    98,                      // Payload type

[danilchap, 2017/05/19 08:39:03]

likely add dots at the end of the comments.

[nisse-webrtc, 2017/05/19 10:59:14]

Done.

+    0x12, 0x34,              // Seqno,
+    0x11, 0x11, 0x11, 0x11,  // Timestamp
+    0x22, 0x22, 0x22, 0x22,  // SSRC
+    // RTX header
+    0x56, 0x57,              // Orig seqno
+    // Payload
+    0xee,
+};
+
+std::map<int, int> PTMapping() {

[danilchap, 2017/05/19 08:39:03]

it is easier to read if you do not abbreviate: so may be name it
PayloadTypeMapping()
https://google.github.io/styleguide/cppguide.html#General_Naming_Rules

[nisse-webrtc, 2017/05/19 10:59:14]

Done.

+  std::map<int, int> m;

[danilchap, 2017/05/19 08:39:02]

I wonder if
return {{kRtxPayloadType, kMediaPayloadType}};
would compile on all platforms.

+  m[kRtxPayloadType] = kMediaPayloadType;
+  return m;
+}
+
+template <typename T>
+rtc::ArrayView<T> Truncate(rtc::ArrayView<T> a, size_t drop) {
+  return a.subview(0, a.size() - drop);
+}
+
+}  // namespace
+
+TEST(RtxReceiveStreamTest, RestoresPacketPayload) {
+  testing::StrictMock<MockRtpPacketSink> media_sink;

[danilchap, 2017/05/19 08:39:02]

personally I prefer to add
using ::testing::_;
using ::testing::StrictMock;
and everything else used from testing namespace at the top of the unnamed
namespace.

[nisse-webrtc, 2017/05/19 10:59:14]

Done for some of the symbols.

+  RtxReceiveStream rtx_sink(&media_sink, PTMapping(), kMediaSSRC);
+  RtpPacketReceived rtx_packet;
+  EXPECT_TRUE(rtx_packet.Parse(rtc::ArrayView<const uint8_t>(kRtxPacket)));
+
+  EXPECT_CALL(media_sink,
+              OnRtpPacket(testing::AllOf(

[danilchap, 2017/05/19 08:39:02]

Alternative suggestion (not sure it is better though):
EXPECT_CALL(media_sink, OnRtpPacket(_)).WillOnce(testing::Invoke([](const
RtpPacketReceived& packet) {
  EXPECT_EQ(packet.SequenceNumber(), 0x5657);
  EXPECT_EQ(packet.Ssrc(), kMediaSSRC);
  EXPECT_THAT(packet.payload(), ElementsAre(0xee));
});

[nisse-webrtc, 2017/05/19 10:59:14]

I've changed to this alternative. It provides clearer messages when expectations
fail.

+                  testing::Truly([](const RtpPacketReceived& packet) {
+                    return packet.SequenceNumber() == 0x5657;
+                  }),
+                  testing::Truly([](const RtpPacketReceived& packet) {

[danilchap, 2017/05/19 08:39:02]

Or... yet another (again, not sure it is more readable):
instead of testing::Truly
testing::Property(&RtpPacketReceived::Ssrc, kMediaSSRC)

+                    return packet.Ssrc() == kMediaSSRC;
+                  }),
+                  testing::Truly([](const RtpPacketReceived& packet) {
+                    return packet.payload().size() == 1 &&
+                           packet.payload()[0] == 0xee;
+                  }))));
+  rtx_sink.OnRtpPacket(rtx_packet);
+}
+
+TEST(RtxReceiveStreamTest, IgnoresUnknownPayloadType) {
+  testing::StrictMock<MockRtpPacketSink> media_sink;
+  RtxReceiveStream rtx_sink(&media_sink, std::map<int, int>(), kMediaSSRC);
+  RtpPacketReceived rtx_packet;
+  EXPECT_TRUE(rtx_packet.Parse(rtc::ArrayView<const uint8_t>(kRtxPacket)));
+  rtx_sink.OnRtpPacket(rtx_packet);
+}
+
+TEST(RtxReceiveStreamTest, IgnoresTruncatedPacket) {
+  testing::StrictMock<MockRtpPacketSink> media_sink;
+  RtxReceiveStream rtx_sink(&media_sink, PTMapping(), kMediaSSRC);
+  RtpPacketReceived rtx_packet;
+  EXPECT_TRUE(
+      rtx_packet.Parse(Truncate(rtc::ArrayView<const uint8_t>(kRtxPacket), 2)));
+  rtx_sink.OnRtpPacket(rtx_packet);
+}
+
+}  // namespace webrtc