Add PeerConnectionInterface::UpdateCallBitrate with call tests.

webrtc/call/call.cc

 /*
  *  Copyright (c) 2013 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 73 matching lines @@
 }
 
 bool UseSendSideBwe(const FlexfecReceiveStream::Config& config) {
   return UseSendSideBwe(config.rtp_header_extensions, config.transport_cc);
 }
 
 class RtpTransportControllerSend : public RtpTransportControllerSendInterface {
  public:
   RtpTransportControllerSend(Clock* clock, webrtc::RtcEventLog* event_log);
 
-  void RegisterNetworkObserver(
-      SendSideCongestionController::Observer* observer);
-
   // Implements RtpTransportControllerSendInterface
   PacketRouter* packet_router() override { return &packet_router_; }
   SendSideCongestionController* send_side_cc() override {
     return &send_side_cc_;
   }
   TransportFeedbackObserver* transport_feedback_observer() override {
     return &send_side_cc_;
   }
   RtpPacketSender* packet_sender() override { return send_side_cc_.pacer(); }
 
  private:
   PacketRouter packet_router_;
   SendSideCongestionController send_side_cc_;
 };
 
 RtpTransportControllerSend::RtpTransportControllerSend(
     Clock* clock,
     webrtc::RtcEventLog* event_log)
     : send_side_cc_(clock, nullptr /* observer */, event_log, &packet_router_) {
 }
 
-void RtpTransportControllerSend::RegisterNetworkObserver(
-    SendSideCongestionController::Observer* observer) {
-  // Must be called only once.
-  send_side_cc_.RegisterNetworkObserver(observer);
-}
-
 }  // namespace
 
 namespace internal {
 
 class Call : public webrtc::Call,
              public PacketReceiver,
              public RecoveredPacketReceiver,
              public SendSideCongestionController::Observer,
              public BitrateAllocator::LimitObserver {
  public:
   Call(const Call::Config& config,
-       std::unique_ptr<RtpTransportControllerSend> transport_send);
+       std::unique_ptr<RtpTransportControllerSendInterface> transport_send);
   virtual ~Call();
 
   // Implements webrtc::Call.
   PacketReceiver* Receiver() override;
 
   webrtc::AudioSendStream* CreateAudioSendStream(
       const webrtc::AudioSendStream::Config& config) override;
   void DestroyAudioSendStream(webrtc::AudioSendStream* send_stream) override;
 
   webrtc::AudioReceiveStream* CreateAudioReceiveStream(
@@ skipping 23 matching lines @@
                                const uint8_t* packet,
                                size_t length,
                                const PacketTime& packet_time) override;
 
   // Implements RecoveredPacketReceiver.
   bool OnRecoveredPacket(const uint8_t* packet, size_t length) override;
 
   void SetBitrateConfig(
       const webrtc::Call::Config::BitrateConfig& bitrate_config) override;
 
+  RTCError SetBitrateConfigMask(
+      const webrtc::Call::Config::BitrateConfigMask& bitrate_config) override;
+
   void SignalChannelNetworkState(MediaType media, NetworkState state) override;
 
   void OnTransportOverheadChanged(MediaType media,
                                   int transport_overhead_per_packet) override;
 
   void OnNetworkRouteChanged(const std::string& transport_name,
                              const rtc::NetworkRoute& network_route) override;
 
   void OnSentPacket(const rtc::SentPacket& sent_packet) override;
 
@@ skipping 26 matching lines @@
                                                   size_t length,
                                                   const PacketTime& packet_time)
       SHARED_LOCKS_REQUIRED(receive_crit_);
 
   void UpdateSendHistograms(int64_t first_sent_packet_ms)
       EXCLUSIVE_LOCKS_REQUIRED(&bitrate_crit_);
   void UpdateReceiveHistograms();
   void UpdateHistograms();
   void UpdateAggregateNetworkState();
 
+  // Updates the BitrateConfig cached in |config_| by combining
+  // |base_bitrate_config_| and |bitrate_config_mask_| and updates the
+  // congestion controller if the combined config changed or if
+  // |force_update| is true.
+  RTCError UpdateCurrentBitrateConfig(bool force_update);
+
+  // UpdateCurrentBitrateConfig, but doesn't verify BitrateConfig invariants.
+  void UpdateCurrentBitrateConfigUnchecked();
+
   Clock* const clock_;
 
   const int num_cpu_cores_;
   const std::unique_ptr<ProcessThread> module_process_thread_;
   const std::unique_ptr<ProcessThread> pacer_thread_;
   const std::unique_ptr<CallStats> call_stats_;
   const std::unique_ptr<BitrateAllocator> bitrate_allocator_;
   Call::Config config_;
   rtc::ThreadChecker configuration_thread_checker_;
 
