Add rtcp::TransportFeedback::GetReceivedPackets()

webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc

 /*
  *  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/pacing/packet_router.h"
 #include "webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h"
 #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
 #include "webrtc/system_wrappers/include/clock.h"
 #include "webrtc/test/gmock.h"
 #include "webrtc/test/gtest.h"
 
 using ::testing::_;
-using ::testing::InSequence;
+using ::testing::ElementsAre;
 using ::testing::Invoke;
 using ::testing::Return;
 
 namespace webrtc {
+namespace {
+
+std::vector<uint16_t> SequenceNumbers(const rtcp::TransportFeedback& packet) {

[terelius, 2017/01/17 16:32:23]

It is unclear whether packet refers to the RTCP feedback or RTP media packet.
Maybe rename |packet| to |feedback_packet|? That way it will match the name in
remote_estimator_proxy.cc

[danilchap, 2017/01/17 17:20:14]

Done.

+  std::vector<uint16_t> sequence_numbers;
+  for (const auto& received_packet : packet.GetReceivedPackets()) {
+    sequence_numbers.push_back(received_packet.sequence_number());
+  }
+  return sequence_numbers;
+}
+

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.

+std::vector<int64_t> TimestampsMs(const rtcp::TransportFeedback& packet) {
+  std::vector<int64_t> timestamps;
+  int64_t timestamp_us = packet.GetBaseTimeUs();
+  for (const auto& received_packet : packet.GetReceivedPackets()) {
+    timestamps.push_back((timestamp_us += received_packet.delta_us()) / 1000);
+  }
+  return timestamps;
+}
 
 class MockPacketRouter : public PacketRouter {
  public:
   MOCK_METHOD1(SendFeedback, bool(rtcp::TransportFeedback* packet));

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.

 };
 
 class RemoteEstimatorProxyTest : public ::testing::Test {
  public:
   RemoteEstimatorProxyTest() : clock_(0), proxy_(&clock_, &router_) {}
 
  protected:
   void IncomingPacket(uint16_t seq, int64_t time_ms) {
     RTPHeader header;
     header.extension.hasTransportSequenceNumber = true;
@@ skipping 17 matching lines @@
   const uint16_t kBaseSeq = 10;
   const int64_t kBaseTimeMs = 123;
   const int64_t kMaxSmallDeltaMs =
       (rtcp::TransportFeedback::kDeltaScaleFactor * 0xFF) / 1000;
 };
 
 TEST_F(RemoteEstimatorProxyTest, SendsSinglePacketFeedback) {
   IncomingPacket(kBaseSeq, kBaseTimeMs);
 
   EXPECT_CALL(router_, SendFeedback(_))
-      .Times(1)
       .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) {

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.
To avoid growing these lines too much noticed that moving constants outside
fixture eliminate need to capture this.

-        packet->Build();
         EXPECT_EQ(kBaseSeq, packet->GetBaseSequence());
         EXPECT_EQ(kMediaSsrc, packet->media_ssrc());
 
-        std::vector<rtcp::TransportFeedback::StatusSymbol> status_vec =
-            packet->GetStatusVector();
-        EXPECT_EQ(1u, status_vec.size());
-        EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta,
-                  status_vec[0]);
-        std::vector<int64_t> delta_vec = packet->GetReceiveDeltasUs();
-        EXPECT_EQ(1u, delta_vec.size());
-        EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000);
+        EXPECT_THAT(SequenceNumbers(*packet), ElementsAre(kBaseSeq));
+        EXPECT_THAT(TimestampsMs(*packet), ElementsAre(kBaseTimeMs));
         return true;
       }));
 
   Process();
 }
 
 TEST_F(RemoteEstimatorProxyTest, DuplicatedPackets) {
   IncomingPacket(kBaseSeq, kBaseTimeMs);
   IncomingPacket(kBaseSeq, kBaseTimeMs + 1000);
 
   EXPECT_CALL(router_, SendFeedback(_))
-      .Times(1)
       .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) {

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.

-        packet->Build();
         EXPECT_EQ(kBaseSeq, packet->GetBaseSequence());
         EXPECT_EQ(kMediaSsrc, packet->media_ssrc());
 
-        std::vector<rtcp::TransportFeedback::StatusSymbol> status_vec =
-            packet->GetStatusVector();
-        EXPECT_EQ(1u, status_vec.size());
-        EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta,
-                  status_vec[0]);
-        std::vector<int64_t> delta_vec = packet->GetReceiveDeltasUs();
-        EXPECT_EQ(1u, delta_vec.size());
-        EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000);
+        EXPECT_THAT(SequenceNumbers(*packet), ElementsAre(kBaseSeq));
+        EXPECT_THAT(TimestampsMs(*packet), ElementsAre(kBaseTimeMs));
         return true;
       }));
 
   Process();
 }
 
 TEST_F(RemoteEstimatorProxyTest, FeedbackWithMissingStart) {
   // First feedback.
   IncomingPacket(kBaseSeq, kBaseTimeMs);
   IncomingPacket(kBaseSeq + 1, kBaseTimeMs + 1000);
-  EXPECT_CALL(router_, SendFeedback(_)).Times(1).WillOnce(Return(true));
+  EXPECT_CALL(router_, SendFeedback(_)).WillOnce(Return(true));
   Process();
 
   // Second feedback starts with a missing packet (DROP kBaseSeq + 2).
   IncomingPacket(kBaseSeq + 3, kBaseTimeMs + 3000);
 
   EXPECT_CALL(router_, SendFeedback(_))
-      .Times(1)
       .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) {

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.

-        packet->Build();
         EXPECT_EQ(kBaseSeq + 2, packet->GetBaseSequence());
         EXPECT_EQ(kMediaSsrc, packet->media_ssrc());
 
-        std::vector<rtcp::TransportFeedback::StatusSymbol> status_vec =
-            packet->GetStatusVector();
-        EXPECT_EQ(2u, status_vec.size());
-        EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kNotReceived,
-                  status_vec[0]);
-        EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta,
-                  status_vec[1]);
-        std::vector<int64_t> delta_vec = packet->GetReceiveDeltasUs();
-        EXPECT_EQ(1u, delta_vec.size());
-        EXPECT_EQ(kBaseTimeMs + 3000,
-                  (packet->GetBaseTimeUs() + delta_vec[0]) / 1000);
+        EXPECT_THAT(SequenceNumbers(*packet), ElementsAre(kBaseSeq + 3));
+        EXPECT_THAT(TimestampsMs(*packet), ElementsAre(kBaseTimeMs + 3000));
         return true;
       }));
 
   Process();
 }
 
 TEST_F(RemoteEstimatorProxyTest, SendsFeedbackWithVaryingDeltas) {
   IncomingPacket(kBaseSeq, kBaseTimeMs);
   IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kMaxSmallDeltaMs);
   IncomingPacket(kBaseSeq + 2, kBaseTimeMs + (2 * kMaxSmallDeltaMs) + 1);
 
   EXPECT_CALL(router_, SendFeedback(_))
-      .Times(1)
       .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) {

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.

-        packet->Build();
         EXPECT_EQ(kBaseSeq, packet->GetBaseSequence());
         EXPECT_EQ(kMediaSsrc, packet->media_ssrc());
 
-        std::vector<rtcp::TransportFeedback::StatusSymbol> status_vec =
-            packet->GetStatusVector();
-        EXPECT_EQ(3u, status_vec.size());
-        EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta,
-                  status_vec[0]);
-        EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta,
-                  status_vec[1]);
-        EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedLargeDelta,
-                  status_vec[2]);
-
-        std::vector<int64_t> delta_vec = packet->GetReceiveDeltasUs();
-        EXPECT_EQ(3u, delta_vec.size());
-        EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000);
-        EXPECT_EQ(kMaxSmallDeltaMs, delta_vec[1] / 1000);
-        EXPECT_EQ(kMaxSmallDeltaMs + 1, delta_vec[2] / 1000);
+        EXPECT_THAT(SequenceNumbers(*packet),
+                    ElementsAre(kBaseSeq, kBaseSeq + 1, kBaseSeq + 2));
+        EXPECT_THAT(TimestampsMs(*packet),
+                    ElementsAre(kBaseTimeMs, kBaseTimeMs + kMaxSmallDeltaMs,
+                                kBaseTimeMs + (2 * kMaxSmallDeltaMs) + 1));
         return true;
       }));
 
   Process();
 }
 
 TEST_F(RemoteEstimatorProxyTest, SendsFragmentedFeedback) {
   const int64_t kTooLargeDelta =
       rtcp::TransportFeedback::kDeltaScaleFactor * (1 << 16);
 
   IncomingPacket(kBaseSeq, kBaseTimeMs);
   IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kTooLargeDelta);
 
-  InSequence s;
   EXPECT_CALL(router_, SendFeedback(_))
-      .Times(1)
       .WillOnce(Invoke([kTooLargeDelta, this](rtcp::TransportFeedback* packet) {

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.

-        packet->Build();
         EXPECT_EQ(kBaseSeq, packet->GetBaseSequence());
         EXPECT_EQ(kMediaSsrc, packet->media_ssrc());
 
-        std::vector<rtcp::TransportFeedback::StatusSymbol> status_vec =
-            packet->GetStatusVector();
-        EXPECT_EQ(1u, status_vec.size());
-        EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta,
-                  status_vec[0]);
-        std::vector<int64_t> delta_vec = packet->GetReceiveDeltasUs();
-        EXPECT_EQ(1u, delta_vec.size());
-        EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000);
+        EXPECT_THAT(SequenceNumbers(*packet), ElementsAre(kBaseSeq));
+        EXPECT_THAT(TimestampsMs(*packet), ElementsAre(kBaseTimeMs));
         return true;
       }))
-      .RetiresOnSaturation();
-
-  EXPECT_CALL(router_, SendFeedback(_))
-      .Times(1)
       .WillOnce(Invoke([kTooLargeDelta, this](rtcp::TransportFeedback* packet) {

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.

-        packet->Build();
         EXPECT_EQ(kBaseSeq + 1, packet->GetBaseSequence());
         EXPECT_EQ(kMediaSsrc, packet->media_ssrc());
 
-        std::vector<rtcp::TransportFeedback::StatusSymbol> status_vec =
-            packet->GetStatusVector();
-        EXPECT_EQ(1u, status_vec.size());
-        EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta,
-                  status_vec[0]);
-        std::vector<int64_t> delta_vec = packet->GetReceiveDeltasUs();
-        EXPECT_EQ(1u, delta_vec.size());
-        EXPECT_EQ(kBaseTimeMs + kTooLargeDelta,
-                  (packet->GetBaseTimeUs() + delta_vec[0]) / 1000);
+        EXPECT_THAT(SequenceNumbers(*packet), ElementsAre(kBaseSeq + 1));
+        EXPECT_THAT(TimestampsMs(*packet),
+                    ElementsAre(kBaseTimeMs + kTooLargeDelta));
         return true;
-      }))
-      .RetiresOnSaturation();
+      }));
 
   Process();
 }
 
 TEST_F(RemoteEstimatorProxyTest, GracefullyHandlesReorderingAndWrap) {

[terelius, 2017/01/17 16:32:23]

Based on this test, it looks like "gracefully handling reordered packets and
wrapping seq. numbers" means "not sending any feedback for some packets". Is
this correct?

[danilchap, 2017/01/17 17:20:14]

Yes, currently 'gracefully' mean 'ignore but do not disturb handling of other
packets'. (RemoteEstimatorProxy::OnPacketArrival is responsible for that).

   const int64_t kDeltaMs = 1000;
   const uint16_t kLargeSeq = 62762;
   IncomingPacket(kBaseSeq, kBaseTimeMs);
   IncomingPacket(kLargeSeq, kBaseTimeMs + kDeltaMs);
 
   EXPECT_CALL(router_, SendFeedback(_))
-      .Times(1)
       .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) {

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.

-        packet->Build();
         EXPECT_EQ(kBaseSeq, packet->GetBaseSequence());
         EXPECT_EQ(kMediaSsrc, packet->media_ssrc());
 
-        std::vector<int64_t> delta_vec = packet->GetReceiveDeltasUs();
-        EXPECT_EQ(1u, delta_vec.size());
-        EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000);
+        EXPECT_THAT(TimestampsMs(*packet), ElementsAre(kBaseTimeMs));
         return true;
       }));
 
   Process();
 }
 
 TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) {
   IncomingPacket(kBaseSeq, kBaseTimeMs);
   IncomingPacket(kBaseSeq + 2, kBaseTimeMs + 2);
 
   EXPECT_CALL(router_, SendFeedback(_))
-      .Times(1)
       .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) {

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.

-        packet->Build();
         EXPECT_EQ(kBaseSeq, packet->GetBaseSequence());
         EXPECT_EQ(kMediaSsrc, packet->media_ssrc());
 
-        std::vector<int64_t> delta_vec = packet->GetReceiveDeltasUs();
-        EXPECT_EQ(2u, delta_vec.size());
-        EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000);
-        EXPECT_EQ(2, delta_vec[1] / 1000);
+        EXPECT_THAT(SequenceNumbers(*packet),
+                    ElementsAre(kBaseSeq, kBaseSeq + 2));
+        EXPECT_THAT(TimestampsMs(*packet),
+                    ElementsAre(kBaseTimeMs, kBaseTimeMs + 2));
         return true;
       }));
 
   Process();
 
   IncomingPacket(kBaseSeq + 1, kBaseTimeMs + 1);
 
   EXPECT_CALL(router_, SendFeedback(_))
-      .Times(1)
       .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) {

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.

-        packet->Build();
         EXPECT_EQ(kBaseSeq + 1, packet->GetBaseSequence());
         EXPECT_EQ(kMediaSsrc, packet->media_ssrc());
 
-        std::vector<int64_t> delta_vec = packet->GetReceiveDeltasUs();
-        EXPECT_EQ(2u, delta_vec.size());
-        EXPECT_EQ(kBaseTimeMs + 1,
-                  (packet->GetBaseTimeUs() + delta_vec[0]) / 1000);
-        EXPECT_EQ(1, delta_vec[1] / 1000);
+        EXPECT_THAT(SequenceNumbers(*packet),
+                    ElementsAre(kBaseSeq + 1, kBaseSeq + 2));
+        EXPECT_THAT(TimestampsMs(*packet),
+                    ElementsAre(kBaseTimeMs + 1, kBaseTimeMs + 2));
         return true;
       }));
 
   Process();
 }
 
 TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) {
   const int64_t kTimeoutTimeMs =
       kBaseTimeMs + RemoteEstimatorProxy::kBackWindowMs;
 
   IncomingPacket(kBaseSeq + 2, kBaseTimeMs);
 
   EXPECT_CALL(router_, SendFeedback(_))
-      .Times(1)
       .WillOnce(Invoke([kTimeoutTimeMs, this](rtcp::TransportFeedback* packet) {

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.

-        packet->Build();
         EXPECT_EQ(kBaseSeq + 2, packet->GetBaseSequence());
 
-        std::vector<int64_t> delta_vec = packet->GetReceiveDeltasUs();
-        EXPECT_EQ(1u, delta_vec.size());
-        EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000);
+        EXPECT_THAT(TimestampsMs(*packet), ElementsAre(kBaseTimeMs));
         return true;
       }));
 
   Process();
 
   IncomingPacket(kBaseSeq + 3, kTimeoutTimeMs);  // kBaseSeq + 2 times out here.
 
   EXPECT_CALL(router_, SendFeedback(_))
-      .Times(1)
       .WillOnce(Invoke([kTimeoutTimeMs, this](rtcp::TransportFeedback* packet) {
-        packet->Build();
         EXPECT_EQ(kBaseSeq + 3, packet->GetBaseSequence());
 
-        std::vector<int64_t> delta_vec = packet->GetReceiveDeltasUs();
-        EXPECT_EQ(1u, delta_vec.size());
-        EXPECT_EQ(kTimeoutTimeMs,
-                  (packet->GetBaseTimeUs() + delta_vec[0]) / 1000);
+        EXPECT_THAT(TimestampsMs(*packet), ElementsAre(kTimeoutTimeMs));
         return true;
       }));
 
   Process();
 
   // New group, with sequence starting below the first so that they may be
   // retransmitted.
   IncomingPacket(kBaseSeq, kBaseTimeMs - 1);
   IncomingPacket(kBaseSeq + 1, kTimeoutTimeMs - 1);
 
   EXPECT_CALL(router_, SendFeedback(_))
-      .Times(1)
       .WillOnce(Invoke([kTimeoutTimeMs, this](rtcp::TransportFeedback* packet) {

[terelius, 2017/01/17 16:32:23]

feedback_packet?

[danilchap, 2017/01/17 17:20:14]

Done.

-        packet->Build();
         EXPECT_EQ(kBaseSeq, packet->GetBaseSequence());
 
-        // Four status entries (kBaseSeq + 3 missing).
-        EXPECT_EQ(4u, packet->GetStatusVector().size());
-
-        // Only three actual timestamps.
-        std::vector<int64_t> delta_vec = packet->GetReceiveDeltasUs();
-        EXPECT_EQ(3u, delta_vec.size());
-        EXPECT_EQ(kBaseTimeMs - 1,
-                  (packet->GetBaseTimeUs() + delta_vec[0]) / 1000);
-        EXPECT_EQ(kTimeoutTimeMs - kBaseTimeMs, delta_vec[1] / 1000);
-        EXPECT_EQ(1, delta_vec[2] / 1000);
+        EXPECT_THAT(SequenceNumbers(*packet),
+                    ElementsAre(kBaseSeq, kBaseSeq + 1, kBaseSeq + 3));
+        EXPECT_THAT(
+            TimestampsMs(*packet),
+            ElementsAre(kBaseTimeMs - 1, kTimeoutTimeMs - 1, kTimeoutTimeMs));
         return true;
       }));
 
   Process();
 }
 
 TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsZeroBeforeFirstProcess) {
   EXPECT_EQ(0, proxy_.TimeUntilNextProcess());
 }
 
@@ skipping 27 matching lines @@
             proxy_.TimeUntilNextProcess());
 }
 
 TEST_F(RemoteEstimatorProxyTest, TwccReportsUse5PercentOfAvailableBandwidth) {
   Process();
   proxy_.OnBitrateChanged(80000);
   // 80kbps * 0.05 = TwccReportSize(68B * 8b/B) * 1000ms / SendInterval(136ms)
   EXPECT_EQ(136, proxy_.TimeUntilNextProcess());
 }
 
+}  // namespace
 }  // namespace webrtc