Structure of BBR's implementation,some main classes and functions which are going to be used

webrtc/modules/remote_bitrate_estimator/test/estimators/bbr.h

+/*
+ *  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.
+ *
+ */
+
+#ifndef WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_TEST_ESTIMATORS_BBR_H_
+#define WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_TEST_ESTIMATORS_BBR_H_
+
+#include <climits>
+#include <map>
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "webrtc/logging/rtc_event_log/mock/mock_rtc_event_log.h"
+#include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h"
+#include "webrtc/modules/remote_bitrate_estimator/test/bwe.h"
+#include "webrtc/modules/remote_bitrate_estimator/test/estimators/stats.h"

[stefan-webrtc, 2017/05/30 09:45:13]

Do we have to include this here, or can we forward declare instead?

[gnish2, 2017/05/30 15:13:21]

Done.

+
+namespace webrtc {
+namespace testing {
+namespace bwe {
+
+class BbrBweSender : public BweSender {
+ public:
+  BbrBweSender(int kbps, BitrateObserver* observer, Clock* clock);
+  virtual ~BbrBweSender();
+
+  enum Mode {
+    // Startup phase.
+    STARTUP,
+    // Queue draining phase,which where created during startup.
+    DRAIN,
+    // Cruising.
+    PROBE_BW,
+    // Temporarily decreasing throughtput in order to measure
+    // minimum RTT.
+    PROBE_RTT
+  };
+
+  void OnPacketsSent(const Packets& packets) override;

[stefan-webrtc, 2017/05/30 09:45:14]

Please group overrides and non-overrides separately.

[gnish2, 2017/05/30 15:13:21]

Done.

+  void EnterStartup();

[philipel, 2017/05/30 09:43:38]

All functions that you expect to only be used by this class should be private.

[gnish2, 2017/05/30 15:13:21]

Done.

+  void EnterDrain(const FeedbackPacket& feedback);
+  void EnterCruise(const FeedbackPacket& feedback);
+  void EnterDecreasingThroughput(const FeedbackPacket& feedback);
+  bool UpdateBandwidthAndMinRtt(
+      int64_t now,
+      const std::vector<std::pair<uint64_t, int64_t>>& feedback_vector,
+      int64_t bytes_acked);
+  void TryExitingStartup();
+  void TryExitingDrain(int64_t now);
+  void EnterProbeBw(int64_t now);
+  void EnterProbeRtt(int64_t now);
+  void TryExitingProbeRtt(int64_t now);
+  void TryUpdatingCyclePhase(int64_t now);
+  int GetPacketSize(const uint64_t sequence_number);

[stefan-webrtc, 2017/05/30 09:45:14]

No need for const for pass-by-value parameters.

Should this return size_t?

[gnish2, 2017/05/30 15:13:21]

Done.

+  int GetFeedbackIntervalMs() const override;

[stefan-webrtc, 2017/05/30 09:45:14]

Return int64_t which is typically used for time.

[gnish2, 2017/05/30 15:13:21]

This is overrode function,which's return type in simulation tool (in bwe.h) is
declared as int.So I cannot change it to int64_t unless I change it in bwe.h.

+  void GiveFeedback(const FeedbackPacket& feedback) override;
+
+  int64_t TimeUntilNextProcess() override;
+  void Process() override;
+
+ private:
+  Clock* const clock_;
+  BitrateObserver* const observer_;
+  int64_t last_feedback_ms_ = 0;

[stefan-webrtc, 2017/05/30 09:45:13]

Please initialize in initializer list.

[gnish2, 2017/05/30 15:13:21]

Done.

+  std::map<uint16_t, int64_t> send_time_;

[stefan-webrtc, 2017/05/30 09:45:14]

Map from what to what? Add a comment above.

[gnish2, 2017/05/30 15:13:21]

Done.

+  Mode mode_;
+  ConnectionState* stats_;
+  uint64_t round_trip_end_;
+  int64_t rt_count_;

[stefan-webrtc, 2017/05/30 09:45:13]

rtt_count_?

[gnish2, 2017/05/30 15:13:21]

Done.

+  int64_t last_packet_sent_;
+};
+
+class BbrBweReceiver : public BweReceiver {
+ public:
+  explicit BbrBweReceiver(int flow_id);
+  virtual ~BbrBweReceiver();
+  void ReceivePacket(int64_t arrival_time_ms,
+                     const MediaPacket& media_packet) override;
+  FeedbackPacket* GetFeedback(int64_t now_ms) override;
+  static const int64_t kReceivingRateTimeWindowMs;
+  int64_t last_feedback_ms_;

[stefan-webrtc, 2017/05/30 09:45:14]

private?

[gnish2, 2017/05/30 15:13:21]

Done.

+
+ private:
+  SimulatedClock clock_;
+  std::vector<std::pair<uint64_t, int64_t>> packet_feedbacks_;
+};
+}  // namespace bwe
+}  // namespace testing
+}  // namespace webrtc
+
+#endif  // WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_TEST_ESTIMATORS_BBR_H_