Refactor how padding is calculated.

webrtc/call/bitrate_allocator.cc

 /*
  *  Copyright (c) 2015 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 "webrtc/call/bitrate_allocator.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "webrtc/base/checks.h"
 #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h"
 
 namespace webrtc {
 
 // Allow packets to be transmitted in up to 2 times max video bitrate if the
 // bandwidth estimate allows it.
 const int kTransmissionMaxBitrateMultiplier = 2;
 const int kDefaultBitrateBps = 300000;
 
-BitrateAllocator::BitrateAllocator()
-    : bitrate_observer_configs_(),
+BitrateAllocator::BitrateAllocator(LimitObserver* limit_observer)
+    : limit_observer_(limit_observer),
+      bitrate_observer_configs_(),
       enforce_min_bitrate_(true),
       last_bitrate_bps_(kDefaultBitrateBps),
       last_non_zero_bitrate_bps_(kDefaultBitrateBps),
       last_fraction_loss_(0),
       last_rtt_(0) {}
 
-uint32_t BitrateAllocator::OnNetworkChanged(uint32_t bitrate,
-                                            uint8_t fraction_loss,
-                                            int64_t rtt) {
+void BitrateAllocator::OnNetworkChanged(uint32_t bitrate,
+                                        uint8_t fraction_loss,
+                                        int64_t rtt) {
   rtc::CritScope lock(&crit_sect_);
   last_bitrate_bps_ = bitrate;
   last_non_zero_bitrate_bps_ =
       bitrate > 0 ? bitrate : last_non_zero_bitrate_bps_;
   last_fraction_loss_ = fraction_loss;
   last_rtt_ = rtt;
 
-  uint32_t allocated_bitrate_bps = 0;
   ObserverAllocation allocation = AllocateBitrates(bitrate);
   for (const auto& kv : allocation) {
     kv.first->OnBitrateUpdated(kv.second, last_fraction_loss_, last_rtt_);
-    allocated_bitrate_bps += kv.second;
   }
-  return allocated_bitrate_bps;
 }
 
 int BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer,
                                   uint32_t min_bitrate_bps,
                                   uint32_t max_bitrate_bps,
+                                  uint32_t pad_up_bitrate_bps,
                                   bool enforce_min_bitrate) {
   rtc::CritScope lock(&crit_sect_);
   // TODO(mflodman): Enforce this per observer.
   EnforceMinBitrate(enforce_min_bitrate);
 
   auto it = FindObserverConfig(observer);
 
   // Allow the max bitrate to be exceeded for FEC and retransmissions.
   // TODO(holmer): We have to get rid of this hack as it makes it difficult to
   // properly allocate bitrate. The allocator should instead distribute any
   // extra bitrate after all streams have maxed out.
   max_bitrate_bps *= kTransmissionMaxBitrateMultiplier;
   if (it != bitrate_observer_configs_.end()) {
     // Update current configuration.
     it->min_bitrate_bps = min_bitrate_bps;
     it->max_bitrate_bps = max_bitrate_bps;
+    it->pad_up_bitrate_bps = pad_up_bitrate_bps;
   } else {
     // Add new settings.
-    bitrate_observer_configs_.push_back(ObserverConfig(
-        observer, min_bitrate_bps, max_bitrate_bps, enforce_min_bitrate));
+    bitrate_observer_configs_.push_back(
+        ObserverConfig(observer, min_bitrate_bps, max_bitrate_bps,
+                       pad_up_bitrate_bps, enforce_min_bitrate));
   }
 
   int new_observer_bitrate_bps = 0;
   if (last_bitrate_bps_ > 0) {  // We have a bitrate to allocate.
     ObserverAllocation allocation = AllocateBitrates(last_bitrate_bps_);
     for (auto& kv : allocation) {
       // Update all observers with the new allocation.
       kv.first->OnBitrateUpdated(kv.second, last_fraction_loss_, last_rtt_);
       if (kv.first == observer)
         new_observer_bitrate_bps = kv.second;
     }
   } else {
     // Currently, an encoder is not allowed to produce frames.
     // But we still have to return the initial config bitrate + let the
     // observer know that it can not produce frames.
     ObserverAllocation allocation =
         AllocateBitrates(last_non_zero_bitrate_bps_);
     observer->OnBitrateUpdated(0, last_fraction_loss_, last_rtt_);
     new_observer_bitrate_bps = allocation[observer];
   }
+
+  UpdateAllocationLimits();
+
   return new_observer_bitrate_bps;
 }
 
+void BitrateAllocator::NotifyObserverInactive(

[mflodman, 2016/06/08 10:18:31]

Can we in this case just remove the observer instead?

Or in another way, if it is inactive, wouldn't it be better to remove it from
the allocation?

[perkj_webrtc, 2016/06/08 15:35:27]

Seems like that is what stefan suggests too.

+    BitrateAllocatorObserver* observer,
+    bool inactive) {
+  rtc::CritScope lock(&crit_sect_);
+  auto iter = std::find_if(
+      bitrate_observer_configs_.begin(), bitrate_observer_configs_.end(),
+      [observer](const ObserverConfig& c) { return c.observer == observer; });
+  RTC_DCHECK(iter != bitrate_observer_configs_.end());
+  iter->active = !inactive;
+
+  UpdateAllocationLimits();
+}
+
+void BitrateAllocator::UpdateAllocationLimits() {
+  uint32_t total_requested_padding_bitrate = 0;
+  uint32_t total_requested_min_bitrate = 0;
+
+  for (const auto& config : bitrate_observer_configs_) {
+    if (enforce_min_bitrate_) {
+      total_requested_min_bitrate += config.min_bitrate_bps;
+    }
+    if (config.active)
+      total_requested_padding_bitrate += config.pad_up_bitrate_bps;
+  }
+  crit_sect_.Leave();

[stefan-webrtc, 2016/06/08 09:25:13]

I don't like this... Can we make sure we can OnAllocationLimitsChanged() after
the crit is no longer active? You should be able to scope the crit in
AddObserver() so that it is no longer active once you call
OnAllocationLimitsChanged at the end. Same for NotifyObserverInactive.

[perkj_webrtc, 2016/06/08 15:35:27]

Yes, but with code duplication. Do you have a suggestion?

+  limit_observer_->OnAllocationLimitsChanged(total_requested_min_bitrate,
+                                             total_requested_padding_bitrate);
+  crit_sect_.Enter();
+}
+
 void BitrateAllocator::RemoveObserver(BitrateAllocatorObserver* observer) {
   rtc::CritScope lock(&crit_sect_);
   auto it = FindObserverConfig(observer);
   if (it != bitrate_observer_configs_.end()) {
     bitrate_observer_configs_.erase(it);
   }
+  UpdateAllocationLimits();
 }
 
 void BitrateAllocator::EnforceMinBitrate(bool enforce_min_bitrate) {
   enforce_min_bitrate_ = enforce_min_bitrate;
 }
 
 BitrateAllocator::ObserverConfigList::iterator
 BitrateAllocator::FindObserverConfig(
     const BitrateAllocatorObserver* observer) {
   for (auto it = bitrate_observer_configs_.begin();
@@ skipping 82 matching lines @@
     for (const auto& observer_config : bitrate_observer_configs_) {
       uint32_t allocated_bitrate =
           std::min(remainder, observer_config.min_bitrate_bps);
       allocation[observer_config.observer] = allocated_bitrate;
       remainder -= allocated_bitrate;
     }
   }
   return allocation;
 }
 }  // namespace webrtc