Use |probe_cluster_id| to cluster packets.

webrtc/modules/congestion_controller/delay_based_bandwidth_estimator.h

 /*
- *  Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
+ *  Copyright (c) 2016 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_REMOTE_BITRATE_ESTIMATOR_ABS_SEND_TIME_H_
-#define WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_REMOTE_BITRATE_ESTIMATOR_ABS_SEND_TIME_H_
+#ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_DELAY_BASED_BANDWIDTH_ESTIMATOR_H_
+#define WEBRTC_MODULES_CONGESTION_CONTROLLER_DELAY_BASED_BANDWIDTH_ESTIMATOR_H_
 
 #include <list>
 #include <map>
 #include <memory>
 #include <vector>
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/base/criticalsection.h"
 #include "webrtc/base/rate_statistics.h"
 #include "webrtc/base/thread_checker.h"
 #include "webrtc/modules/remote_bitrate_estimator/aimd_rate_control.h"
 #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
 #include "webrtc/modules/remote_bitrate_estimator/inter_arrival.h"
 #include "webrtc/modules/remote_bitrate_estimator/overuse_detector.h"
 #include "webrtc/modules/remote_bitrate_estimator/overuse_estimator.h"
 #include "webrtc/system_wrappers/include/critical_section_wrapper.h"
 
 namespace webrtc {
 
-struct Probe {
-  Probe(int64_t send_time_ms, int64_t recv_time_ms, size_t payload_size)
-      : send_time_ms(send_time_ms),
-        recv_time_ms(recv_time_ms),
-        payload_size(payload_size) {}
-  int64_t send_time_ms;
-  int64_t recv_time_ms;
-  size_t payload_size;
-};
-
-struct Cluster {
-  Cluster()
-      : send_mean_ms(0.0f),
-        recv_mean_ms(0.0f),
-        mean_size(0),
-        count(0),
-        num_above_min_delta(0) {}
-
-  int GetSendBitrateBps() const {
-    RTC_CHECK_GT(send_mean_ms, 0.0f);
-    return mean_size * 8 * 1000 / send_mean_ms;
-  }
-
-  int GetRecvBitrateBps() const {
-    RTC_CHECK_GT(recv_mean_ms, 0.0f);
-    return mean_size * 8 * 1000 / recv_mean_ms;
-  }
-
-  float send_mean_ms;
-  float recv_mean_ms;
-  // TODO(holmer): Add some variance metric as well?
-  size_t mean_size;
-  int count;
-  int num_above_min_delta;
-};
-
-class RemoteBitrateEstimatorAbsSendTime : public RemoteBitrateEstimator {
+class DelayBasedBwe : public RemoteBitrateEstimator {

[danilchap, 2016/06/03 16:25:29]

it is easiser to follow code when class name match filename, i.e.
rename class to DelayBasedBandwidthEstimator or file to delay_based_bwe.h
(or both of them to something like LocalBandwidthEstimatorDelayBased)

[philipel, 2016/06/08 11:46:38]

Renamed files to delay_based_bwe.cc/h.

  public:
-  explicit RemoteBitrateEstimatorAbsSendTime(RemoteBitrateObserver* observer);
-  virtual ~RemoteBitrateEstimatorAbsSendTime() {}
+  explicit DelayBasedBwe(RemoteBitrateObserver* observer);
+  virtual ~DelayBasedBwe() {}
 
   void IncomingPacketFeedbackVector(
       const std::vector<PacketInfo>& packet_feedback_vector) override;
 
   void IncomingPacket(int64_t arrival_time_ms,
                       size_t payload_size,
                       const RTPHeader& header,
                       bool was_paced) override;
   // This class relies on Process() being called periodically (at least once
   // every other second) for streams to be timed out properly. Therefore it
   // shouldn't be detached from the ProcessThread except if it's about to be
   // deleted.
   void Process() override;
   int64_t TimeUntilNextProcess() override;
   void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override;
   void RemoveStream(uint32_t ssrc) override;
   bool LatestEstimate(std::vector<uint32_t>* ssrcs,
                       uint32_t* bitrate_bps) const override;
   void SetMinBitrate(int min_bitrate_bps) override;
 
  private:
+  struct Probe {
+    Probe(int64_t send_time_ms,
+          int64_t recv_time_ms,
+          size_t payload_size,
+          int probe_cluster_id)
+        : send_time_ms(send_time_ms),
+          recv_time_ms(recv_time_ms),
+          payload_size(payload_size),
+          probe_cluster_id(probe_cluster_id) {}
+    int64_t send_time_ms;
+    int64_t recv_time_ms;
+    size_t payload_size;
+    int probe_cluster_id;

[danilchap, 2016/06/03 16:25:29]

this code (specially this struct) already knows it is about probing. May be name
member cluster_id to make it shorter

[philipel, 2016/06/08 11:46:38]

Done.

+  };
+
+  struct Cluster {

[danilchap, 2016/06/03 16:25:28]

Probe can't be easily forward-declared, but Cluster can.
Not sure if you want to start to move classes around in this or next CL thought.

[philipel, 2016/06/08 11:46:38]

I would keep them grouped rather than to have one in the .h file and the other
in the .cc file. If we want to move it lets do it in another CL.

+    Cluster()
+        : send_mean_ms(0.0f),
+          recv_mean_ms(0.0f),
+          mean_size(0),
+          count(0),
+          num_above_min_delta(0) {}
+
+    int GetSendBitrateBps() const {
+      RTC_CHECK_GT(send_mean_ms, 0.0f);
+      return mean_size * 8 * 1000 / send_mean_ms;
+    }
+
+    int GetRecvBitrateBps() const {
+      RTC_CHECK_GT(recv_mean_ms, 0.0f);
+      return mean_size * 8 * 1000 / recv_mean_ms;
+    }
+
+    float send_mean_ms;
+    float recv_mean_ms;
+    // TODO(holmer): Add some variance metric as well?
+    size_t mean_size;
+    int count;
+    int num_above_min_delta;
+  };
+
   typedef std::map<uint32_t, int64_t> Ssrcs;
   enum class ProbeResult { kBitrateUpdated, kNoUpdate };
 
-  static bool IsWithinClusterBounds(int send_delta_ms,
-                                    const Cluster& cluster_aggregate);
-
   static void AddCluster(std::list<Cluster>* clusters, Cluster* cluster);
 
   void IncomingPacketInfo(int64_t arrival_time_ms,
                           uint32_t send_time_24bits,
                           size_t payload_size,
                           uint32_t ssrc,
-                          bool was_paced);
+                          bool was_paced,
+                          int probe_cluster_id);
 
   void ComputeClusters(std::list<Cluster>* clusters) const;
 
   std::list<Cluster>::const_iterator FindBestProbe(
       const std::list<Cluster>& clusters) const;
 
   // Returns true if a probe which changed the estimate was detected.
   ProbeResult ProcessClusters(int64_t now_ms) EXCLUSIVE_LOCKS_REQUIRED(&crit_);
 
   bool IsBitrateImproving(int probe_bitrate_bps) const
@@ skipping 11 matching lines @@
   std::vector<int64_t> recent_update_time_ms_;
   std::list<Probe> probes_;
   size_t total_probes_received_;
   int64_t first_packet_time_ms_;
   int64_t last_update_ms_;
 
   rtc::CriticalSection crit_;
   Ssrcs ssrcs_ GUARDED_BY(&crit_);
   AimdRateControl remote_rate_ GUARDED_BY(&crit_);
 
-  RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RemoteBitrateEstimatorAbsSendTime);
+  RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(DelayBasedBwe);
 };
 
 }  // namespace webrtc
 
-#endif  // WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_REMOTE_BITRATE_ESTIMATOR_ABS_SEND_TIME_H_
+#endif  // WEBRTC_MODULES_CONGESTION_CONTROLLER_DELAY_BASED_BANDWIDTH_ESTIMATOR_H_