Fix perf regression in screenshare temporal layer bitrate allocation

webrtc/modules/video_coding/utility/simulcast_rate_allocator.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 84 matching lines @@
        ++simulcast_id) {
     auto tl_it = temporal_layers_.find(simulcast_id);
     if (tl_it == temporal_layers_.end())
       continue;  // TODO(sprang): If > 1 SS, assume default TL alloc?
 
     uint32_t target_bitrate_kbps =
         allocated_bitrates_bps.GetBitrate(simulcast_id, 0) / 1000;
     RTC_DCHECK_EQ(
         target_bitrate_kbps,
         allocated_bitrates_bps.GetSpatialLayerSum(simulcast_id) / 1000);
+    const int num_temporal_streams = std::max<uint8_t>(
+        1, codec_.numberOfSimulcastStreams == 0
+               ? codec_.VP8().numberOfTemporalLayers
+               : codec_.simulcastStream[0].numberOfTemporalLayers);
+
     uint32_t max_bitrate_kbps;
     if (num_spatial_streams == 1) {
       max_bitrate_kbps = codec_.maxBitrate;
-      const int num_temporal_streams =
-          codec_.numberOfSimulcastStreams == 0
-              ? codec_.VP8().numberOfTemporalLayers
-              : codec_.simulcastStream[0].numberOfTemporalLayers;
 
       // TODO(holmer): This is a temporary hack for screensharing, where we
       // interpret the startBitrate as the encoder target bitrate. This is
       // to allow for a different max bitrate, so if the codec can't meet
       // the target we still allow it to overshoot up to the max before dropping
       // frames. This hack should be improved.
       if (codec_.mode == kScreensharing && codec_.targetBitrate > 0 &&
           num_temporal_streams == 2) {
         int tl0_bitrate = std::min(codec_.targetBitrate, target_bitrate_kbps);
         max_bitrate_kbps = std::min(codec_.maxBitrate, target_bitrate_kbps);
         target_bitrate_kbps = tl0_bitrate;
       }
     } else {
       max_bitrate_kbps = codec_.simulcastStream[simulcast_id].maxBitrate;
     }
 
     std::vector<uint32_t> tl_allocation = tl_it->second->OnRatesUpdated(
         target_bitrate_kbps, max_bitrate_kbps, framerate);
+    RTC_DCHECK_GT(tl_allocation.size(), 0);
+    RTC_DCHECK_LE(tl_allocation.size(), num_temporal_streams);
 
+    uint64_t tl_allocation_sum_kbps = 0;
     for (size_t tl_index = 0; tl_index < tl_allocation.size(); ++tl_index) {
+      uint32_t layer_rate_kbps = tl_allocation[tl_index];
+      RTC_DCHECK_LE(tl_allocation_sum_kbps, target_bitrate_kbps);

[danilchap, 2016/11/25 14:40:31]

may be move this check after the loop, or at least after you increment
tl_allocation_sum_kbps. (right now it doesn't check sum of layers, only  all
layers except last one)

[sprang_webrtc, 2016/11/25 14:46:36]

Done.

       allocated_bitrates_bps.SetBitrate(simulcast_id, tl_index,
-                                        tl_allocation[tl_index] * 1000);
+                                        layer_rate_kbps * 1000);
+      tl_allocation_sum_kbps += layer_rate_kbps;
     }
   }
 
   return allocated_bitrates_bps;
 }
 
 uint32_t SimulcastRateAllocator::GetPreferredBitrateBps(uint32_t framerate) {
+  // Create a temporary instance without temporal layers, as they may be
+  // stateful, and updating the bitrate to max here can cause side effects.
+  SimulcastRateAllocator temp_allocator(codec_, nullptr);
   BitrateAllocation allocation =
-      GetAllocation(codec_.maxBitrate * 1000, framerate);
+      temp_allocator.GetAllocation(codec_.maxBitrate * 1000, framerate);
   return allocation.get_sum_bps();
 }
 
 const VideoCodec& webrtc::SimulcastRateAllocator::GetCodec() const {
   return codec_;
 }
 
 }  // namespace webrtc