Refactor EncodeParameters usage, remove unused rtt/loss

Bug: webrtc:10126
Change-Id: Ib93f5e65b25540576c026197f72a5902cf43fc16
Reviewed-on: https://webrtc-review.googlesource.com/c/114281
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26001}

modules/video_coding/BUILD.gn

@@ -173,6 +173,7 @@ rtc_static_library("video_coding") {
     "..:module_api_public",
     "../..:webrtc_common",
     "../../api:fec_controller_api",
+    "../../api/units:data_rate",
     "../../api/video:builtin_video_bitrate_allocator_factory",
     "../../api/video:encoded_frame",
     "../../api/video:video_bitrate_allocator",

modules/video_coding/generic_encoder.cc

@@ -46,7 +46,6 @@ VCMGenericEncoder::VCMGenericEncoder(
     : encoder_(encoder),
       vcm_encoded_frame_callback_(encoded_frame_callback),
       internal_source_(internal_source),
-      encoder_params_({VideoBitrateAllocation(), 0, 0, 0}),
       streams_or_svc_num_(0),
       codec_type_(VideoCodecType::kVideoCodecGeneric) {}
 

modules/video_coding/generic_encoder.h

@@ -15,9 +15,9 @@
 #include <list>
 #include <vector>
 
+#include "api/units/data_rate.h"
 #include "modules/video_coding/include/video_codec_interface.h"
 #include "modules/video_coding/include/video_coding_defines.h"
-
 #include "rtc_base/criticalsection.h"
 #include "rtc_base/race_checker.h"
 
@@ -28,9 +28,21 @@ class MediaOptimization;
 }  // namespace media_optimization
 
 struct EncoderParameters {
+  EncoderParameters() : total_bitrate(DataRate::Zero()), input_frame_rate(0) {}
+  EncoderParameters(DataRate total_bitrate,
+                    const VideoBitrateAllocation& allocation,
+                    uint32_t framerate)
+      : total_bitrate(total_bitrate),
+        target_bitrate(allocation),
+        input_frame_rate(framerate) {}
+
+  // Total bitrate allocated for this encoder.
+  DataRate total_bitrate;
+  // The bitrate allocation, across spatial and/or temporal layers. Note that
+  // the sum of these might be less than |total_bitrate| if the allocator has
+  // capped the bitrate for some configuration.
   VideoBitrateAllocation target_bitrate;
-  uint8_t loss_rate;
+  // The input frame rate to the encoder in fps, measured in the video sender.
-  int64_t rtt;
   uint32_t input_frame_rate;
 };
 

modules/video_coding/video_coding_impl.cc

@@ -121,8 +121,8 @@ class VideoCodingModuleImpl : public VideoCodingModule {
   int32_t SetChannelParameters(uint32_t target_bitrate,  // bits/s.
                                uint8_t lossRate,
                                int64_t rtt) override {
-    return sender_.SetChannelParameters(target_bitrate, lossRate, rtt,
+    return sender_.SetChannelParameters(target_bitrate, rate_allocator_.get(),
-                                        rate_allocator_.get(), nullptr);
+                                        nullptr);
   }
 
   int32_t SetVideoProtection(VCMVideoProtection videoProtection,

modules/video_coding/video_coding_impl.h

@@ -79,8 +79,6 @@ class VideoSender {
   // cause an immediate call to VideoEncoder::SetRateAllocation().
   int32_t SetChannelParameters(
       uint32_t target_bitrate_bps,
-      uint8_t loss_rate,
-      int64_t rtt,
       VideoBitrateAllocator* bitrate_allocator,
       VideoBitrateAllocationObserver* bitrate_updated_callback);
 
@@ -107,6 +105,10 @@ class VideoSender {
       uint32_t target_bitrate_bps);
   void SetEncoderParameters(EncoderParameters params, bool has_internal_source)
       RTC_EXCLUSIVE_LOCKS_REQUIRED(encoder_crit_);
+  VideoBitrateAllocation GetAllocation(
+      uint32_t bitrate_bps,
+      uint32_t framerate_fps,
+      VideoBitrateAllocator* bitrate_allocator) const;
 
   rtc::CriticalSection encoder_crit_;
   VCMGenericEncoder* _encoder;

modules/video_coding/video_sender.cc

@@ -55,7 +55,6 @@ VideoSender::VideoSender(Clock* clock,
       frame_dropper_requested_(true),
       force_disable_frame_dropper_(false),
       current_codec_(),
-      encoder_params_({VideoBitrateAllocation(), 0, 0, 0}),
       encoder_has_internal_source_(false),
       next_frame_types_(1, kVideoFrameDelta) {
   // Allow VideoSender to be created on one thread but used on another, post
@@ -167,22 +166,31 @@ EncoderParameters VideoSender::UpdateEncoderParameters(
   if (input_frame_rate == 0)
     input_frame_rate = current_codec_.maxFramerate;
 
+  EncoderParameters new_encoder_params = {
+      DataRate::bps(target_bitrate_bps),
+      GetAllocation(video_target_rate_bps, input_frame_rate, bitrate_allocator),
+      input_frame_rate};
+  return new_encoder_params;
+}
+
+VideoBitrateAllocation VideoSender::GetAllocation(
+    uint32_t bitrate_bps,
+    uint32_t framerate_fps,
+    VideoBitrateAllocator* bitrate_allocator) const {
   VideoBitrateAllocation bitrate_allocation;
   // Only call allocators if bitrate > 0 (ie, not suspended), otherwise they
   // might cap the bitrate to the min bitrate configured.
-  if (target_bitrate_bps > 0) {
+  if (bitrate_bps > 0) {
     if (bitrate_allocator) {
-      bitrate_allocation = bitrate_allocator->GetAllocation(
+      bitrate_allocation =
-          video_target_rate_bps, input_frame_rate);
+          bitrate_allocator->GetAllocation(bitrate_bps, framerate_fps);
     } else {
       DefaultVideoBitrateAllocator default_allocator(current_codec_);
-      bitrate_allocation = default_allocator.GetAllocation(
+      bitrate_allocation =
-          video_target_rate_bps, input_frame_rate);
+          default_allocator.GetAllocation(bitrate_bps, framerate_fps);
     }
   }
-  EncoderParameters new_encoder_params = {bitrate_allocation, params.loss_rate,
+  return bitrate_allocation;
-                                          params.rtt, input_frame_rate};
-  return new_encoder_params;
 }
 
 void VideoSender::UpdateChannelParameters(
@@ -193,22 +201,19 @@ void VideoSender::UpdateChannelParameters(
     rtc::CritScope cs(&params_crit_);
     encoder_params_ =
         UpdateEncoderParameters(encoder_params_, bitrate_allocator,
-                                encoder_params_.target_bitrate.get_sum_bps());
+                                encoder_params_.total_bitrate.bps());
     target_rate = encoder_params_.target_bitrate;
   }
-  if (bitrate_updated_callback && target_rate.get_sum_bps() > 0)
+  if (bitrate_updated_callback && target_rate.get_sum_bps() > 0) {
     bitrate_updated_callback->OnBitrateAllocationUpdated(target_rate);
+  }
 }
 
 int32_t VideoSender::SetChannelParameters(
     uint32_t target_bitrate_bps,
-    uint8_t loss_rate,
-    int64_t rtt,
     VideoBitrateAllocator* bitrate_allocator,
     VideoBitrateAllocationObserver* bitrate_updated_callback) {
   EncoderParameters encoder_params;
-  encoder_params.loss_rate = loss_rate;
-  encoder_params.rtt = rtt;
   encoder_params = UpdateEncoderParameters(encoder_params, bitrate_allocator,
                                            target_bitrate_bps);
   if (bitrate_updated_callback && target_bitrate_bps > 0) {
@@ -286,11 +291,10 @@ int32_t VideoSender::AddVideoFrame(
   _mediaOpt.EnableFrameDropper(frame_dropping_enabled);
 
   if (_mediaOpt.DropFrame()) {
-    RTC_LOG(LS_VERBOSE) << "Drop Frame "
+    RTC_LOG(LS_VERBOSE) << "Drop Frame: "
                         << "target bitrate "
                         << encoder_params.target_bitrate.get_sum_bps()
-                        << " loss rate " << encoder_params.loss_rate << " rtt "
+                        << ", input frame rate "
-                        << encoder_params.rtt << " input frame rate "
                         << encoder_params.input_frame_rate;
     post_encode_callback_->OnDroppedFrame(
         EncodedImageCallback::DropReason::kDroppedByMediaOptimizations);

modules/video_coding/video_sender_unittest.cc

@@ -314,14 +314,14 @@ TEST_F(TestVideoSenderWithMockEncoder, TestSetRate) {
               SetRateAllocation(new_rate_allocation, settings_.maxFramerate))
       .Times(1)
       .WillOnce(Return(0));
-  sender_->SetChannelParameters(new_bitrate_kbps * 1000, 0, 200,
+  sender_->SetChannelParameters(new_bitrate_kbps * 1000, rate_allocator_.get(),
-                                rate_allocator_.get(), nullptr);
+                                nullptr);
   AddFrame();
   clock_.AdvanceTimeMilliseconds(kFrameIntervalMs);
 
   // Expect no call to encoder_.SetRates if the new bitrate is zero.
   EXPECT_CALL(encoder_, SetRateAllocation(_, _)).Times(0);
-  sender_->SetChannelParameters(0, 0, 200, rate_allocator_.get(), nullptr);
+  sender_->SetChannelParameters(0, rate_allocator_.get(), nullptr);
   AddFrame();
 }
 
@@ -358,21 +358,19 @@ TEST_F(TestVideoSenderWithMockEncoder, TestEncoderParametersForInternalSource) {
   EXPECT_CALL(encoder_, SetRateAllocation(new_rate_allocation, _))
       .Times(1)
       .WillOnce(Return(0));
-  sender_->SetChannelParameters(new_bitrate_kbps * 1000, 0, 200,
+  sender_->SetChannelParameters(new_bitrate_kbps * 1000, rate_allocator_.get(),
-                                rate_allocator_.get(), nullptr);
+                                nullptr);
 }
 
 TEST_F(TestVideoSenderWithMockEncoder,
        NoRedundantSetChannelParameterOrSetRatesCalls) {
-  const uint8_t kLossRate = 4;
-  const uint8_t kRtt = 200;
   const int64_t kRateStatsWindowMs = 2000;
   const uint32_t kInputFps = 20;
   int64_t start_time = clock_.TimeInMilliseconds();
   // Expect initial call to SetChannelParameters. Rates are initialized through
   // InitEncode and expects no additional call before the framerate (or bitrate)
   // updates.
-  sender_->SetChannelParameters(settings_.startBitrate * 1000, kLossRate, kRtt,
+  sender_->SetChannelParameters(settings_.startBitrate * 1000,
                                 rate_allocator_.get(), nullptr);
   while (clock_.TimeInMilliseconds() < start_time + kRateStatsWindowMs) {
     AddFrame();
@@ -387,8 +385,8 @@ TEST_F(TestVideoSenderWithMockEncoder,
   EXPECT_CALL(encoder_, SetRateAllocation(new_rate_allocation, kInputFps))
       .Times(1)
       .WillOnce(Return(0));
-  sender_->SetChannelParameters(new_bitrate_bps, kLossRate, kRtt,
+  sender_->SetChannelParameters(new_bitrate_bps, rate_allocator_.get(),
-                                rate_allocator_.get(), nullptr);
+                                nullptr);
   AddFrame();
 }
 
@@ -440,7 +438,7 @@ class TestVideoSenderWithVp8 : public TestVideoSender {
       // buffer since it will fail to calculate the framerate.
       if (i != 0) {
         EXPECT_EQ(VCM_OK, sender_->SetChannelParameters(
-                              available_bitrate_kbps_ * 1000, 0, 200,
+                              available_bitrate_kbps_ * 1000,
                               rate_allocator_.get(), nullptr));
       }
     }

video/video_stream_encoder.cc

@@ -1003,8 +1003,7 @@ void VideoStreamEncoder::OnBitrateUpdated(uint32_t bitrate_bps,
     has_seen_first_significant_bwe_change_ = true;
   }
 
-  video_sender_.SetChannelParameters(bitrate_bps, fraction_lost,
+  video_sender_.SetChannelParameters(bitrate_bps, rate_allocator_.get(),
-                                     round_trip_time_ms, rate_allocator_.get(),
                                      bitrate_observer_);
 
   encoder_start_bitrate_bps_ =