Reland Refactor reporting of VideoBitrateAllocation

Original description
 Move reporting of target bitrate to just after the encoder has been
 updated. Originall submitted as refs/heads/master@{#32275}

Patch 1 contains the original cl
,patch 2 the fix to send rtcp even if BWE does not change.

Bug: webrtc:12000
Change-Id: I16766e08229fe1f6f65f449e0e074bed03338693
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/186948
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32340}

api/video_codecs/video_encoder.h

@@ -267,6 +267,9 @@ class RTC_EXPORT VideoEncoder {
 
     // Target bitrate, per spatial/temporal layer.
     // A target bitrate of 0bps indicates a layer should not be encoded at all.
+    VideoBitrateAllocation target_bitrate;
+    // Adjusted target bitrate, per spatial/temporal layer. May be lower or
+    // higher than the target depending on encoder behaviour.
     VideoBitrateAllocation bitrate;
     // Target framerate, in fps. A value <= 0.0 is invalid and should be
     // interpreted as framerate target not available. In this case the encoder

video/video_stream_encoder.cc

@@ -874,7 +874,7 @@ void VideoStreamEncoder::ReconfigureEncoder() {
     last_encoder_rate_settings_.reset();
     rate_settings.rate_control.framerate_fps = GetInputFramerateFps();
 
-    SetEncoderRates(UpdateBitrateAllocationAndNotifyObserver(rate_settings));
+    SetEncoderRates(UpdateBitrateAllocation(rate_settings));
   }
 
   encoder_stats_observer_->OnEncoderReconfigured(encoder_config_, streams);
@@ -1053,7 +1053,7 @@ void VideoStreamEncoder::TraceFrameDropEnd() {
 }
 
 VideoStreamEncoder::EncoderRateSettings
-VideoStreamEncoder::UpdateBitrateAllocationAndNotifyObserver(
+VideoStreamEncoder::UpdateBitrateAllocation(
     const EncoderRateSettings& rate_settings) {
   VideoBitrateAllocation new_allocation;
   // Only call allocators if bitrate > 0 (ie, not suspended), otherwise they
@@ -1064,24 +1064,8 @@ VideoStreamEncoder::UpdateBitrateAllocationAndNotifyObserver(
         rate_settings.rate_control.framerate_fps));
   }
 
-  if (bitrate_observer_ && new_allocation.get_sum_bps() > 0) {
-    if (encoder_ && encoder_initialized_) {
-      // Avoid too old encoder_info_.
-      const int64_t kMaxDiffMs = 100;
-      const bool updated_recently =
-          (last_encode_info_ms_ && ((clock_->TimeInMilliseconds() -
-                                     *last_encode_info_ms_) < kMaxDiffMs));
-      // Update allocation according to info from encoder.
-      bitrate_observer_->OnBitrateAllocationUpdated(
-          UpdateAllocationFromEncoderInfo(
-              new_allocation,
-              updated_recently ? encoder_info_ : encoder_->GetEncoderInfo()));
-    } else {
-      bitrate_observer_->OnBitrateAllocationUpdated(new_allocation);
-    }
-  }
-
   EncoderRateSettings new_rate_settings = rate_settings;
+  new_rate_settings.rate_control.target_bitrate = new_allocation;
   new_rate_settings.rate_control.bitrate = new_allocation;
   // VideoBitrateAllocator subclasses may allocate a bitrate higher than the
   // target in order to sustain the min bitrate of the video codec. In this
@@ -1102,9 +1086,6 @@ VideoStreamEncoder::UpdateBitrateAllocationAndNotifyObserver(
     new_rate_settings.rate_control.bitrate = adjusted_allocation;
   }
 
-  encoder_stats_observer_->OnBitrateAllocationUpdated(
-      send_codec_, new_rate_settings.rate_control.bitrate);
-
   return new_rate_settings;
 }
 
@@ -1147,11 +1128,22 @@ void VideoStreamEncoder::SetEncoderRates(
 
   if (rate_control_changed) {
     encoder_->SetRates(rate_settings.rate_control);
+
+    encoder_stats_observer_->OnBitrateAllocationUpdated(
+        send_codec_, rate_settings.rate_control.bitrate);
     frame_encode_metadata_writer_.OnSetRates(
         rate_settings.rate_control.bitrate,
         static_cast<uint32_t>(rate_settings.rate_control.framerate_fps + 0.5));
     stream_resource_manager_.SetEncoderRates(rate_settings.rate_control);
   }
+  if (bitrate_observer_) {
+    bitrate_observer_->OnBitrateAllocationUpdated(
+        // Update allocation according to info from encoder. An encoder may
+        // choose to not use all layers due to for example HW.
+        UpdateAllocationFromEncoderInfo(
+            rate_settings.rate_control.target_bitrate,
+            encoder_->GetEncoderInfo()));
+  }
 }
 
 void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame,
@@ -1202,8 +1194,7 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame,
       EncoderRateSettings new_rate_settings = *last_encoder_rate_settings_;
       new_rate_settings.rate_control.framerate_fps =
           static_cast<double>(framerate_fps);
-      SetEncoderRates(
-          UpdateBitrateAllocationAndNotifyObserver(new_rate_settings));
+      SetEncoderRates(UpdateBitrateAllocation(new_rate_settings));
     }
     last_parameters_update_ms_.emplace(now_ms);
   }
@@ -1738,7 +1729,7 @@ void VideoStreamEncoder::OnBitrateUpdated(DataRate target_bitrate,
   EncoderRateSettings new_rate_settings{
       VideoBitrateAllocation(), static_cast<double>(framerate_fps),
       link_allocation, target_bitrate, stable_target_bitrate};
-  SetEncoderRates(UpdateBitrateAllocationAndNotifyObserver(new_rate_settings));
+  SetEncoderRates(UpdateBitrateAllocation(new_rate_settings));
 
   if (target_bitrate.bps() != 0)
     encoder_target_bitrate_bps_ = target_bitrate.bps();

video/video_stream_encoder.h

@@ -192,9 +192,8 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
   void TraceFrameDropEnd();
 
   // Returns a copy of |rate_settings| with the |bitrate| field updated using
-  // the current VideoBitrateAllocator, and notifies any listeners of the new
-  // allocation.
-  EncoderRateSettings UpdateBitrateAllocationAndNotifyObserver(
+  // the current VideoBitrateAllocator.
+  EncoderRateSettings UpdateBitrateAllocation(
       const EncoderRateSettings& rate_settings) RTC_RUN_ON(&encoder_queue_);
 
   uint32_t GetInputFramerateFps() RTC_RUN_ON(&encoder_queue_);

video/video_stream_encoder_unittest.cc

@@ -58,6 +58,7 @@ namespace webrtc {
 
 using ::testing::_;
 using ::testing::AllOf;
+using ::testing::AtLeast;
 using ::testing::Eq;
 using ::testing::Field;
 using ::testing::Ge;
@@ -3950,9 +3951,9 @@ TEST_F(VideoStreamEncoderTest, CallsBitrateObserver) {
 
   // Called after a process interval.
   EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate))
-      .Times(1);
+      .Times(AtLeast(3));
   const int64_t start_time_ms = CurrentTimeMs();
-  while (CurrentTimeMs() - start_time_ms < kProcessIntervalMs) {
+  while (CurrentTimeMs() - start_time_ms < 5 * kProcessIntervalMs) {
     video_source_.IncomingCapturedFrame(
         CreateFrame(CurrentTimeMs(), codec_width_, codec_height_));
     WaitForEncodedFrame(CurrentTimeMs());