Reland "Moved BitrateConfig out of Call::Config."

This is a reland of 5897fe2. Adding back CallConfig::kDefaultStartBitrateBps as deprecated. Also making BitrateContraints::kDefaultStartBitrateBps private to stop it from being used in other places. Original change's description: > Moved BitrateConfig out of Call::Config. > > This prepares for a CL extracting the bitrate configuration logic from > the Call class. > > Also renaming BitrateConfig to BitrateConstraints. > > Bug: webrtc:8415 > Change-Id: I7e472683034c57bdc8093cdf5e78e477d1732480 > Reviewed-on: https://webrtc-review.googlesource.com/54400 > Commit-Queue: Sebastian Jansson <[email protected]> > Reviewed-by: Stefan Holmer <[email protected]> > Reviewed-by: Niels Moller <[email protected]> > Cr-Commit-Position: refs/heads/master@{#22104} Bug: webrtc:8415 Change-Id: Iacfe2d6daedff710832ab89210c7c66d4403c93b Reviewed-on: https://webrtc-review.googlesource.com/55980 Commit-Queue: Sebastian Jansson <[email protected]> Reviewed-by: Stefan Holmer <[email protected]> Reviewed-by: Niels Moller <[email protected]> Cr-Commit-Position: refs/heads/master@{#22123}
jiashilei-1985 · Feb 21, 2018 · fc8d26b · fc8d26b
1 parent 9f016a0
commit fc8d26b
Show file tree

Hide file tree

