Refactor how padding is calculated.

webrtc/video/video_send_stream.cc

Index: webrtc/video/video_send_stream.cc
diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc
index d1811d2ced341e471ec72a1c622caee1a4254a32..6eb6b3633285162f6fd3aa0639294f16b909ed93 100644
--- a/webrtc/video/video_send_stream.cc
+++ b/webrtc/video/video_send_stream.cc
@@ -344,6 +344,28 @@ VideoCodec VideoEncoderConfigToVideoCodec(const VideoEncoderConfig& config,
   return video_codec;
 }
 
+int CalulcateMaxPadBitrateBps(const VideoEncoderConfig& config,
+                              bool pad_to_min_bitrate) {
+  int pad_up_to_bitrate_bps = 0;
+  // Calulate max padding bitrate for a multi layer codec.

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

Calculate

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

Done.

+  if (config.streams.size() > 1) {
+    // Pad to min bitrate of the highest layer.
+    pad_up_to_bitrate_bps =
+        config.streams[config.streams.size() - 1].min_bitrate_bps;
+    // + target_bitrate_bps of the lower layers.

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

Add instead of +.

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

Done.

+    for (size_t i = 0; i < config.streams.size() - 1; ++i) {

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

Remove {}

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

Done.

+      pad_up_to_bitrate_bps += config.streams[i].target_bitrate_bps;
+    }
+  } else if (pad_to_min_bitrate) {
+    pad_up_to_bitrate_bps = config.streams[0].min_bitrate_bps;
+  }
+
+  pad_up_to_bitrate_bps =
+      std::max(pad_up_to_bitrate_bps, config.min_transmit_bitrate_bps);
+
+  return pad_up_to_bitrate_bps;
+}
+
 }  // namespace
 
 namespace internal {
@@ -536,7 +558,8 @@ void VideoSendStream::EncoderProcess() {
                       config_.encoder_settings.internal_source));
 
   while (true) {
-    encoder_wakeup_event_.Wait(rtc::Event::kForever);
+    const int kEncodeCheckForActiviyPeriodMs = 1000;

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

Activity

And please add a comment why we have to wake up periodically.

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

Done.

+    encoder_wakeup_event_.Wait(kEncodeCheckForActiviyPeriodMs);
     if (rtc::AtomicOps::AcquireLoad(&stop_encoder_thread_))
       break;
     rtc::Optional<EncoderSettings> encoder_settings;
@@ -552,12 +575,13 @@ void VideoSendStream::EncoderProcess() {
           bitrate_allocator_->AddObserver(
               this, encoder_settings->video_codec.minBitrate * 1000,
               encoder_settings->video_codec.maxBitrate * 1000,
+              CalulcateMaxPadBitrateBps(encoder_settings->config,
+                                        config_.suspend_below_min_bitrate),
               !config_.suspend_below_min_bitrate) /
           1000;
 
-      payload_router_.SetSendStreams(encoder_settings->streams);
+      payload_router_.SetSendStreams(encoder_settings->config.streams);
       vie_encoder_.SetEncoder(encoder_settings->video_codec,
-                              encoder_settings->min_transmit_bitrate_bps,
                               payload_router_.MaxPayloadLength());
 
       // vie_encoder_.SetEncoder must be called before this.
@@ -565,13 +589,13 @@ void VideoSendStream::EncoderProcess() {
         video_sender_->SuspendBelowMinBitrate();
 
       // Clear stats for disabled layers.
-      for (size_t i = encoder_settings->streams.size();
+      for (size_t i = encoder_settings->config.streams.size();
            i < config_.rtp.ssrcs.size(); ++i) {
         stats_proxy_.OnInactiveSsrc(config_.rtp.ssrcs[i]);
       }
 
       size_t number_of_temporal_layers =
-          encoder_settings->streams.back()
+          encoder_settings->config.streams.back()
               .temporal_layer_thresholds_bps.size() +
           1;
       protection_bitrate_calculator_.SetEncodingData(
@@ -597,6 +621,10 @@ void VideoSendStream::EncoderProcess() {
       }
       vie_encoder_.EncodeVideoFrame(frame);
     }
+
+    // TODO(perkj): This is temporary. Activity should be notified by ViEncoder

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

ViEEncoder

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

Acknowledged.

+    // once it owns its own thread.
+    vie_encoder_.CheckForActivity();

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

If that is done, can we also update the congestion controller from here rather
than from the allocator? The CC API would then be to add min bitrate and padding
needed per stream rather than once for all. Doing that, we don't need to add the
padding concept at all to the allocator.

WDYT?

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

Can this method return true / false instead and we remove the observer in case
it's inactive? That way we don't need the sink implementation and we can avoid
adding the active/inactive state in the allocator.

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

I don't think I can call the congestion controller directly. Its only the
BitrateAllocator that can sum all observers requested padding rates. - the
CongestionController does not know about the streams. Call however, do know
about the streams so it can potentially sum if we really want. But I don't think
it make sense to add Call* as a dependency to VideoSendStream as well.

[mflodman, 2016/06/13 05:02:43]

My intention was to let the CongestionController do the addition of bitrates and
the send stream update only its own min and pad bitrates at the same time as an
observer is registered / deregistered. This might still be better as you've done
it though. Let's just discuss for one minute to make sure we understand each
other and agree.

   }
   vie_encoder_.DeRegisterExternalEncoder(config_.encoder_settings.payload_type);
 }
@@ -611,8 +639,8 @@ void VideoSendStream::ReconfigureVideoEncoder(
       config_.encoder_settings.payload_type);
   {
     rtc::CritScope lock(&encoder_settings_crit_);
-    pending_encoder_settings_ = rtc::Optional<EncoderSettings>(
-        {video_codec, config.min_transmit_bitrate_bps, config.streams});
+    pending_encoder_settings_ =
+        rtc::Optional<EncoderSettings>({video_codec, config});
   }
   encoder_wakeup_event_.Set();
 }
@@ -667,6 +695,12 @@ int32_t VideoSendStream::Encoded(const EncodedImage& encoded_image,
   return return_value;
 }
 
+void VideoSendStream::OnEncoderActivityChanged(bool active) {
+  // Notify the |bitrate_allocator_| if the encoder have timed out so it can
+  // stop sending padding if applicable.
+  bitrate_allocator_->NotifyObserverInactive(this, !active);
+}
+
 void VideoSendStream::ConfigureProtection() {
   // Enable NACK, FEC or both.
   const bool enable_protection_nack = config_.rtp.nack.rtp_history_ms > 0;
@@ -791,10 +825,6 @@ void VideoSendStream::SignalNetworkState(NetworkState state) {
   }
 }
 
-int VideoSendStream::GetPaddingNeededBps() const {
-  return vie_encoder_.GetPaddingNeededBps();
-}
-
 void VideoSendStream::OnBitrateUpdated(uint32_t bitrate_bps,
                                        uint8_t fraction_loss,
                                        int64_t rtt) {