Extract bitrate allocation of spatial/temporal layers out of codec impl.

webrtc/video/vie_encoder.cc

Index: webrtc/video/vie_encoder.cc
diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc
index 95a6f5a18c9f54a8139292fcfd05167cfdc8af3c..fd21364b83b427db1ab426f70ceb7f270c00e09f 100644
--- a/webrtc/video/vie_encoder.cc
+++ b/webrtc/video/vie_encoder.cc
@@ -19,8 +19,11 @@
 #include "webrtc/base/trace_event.h"
 #include "webrtc/base/timeutils.h"
 #include "webrtc/modules/pacing/paced_sender.h"
+#include "webrtc/modules/video_coding/codecs/vp8/screenshare_layers.h"
 #include "webrtc/modules/video_coding/include/video_coding.h"
 #include "webrtc/modules/video_coding/include/video_coding_defines.h"
+#include "webrtc/modules/video_coding/utility/default_video_bitrate_allocator.h"
+#include "webrtc/modules/video_coding/utility/simulcast_rate_allocator.h"
 #include "webrtc/video/overuse_frame_detector.h"
 #include "webrtc/video/send_statistics_proxy.h"
 #include "webrtc/video_frame.h"
@@ -186,6 +189,21 @@ CpuOveruseOptions GetCpuOveruseOptions(bool full_overuse_time) {
   return options;
 }
 
+struct ScreenshareTemporalLayersFactory : webrtc::TemporalLayersFactory {
+  ScreenshareTemporalLayersFactory() {}
+  virtual ~ScreenshareTemporalLayersFactory() {}
+
+  virtual webrtc::TemporalLayers* Create(int simulcast_id,
+                                         int num_temporal_layers,
+                                         uint8_t initial_tl0_pic_idx) const {
+    webrtc::TemporalLayers* tl = new webrtc::ScreenshareLayers(
+        num_temporal_layers, rand(), webrtc::Clock::GetRealTimeClock());
+    if (listener_)
+      listener_->OnTemporalLayersCreated(simulcast_id, tl);
+    return tl;
+  }
+};
+
 }  //  namespace
 
 class ViEEncoder::ConfigureEncoderTask : public rtc::QueuedTask {
@@ -351,6 +369,7 @@ void ViEEncoder::Stop() {
   source_proxy_->SetSource(nullptr);
   encoder_queue_.PostTask([this] {
     RTC_DCHECK_RUN_ON(&encoder_queue_);
+    rate_allocator_.reset();
     video_sender_.RegisterExternalEncoder(nullptr, settings_.payload_type,
                                           false);
     overuse_detector_.StopCheckForOveruse();
@@ -438,6 +457,31 @@ void ViEEncoder::ReconfigureEncoder() {
   codec.startBitrate = std::min(codec.startBitrate, codec.maxBitrate);
   codec.expect_encode_from_texture = last_frame_info_->is_texture;
 
+  switch (codec.codecType) {

[perkj_webrtc, 2016/10/21 08:24:29]

Can we try to keep ViEEncoder codec agnostic? 
Ie- create a static Create method in the BitrateAllocator that creates the
correct instance of a BitrateAllocator given the codecType.

[sprang_webrtc, 2016/10/25 10:44:25]

I created a separate factory class for this, since it couldn't place it directly
in the interface.

+    case kVideoCodecVP8: {
+      SimulcastRateAllocator* rate_allocator =
+          new SimulcastRateAllocator(codec);
+      rate_allocator_.reset(rate_allocator);

[perkj_webrtc, 2016/10/21 08:24:29]

rate_allocator_.reset(new SimulcastRateAllocator(codec));

[sprang_webrtc, 2016/10/25 10:44:25]

Done. This was to avoid a few static_cast when accessing simulcast-specific
fields, but since there's only one usage left of those I'll bring the cast back.

+
+      // Set up default VP8 temporal layer factory, if not provided.
+      if (!codec.codecSpecific.VP8.tl_factory) {
+        if (codec.mode == kScreensharing &&
+            codec.numberOfSimulcastStreams == 1 &&
+            codec.codecSpecific.VP8.numberOfTemporalLayers == 2) {
+          // Conference mode temporal layering for screen content.
+          vp8_tl_factory_.reset(new ScreenshareTemporalLayersFactory());
+        } else {
+          // Standard video temporal layers.
+          vp8_tl_factory_.reset(new TemporalLayersFactory());
+        }
+        codec.codecSpecific.VP8.tl_factory = vp8_tl_factory_.get();
+      }
+      vp8_tl_factory_->SetListener(rate_allocator);

[perkj_webrtc, 2016/10/21 08:24:29]

Can we hide this from ViEEncoder by hiding it in SimulcastRateAllocator? 
SimulcastRateAllocator can own and setup the tl_factory. 
You can add a method on the rate allocator that lets it modify VideoCodec.

[sprang_webrtc, 2016/10/25 10:44:25]

I can do it so it can optionally hold the tl factory, but they can still be
propagated from the outside so can't always rely on that.

+    } break;
+    default:
+      rate_allocator_.reset(new DefaultVideoBitrateAllocator(codec));
+  }
+
   bool success = video_sender_.RegisterSendCodec(
                      &codec, number_of_cores_,
                      static_cast<uint32_t>(max_data_payload_length_)) == VCM_OK;
@@ -446,17 +490,16 @@ void ViEEncoder::ReconfigureEncoder() {
     RTC_DCHECK(success);
   }
 
-  rate_allocator_.reset(new SimulcastRateAllocator(codec));
   if (stats_proxy_) {
-    stats_proxy_->OnEncoderReconfigured(encoder_config_,
-                                        rate_allocator_->GetPreferedBitrate());
+    int frame_rate = stats_proxy_->GetSendFrameRate();
+    if (frame_rate == 0)
+      frame_rate = codec.maxFramerate;

[perkj_webrtc, 2016/10/21 08:24:29]

codec.maxFramerate does not have a real meaning. It is just a fake hard coded
value.

It seems a little bit weird to require a  frame rate in order to get the
preferedbitrate. The frame rate is hard to know. 

[sprang_webrtc, 2016/10/25 10:44:25]

I know, this is just a placeholder until we have valid stats. 
The rate allocator wants the frame rate because if it is too low we may change
the temporal layer layout. With the current implementations that may not change
the preferred bitrate, but we will still update the registered TemporalLayers
instances though - so it can impact the encoder.

+    stats_proxy_->OnEncoderReconfigured(
+        encoder_config_, rate_allocator_->GetPreferedBitrate(frame_rate));
   }
 
   pending_encoder_reconfiguration_ = false;
-  if (stats_proxy_) {
-    stats_proxy_->OnEncoderReconfigured(encoder_config_,
-                                        rate_allocator_->GetPreferedBitrate());
-  }
+
   sink_->OnEncoderConfigurationChanged(
       std::move(streams), encoder_config_.min_transmit_bitrate_bps);
 }
@@ -679,7 +722,7 @@ void ViEEncoder::OnBitrateUpdated(uint32_t bitrate_bps,
                   << " rtt " << round_trip_time_ms;
 
   video_sender_.SetChannelParameters(bitrate_bps, fraction_lost,
-                                     round_trip_time_ms);
+                                     round_trip_time_ms, rate_allocator_.get());
 
   encoder_start_bitrate_bps_ =
       bitrate_bps != 0 ? bitrate_bps : encoder_start_bitrate_bps_;