Showing 23 changed files with 150 additions and 108 deletions.
diff --git a/call/BUILD.gn b/call/BUILD.gn
@@ -42,6 +42,8 @@ rtc_source_set("call_interfaces") {
 # when interfaces have stabilized. See also TODO for |mock_rtp_interfaces|.
 rtc_source_set("rtp_interfaces") {
   sources = [
+    "bitrate_constraints.cc",
+    "bitrate_constraints.h",
     "rtcp_packet_sink_interface.h",
     "rtp_config.cc",
     "rtp_config.h",
@@ -51,6 +53,7 @@ rtc_source_set("rtp_interfaces") {
   ]
   deps = [
     "../api:array_view",
+    "../api:optional",
     "../rtc_base:rtc_base_approved",
   ]
 }

diff --git a/call/bitrate_constraints.cc b/call/bitrate_constraints.cc
@@ -0,0 +1,18 @@
+/*
+ *  Copyright (c) 2018 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.
+ */
+
+#include "call/bitrate_constraints.h"
+
+namespace webrtc {
+BitrateConstraintsMask::BitrateConstraintsMask() = default;
+BitrateConstraintsMask::~BitrateConstraintsMask() = default;
+BitrateConstraintsMask::BitrateConstraintsMask(const BitrateConstraintsMask&) =
+    default;
+}  // namespace webrtc
diff --git a/call/bitrate_constraints.h b/call/bitrate_constraints.h
@@ -0,0 +1,55 @@
+/*
+ *  Copyright (c) 2018 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 CALL_BITRATE_CONSTRAINTS_H_
+#define CALL_BITRATE_CONSTRAINTS_H_
+
+#include <algorithm>
+
+#include "api/optional.h"
+
+namespace webrtc {
+// TODO(srte): BitrateConstraints and BitrateConstraintsMask should be merged.
+// Both represent the same kind data, but are using different default
+// initializer and representation of unset values.
+struct BitrateConstraints {
+  int min_bitrate_bps = 0;
+  int start_bitrate_bps = kDefaultStartBitrateBps;
+  int max_bitrate_bps = -1;
+
+ private:
+  static constexpr int kDefaultStartBitrateBps = 300000;
+};
+
+// BitrateConstraintsMask is used for the local client's bitrate preferences.
+// Semantically it carries the same kind of information as BitrateConstraints,
+// but is used in a slightly different way.
+struct BitrateConstraintsMask {
+  BitrateConstraintsMask();
+  ~BitrateConstraintsMask();
+  BitrateConstraintsMask(const BitrateConstraintsMask&);
+  rtc::Optional<int> min_bitrate_bps;
+  rtc::Optional<int> start_bitrate_bps;
+  rtc::Optional<int> max_bitrate_bps;
+};
+
+// Like std::min, but considers non-positive values to be unset.
+template <typename T>
+static T MinPositive(T a, T b) {
+  if (a <= 0) {
+    return b;
+  }
+  if (b <= 0) {
+    return a;
+  }
+  return std::min(a, b);
+}
+}  // namespace webrtc
+#endif  // CALL_BITRATE_CONSTRAINTS_H_
diff --git a/call/call.cc b/call/call.cc
@@ -213,10 +213,10 @@ class Call : public webrtc::Call,
   void OnRecoveredPacket(const uint8_t* packet, size_t length) override;
 
   void SetBitrateConfig(
-      const webrtc::Call::Config::BitrateConfig& bitrate_config) override;
+      const webrtc::BitrateConstraints& bitrate_config) override;
 
   void SetBitrateConfigMask(
-      const webrtc::Call::Config::BitrateConfigMask& bitrate_config) override;
+      const webrtc::BitrateConstraintsMask& bitrate_config) override;
 
   void SetBitrateAllocationStrategy(
       std::unique_ptr<rtc::BitrateAllocationStrategy>
@@ -261,7 +261,7 @@ class Call : public webrtc::Call,
   void UpdateHistograms();
   void UpdateAggregateNetworkState();
 
-  // Applies update to the BitrateConfig cached in |config_|, restarting
+  // Applies update to the BitrateConstraints cached in |config_|, restarting
   // bandwidth estimation from |new_start| if set.
   void UpdateCurrentBitrateConfig(const rtc::Optional<int>& new_start);
 
@@ -374,11 +374,11 @@ class Call : public webrtc::Call,
 
   // The config mask set by SetBitrateConfigMask.
   // 0 <= min <= start <= max
-  Config::BitrateConfigMask bitrate_config_mask_;
+  BitrateConstraintsMask bitrate_config_mask_;
 
   // The config set by SetBitrateConfig.
   // min >= 0, start != 0, max == -1 || max > 0
-  Config::BitrateConfig base_bitrate_config_;
+  BitrateConstraints base_bitrate_config_;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(Call);
 };
@@ -954,8 +954,7 @@ Call::Stats Call::GetStats() const {
   return stats;
 }
 
-void Call::SetBitrateConfig(
-    const webrtc::Call::Config::BitrateConfig& bitrate_config) {
+void Call::SetBitrateConfig(const BitrateConstraints& bitrate_config) {
   TRACE_EVENT0("webrtc", "Call::SetBitrateConfig");
   RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_);
   RTC_DCHECK_GE(bitrate_config.min_bitrate_bps, 0);
@@ -978,8 +977,7 @@ void Call::SetBitrateConfig(
   UpdateCurrentBitrateConfig(new_start);
 }
 
-void Call::SetBitrateConfigMask(
-    const webrtc::Call::Config::BitrateConfigMask& mask) {
+void Call::SetBitrateConfigMask(const BitrateConstraintsMask& mask) {
   TRACE_EVENT0("webrtc", "Call::SetBitrateConfigMask");
   RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_);
 
@@ -988,7 +986,7 @@ void Call::SetBitrateConfigMask(
 }
 
 void Call::UpdateCurrentBitrateConfig(const rtc::Optional<int>& new_start) {
-  Config::BitrateConfig updated;
+  BitrateConstraints updated;
   updated.min_bitrate_bps =
       std::max(bitrate_config_mask_.min_bitrate_bps.value_or(0),
                base_bitrate_config_.min_bitrate_bps);

diff --git a/call/call.h b/call/call.h
@@ -20,6 +20,7 @@
 #include "call/audio_receive_stream.h"
 #include "call/audio_send_stream.h"
 #include "call/audio_state.h"
+#include "call/bitrate_constraints.h"
 #include "call/flexfec_receive_stream.h"
 #include "call/rtp_transport_controller_send_interface.h"
 #include "call/video_receive_stream.h"
@@ -43,19 +44,6 @@ enum class MediaType {
   DATA
 };
 
-// Like std::min, but considers non-positive values to be unset.
-// TODO(zstein): Remove once all callers use rtc::Optional.
-template <typename T>
-static T MinPositive(T a, T b) {
-  if (a <= 0) {
-    return b;
-  }
-  if (b <= 0) {
-    return a;
-  }
-  return std::min(a, b);
-}
-
 class PacketReceiver {
  public:
   enum DeliveryStatus {
@@ -77,26 +65,11 @@ struct CallConfig {
     RTC_DCHECK(event_log);
   }
 
-  static constexpr int kDefaultStartBitrateBps = 300000;
+  RTC_DEPRECATED static constexpr int kDefaultStartBitrateBps = 300000;
 
   // Bitrate config used until valid bitrate estimates are calculated. Also
   // used to cap total bitrate used. This comes from the remote connection.
-  struct BitrateConfig {
-    int min_bitrate_bps = 0;
-    int start_bitrate_bps = kDefaultStartBitrateBps;
-    int max_bitrate_bps = -1;
-  } bitrate_config;
-
-  // The local client's bitrate preferences. The actual configuration used
-  // is a combination of this and |bitrate_config|. The combination is
-  // currently more complicated than a simple mask operation (see
-  // SetBitrateConfig and SetBitrateConfigMask). Assumes that 0 <= min <=
-  // start <= max holds for set parameters.
-  struct BitrateConfigMask {
-    rtc::Optional<int> min_bitrate_bps;
-    rtc::Optional<int> start_bitrate_bps;
-    rtc::Optional<int> max_bitrate_bps;
-  };
+  BitrateConstraints bitrate_config;
 
   // AudioState which is possibly shared between multiple calls.
   // TODO(solenberg): Change this to a shared_ptr once we can use C++11.
@@ -184,15 +157,14 @@ class Call {
   // This is due to how the 'x-google-start-bitrate' flag is currently
   // implemented. Passing -1 leaves the start bitrate unchanged. Behavior is not
   // guaranteed for other negative values or 0.
-  virtual void SetBitrateConfig(
-      const Config::BitrateConfig& bitrate_config) = 0;
+  virtual void SetBitrateConfig(const BitrateConstraints& bitrate_config) = 0;
 
   // The greater min and smaller max set by this and SetBitrateConfig will be
   // used. The latest non-negative start value form either call will be used.
   // Specifying a start bitrate will reset the current bitrate estimate.
   // Assumes 0 <= min <= start <= max holds for set parameters.
   virtual void SetBitrateConfigMask(
-      const Config::BitrateConfigMask& bitrate_mask) = 0;
+      const BitrateConstraintsMask& bitrate_mask) = 0;
 
   virtual void SetBitrateAllocationStrategy(
       std::unique_ptr<rtc::BitrateAllocationStrategy>

diff --git a/call/call_perf_tests.cc b/call/call_perf_tests.cc
@@ -888,7 +888,7 @@ void CallPerfTest::TestMinAudioVideoBitrate(
 
     void OnCallsCreated(Call* sender_call, Call* receiver_call) override {
       sender_call_ = sender_call;
-      Call::Config::BitrateConfig bitrate_config;
+      BitrateConstraints bitrate_config;
       bitrate_config.min_bitrate_bps = min_bwe_;
       bitrate_config.start_bitrate_bps = start_bwe_;
       bitrate_config.max_bitrate_bps = max_bwe_;