Add PeerConnectionInterface::UpdateCallBitrate with call tests.

webrtc/call/call.cc

Index: webrtc/call/call.cc
diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc
index e594dcc6cadce83560ac48f325c60c4637a566d2..57e991ee7c1eb12882b2d161792289595d04d3c8 100644
--- a/webrtc/call/call.cc
+++ b/webrtc/call/call.cc
@@ -142,6 +142,9 @@ class Call : public webrtc::Call,
   void SetBitrateConfig(
       const webrtc::Call::Config::BitrateConfig& bitrate_config) override;
 
+  void SetBitrateConfigMask(
+      const webrtc::Call::Config::BitrateConfigMask& bitrate_config) override;
+
   void SignalChannelNetworkState(MediaType media, NetworkState state) override;
 
   void OnTransportOverheadChanged(MediaType media,
@@ -188,6 +191,17 @@ class Call : public webrtc::Call,
   void UpdateHistograms();
   void UpdateAggregateNetworkState();
 
+  // These values only get set if there is actually something to change.
+  struct BitrateUpdate {
+    rtc::Optional<int> min_bitrate_bps;
+    rtc::Optional<int> start_bitrate_bps;
+    rtc::Optional<int> max_bitrate_bps;
+  };
+
+  // Applies update to the BitrateConfig cached in |config_| and updates the
+  // congestion controller if update is not empty.
+  void UpdateCurrentBitrateConfig(const BitrateUpdate& update);
+
   Clock* const clock_;
 
   const int num_cpu_cores_;
@@ -282,6 +296,14 @@ class Call : public webrtc::Call,
   // and deleted before any other members.
   rtc::TaskQueue worker_queue_;
 
+  // The config mask set by SetBitrateConfigMask.
+  // 0 <= min <= start <= max
+  Config::BitrateConfigMask bitrate_config_mask_;
+
+  // The config set by SetBitrateConfig.
+  // min >= 0, start != 0, max == -1 || max > 0
+  Config::BitrateConfig base_bitrate_config_;
+
   RTC_DISALLOW_COPY_AND_ASSIGN(Call);
 };
 }  // namespace internal
