Add 120ms frame ability to ANA

webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.cc

 /*
  *  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.
  */
 
@@ skipping 63 matching lines @@
 
 std::unique_ptr<FrameLengthController> CreateFrameLengthController(
     const audio_network_adaptor::config::FrameLengthController& config,
     rtc::ArrayView<const int> encoder_frame_lengths_ms,
     int initial_frame_length_ms) {
   RTC_CHECK(config.has_fl_increasing_packet_loss_fraction());
   RTC_CHECK(config.has_fl_decreasing_packet_loss_fraction());
   RTC_CHECK(config.has_fl_20ms_to_60ms_bandwidth_bps());
   RTC_CHECK(config.has_fl_60ms_to_20ms_bandwidth_bps());
 
+  std::map<FrameLengthController::Config::FrameLengthChange, int>
+      fl_changing_bandwidths_bps;
+  fl_changing_bandwidths_bps.insert(
+      std::make_pair(FrameLengthController::Config::FrameLengthChange(20, 60),
+                     config.fl_20ms_to_60ms_bandwidth_bps()));
+  fl_changing_bandwidths_bps.insert(
+      std::make_pair(FrameLengthController::Config::FrameLengthChange(60, 20),
+                     config.fl_60ms_to_20ms_bandwidth_bps()));

[kwiberg-webrtc, 2017/02/02 12:34:02]

I think you can replace line 84-91 with something like

  std::map<FrameLengthController::Config::FrameLengthChange, int>
fl_changing_bandwidths_bps =
{{FrameLengthController::Config::FrameLengthChange(20, 60),
config.fl_20ms_to_60ms_bandwidth_bps()},
{FrameLengthController::Config::FrameLengthChange(60, 20),
config.fl_60ms_to_20ms_bandwidth_bps()}};

[michaelt, 2017/02/02 13:04:34]

Cool

[kwiberg-webrtc, 2017/02/02 13:18:40]

Nice. I suspect it's a bit more efficient, but mainly it's more readable.

+  if (config.has_fl_60ms_to_120ms_bandwidth_bps() &&
+      config.has_fl_120ms_to_60ms_bandwidth_bps()) {
+    fl_changing_bandwidths_bps.insert(std::make_pair(
+        FrameLengthController::Config::FrameLengthChange(60, 120),
+        config.fl_60ms_to_120ms_bandwidth_bps()));
+    fl_changing_bandwidths_bps.insert(std::make_pair(
+        FrameLengthController::Config::FrameLengthChange(120, 60),
+        config.fl_120ms_to_60ms_bandwidth_bps()));
+  }
+
   FrameLengthController::Config ctor_config(
       std::vector<int>(), initial_frame_length_ms,
       config.fl_increasing_packet_loss_fraction(),
-      config.fl_decreasing_packet_loss_fraction(),
-      config.fl_20ms_to_60ms_bandwidth_bps(),
-      config.fl_60ms_to_20ms_bandwidth_bps());
+      config.fl_decreasing_packet_loss_fraction(), fl_changing_bandwidths_bps);

[kwiberg-webrtc, 2017/02/02 12:34:02]

If you std::move fl_changing_bandwidths_bps here and change the argument type
from const T& to just T, you won't have to pay for copying and destroying an
extra map instance. I guess this code may not be performance critical, but it's
good to get into the habit...

[michaelt, 2017/02/02 13:04:34]

Done.

 
   for (auto frame_length : encoder_frame_lengths_ms)
     ctor_config.encoder_frame_lengths_ms.push_back(frame_length);
 
   return std::unique_ptr<FrameLengthController>(
       new FrameLengthController(ctor_config));
 }
 
 std::unique_ptr<ChannelController> CreateChannelController(
     const audio_network_adaptor::config::ChannelController& config,
@@ skipping 225 matching lines @@
       NormalizeUplinkBandwidth(scoring_point.uplink_bandwidth_bps) -
       NormalizeUplinkBandwidth(uplink_bandwidth_bps);
   float diff_normalized_packet_loss =
       NormalizePacketLossFraction(scoring_point.uplink_packet_loss_fraction) -
       NormalizePacketLossFraction(uplink_packet_loss_fraction);
   return std::pow(diff_normalized_bitrate_bps, 2) +
          std::pow(diff_normalized_packet_loss, 2);
 }
 
 }  // namespace webrtc