Adding reordering logic in audio network adaptor.

webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h

Index: webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h
diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h b/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h
index 6027c42fd24a55806d1da0fc4b40f83e73ff81b7..aa5a5a8faa005e4e3bdc9db2c748feecfa998f68 100644
--- a/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h
+++ b/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h
@@ -11,11 +11,13 @@
 #ifndef WEBRTC_MODULES_AUDIO_CODING_AUDIO_NETWORK_ADAPTOR_CONTROLLER_MANAGER_H_
 #define WEBRTC_MODULES_AUDIO_CODING_AUDIO_NETWORK_ADAPTOR_CONTROLLER_MANAGER_H_
 
+#include <map>
 #include <memory>
 #include <vector>
 
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/modules/audio_coding/audio_network_adaptor/controller.h"
+#include "webrtc/system_wrappers/include/clock.h"

[hlundin-webrtc, 2016/09/20 08:44:01]

Forward-declare here, and include in .cc and unit test instead.

[minyue-webrtc, 2016/09/21 07:42:58]

Done.

 
 namespace webrtc {
 
@@ -33,15 +35,23 @@ class ControllerManager {
 class ControllerManagerImpl final : public ControllerManager {
  public:
   struct Config {
-    Config();
+    Config(int min_reordering_time_ms,
+           float min_reordering_squared_distance,
+           Clock* clock);
     ~Config();
+    int min_reordering_time_ms;
+    float min_reordering_squared_distance;
+    Clock* clock;
   };
 
   explicit ControllerManagerImpl(const Config& config);
 
   // Dependency injection for testing.
-  ControllerManagerImpl(const Config& config,
-                        std::vector<std::unique_ptr<Controller>> controllers);
+  ControllerManagerImpl(
+      const Config& config,
+      const std::map<const Controller*, std::pair<int, float>>&
+          chracteristic_points,
+      std::vector<std::unique_ptr<Controller>> controllers);

[michaelt, 2016/09/19 08:22:45]

How a bout a vector with a pair with std::unique_ptr<Controller> and
rtc::Optional<ScoringPoint>. I think this way the interface would be more clear
to me ?

[minyue-webrtc, 2016/09/19 10:06:58]

I like the idea, but I want to keep ScoringPoint private, so it will become
std::vector<std::pair<std::unique_ptr<Controller>>, rtc::Optional<std::pair<int,
float>>>>

It looks cumbersome, and we cannot assign values to |controllers_| easily.

Another solution can be to use 2 arguments:
std::vector<std::unique_ptr<Controller>> controllers
std::vector<rtc::Optional<std::pair<int, float>>> chracteristic_points   //
one-one mapped to |controllers|

But this is not much better than std::map<const Controller*, std::pair<int,
float>>. An additional benefit of std::map<const Controller*, std::pair<int,
float>> is that it matches the member variable |controller_scoring_points_| in a
nice way.

[hlundin-webrtc, 2016/09/20 08:44:00]

It's not immediately clear that |controllers| is actually moved from. I would
argue (in conflict with the style guide), that you you apply rvalue notation
here, i.e., "std::vector<std::unique_ptr<Controller>>&& controllers".

[minyue-webrtc, 2016/09/21 07:42:58]

rvalue notation is very new to me. But I think it fits here extremely well,
adopted, thanks!

 
   ~ControllerManagerImpl() override;
 
@@ -52,12 +62,33 @@ class ControllerManagerImpl final : public ControllerManager {
   std::vector<Controller*> GetControllers() const override;
 
  private:
+  // Scoring point is a subset of NetworkMetrics that is used for comparing the
+  // significance of controllers.
+  struct ScoringPoint {
+    ScoringPoint(int uplink_bandwidth_bps, float uplink_packet_loss_fraction);
+
+    // Calculate the normalized [0,1] distance between two scoring points.
+    float SquaredDistanceTo(const ScoringPoint& scoring_point) const;
+
+    int uplink_bandwidth_bps;
+    float uplink_packet_loss_fraction;
+  };
+
   const Config config_;
 
   std::vector<std::unique_ptr<Controller>> controllers_;
 
+  rtc::Optional<int64_t> last_reordering_time_ms_;
+  ScoringPoint last_scoring_point_;
+
   std::vector<Controller*> default_sorted_controllers_;
 
+  std::vector<Controller*> sorted_controllers_;
+
+  // |scoring_points_| saves the characteristic scoring points of various
+  // controllers.
+  std::map<const Controller*, ScoringPoint> controller_scoring_points_;
+
   RTC_DISALLOW_COPY_AND_ASSIGN(ControllerManagerImpl);
 };