@@ -337,7 +359,8 @@ Call::Call(const Call::Config& config,
       receive_side_cc_(clock_, transport_send->packet_router()),
       video_send_delay_stats_(new SendDelayStats(clock_)),
       start_ms_(clock_->TimeInMilliseconds()),
-      worker_queue_("call_worker_queue") {
+      worker_queue_("call_worker_queue"),
+      base_bitrate_config_(config.bitrate_config) {
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   RTC_DCHECK(config.event_log != nullptr);
   RTC_DCHECK_GE(config.bitrate_config.min_bitrate_bps, 0);
@@ -862,29 +885,110 @@ void Call::SetBitrateConfig(
     const webrtc::Call::Config::BitrateConfig& bitrate_config) {
   TRACE_EVENT0("webrtc", "Call::SetBitrateConfig");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
+
   RTC_DCHECK_GE(bitrate_config.min_bitrate_bps, 0);
-  if (bitrate_config.max_bitrate_bps != -1)
+  RTC_DCHECK_NE(bitrate_config.start_bitrate_bps, 0);
+  if (bitrate_config.max_bitrate_bps != -1) {
     RTC_DCHECK_GT(bitrate_config.max_bitrate_bps, 0);
-  if (config_.bitrate_config.min_bitrate_bps ==
-          bitrate_config.min_bitrate_bps &&
-      (bitrate_config.start_bitrate_bps <= 0 ||
-       config_.bitrate_config.start_bitrate_bps ==
-           bitrate_config.start_bitrate_bps) &&
-      config_.bitrate_config.max_bitrate_bps ==
-          bitrate_config.max_bitrate_bps) {
-    // Nothing new to set, early abort to avoid encoder reconfigurations.
+  }
+
+  BitrateUpdate update;
+
+  const int min = bitrate_config.min_bitrate_bps;
+  if (min != base_bitrate_config_.min_bitrate_bps) {
+    base_bitrate_config_.min_bitrate_bps = min;
+    if (!bitrate_config_mask_.min_bitrate_bps ||
+        min > *bitrate_config_mask_.min_bitrate_bps) {
+      update.min_bitrate_bps = rtc::Optional<int>(min);
+      LOG(LS_INFO) << "WebRTC.Call.SetBitrateConfig: "
+                   << "Planning to update min to " << min;
+    }
+  }
+
+  const int start = bitrate_config.start_bitrate_bps;
+  if (start != -1 && start != base_bitrate_config_.start_bitrate_bps) {
+    base_bitrate_config_.start_bitrate_bps = start;
+    update.start_bitrate_bps = rtc::Optional<int>(start);
+    LOG(LS_INFO) << "WebRTC.Call.SetBitrateConfig: "
+                 << "Planning to update start to " << start;
+  }
+  const int max = bitrate_config.max_bitrate_bps;
+  if (max != -1 && max != base_bitrate_config_.max_bitrate_bps) {
+    base_bitrate_config_.max_bitrate_bps = max;
+    if (!bitrate_config_mask_.max_bitrate_bps ||
+        max < *bitrate_config_mask_.max_bitrate_bps) {
+      update.max_bitrate_bps = rtc::Optional<int>(max);
+      LOG(LS_INFO) << "WebRTC.Call.SetBitrateConfig: "
+                   << "Planning to update max to " << max;
+    }
+  }
+
+  UpdateCurrentBitrateConfig(update);
+}
+
+void Call::SetBitrateConfigMask(
+    const webrtc::Call::Config::BitrateConfigMask& mask) {
+  TRACE_EVENT0("webrtc", "Call::SetBitrateConfigMask");
+  RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
+
+  bitrate_config_mask_ = mask;
+  BitrateUpdate update;
+
+  if (mask.min_bitrate_bps &&
+      *mask.min_bitrate_bps > base_bitrate_config_.min_bitrate_bps) {
+    update.min_bitrate_bps = mask.min_bitrate_bps;
+  }
+  update.start_bitrate_bps = mask.start_bitrate_bps;
+  if (mask.max_bitrate_bps &&
+      (base_bitrate_config_.max_bitrate_bps == -1 ||
+       *mask.max_bitrate_bps < base_bitrate_config_.max_bitrate_bps)) {
+    update.max_bitrate_bps = mask.max_bitrate_bps;
+  }
+
+  UpdateCurrentBitrateConfig(update);
+}
+
+void Call::UpdateCurrentBitrateConfig(const BitrateUpdate& update) {
+  if (!update.min_bitrate_bps && !update.start_bitrate_bps &&
+      !update.max_bitrate_bps) {
+    // Nothing to update.
+    LOG(LS_INFO) << "WebRTC.Call.UpdateCurrentBitrateConfig: "
+                 << "nothing to update";

[Taylor Brandstetter, 2017/05/11 23:14:08]

This could be spammy if it happens for every SDP offer/answer; can you change it
to LS_VERBOSE?

[Zach Stein, 2017/05/23 20:58:53]

Done.

     return;
   }
-  config_.bitrate_config.min_bitrate_bps = bitrate_config.min_bitrate_bps;
-  // Start bitrate of -1 means we should keep the old bitrate, which there is
-  // no point in remembering for the future.
-  if (bitrate_config.start_bitrate_bps > 0)
-    config_.bitrate_config.start_bitrate_bps = bitrate_config.start_bitrate_bps;
-  config_.bitrate_config.max_bitrate_bps = bitrate_config.max_bitrate_bps;
-  RTC_DCHECK_NE(bitrate_config.start_bitrate_bps, 0);
+
+  Config::BitrateConfig updated;
+  const Config::BitrateConfig& current = config_.bitrate_config;
+  updated.min_bitrate_bps =
+      update.min_bitrate_bps.value_or(current.min_bitrate_bps);
+  updated.start_bitrate_bps =
+      update.start_bitrate_bps.value_or(current.start_bitrate_bps);
+  updated.max_bitrate_bps =
+      update.max_bitrate_bps.value_or(current.max_bitrate_bps);
+
+  // TODO(zstein): This clamp logic is different than send_side_cc's (moves min
+  // instead of max, allows start to be greather than min) - check that it is a
+  // sensible difference.
+
+  // Clamp min under max and start under max and above min.
+  if (updated.max_bitrate_bps != -1) {
+    updated.min_bitrate_bps =
+        std::min(updated.min_bitrate_bps, updated.max_bitrate_bps);
+  }
+  updated.start_bitrate_bps =
+      cricket::MinPositive(updated.start_bitrate_bps, updated.max_bitrate_bps);
+  updated.start_bitrate_bps =
+      std::max(updated.start_bitrate_bps, updated.min_bitrate_bps);
+
+  int effective_start =
+      update.start_bitrate_bps ? updated.start_bitrate_bps : -1;
+  LOG(INFO) << "WebRTC.Call.UpdateCurrentBitrateConfig: "
+            << "calling SetBweBitrates with args (" << updated.min_bitrate_bps
+            << ", " << effective_start << ", " << updated.max_bitrate_bps
+            << ")";
   transport_send_->send_side_cc()->SetBweBitrates(
-      bitrate_config.min_bitrate_bps, bitrate_config.start_bitrate_bps,
-      bitrate_config.max_bitrate_bps);
+      updated.min_bitrate_bps, effective_start, updated.max_bitrate_bps);
+  config_.bitrate_config = updated;
 }
 
 void Call::SignalChannelNetworkState(MediaType media, NetworkState state) {