ProbingEstimator: Erase history based on time threshold

webrtc/modules/congestion_controller/probe_bitrate_estimator.h

Index: webrtc/modules/congestion_controller/probe_bitrate_estimator.h
diff --git a/webrtc/modules/congestion_controller/probe_bitrate_estimator.h b/webrtc/modules/congestion_controller/probe_bitrate_estimator.h
index 7f0c4569c97f039976e822d94887987d66e8e994..3298106b557079ba4c72f9bddb26bb825272856f 100644
--- a/webrtc/modules/congestion_controller/probe_bitrate_estimator.h
+++ b/webrtc/modules/congestion_controller/probe_bitrate_estimator.h
@@ -18,25 +18,16 @@
 
 namespace webrtc {
 
-struct ProbingResult {
-  static constexpr int kNoEstimate = -1;
-
-  ProbingResult();
-  ProbingResult(int bps, int64_t timestamp);
-  bool valid() const;
-
-  int bps;
-  int64_t timestamp;
-};
-
 class ProbeBitrateEstimator {
  public:
   ProbeBitrateEstimator();
 
-  // Should be called for every packet we receive feedback about. If the
-  // packet was used for probing it will validate/calculate the resulting
-  // bitrate and return the result.
-  ProbingResult PacketFeedback(const PacketInfo& packet_info);
+  // Should be called for every probe packet we receive feedback about.
+  // Estimates highest bitrate across all probe clusters received.
+  // |min_probe_clusters| specifies minimum number of probe clusters to
+  // have a valid BW value.
+  int HandleProbeAndEstimateBitrate(const PacketInfo& packet_info,
+                                    int min_probe_clusters);
 
  private:
   struct AggregatedCluster {
@@ -46,10 +37,16 @@ class ProbeBitrateEstimator {
     int64_t first_receive_ms = std::numeric_limits<int64_t>::max();
     int64_t last_receive_ms = 0;
     size_t size = 0;
+    int bps = 0;
   };
 
+  // Erases old cluster data that was seen before |timestamp_ms|.
+  void EraseOldClusters(int64_t timestamp_ms);
+
+  int HighestBitrateOnClusters();

[philipel, 2016/08/16 15:21:06]

I'm still not sure why we need this functionality, is it some type of filtering
or validation?

[Irfan, 2016/08/16 17:02:10]

The initial phase has probing on multiple clusters at the same time (3x and 6x
times the estimate). We need to evaluate the highest across clusters at start up
before we move on with one cluster evaluation during exponential probing phase.

Let me know if this helps clarify.

[philipel, 2016/08/19 13:41:31]

Why don't we just return the newest result possible? If we for example have a
800 kbps connection and probe at 900 and 1800, and the result from the first
probe is 950 and the result from the second probe is 750, should we still report
950 as the result from the second probe?

+
   std::map<int, AggregatedCluster> clusters_;
-  int last_valid_cluster_id_;
+  int valid_clusters_ = 0;
 };
 
 }  // namespace webrtc