Refactor how padding is calculated.

webrtc/video/video_send_stream.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 326 matching lines @@
   }
   if (video_codec.maxBitrate < kEncoderMinBitrateKbps)
     video_codec.maxBitrate = kEncoderMinBitrateKbps;
 
   RTC_DCHECK_GT(streams[0].max_framerate, 0);
   video_codec.maxFramerate = streams[0].max_framerate;
 
   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 {
 VideoSendStream::VideoSendStream(
     int num_cpu_cores,
     ProcessThread* module_process_thread,
     CallStats* call_stats,
     CongestionController* congestion_controller,
     BitrateAllocator* bitrate_allocator,
     SendDelayStats* send_delay_stats,
@@ skipping 172 matching lines @@
   return false;
 }
 
 void VideoSendStream::EncoderProcess() {
   RTC_CHECK_EQ(0, vie_encoder_.RegisterExternalEncoder(
                       config_.encoder_settings.encoder,
                       config_.encoder_settings.payload_type,
                       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;
     {
       rtc::CritScope lock(&encoder_settings_crit_);
       if (pending_encoder_settings_) {
         encoder_settings = pending_encoder_settings_;
         pending_encoder_settings_ = rtc::Optional<EncoderSettings>();
       }
     }
     if (encoder_settings) {
       encoder_settings->video_codec.startBitrate =
           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.
       if (config_.suspend_below_min_bitrate)
         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(
           encoder_settings->video_codec.startBitrate * 1000,
           encoder_settings->video_codec.width,
           encoder_settings->video_codec.height,
           encoder_settings->video_codec.maxFramerate, number_of_temporal_layers,
           payload_router_.MaxPayloadLength());
 
       // We might've gotten new settings while configuring the encoder settings,
       // restart from the top to see if that's the case before trying to encode
       // a frame (which might correspond to the last frame size).
       encoder_wakeup_event_.Set();
       continue;
     }
 
     VideoFrame frame;
     if (input_.GetVideoFrame(&frame)) {
       // TODO(perkj): |pre_encode_callback| is only used by tests. Tests should
       // register as a sink to the VideoSource instead.
       if (config_.pre_encode_callback) {
         config_.pre_encode_callback->OnFrame(frame);
       }
       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);
 }
 
 void VideoSendStream::ReconfigureVideoEncoder(
     const VideoEncoderConfig& config) {
   TRACE_EVENT0("webrtc", "VideoSendStream::(Re)configureVideoEncoder");
   LOG(LS_INFO) << "(Re)configureVideoEncoder: " << config.ToString();
   RTC_DCHECK_GE(config_.rtp.ssrcs.size(), config.streams.size());
   VideoCodec video_codec = VideoEncoderConfigToVideoCodec(
       config, config_.encoder_settings.payload_name,
       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();
 }
 
 VideoSendStream::Stats VideoSendStream::GetStats() {
   return stats_proxy_.GetStats();
 }
 
 void VideoSendStream::OveruseDetected() {
   if (config_.overuse_callback)
@@ skipping 34 matching lines @@
     }
     if (file_writer) {
       bool ok = file_writer->WriteFrame(encoded_image);
       RTC_DCHECK(ok);
     }
   }
 
   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;
   bool enable_protection_fec = config_.rtp.fec.ulpfec_payload_type != -1;
   // Payload types without picture ID cannot determine that a stream is complete
   // without retransmitting FEC, so using FEC + NACK for H.264 (for instance) is
   // a waste of bandwidth since FEC packets still have to be transmitted. Note
   // that this is not the case with FLEXFEC.
   if (enable_protection_nack &&
       !PayloadTypeSupportsSkippingFecPackets(
@@ skipping 104 matching lines @@
   return rtp_states;
 }
 
 void VideoSendStream::SignalNetworkState(NetworkState state) {
   for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
     rtp_rtcp->SetRTCPStatus(state == kNetworkUp ? config_.rtp.rtcp_mode
                                                 : RtcpMode::kOff);
   }
 }
 
-int VideoSendStream::GetPaddingNeededBps() const {
-  return vie_encoder_.GetPaddingNeededBps();
-}
-
 void VideoSendStream::OnBitrateUpdated(uint32_t bitrate_bps,
                                        uint8_t fraction_loss,
                                        int64_t rtt) {
   payload_router_.SetTargetSendBitrate(bitrate_bps);
   // Get the encoder target rate. It is the estimated network rate -
   // protection overhead.
   uint32_t encoder_target_rate = protection_bitrate_calculator_.SetTargetRates(
       bitrate_bps, stats_proxy_.GetSendFrameRate(), fraction_loss, rtt);
 
   vie_encoder_.OnBitrateUpdated(encoder_target_rate, fraction_loss, rtt);
@@ skipping 17 matching lines @@
                           &module_nack_rate);
     *sent_video_rate_bps += module_video_rate;
     *sent_nack_rate_bps += module_nack_rate;
     *sent_fec_rate_bps += module_fec_rate;
   }
   return 0;
 }
 
 }  // namespace internal
 }  // namespace webrtc