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

webrtc/modules/video_coding/video_sender.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.
  */
 
 
 #include <algorithm>  // std::max
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/common_types.h"
+#include "webrtc/common_video/include/video_bitrate_allocator.h"

[stefan-webrtc, 2016/11/02 10:26:36]

Do you have to include both this one and default_video_bitrate_allocator.h?

[sprang_webrtc, 2016/11/02 13:28:33]

I don't have to, but I think this follows the "include what you use" rule. We
use both a raw VideoBitrateAllocator interface and the
DefaultVideoBitrateAllocator class, so I prefer not to include the interface
implicitly.

[stefan-webrtc, 2016/11/03 13:34:58]

Acknowledged.

 #include "webrtc/common_video/libyuv/include/webrtc_libyuv.h"
+#include "webrtc/modules/video_coding/codecs/vp8/temporal_layers.h"
 #include "webrtc/modules/video_coding/include/video_codec_interface.h"
 #include "webrtc/modules/video_coding/encoded_frame.h"
+#include "webrtc/modules/video_coding/utility/default_video_bitrate_allocator.h"
 #include "webrtc/modules/video_coding/utility/quality_scaler.h"
 #include "webrtc/modules/video_coding/video_coding_impl.h"
 #include "webrtc/system_wrappers/include/clock.h"
 
 namespace webrtc {
 namespace vcm {
 
 VideoSender::VideoSender(Clock* clock,
                          EncodedImageCallback* post_encode_callback,
                          VCMSendStatisticsCallback* send_stats_callback)
     : clock_(clock),
       _encoder(nullptr),
       _mediaOpt(clock_),
       _encodedFrameCallback(post_encode_callback, &_mediaOpt),
       send_stats_callback_(send_stats_callback),
       _codecDataBase(&_encodedFrameCallback),
       frame_dropper_enabled_(true),
       _sendStatsTimer(1000, clock_),
       current_codec_(),
-      encoder_params_({0, 0, 0, 0}),
+      encoder_params_({BitrateAllocation(), 0, 0, 0}),
       encoder_has_internal_source_(false),
       next_frame_types_(1, kVideoFrameDelta) {
   _mediaOpt.Reset();
   // Allow VideoSender to be created on one thread but used on another, post
   // construction. This is currently how this class is being used by at least
   // one external project (diffractor).
   sequenced_checker_.Detach();
 }
 
 VideoSender::~VideoSender() {}
@@ skipping 115 matching lines @@
 }
 
 // Get encode bitrate
 int VideoSender::Bitrate(unsigned int* bitrate) const {
   RTC_DCHECK(sequenced_checker_.CalledSequentially());
   // Since we're running on the thread that's the only thread known to modify
   // the value of _encoder, we don't need to grab the lock here.
 
   if (!_encoder)
     return VCM_UNINITIALIZED;
-  *bitrate = _encoder->GetEncoderParameters().target_bitrate;
+  *bitrate = _encoder->GetEncoderParameters().target_bitrate.get_sum_bps();
   return 0;
 }
 
 // Get encode frame rate
 int VideoSender::FrameRate(unsigned int* framerate) const {
   RTC_DCHECK(sequenced_checker_.CalledSequentially());
   // Since we're running on the thread that's the only thread known to modify
   // the value of _encoder, we don't need to grab the lock here.
 
   if (!_encoder)
     return VCM_UNINITIALIZED;
 
   *framerate = _encoder->GetEncoderParameters().input_frame_rate;
   return 0;
 }
 
-int32_t VideoSender::SetChannelParameters(uint32_t target_bitrate,
-                                          uint8_t lossRate,
-                                          int64_t rtt) {
-  uint32_t target_rate =
-      _mediaOpt.SetTargetRates(target_bitrate, lossRate, rtt);
+int32_t VideoSender::SetChannelParameters(
+    uint32_t target_bitrate_bps,
+    uint8_t lossRate,
+    int64_t rtt,
+    VideoBitrateAllocator* bitrate_allocator) {

[stefan-webrtc, 2016/11/02 10:26:36]

Does this change on every channel parameter update? Should it be set in the ctor
instead?

[sprang_webrtc, 2016/11/02 13:28:33]

The bitrate_allocator is owned by ViEEncoder, and may change on
ReconfigureEncoder(). I included it as a parameter here rather than set up an
explicit setter method and keeping a state.

[stefan-webrtc, 2016/11/03 13:34:58]

Probably good, or perhaps you should pass in a BitrateAllocation instead? And if
it's nullptr you create a default allocation?

[sprang_webrtc, 2016/11/03 14:36:59]

The reason I pass down the allocator reference is that it needs the
input_frame_rate as argument. That is calculated by _mediaOpt in VideoSender...
The default allocator path is just used in tests.

I hope we can clean this up if we merge VideoSender ViEEncoder.

[stefan-webrtc, 2016/11/03 15:32:08]

I don't know how, but I missed that.

+  uint32_t video_target_rate_bps =
+      _mediaOpt.SetTargetRates(target_bitrate_bps, lossRate, rtt);
+  uint32_t input_frame_rate = _mediaOpt.InputFrameRate();
+  BitrateAllocation bitrate_allocation;
+  if (bitrate_allocator) {
+    bitrate_allocation = bitrate_allocator->GetAllocation(video_target_rate_bps,
+                                                          input_frame_rate);
+  } else {
+    DefaultVideoBitrateAllocator default_allocator(current_codec_);
+    bitrate_allocation = default_allocator.GetAllocation(video_target_rate_bps,
+                                                         input_frame_rate);
+  }
 
-  uint32_t input_frame_rate = _mediaOpt.InputFrameRate();
-
-  EncoderParameters encoder_params = {target_rate, lossRate, rtt,
+  EncoderParameters encoder_params = {bitrate_allocation, lossRate, rtt,
                                       input_frame_rate};
   bool encoder_has_internal_source;
   {
     rtc::CritScope cs(&params_crit_);
     encoder_params_ = encoder_params;
     encoder_has_internal_source = encoder_has_internal_source_;
   }
 
   // For encoders with internal sources, we need to tell the encoder directly,
   // instead of waiting for an AddVideoFrame that will never come (internal
@@ skipping 10 matching lines @@
 
 void VideoSender::SetEncoderParameters(EncoderParameters params,
                                        bool has_internal_source) {
   // |target_bitrate == 0 | means that the network is down or the send pacer is
   // full. We currently only report this if the encoder has an internal source.
   // If the encoder does not have an internal source, higher levels are expected
   // to not call AddVideoFrame. We do this since its unclear how current
   // encoder implementations behave when given a zero target bitrate.
   // TODO(perkj): Make sure all known encoder implementations handle zero
   // target bitrate and remove this check.
-  if (!has_internal_source && params.target_bitrate == 0)
+  if (!has_internal_source && params.target_bitrate.get_sum_bps() == 0)
     return;
 
   if (params.input_frame_rate == 0) {
     // No frame rate estimate available, use default.
     params.input_frame_rate = current_codec_.maxFramerate;
   }
   if (_encoder != nullptr)
     _encoder->SetEncoderParameters(params);
 }
 
@@ skipping 19 matching lines @@
     encoder_params = encoder_params_;
     next_frame_types = next_frame_types_;
     encoder_has_internal_source = encoder_has_internal_source_;
   }
   rtc::CritScope lock(&encoder_crit_);
   if (_encoder == nullptr)
     return VCM_UNINITIALIZED;
   SetEncoderParameters(encoder_params, encoder_has_internal_source);
   if (_mediaOpt.DropFrame()) {
     LOG(LS_VERBOSE) << "Drop Frame "
-                    << "target bitrate " << encoder_params.target_bitrate
+                    << "target bitrate "
+                    << encoder_params.target_bitrate.get_sum_bps()
                     << " loss rate " << encoder_params.loss_rate << " rtt "
                     << encoder_params.rtt << " input frame rate "
                     << encoder_params.input_frame_rate;
     _encoder->OnDroppedFrame();
     return VCM_OK;
   }
   // TODO(pbos): Make sure setting send codec is synchronized with video
   // processing so frame size always matches.
   if (!_codecDataBase.MatchesCurrentResolution(videoFrame.width(),
                                                videoFrame.height())) {
@@ skipping 68 matching lines @@
 }
 
 int32_t VideoSender::EnableFrameDropper(bool enable) {
   rtc::CritScope lock(&encoder_crit_);
   frame_dropper_enabled_ = enable;
   _mediaOpt.EnableFrameDropper(enable);
   return VCM_OK;
 }
 }  // namespace vcm
 }  // namespace webrtc