@@ skipping 74 matching lines @@
   std::unique_ptr<RtpTransportControllerSendInterface> transport_send_;
   ReceiveSideCongestionController receive_side_cc_;
   const std::unique_ptr<SendDelayStats> video_send_delay_stats_;
   const int64_t start_ms_;
   // TODO(perkj): |worker_queue_| is supposed to replace
   // |module_process_thread_|.
   // |worker_queue| is defined last to ensure all pending tasks are cancelled
   // 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
 
 std::string Call::Stats::ToString(int64_t time_ms) const {
   std::stringstream ss;
   ss << "Call stats: " << time_ms << ", {";
   ss << "send_bw_bps: " << send_bandwidth_bps << ", ";
   ss << "recv_bw_bps: " << recv_bandwidth_bps << ", ";
   ss << "max_pad_bps: " << max_padding_bitrate_bps << ", ";
   ss << "pacer_delay_ms: " << pacer_delay_ms << ", ";
   ss << "rtt_ms: " << rtt_ms;
   ss << '}';
   return ss.str();
 }
 
 Call* Call::Create(const Call::Config& config) {
   return new internal::Call(
       config, std::unique_ptr<RtpTransportControllerSend>(
                   new RtpTransportControllerSend(Clock::GetRealTimeClock(),
                                                  config.event_log)));
 }
 
+Call* Call::Create(
+    const Call::Config& config,
+    std::unique_ptr<RtpTransportControllerSendInterface> transport_send) {
+  return new internal::Call(config, std::move(transport_send));
+}
+
 namespace internal {
 
 Call::Call(const Call::Config& config,
-           std::unique_ptr<RtpTransportControllerSend> transport_send)
+           std::unique_ptr<RtpTransportControllerSendInterface> transport_send)
     : clock_(Clock::GetRealTimeClock()),
       num_cpu_cores_(CpuInfo::DetectNumberOfCores()),
       module_process_thread_(ProcessThread::Create("ModuleProcessThread")),
       pacer_thread_(ProcessThread::Create("PacerThread")),
       call_stats_(new CallStats(clock_)),
       bitrate_allocator_(new BitrateAllocator(this)),
       config_(config),
       audio_network_state_(kNetworkDown),
       video_network_state_(kNetworkDown),
       receive_crit_(RWLockWrapper::CreateRWLock()),
       send_crit_(RWLockWrapper::CreateRWLock()),
       event_log_(config.event_log),
       received_bytes_per_second_counter_(clock_, nullptr, true),
       received_audio_bytes_per_second_counter_(clock_, nullptr, true),
       received_video_bytes_per_second_counter_(clock_, nullptr, true),
       received_rtcp_bytes_per_second_counter_(clock_, nullptr, true),
       min_allocated_send_bitrate_bps_(0),
       configured_max_padding_bitrate_bps_(0),
       estimated_send_bitrate_kbps_counter_(clock_, nullptr, true),
       pacer_bitrate_kbps_counter_(clock_, nullptr, true),
       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);
   RTC_DCHECK_GE(config.bitrate_config.start_bitrate_bps,
                 config.bitrate_config.min_bitrate_bps);
   if (config.bitrate_config.max_bitrate_bps != -1) {
     RTC_DCHECK_GE(config.bitrate_config.max_bitrate_bps,
                   config.bitrate_config.start_bitrate_bps);
   }
   Trace::CreateTrace();
-  transport_send->RegisterNetworkObserver(this);
+  transport_send->send_side_cc()->RegisterNetworkObserver(this);
   transport_send_ = std::move(transport_send);
   transport_send_->send_side_cc()->SignalNetworkState(kNetworkDown);
   transport_send_->send_side_cc()->SetBweBitrates(
       config_.bitrate_config.min_bitrate_bps,
       config_.bitrate_config.start_bitrate_bps,
       config_.bitrate_config.max_bitrate_bps);
   call_stats_->RegisterStatsObserver(&receive_side_cc_);
   call_stats_->RegisterStatsObserver(transport_send_->send_side_cc());
 
   module_process_thread_->Start();
@@ skipping 493 matching lines @@
     rtc::CritScope cs(&bitrate_crit_);
     stats.max_padding_bitrate_bps = configured_max_padding_bitrate_bps_;
   }
   return stats;
 }
 
 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) {
+  }
+
+  const bool same_min =
+      base_bitrate_config_.min_bitrate_bps == bitrate_config.min_bitrate_bps;
+  const bool same_start = base_bitrate_config_.start_bitrate_bps ==
+                          bitrate_config.start_bitrate_bps;
+  const bool same_max =
+      base_bitrate_config_.max_bitrate_bps == bitrate_config.max_bitrate_bps;
+  // TODO(zstein): s/same_max/bitrate_config.max_bitrate_bps == -1 || same_max/
+  if (same_min && (bitrate_config.start_bitrate_bps <= 0 || same_start) &&
+      same_max) {
     // Nothing new to set, early abort to avoid encoder reconfigurations.
     return;
   }
-  config_.bitrate_config.min_bitrate_bps = bitrate_config.min_bitrate_bps;
+
+  base_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);
+  if (bitrate_config.start_bitrate_bps > 0) {
+    base_bitrate_config_.start_bitrate_bps = bitrate_config.start_bitrate_bps;
+  }
+  base_bitrate_config_.max_bitrate_bps = bitrate_config.max_bitrate_bps;
+
+  UpdateCurrentBitrateConfigUnchecked();
+}
+
+RTCError Call::SetBitrateConfigMask(
+    const webrtc::Call::Config::BitrateConfigMask& mask) {
+  TRACE_EVENT0("webrtc", "Call::SetBitrateConfigMask");
+  RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
+
+  bitrate_config_mask_ = mask;
+
+  const bool force_update = static_cast<bool>(mask.start_bitrate_bps);
+
+  return UpdateCurrentBitrateConfig(force_update);
+}
+
+Call::Config::BitrateConfig ComputeMasked(
+    const Call::Config::BitrateConfig& base,
+    const Call::Config::BitrateConfigMask& mask) {
+  Call::Config::BitrateConfig masked;
+  masked.min_bitrate_bps = mask.min_bitrate_bps.value_or(base.min_bitrate_bps);
+  masked.start_bitrate_bps =
+      mask.start_bitrate_bps.value_or(base.start_bitrate_bps);
+
+  const rtc::Optional<int>& mask_max = mask.max_bitrate_bps;
+  const int base_max = base.max_bitrate_bps;
+  const bool mask_has_max = static_cast<bool>(mask_max);
+  const bool base_has_max = base_max != -1;
+  if (mask_has_max && base_has_max) {
+    masked.max_bitrate_bps = std::min(*mask_max, base_max);
+  } else if (mask_has_max) {
+    masked.max_bitrate_bps = *mask_max;
+  } else if (base_has_max) {
+    masked.max_bitrate_bps = base_max;
+  } else {
+    masked.max_bitrate_bps = -1;
+  }
+
+  return masked;
+}
+
+void Call::UpdateCurrentBitrateConfigUnchecked() {
+  Config::BitrateConfig masked =
+      ComputeMasked(base_bitrate_config_, bitrate_config_mask_);
+
+  // We only get here if SetBitrateConfig changed something.
+
+  config_.bitrate_config = masked;
   transport_send_->send_side_cc()->SetBweBitrates(
-      bitrate_config.min_bitrate_bps, bitrate_config.start_bitrate_bps,
-      bitrate_config.max_bitrate_bps);
+      masked.min_bitrate_bps, masked.start_bitrate_bps, masked.max_bitrate_bps);
+}
+
+RTCError Call::UpdateCurrentBitrateConfig(bool force_update) {
+  Config::BitrateConfig masked =
+      ComputeMasked(base_bitrate_config_, bitrate_config_mask_);
+
+  Config::BitrateConfig& current = config_.bitrate_config;
+  if (!force_update && masked.min_bitrate_bps == current.min_bitrate_bps &&
+      masked.start_bitrate_bps == current.start_bitrate_bps &&
+      masked.max_bitrate_bps == current.max_bitrate_bps) {
+    return RTCError::OK();
+  }
+
+  RTC_DCHECK_GE(masked.min_bitrate_bps, 0);
+  if (masked.start_bitrate_bps > 0 &&
+      masked.start_bitrate_bps < masked.min_bitrate_bps) {
+    LOG_AND_RETURN_ERROR(
+        RTCErrorType::INVALID_STATE,
+        "effective start_bitrate_bps < effective min_bitrate_bps");
+  }
+  if (masked.max_bitrate_bps != -1 && masked.start_bitrate_bps > 0 &&
+      masked.max_bitrate_bps < masked.start_bitrate_bps) {
+    LOG_AND_RETURN_ERROR(
+        RTCErrorType::INVALID_STATE,
+        "effective max_bitrate_bps < effective start_bitrate_bps");
+  }
+  if (masked.max_bitrate_bps != -1 &&
+      masked.max_bitrate_bps < masked.min_bitrate_bps) {
+    LOG_AND_RETURN_ERROR(
+        RTCErrorType::INVALID_STATE,
+        "effective max_bitrate_bps < effective min_bitrate_bps");
+  }
+
+  current = masked;
+  transport_send_->send_side_cc()->SetBweBitrates(current.min_bitrate_bps,
+                                                  current.start_bitrate_bps,
+                                                  current.max_bitrate_bps);
+  return RTCError::OK();
 }

[Taylor_Brandstetter, 2017/04/26 15:46:11]

I still think the code in this file could be simplified. Here's what I was
thinking:
https://codereview.webrtc.org/2844693002/diff2/1:20001/webrtc/call/call.cc

Note that I also replaced the errors in the above method with clamping

[Zach Stein, 2017/05/04 22:32:43]

Acknowledged.

 
 void Call::SignalChannelNetworkState(MediaType media, NetworkState state) {
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   switch (media) {
     case MediaType::AUDIO:
       audio_network_state_ = state;
       break;
     case MediaType::VIDEO:
       video_network_state_ = state;
       break;
@@ skipping 389 matching lines @@
       (use_send_side_bwe && header.extension.hasTransportSequenceNumber)) {
     receive_side_cc_.OnReceivedPacket(
         packet.arrival_time_ms(), packet.payload_size() + packet.padding_size(),
         header);
   }
 }
 
 }  // namespace internal
 
 }  // namespace webrtc