Revert "Refactor and remove media_optimization::MediaOptimization."

This reverts commit 07276e4f89a93b1479d7aeefa53b4fc32daf001b.

Reason for revert: Speculative revert due to downstream crashes.

Original change's description:
> Refactor and remove media_optimization::MediaOptimization.
> 
> This CL removes MediaOptmization and folds some of its functionality
> into VideoStreamEncoder.
> 
> The FPS tracking is now handled by a RateStatistics instance. Frame
> dropping is still handled by FrameDropper. Both of these now live
> directly in VideoStreamEncoder.
> There is no intended change in behavior from this CL, but due to a new
> way of measuring frame rate, some minor perf changes can be expected.
> 
> A small change in behavior is that OnBitrateUpdated is now called
> directly rather than on the next frame. Since both encoding frame and
> setting rate allocations happen on the encoder worker thread, there's
> really no reason to cache bitrates and wait until the next frame.
> An edge case though is that if a new bitrate is set before the first
> frame, we must remember that bitrate and then apply it after the video
> bitrate allocator has been first created.
> 
> In addition to existing unit tests, manual tests have been used to
> confirm that frame dropping works as expected with misbehaving encoders.
> 
> Bug: webrtc:10164
> Change-Id: I7ee9c8d3c4f2bcf23c8c420310b05a4d35d94744
> Reviewed-on: https://webrtc-review.googlesource.com/c/115620
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Niels Moller <nisse@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#26147}

TBR=nisse@webrtc.org,sprang@webrtc.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:10164
Change-Id: Ie0dae19dd012bc09e793c9661a45823fd760c20c
Reviewed-on: https://webrtc-review.googlesource.com/c/116780
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26191}

modules/video_coding/BUILD.gn

@@ -133,6 +133,8 @@ rtc_static_library("video_coding") {
     "jitter_estimator.h",
     "media_opt_util.cc",
     "media_opt_util.h",
+    "media_optimization.cc",
+    "media_optimization.h",
     "nack_fec_tables.h",
     "packet_buffer.cc",
     "packet_buffer.h",

modules/video_coding/generic_encoder.cc

@@ -22,6 +22,7 @@
 #include "api/video/video_timing.h"
 #include "modules/include/module_common_types_public.h"
 #include "modules/video_coding/include/video_coding_defines.h"
+#include "modules/video_coding/media_optimization.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/experiments/alr_experiment.h"
 #include "rtc_base/logging.h"
@@ -45,7 +46,6 @@ VCMGenericEncoder::VCMGenericEncoder(
     : encoder_(encoder),
       vcm_encoded_frame_callback_(encoded_frame_callback),
       internal_source_(internal_source),
-      input_frame_rate_(0),
       streams_or_svc_num_(0),
       codec_type_(VideoCodecType::kVideoCodecGeneric) {}
 
@@ -103,34 +103,38 @@ int32_t VCMGenericEncoder::Encode(const VideoFrame& frame,
   return encoder_->Encode(frame, codec_specific, &frame_types);
 }
 
-void VCMGenericEncoder::SetEncoderParameters(
+void VCMGenericEncoder::SetEncoderParameters(const EncoderParameters& params) {
-    const VideoBitrateAllocation& target_bitrate,
-    uint32_t input_frame_rate) {
   RTC_DCHECK_RUNS_SERIALIZED(&race_checker_);
   bool rates_have_changed;
   {
     rtc::CritScope lock(&params_lock_);
-    rates_have_changed = target_bitrate != bitrate_allocation_ ||
+    rates_have_changed =
-                         input_frame_rate != input_frame_rate_;
+        params.target_bitrate != encoder_params_.target_bitrate ||
-    bitrate_allocation_ = target_bitrate;
+        params.input_frame_rate != encoder_params_.input_frame_rate;
-    input_frame_rate_ = input_frame_rate;
+    encoder_params_ = params;
   }
   if (rates_have_changed) {
-    int res = encoder_->SetRateAllocation(target_bitrate, input_frame_rate);
+    int res = encoder_->SetRateAllocation(params.target_bitrate,
+                                          params.input_frame_rate);
     if (res != 0) {
       RTC_LOG(LS_WARNING) << "Error set encoder rate (total bitrate bps = "
-                          << target_bitrate.get_sum_bps()
+                          << params.target_bitrate.get_sum_bps()
-                          << ", framerate = " << input_frame_rate
+                          << ", framerate = " << params.input_frame_rate
                           << "): " << res;
     }
-    vcm_encoded_frame_callback_->OnFrameRateChanged(input_frame_rate);
+    vcm_encoded_frame_callback_->OnFrameRateChanged(params.input_frame_rate);
     for (size_t i = 0; i < streams_or_svc_num_; ++i) {
       vcm_encoded_frame_callback_->OnTargetBitrateChanged(
-          target_bitrate.GetSpatialLayerSum(i) / 8, i);
+          params.target_bitrate.GetSpatialLayerSum(i) / 8, i);
     }
   }
 }
 
+EncoderParameters VCMGenericEncoder::GetEncoderParameters() const {
+  rtc::CritScope lock(&params_lock_);
+  return encoder_params_;
+}
+
 int32_t VCMGenericEncoder::RequestFrame(
     const std::vector<FrameType>& frame_types) {
   RTC_DCHECK_RUNS_SERIALIZED(&race_checker_);
@@ -160,9 +164,11 @@ VideoEncoder::EncoderInfo VCMGenericEncoder::GetEncoderInfo() const {
 }
 
 VCMEncodedFrameCallback::VCMEncodedFrameCallback(
-    EncodedImageCallback* post_encode_callback)
+    EncodedImageCallback* post_encode_callback,
+    media_optimization::MediaOptimization* media_opt)
     : internal_source_(false),
       post_encode_callback_(post_encode_callback),
+      media_opt_(media_opt),
       framerate_(1),
       last_timing_frame_time_ms_(-1),
       timing_frames_thresholds_({-1, 0}),
@@ -400,8 +406,20 @@ EncodedImageCallback::Result VCMEncodedFrameCallback::OnEncodedImage(
   RTC_CHECK(videocontenttypehelpers::SetSimulcastId(
       &image_copy.content_type_, static_cast<uint8_t>(spatial_idx + 1)));
 
-  return post_encode_callback_->OnEncodedImage(image_copy, codec_specific,
+  Result result = post_encode_callback_->OnEncodedImage(
-                                               fragmentation_header);
+      image_copy, codec_specific, fragmentation_header);
+  if (result.error != Result::OK)
+    return result;
+
+  if (media_opt_) {
+    media_opt_->UpdateWithEncodedData(image_copy._length,
+                                      image_copy._frameType);
+    if (internal_source_) {
+      // Signal to encoder to drop next frame.
+      result.drop_next_frame = media_opt_->DropFrame();
+    }
+  }
+  return result;
 }
 
 }  // namespace webrtc

modules/video_coding/generic_encoder.h

@@ -23,9 +23,33 @@
 
 namespace webrtc {
 
+namespace media_optimization {
+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;
+  // The input frame rate to the encoder in fps, measured in the video sender.
+  uint32_t input_frame_rate;
+};
+
 class VCMEncodedFrameCallback : public EncodedImageCallback {
  public:
-  explicit VCMEncodedFrameCallback(EncodedImageCallback* post_encode_callback);
+  VCMEncodedFrameCallback(EncodedImageCallback* post_encode_callback,
+                          media_optimization::MediaOptimization* media_opt);
   ~VCMEncodedFrameCallback() override;
 
   // Implements EncodedImageCallback.
@@ -76,6 +100,7 @@ class VCMEncodedFrameCallback : public EncodedImageCallback {
   rtc::CriticalSection timing_params_lock_;
   bool internal_source_;
   EncodedImageCallback* const post_encode_callback_;
+  media_optimization::MediaOptimization* const media_opt_;
 
   struct EncodeStartTimeRecord {
     EncodeStartTimeRecord(uint32_t timestamp,
@@ -128,8 +153,8 @@ class VCMGenericEncoder {
                  const CodecSpecificInfo* codec_specific,
                  const std::vector<FrameType>& frame_types);
 
-  void SetEncoderParameters(const VideoBitrateAllocation& target_bitrate,
+  void SetEncoderParameters(const EncoderParameters& params);
-                            uint32_t input_frame_rate);
+  EncoderParameters GetEncoderParameters() const;
 
   int32_t RequestFrame(const std::vector<FrameType>& frame_types);
   bool InternalSource() const;
@@ -142,8 +167,7 @@ class VCMGenericEncoder {
   VCMEncodedFrameCallback* const vcm_encoded_frame_callback_;
   const bool internal_source_;
   rtc::CriticalSection params_lock_;
-  VideoBitrateAllocation bitrate_allocation_ RTC_GUARDED_BY(params_lock_);
+  EncoderParameters encoder_params_ RTC_GUARDED_BY(params_lock_);
-  uint32_t input_frame_rate_ RTC_GUARDED_BY(params_lock_);
   size_t streams_or_svc_num_ RTC_GUARDED_BY(race_checker_);
   VideoCodecType codec_type_ RTC_GUARDED_BY(race_checker_);
 };

modules/video_coding/generic_encoder_unittest.cc

@@ -76,7 +76,7 @@ std::vector<std::vector<FrameType>> GetTimingFrames(
     const int num_streams,
     const int num_frames) {
   FakeEncodedImageCallback sink;
-  VCMEncodedFrameCallback callback(&sink);
+  VCMEncodedFrameCallback callback(&sink, nullptr);
   const size_t kFramerate = 30;
   callback.SetTimingFramesThresholds(
       {delay_ms, kDefaultOutlierFrameSizePercent});
@@ -192,7 +192,7 @@ TEST(TestVCMEncodedFrameCallback, NoTimingFrameIfNoEncodeStartTime) {
   image.SetTimestamp(static_cast<uint32_t>(timestamp * 90));
   codec_specific.codecType = kVideoCodecGeneric;
   FakeEncodedImageCallback sink;
-  VCMEncodedFrameCallback callback(&sink);
+  VCMEncodedFrameCallback callback(&sink, nullptr);
   VideoCodec::TimingFrameTriggerThresholds thresholds;
   thresholds.delay_ms = 1;  // Make all frames timing frames.
   callback.SetTimingFramesThresholds(thresholds);
@@ -223,7 +223,7 @@ TEST(TestVCMEncodedFrameCallback, AdjustsCaptureTimeForInternalSourceEncoder) {
   image.SetTimestamp(static_cast<uint32_t>(timestamp * 90));
   codec_specific.codecType = kVideoCodecGeneric;
   FakeEncodedImageCallback sink;
-  VCMEncodedFrameCallback callback(&sink);
+  VCMEncodedFrameCallback callback(&sink, nullptr);
   callback.SetInternalSource(true);
   VideoCodec::TimingFrameTriggerThresholds thresholds;
   thresholds.delay_ms = 1;  // Make all frames timing frames.
@@ -258,7 +258,7 @@ TEST(TestVCMEncodedFrameCallback, NotifiesAboutDroppedFrames) {
   const int64_t kTimestampMs4 = 47721870;
   codec_specific.codecType = kVideoCodecGeneric;
   FakeEncodedImageCallback sink;
-  VCMEncodedFrameCallback callback(&sink);
+  VCMEncodedFrameCallback callback(&sink, nullptr);
   // Any non-zero bitrate needed to be set before the first frame.
   callback.OnTargetBitrateChanged(500, 0);
   image.capture_time_ms_ = kTimestampMs1;
@@ -293,7 +293,7 @@ TEST(TestVCMEncodedFrameCallback, RestoresCaptureTimestamps) {
   const int64_t kTimestampMs = 123456;
   codec_specific.codecType = kVideoCodecGeneric;
   FakeEncodedImageCallback sink;
-  VCMEncodedFrameCallback callback(&sink);
+  VCMEncodedFrameCallback callback(&sink, nullptr);
   // Any non-zero bitrate needed to be set before the first frame.
   callback.OnTargetBitrateChanged(500, 0);
   image.capture_time_ms_ = kTimestampMs;  // Incorrect timesetamp.

modules/video_coding/media_optimization.cc

@@ -0,0 +1,145 @@
+/*
+ *  Copyright (c) 2012 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 "modules/video_coding/media_optimization.h"
+
+#include <string.h>
+#include <algorithm>
+#include <limits>
+
+#include "modules/video_coding/utility/frame_dropper.h"
+#include "system_wrappers/include/clock.h"
+
+namespace webrtc {
+namespace media_optimization {
+namespace {
+const int64_t kFrameHistoryWinMs = 2000;
+}  // namespace
+
+MediaOptimization::MediaOptimization(Clock* clock)
+    : clock_(clock),
+      max_bit_rate_(0),
+      max_frame_rate_(0),
+      frame_dropper_(),
+      incoming_frame_rate_(0) {
+  memset(incoming_frame_times_, -1, sizeof(incoming_frame_times_));
+  frame_dropper_.SetRates(0, 0);
+}
+
+MediaOptimization::~MediaOptimization() = default;
+
+void MediaOptimization::SetEncodingData(int32_t max_bit_rate,
+                                        uint32_t target_bitrate,
+                                        uint32_t max_frame_rate) {
+  rtc::CritScope lock(&crit_sect_);
+  // Everything codec specific should be reset here since the codec has changed.
+  max_bit_rate_ = max_bit_rate;
+  max_frame_rate_ = static_cast<float>(max_frame_rate);
+  float target_bitrate_kbps = static_cast<float>(target_bitrate) / 1000.0f;
+  frame_dropper_.Reset();
+  frame_dropper_.SetRates(target_bitrate_kbps, max_frame_rate_);
+}
+
+uint32_t MediaOptimization::SetTargetRates(uint32_t target_bitrate) {
+  rtc::CritScope lock(&crit_sect_);
+
+  // Cap target video bitrate to codec maximum.
+  int video_target_bitrate = target_bitrate;
+  if (max_bit_rate_ > 0 && video_target_bitrate > max_bit_rate_) {
+    video_target_bitrate = max_bit_rate_;
+  }
+  float target_video_bitrate_kbps =
+      static_cast<float>(video_target_bitrate) / 1000.0f;
+
+  float framerate = incoming_frame_rate_;
+  if (framerate == 0.0) {
+    // No framerate estimate available, use configured max framerate instead.
+    framerate = max_frame_rate_;
+  }
+
+  frame_dropper_.SetRates(target_video_bitrate_kbps, framerate);
+
+  return video_target_bitrate;
+}
+
+uint32_t MediaOptimization::InputFrameRate() {
+  rtc::CritScope lock(&crit_sect_);
+  return InputFrameRateInternal();
+}
+
+uint32_t MediaOptimization::InputFrameRateInternal() {
+  ProcessIncomingFrameRate(clock_->TimeInMilliseconds());
+  uint32_t framerate = static_cast<uint32_t>(std::min<float>(
+      std::numeric_limits<uint32_t>::max(), incoming_frame_rate_ + 0.5f));
+  return framerate;
+}
+
+void MediaOptimization::UpdateWithEncodedData(
+    const size_t encoded_image_length,
+    const FrameType encoded_image_frametype) {
+  size_t encoded_length = encoded_image_length;
+  rtc::CritScope lock(&crit_sect_);
+  if (encoded_length > 0) {
+    const bool delta_frame = encoded_image_frametype != kVideoFrameKey;
+    frame_dropper_.Fill(encoded_length, delta_frame);
+  }
+}
+
+void MediaOptimization::EnableFrameDropper(bool enable) {
+  rtc::CritScope lock(&crit_sect_);
+  frame_dropper_.Enable(enable);
+}
+
+bool MediaOptimization::DropFrame() {
+  rtc::CritScope lock(&crit_sect_);
+  UpdateIncomingFrameRate();
+  // Leak appropriate number of bytes.
+  frame_dropper_.Leak(static_cast<uint32_t>(InputFrameRateInternal() + 0.5f));
+  return frame_dropper_.DropFrame();
+}
+
+void MediaOptimization::UpdateIncomingFrameRate() {
+  int64_t now = clock_->TimeInMilliseconds();
+  if (incoming_frame_times_[0] == 0) {
+    // No shifting if this is the first time.
+  } else {
+    // Shift all times one step.
+    for (int32_t i = (kFrameCountHistorySize - 2); i >= 0; i--) {
+      incoming_frame_times_[i + 1] = incoming_frame_times_[i];
+    }
+  }
+  incoming_frame_times_[0] = now;
+  ProcessIncomingFrameRate(now);
+}
+
+// Allowing VCM to keep track of incoming frame rate.
+void MediaOptimization::ProcessIncomingFrameRate(int64_t now) {
+  int32_t num = 0;
+  int32_t nr_of_frames = 0;
+  for (num = 1; num < (kFrameCountHistorySize - 1); ++num) {
+    if (incoming_frame_times_[num] <= 0 ||
+        // Don't use data older than 2 s.
+        now - incoming_frame_times_[num] > kFrameHistoryWinMs) {
+      break;
+    } else {
+      nr_of_frames++;
+    }
+  }
+  if (num > 1) {
+    const int64_t diff =
+        incoming_frame_times_[0] - incoming_frame_times_[num - 1];
+    incoming_frame_rate_ = 0.0;  // No frame rate estimate available.
+    if (diff > 0) {
+      incoming_frame_rate_ = nr_of_frames * 1000.0f / static_cast<float>(diff);
+    }
+  }
+}
+}  // namespace media_optimization
+}  // namespace webrtc

modules/video_coding/media_optimization.h

@@ -0,0 +1,78 @@
+/*
+ *  Copyright (c) 2012 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.
+ */
+
+#ifndef MODULES_VIDEO_CODING_MEDIA_OPTIMIZATION_H_
+#define MODULES_VIDEO_CODING_MEDIA_OPTIMIZATION_H_
+
+#include <memory>
+
+#include "modules/include/module_common_types.h"
+#include "modules/video_coding/include/video_coding.h"
+#include "modules/video_coding/media_opt_util.h"
+#include "modules/video_coding/utility/frame_dropper.h"
+#include "rtc_base/criticalsection.h"
+
+namespace webrtc {
+
+class Clock;
+
+namespace media_optimization {
+
+class MediaOptimization {
+ public:
+  explicit MediaOptimization(Clock* clock);
+  ~MediaOptimization();
+
+  // Informs media optimization of initial encoding state.
+  // TODO(perkj): Deprecate SetEncodingData once its not used for stats in
+  // VideoStreamEncoder.
+  void SetEncodingData(int32_t max_bit_rate,
+                       uint32_t bit_rate,
+                       uint32_t max_frame_rate);
+
+  // Sets target rates for the encoder given the channel parameters.
+  // Input: |target bitrate| - the encoder target bitrate in bits/s.
+  uint32_t SetTargetRates(uint32_t target_bitrate);
+
+  void EnableFrameDropper(bool enable);
+  bool DropFrame();
+
+  // Informs Media Optimization of encoded output.
+  // TODO(perkj): Deprecate SetEncodingData once its not used for stats in
+  // VideoStreamEncoder.
+  void UpdateWithEncodedData(const size_t encoded_image_length,
+                             const FrameType encoded_image_frametype);
+
+  // InputFrameRate 0 = no frame rate estimate available.
+  uint32_t InputFrameRate();
+
+ private:
+  enum { kFrameCountHistorySize = 90 };
+
+  void UpdateIncomingFrameRate() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
+  void ProcessIncomingFrameRate(int64_t now)
+      RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
+  uint32_t InputFrameRateInternal() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
+
+  // Protect all members.
+  rtc::CriticalSection crit_sect_;
+
+  Clock* const clock_ RTC_GUARDED_BY(crit_sect_);
+  int32_t max_bit_rate_ RTC_GUARDED_BY(crit_sect_);
+  float max_frame_rate_ RTC_GUARDED_BY(crit_sect_);
+  FrameDropper frame_dropper_ RTC_GUARDED_BY(crit_sect_);
+  float incoming_frame_rate_ RTC_GUARDED_BY(crit_sect_);
+  int64_t incoming_frame_times_[kFrameCountHistorySize] RTC_GUARDED_BY(
+      crit_sect_);
+};
+}  // namespace media_optimization
+}  // namespace webrtc
+
+#endif  // MODULES_VIDEO_CODING_MEDIA_OPTIMIZATION_H_

modules/video_coding/video_coding_impl.h

@@ -24,6 +24,7 @@
 #include "modules/video_coding/generic_decoder.h"
 #include "modules/video_coding/generic_encoder.h"
 #include "modules/video_coding/jitter_buffer.h"
+#include "modules/video_coding/media_optimization.h"
 #include "modules/video_coding/receiver.h"
 #include "modules/video_coding/timing.h"
 #include "rtc_base/onetimeevent.h"
@@ -62,6 +63,7 @@ class VideoSender {
   typedef VideoCodingModule::SenderNackMode SenderNackMode;
 
   VideoSender(Clock* clock, EncodedImageCallback* post_encode_callback);
+
   ~VideoSender();
 
   // Register the send codec to be used.
@@ -73,27 +75,60 @@ class VideoSender {
   void RegisterExternalEncoder(VideoEncoder* externalEncoder,
                                bool internalSource);
 
-  // Update the the encoder with new bitrate allocation and framerate.
+  // Update the channel parameters based on new rates and rtt. This will also
-  int32_t SetChannelParameters(const VideoBitrateAllocation& bitrate_allocation,
+  // cause an immediate call to VideoEncoder::SetRateAllocation().
-                               uint32_t framerate_fps);
+  int32_t SetChannelParameters(
+      uint32_t target_bitrate_bps,
+      VideoBitrateAllocator* bitrate_allocator,
+      VideoBitrateAllocationObserver* bitrate_updated_callback);
+
+  // Updates the channel parameters with a new bitrate allocation, but using the
+  // current targit_bitrate, loss rate and rtt. That is, the distribution or
+  // caps may be updated to a change to a new VideoCodec or allocation mode.
+  // The new parameters will be stored as pending EncoderParameters, and the
+  // encoder will only be updated on the next frame.
+  void UpdateChannelParameters(
+      VideoBitrateAllocator* bitrate_allocator,
+      VideoBitrateAllocationObserver* bitrate_updated_callback);
 
   int32_t AddVideoFrame(const VideoFrame& videoFrame,
                         const CodecSpecificInfo* codecSpecificInfo,
                         absl::optional<VideoEncoder::EncoderInfo> encoder_info);
 
   int32_t IntraFrameRequest(size_t stream_index);
+  int32_t EnableFrameDropper(bool enable);
 
  private:
+  EncoderParameters UpdateEncoderParameters(
+      const EncoderParameters& params,
+      VideoBitrateAllocator* bitrate_allocator,
+      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;
+  media_optimization::MediaOptimization _mediaOpt;
   VCMEncodedFrameCallback _encodedFrameCallback RTC_GUARDED_BY(encoder_crit_);
+  EncodedImageCallback* const post_encode_callback_;
   VCMEncoderDataBase _codecDataBase RTC_GUARDED_BY(encoder_crit_);
 
+  // If frame dropper is not force disabled, frame dropping might still be
+  // disabled if VideoEncoder::GetEncoderInfo() indicates that the encoder has a
+  // trusted rate controller. This is determined on a per-frame basis, as the
+  // encoder behavior might dynamically change.
+  bool force_disable_frame_dropper_ RTC_GUARDED_BY(encoder_crit_);
+
   // Must be accessed on the construction thread of VideoSender.
   VideoCodec current_codec_;
   rtc::SequencedTaskChecker sequenced_checker_;
 
   rtc::CriticalSection params_crit_;
+  EncoderParameters encoder_params_ RTC_GUARDED_BY(params_crit_);
   bool encoder_has_internal_source_ RTC_GUARDED_BY(params_crit_);
   std::vector<FrameType> next_frame_types_ RTC_GUARDED_BY(params_crit_);
 };

modules/video_coding/video_sender.cc

@@ -25,6 +25,7 @@
 #include "modules/video_coding/include/video_coding_defines.h"
 #include "modules/video_coding/include/video_error_codes.h"
 #include "modules/video_coding/internal_defines.h"
+#include "modules/video_coding/media_optimization.h"
 #include "modules/video_coding/utility/default_video_bitrate_allocator.h"
 #include "modules/video_coding/video_coding_impl.h"
 #include "rtc_base/checks.h"
@@ -38,11 +39,20 @@
 namespace webrtc {
 namespace vcm {
 
+namespace {
+
+constexpr char kFrameDropperFieldTrial[] = "WebRTC-FrameDropper";
+
+}  // namespace
+
 VideoSender::VideoSender(Clock* clock,
                          EncodedImageCallback* post_encode_callback)
     : _encoder(nullptr),
-      _encodedFrameCallback(post_encode_callback),
+      _mediaOpt(clock),
+      _encodedFrameCallback(post_encode_callback, &_mediaOpt),
+      post_encode_callback_(post_encode_callback),
       _codecDataBase(&_encodedFrameCallback),
+      force_disable_frame_dropper_(false),
       current_codec_(),
       encoder_has_internal_source_(false),
       next_frame_types_(1, kVideoFrameDelta) {
@@ -83,6 +93,27 @@ int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec,
   // SetSendCodec succeeded, _encoder should be set.
   RTC_DCHECK(_encoder);
 
+  int numLayers;
+  if (sendCodec->codecType == kVideoCodecVP8) {
+    numLayers = sendCodec->VP8().numberOfTemporalLayers;
+  } else if (sendCodec->codecType == kVideoCodecVP9) {
+    numLayers = sendCodec->VP9().numberOfTemporalLayers;
+  } else if (sendCodec->codecType == kVideoCodecGeneric &&
+             sendCodec->numberOfSimulcastStreams > 0) {
+    // This is mainly for unit testing, disabling frame dropping.
+    // TODO(sprang): Add a better way to disable frame dropping.
+    numLayers = sendCodec->simulcastStream[0].numberOfTemporalLayers;
+  } else {
+    numLayers = 1;
+  }
+
+  // Force-disable frame dropper if either:
+  //  * We have screensharing with layers.
+  //  * "WebRTC-FrameDropper" field trial is "Disabled".
+  force_disable_frame_dropper_ =
+      field_trial::IsDisabled(kFrameDropperFieldTrial) ||
+      (numLayers > 1 && sendCodec->mode == VideoCodecMode::kScreensharing);
+
   {
     rtc::CritScope cs(&params_crit_);
     next_frame_types_.clear();
@@ -97,6 +128,9 @@ int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec,
                       << " start bitrate " << sendCodec->startBitrate
                       << " max frame rate " << sendCodec->maxFramerate
                       << " max payload size " << maxPayloadSize;
+  _mediaOpt.SetEncodingData(sendCodec->maxBitrate * 1000,
+                            sendCodec->startBitrate * 1000,
+                            sendCodec->maxFramerate);
   return VCM_OK;
 }
 
@@ -122,58 +156,149 @@ void VideoSender::RegisterExternalEncoder(VideoEncoder* externalEncoder,
                                          internalSource);
 }
 
+EncoderParameters VideoSender::UpdateEncoderParameters(
+    const EncoderParameters& params,
+    VideoBitrateAllocator* bitrate_allocator,
+    uint32_t target_bitrate_bps) {
+  uint32_t video_target_rate_bps = _mediaOpt.SetTargetRates(target_bitrate_bps);
+  uint32_t input_frame_rate = _mediaOpt.InputFrameRate();
+  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 (bitrate_bps > 0) {
+    if (bitrate_allocator) {
+      bitrate_allocation =
+          bitrate_allocator->GetAllocation(bitrate_bps, framerate_fps);
+    } else {
+      DefaultVideoBitrateAllocator default_allocator(current_codec_);
+      bitrate_allocation =
+          default_allocator.GetAllocation(bitrate_bps, framerate_fps);
+    }
+  }
+  return bitrate_allocation;
+}
+
+void VideoSender::UpdateChannelParameters(
+    VideoBitrateAllocator* bitrate_allocator,
+    VideoBitrateAllocationObserver* bitrate_updated_callback) {
+  VideoBitrateAllocation target_rate;
+  {
+    rtc::CritScope cs(&params_crit_);
+    encoder_params_ =
+        UpdateEncoderParameters(encoder_params_, bitrate_allocator,
+                                encoder_params_.total_bitrate.bps());
+    target_rate = encoder_params_.target_bitrate;
+  }
+  if (bitrate_updated_callback && target_rate.get_sum_bps() > 0) {
+    bitrate_updated_callback->OnBitrateAllocationUpdated(target_rate);
+  }
+}
+
 int32_t VideoSender::SetChannelParameters(
-    const VideoBitrateAllocation& bitrate_allocation,
+    uint32_t target_bitrate_bps,
-    uint32_t framerate_fps) {
+    VideoBitrateAllocator* bitrate_allocator,
+    VideoBitrateAllocationObserver* bitrate_updated_callback) {
+  EncoderParameters encoder_params;
+  encoder_params = UpdateEncoderParameters(encoder_params, bitrate_allocator,
+                                           target_bitrate_bps);
+  if (bitrate_updated_callback && target_bitrate_bps > 0) {
+    bitrate_updated_callback->OnBitrateAllocationUpdated(
+        encoder_params.target_bitrate);
+  }
+
   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
+  // source encoders don't get input frames).
+  if (encoder_has_internal_source) {
     rtc::CritScope cs(&encoder_crit_);
     if (_encoder) {
-      // |target_bitrate == 0 | means that the network is down or the send pacer
+      SetEncoderParameters(encoder_params, encoder_has_internal_source);
-      // 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 (!encoder_has_internal_source &&
-          bitrate_allocation.get_sum_bps() == 0) {
-        return VCM_OK;
-      }
-
-      if (framerate_fps == 0) {
-        // No frame rate estimate available, use default.
-        framerate_fps = current_codec_.maxFramerate;
-      }
-      if (_encoder != nullptr)
-        _encoder->SetEncoderParameters(bitrate_allocation, framerate_fps);
     }
   }
 
   return VCM_OK;
 }
 
+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.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);
+}
+
 // Add one raw video frame to the encoder, blocking.
 int32_t VideoSender::AddVideoFrame(
     const VideoFrame& videoFrame,
     const CodecSpecificInfo* codecSpecificInfo,
     absl::optional<VideoEncoder::EncoderInfo> encoder_info) {
+  EncoderParameters encoder_params;
   std::vector<FrameType> next_frame_types;
   bool encoder_has_internal_source = false;
   {
     rtc::CritScope lock(&params_crit_);
+    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 (!encoder_info) {
+    encoder_info = _encoder->GetEncoderInfo();
+  }
+
+  // Frame dropping is enabled iff frame dropping has been requested, and
+  // frame dropping is not force-disabled, and rate controller is not trusted.
+  const bool frame_dropping_enabled =
+      !force_disable_frame_dropper_ &&
+      !encoder_info->has_trusted_rate_controller;
+  _mediaOpt.EnableFrameDropper(frame_dropping_enabled);
+
+  if (_mediaOpt.DropFrame()) {
+    RTC_LOG(LS_VERBOSE) << "Drop Frame: "
+                        << "target bitrate "
+                        << encoder_params.target_bitrate.get_sum_bps()
+                        << ", input frame rate "
+                        << encoder_params.input_frame_rate;
+    post_encode_callback_->OnDroppedFrame(
+        EncodedImageCallback::DropReason::kDroppedByMediaOptimizations);
+    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(),

modules/video_coding/video_sender_unittest.cc

@@ -319,14 +319,14 @@ TEST_F(TestVideoSenderWithMockEncoder, TestSetRate) {
               SetRateAllocation(new_rate_allocation, settings_.maxFramerate))
       .Times(1)
       .WillOnce(Return(0));
-  sender_->SetChannelParameters(new_rate_allocation, settings_.maxFramerate);
+  sender_->SetChannelParameters(new_bitrate_kbps * 1000, rate_allocator_.get(),
+                                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(VideoBitrateAllocation(),
+  sender_->SetChannelParameters(0, rate_allocator_.get(), nullptr);
-                                settings_.maxFramerate);
   AddFrame();
 }
 
@@ -363,7 +363,8 @@ TEST_F(TestVideoSenderWithMockEncoder, TestEncoderParametersForInternalSource) {
   EXPECT_CALL(encoder_, SetRateAllocation(new_rate_allocation, _))
       .Times(1)
       .WillOnce(Return(0));
-  sender_->SetChannelParameters(new_rate_allocation, settings_.maxFramerate);
+  sender_->SetChannelParameters(new_bitrate_kbps * 1000, rate_allocator_.get(),
+                                nullptr);
 }
 
 TEST_F(TestVideoSenderWithMockEncoder,
@@ -374,9 +375,8 @@ TEST_F(TestVideoSenderWithMockEncoder,
   // Expect initial call to SetChannelParameters. Rates are initialized through
   // InitEncode and expects no additional call before the framerate (or bitrate)
   // updates.
-  sender_->SetChannelParameters(
+  sender_->SetChannelParameters(settings_.startBitrate * 1000,
-      rate_allocator_->GetAllocation(settings_.startBitrate * 1000, kInputFps),
+                                rate_allocator_.get(), nullptr);
-      kInputFps);
   while (clock_.TimeInMilliseconds() < start_time + kRateStatsWindowMs) {
     AddFrame();
     clock_.AdvanceTimeMilliseconds(1000 / kInputFps);
@@ -390,7 +390,8 @@ TEST_F(TestVideoSenderWithMockEncoder,
   EXPECT_CALL(encoder_, SetRateAllocation(new_rate_allocation, kInputFps))
       .Times(1)
       .WillOnce(Return(0));
-  sender_->SetChannelParameters(new_rate_allocation, kInputFps);
+  sender_->SetChannelParameters(new_bitrate_bps, rate_allocator_.get(),
+                                nullptr);
   AddFrame();
 }
 
@@ -438,13 +439,12 @@ class TestVideoSenderWithVp8 : public TestVideoSender {
       AddFrame();
       // SetChannelParameters needs to be called frequently to propagate
       // framerate from the media optimization into the encoder.
-      const VideoBitrateAllocation bitrate_allocation =
+      // Note: SetChannelParameters fails if less than 2 frames are in the
-          rate_allocator_->GetAllocation(available_bitrate_kbps_ * 1000,
+      // buffer since it will fail to calculate the framerate.
-                                         static_cast<uint32_t>(framerate));
       if (i != 0) {
-        EXPECT_EQ(VCM_OK,
+        EXPECT_EQ(VCM_OK, sender_->SetChannelParameters(
-                  sender_->SetChannelParameters(
+                              available_bitrate_kbps_ * 1000,
-                      bitrate_allocation, static_cast<uint32_t>(framerate)));
+                              rate_allocator_.get(), nullptr));
       }
     }
   }

video/video_send_stream_tests.cc

@@ -2760,6 +2760,12 @@ TEST_P(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) {
   static const int kMaxBitrateKbps = 413;
   static const int kIncreasedStartBitrateKbps = 451;
   static const int kIncreasedMaxBitrateKbps = 597;
+  // If this field trial is on, we get lower bitrates than expected by this
+  // test, due to the packetization overhead.
+  webrtc::test::ScopedFieldTrials field_trials(
+      std::string(field_trial::GetFieldTrialString()) +
+      "WebRTC-SubtractPacketizationOverhead/Disabled/");
+
   class EncoderBitrateThresholdObserver : public test::SendTest,
                                           public test::FakeEncoder {
    public:
@@ -2823,21 +2829,11 @@ TEST_P(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) {
     }
 
     void WaitForSetRates(uint32_t expected_bitrate) {
-      // Wait for the expected rate to be set. In some cases there can be
+      EXPECT_TRUE(
-      // more than one update pending, in which case we keep waiting
+          bitrate_changed_event_.Wait(VideoSendStreamTest::kDefaultTimeoutMs))
-      // until the correct value has been observed.
-      const int64_t start_time = rtc::TimeMillis();
-      do {
-        rtc::CritScope lock(&crit_);
-        if (target_bitrate_ == expected_bitrate) {
-          return;
-        }
-      } while (bitrate_changed_event_.Wait(
-          std::max(int64_t{1}, VideoSendStreamTest::kDefaultTimeoutMs -
-                                   (rtc::TimeMillis() - start_time))));
-      rtc::CritScope lock(&crit_);
-      EXPECT_EQ(target_bitrate_, expected_bitrate)
           << "Timed out while waiting encoder rate to be set.";
+      rtc::CritScope lock(&crit_);
+      EXPECT_EQ(expected_bitrate, target_bitrate_);
     }
 
     void ModifySenderBitrateConfig(

video/video_stream_encoder.cc

@@ -15,14 +15,12 @@
 #include <numeric>
 #include <utility>
 
-#include "absl/memory/memory.h"
 #include "api/video/encoded_image.h"
 #include "api/video/i420_buffer.h"
 #include "api/video/video_bitrate_allocator_factory.h"
 #include "modules/video_coding/codecs/vp9/svc_rate_allocator.h"
 #include "modules/video_coding/include/video_codec_initializer.h"
 #include "modules/video_coding/include/video_coding.h"
-#include "modules/video_coding/utility/default_video_bitrate_allocator.h"
 #include "rtc_base/arraysize.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/experiments/quality_scaling_experiment.h"
@@ -47,7 +45,6 @@ const int kMinFramerateFps = 2;
 const int64_t kPendingFrameTimeoutMs = 1000;
 
 const char kInitialFramedropFieldTrial[] = "WebRTC-InitialFramedrop";
-constexpr char kFrameDropperFieldTrial[] = "WebRTC-FrameDropper";
 
 // The maximum number of frames to drop at beginning of stream
 // to try and achieve desired bitrate.
@@ -56,10 +53,6 @@ const int kMaxInitialFramedrop = 4;
 // enable DropFrameDueToSize logic.
 const float kFramedropThreshold = 0.3;
 
-// Averaging window spanning 90 frames at default 30fps, matching old media
-// optimization module defaults.
-const int64_t kFrameRateAvergingWindowSizeMs = (1000 / 30) * 90;
-
 // Initial limits for BALANCED degradation preference.
 int MinFps(int pixels) {
   if (pixels <= 320 * 240) {
@@ -387,9 +380,6 @@ VideoStreamEncoder::VideoStreamEncoder(
       dropped_frame_count_(0),
       pending_frame_post_time_us_(0),
       bitrate_observer_(nullptr),
-      force_disable_frame_dropper_(false),
-      input_framerate_(kFrameRateAvergingWindowSizeMs, 1000),
-      pending_frame_drops_(0),
       encoder_queue_("EncoderQueue") {
   RTC_DCHECK(encoder_stats_observer);
   RTC_DCHECK(overuse_detector_);
@@ -555,6 +545,7 @@ void VideoStreamEncoder::ReconfigureEncoder() {
 
   rate_allocator_ =
       settings_.bitrate_allocator_factory->CreateVideoBitrateAllocator(codec);
+  RTC_CHECK(rate_allocator_) << "Failed to create bitrate allocator.";
 
   // Set min_bitrate_bps, max_bitrate_bps, and max padding bit rate for VP9.
   if (encoder_config_.codec_type == kVideoCodecVP9) {
@@ -614,39 +605,8 @@ void VideoStreamEncoder::ReconfigureEncoder() {
     rate_allocator_.reset();
   }
 
-  int num_layers;
+  video_sender_.UpdateChannelParameters(rate_allocator_.get(),
-  if (codec.codecType == kVideoCodecVP8) {
+                                        bitrate_observer_);
-    num_layers = codec.VP8()->numberOfTemporalLayers;
-  } else if (codec.codecType == kVideoCodecVP9) {
-    num_layers = codec.VP9()->numberOfTemporalLayers;
-  } else if (codec.codecType == kVideoCodecGeneric &&
-             codec.numberOfSimulcastStreams > 0) {
-    // This is mainly for unit testing, disabling frame dropping.
-    // TODO(sprang): Add a better way to disable frame dropping.
-    num_layers = codec.simulcastStream[0].numberOfTemporalLayers;
-  } else {
-    num_layers = 1;
-  }
-
-  frame_dropper_.Reset();
-  frame_dropper_.SetRates(codec.startBitrate, max_framerate_);
-  uint32_t framerate_fps = GetInputFramerateFps();
-  // Force-disable frame dropper if either:
-  //  * We have screensharing with layers.
-  //  * "WebRTC-FrameDropper" field trial is "Disabled".
-  force_disable_frame_dropper_ =
-      field_trial::IsDisabled(kFrameDropperFieldTrial) ||
-      (num_layers > 1 && codec.mode == VideoCodecMode::kScreensharing);
-
-  if (rate_allocator_ && last_observed_bitrate_bps_ > 0) {
-    // We have a new rate allocator instance and already configured target
-    // bitrate. Update the rate allocation and notify observsers.
-    VideoBitrateAllocation bitrate_allocation =
-        GetBitrateAllocationAndNotifyObserver(last_observed_bitrate_bps_,
-                                              framerate_fps);
-
-    video_sender_.SetChannelParameters(bitrate_allocation, framerate_fps);
-  }
 
   encoder_stats_observer_->OnEncoderReconfigured(encoder_config_, streams);
 
@@ -816,31 +776,6 @@ void VideoStreamEncoder::TraceFrameDropEnd() {
   encoder_paused_and_dropped_frame_ = false;
 }
 
-VideoBitrateAllocation
-VideoStreamEncoder::GetBitrateAllocationAndNotifyObserver(
-    const uint32_t target_bitrate_bps,
-    uint32_t framerate_fps) {
-  // Only call allocators if bitrate > 0 (ie, not suspended), otherwise they
-  // might cap the bitrate to the min bitrate configured.
-  VideoBitrateAllocation bitrate_allocation;
-  if (rate_allocator_ && target_bitrate_bps > 0) {
-    bitrate_allocation =
-        rate_allocator_->GetAllocation(target_bitrate_bps, framerate_fps);
-  }
-
-  if (bitrate_observer_ && bitrate_allocation.get_sum_bps() > 0) {
-    bitrate_observer_->OnBitrateAllocationUpdated(bitrate_allocation);
-  }
-
-  return bitrate_allocation;
-}
-
-uint32_t VideoStreamEncoder::GetInputFramerateFps() {
-  const uint32_t default_fps = max_framerate_ != -1 ? max_framerate_ : 30;
-  return input_framerate_.Rate(clock_->TimeInMilliseconds())
-      .value_or(default_fps);
-}
-
 void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame,
                                                int64_t time_when_posted_us) {
   RTC_DCHECK_RUN_ON(&encoder_queue_);
@@ -863,8 +798,6 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame,
   // InitialFrameDropOffWhenEncoderDisabledScaling, the return value
   // from GetScalingSettings should enable or disable the frame drop.
 
-  uint32_t framerate_fps = GetInputFramerateFps();
-
   int64_t now_ms = clock_->TimeInMilliseconds();
   if (pending_encoder_reconfiguration_) {
     ReconfigureEncoder();
@@ -872,10 +805,8 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame,
   } else if (!last_parameters_update_ms_ ||
              now_ms - *last_parameters_update_ms_ >=
                  vcm::VCMProcessTimer::kDefaultProcessIntervalMs) {
-    video_sender_.SetChannelParameters(
+    video_sender_.UpdateChannelParameters(rate_allocator_.get(),
-        GetBitrateAllocationAndNotifyObserver(last_observed_bitrate_bps_,
+                                          bitrate_observer_);
-                                              framerate_fps),
-        framerate_fps);
     last_parameters_update_ms_.emplace(now_ms);
   }
 
@@ -917,23 +848,6 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame,
   }
 
   pending_frame_.reset();
-
-  frame_dropper_.Leak(framerate_fps);
-  // Frame dropping is enabled iff frame dropping is not force-disabled, and
-  // rate controller is not trusted.
-  const bool frame_dropping_enabled =
-      !force_disable_frame_dropper_ &&
-      !encoder_info_.has_trusted_rate_controller;
-  frame_dropper_.Enable(frame_dropping_enabled);
-  if (frame_dropping_enabled && frame_dropper_.DropFrame()) {
-    RTC_LOG(LS_VERBOSE) << "Drop Frame: "
-                        << "target bitrate " << last_observed_bitrate_bps_
-                        << ", input frame rate " << framerate_fps;
-    OnDroppedFrame(
-        EncodedImageCallback::DropReason::kDroppedByMediaOptimizations);
-    return;
-  }
-
   EncodeVideoFrame(video_frame, time_when_posted_us);
 }
 
@@ -982,7 +896,6 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame,
   }
   encoder_info_ = info;
 
-  input_framerate_.Update(1u, clock_->TimeInMilliseconds());
   video_sender_.AddVideoFrame(out_frame, nullptr, encoder_info_);
 }
 
@@ -1024,25 +937,14 @@ EncodedImageCallback::Result VideoStreamEncoder::OnEncodedImage(
          encoded_image.timing_.encode_start_ms));
   }
 
-  // Run post encode tasks, such as overuse detection and frame rate/drop
+  encoder_queue_.PostTask(
-  // stats for internal encoders.
+      [this, timestamp, time_sent_us, qp, capture_time_us, encode_duration_us] {
-  const size_t frame_size = encoded_image.size();
+        RTC_DCHECK_RUN_ON(&encoder_queue_);
-  const bool keyframe = encoded_image._frameType == FrameType::kVideoFrameKey;
+        overuse_detector_->FrameSent(timestamp, time_sent_us, capture_time_us,
-  RunPostEncode(timestamp, time_sent_us, capture_time_us, encode_duration_us,
+                                     encode_duration_us);
-                qp, frame_size, keyframe);
+        if (quality_scaler_ && qp >= 0)
-
+          quality_scaler_->ReportQp(qp);
-  if (result.error == Result::OK) {
+      });
-    // In case of an internal encoder running on a separate thread, the
-    // decision to drop a frame might be a frame late and signaled via
-    // atomic flag. This is because we can't easily wait for the worker thread
-    // without risking deadlocks, eg during shutdown when the worker thread
-    // might be waiting for the internal encoder threads to stop.
-    if (pending_frame_drops_.load() > 0) {
-      int pending_drops = pending_frame_drops_.fetch_sub(1);
-      RTC_DCHECK_GT(pending_drops, 0);
-      result.drop_next_frame = true;
-    }
-  }
 
   return result;
 }
@@ -1100,12 +1002,8 @@ void VideoStreamEncoder::OnBitrateUpdated(uint32_t bitrate_bps,
     has_seen_first_significant_bwe_change_ = true;
   }
 
-  uint32_t framerate_fps = GetInputFramerateFps();
+  video_sender_.SetChannelParameters(bitrate_bps, rate_allocator_.get(),
-  frame_dropper_.SetRates((bitrate_bps + 500) / 1000, framerate_fps);
+                                     bitrate_observer_);
-
-  VideoBitrateAllocation bitrate_allocation =
-      GetBitrateAllocationAndNotifyObserver(bitrate_bps, framerate_fps);
-  video_sender_.SetChannelParameters(bitrate_allocation, framerate_fps);
 
   encoder_start_bitrate_bps_ =
       bitrate_bps != 0 ? bitrate_bps : encoder_start_bitrate_bps_;
@@ -1364,44 +1262,6 @@ VideoStreamEncoder::GetConstAdaptCounter() {
   return adapt_counters_[degradation_preference_];
 }
 
-void VideoStreamEncoder::RunPostEncode(uint32_t frame_timestamp,
-                                       int64_t time_sent_us,
-                                       int64_t capture_time_us,
-                                       absl::optional<int> encode_durations_us,
-                                       int qp,
-                                       size_t frame_size_bytes,
-                                       bool keyframe) {
-  if (!encoder_queue_.IsCurrent()) {
-    encoder_queue_.PostTask([this, frame_timestamp, time_sent_us, qp,
-                             capture_time_us, encode_durations_us,
-                             frame_size_bytes, keyframe] {
-      RunPostEncode(frame_timestamp, time_sent_us, capture_time_us,
-                    encode_durations_us, qp, frame_size_bytes, keyframe);
-    });
-    return;
-  }
-
-  RTC_DCHECK_RUN_ON(&encoder_queue_);
-  if (frame_size_bytes > 0) {
-    frame_dropper_.Fill(frame_size_bytes, !keyframe);
-  }
-
-  if (encoder_info_.has_internal_source) {
-    // Update frame dropper after the fact for internal sources.
-    input_framerate_.Update(1u, clock_->TimeInMilliseconds());
-    frame_dropper_.Leak(GetInputFramerateFps());
-    // Signal to encoder to drop next frame.
-    if (frame_dropper_.DropFrame()) {
-      pending_frame_drops_.fetch_add(1);
-    }
-  }
-
-  overuse_detector_->FrameSent(frame_timestamp, time_sent_us, capture_time_us,
-                               encode_durations_us);
-  if (quality_scaler_ && qp >= 0)
-    quality_scaler_->ReportQp(qp);
-}
-
 // Class holding adaptation information.
 VideoStreamEncoder::AdaptCounter::AdaptCounter() {
   fps_counters_.resize(kScaleReasonSize);

video/video_stream_encoder.h

@@ -24,12 +24,10 @@
 #include "api/video/video_stream_encoder_observer.h"
 #include "api/video/video_stream_encoder_settings.h"
 #include "api/video_codecs/video_encoder.h"
-#include "modules/video_coding/utility/frame_dropper.h"
 #include "modules/video_coding/utility/quality_scaler.h"
 #include "modules/video_coding/video_coding_impl.h"
 #include "rtc_base/criticalsection.h"
 #include "rtc_base/event.h"
-#include "rtc_base/rate_statistics.h"
 #include "rtc_base/sequenced_task_checker.h"
 #include "rtc_base/task_queue.h"
 #include "video/overuse_frame_detector.h"
@@ -133,11 +131,6 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
   void TraceFrameDropStart();
   void TraceFrameDropEnd();
 
-  VideoBitrateAllocation GetBitrateAllocationAndNotifyObserver(
-      const uint32_t target_bitrate_bps,
-      uint32_t framerate_fps) RTC_RUN_ON(&encoder_queue_);
-  uint32_t GetInputFramerateFps() RTC_RUN_ON(&encoder_queue_);
-
   // Class holding adaptation information.
   class AdaptCounter final {
    public:
@@ -180,13 +173,6 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
   void UpdateAdaptationStats(AdaptReason reason) RTC_RUN_ON(&encoder_queue_);
   VideoStreamEncoderObserver::AdaptationSteps GetActiveCounts(
       AdaptReason reason) RTC_RUN_ON(&encoder_queue_);
-  void RunPostEncode(uint32_t frame_timestamp,
-                     int64_t time_sent_us,
-                     int64_t capture_time_us,
-                     absl::optional<int> encode_durations_us,
-                     int qp,
-                     size_t frame_size_bytes,
-                     bool keyframe);
 
   rtc::Event shutdown_event_;
 
@@ -281,19 +267,6 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
       RTC_GUARDED_BY(&encoder_queue_);
 
   VideoEncoder::EncoderInfo encoder_info_ RTC_GUARDED_BY(&encoder_queue_);
-  FrameDropper frame_dropper_ RTC_GUARDED_BY(&encoder_queue_);
-
-  // If frame dropper is not force disabled, frame dropping might still be
-  // disabled if VideoEncoder::GetEncoderInfo() indicates that the encoder has a
-  // trusted rate controller. This is determined on a per-frame basis, as the
-  // encoder behavior might dynamically change.
-  bool force_disable_frame_dropper_ RTC_GUARDED_BY(&encoder_queue_);
-  RateStatistics input_framerate_ RTC_GUARDED_BY(&encoder_queue_);
-  // Incremented on worker thread whenever |frame_dropper_| determines that a
-  // frame should be dropped. Decremented on whichever thread runs
-  // OnEncodedImage(), which is only called by one thread but not necessarily
-  // the worker thread.
-  std::atomic<int> pending_frame_drops_;
 
   // All public methods are proxied to |encoder_queue_|. It must must be
   // destroyed first to make sure no tasks are run that use other members.

video/video_stream_encoder_unittest.cc

@@ -553,16 +553,6 @@ class VideoStreamEncoderTest : public ::testing::Test {
       force_init_encode_failed_ = force_failure;
     }
 
-    void SimulateOvershoot(double rate_factor) {
-      rtc::CritScope lock(&local_crit_sect_);
-      rate_factor_ = rate_factor;
-    }
-
-    uint32_t GetLastFramerate() {
-      rtc::CritScope lock(&local_crit_sect_);
-      return last_framerate_;
-    }
-
    private:
     int32_t Encode(const VideoFrame& input_image,
                    const CodecSpecificInfo* codec_specific_info,
@@ -609,25 +599,6 @@ class VideoStreamEncoderTest : public ::testing::Test {
       return res;
     }
 
-    int32_t SetRateAllocation(const VideoBitrateAllocation& rate_allocation,
-                              uint32_t framerate) {
-      rtc::CritScope lock(&local_crit_sect_);
-      VideoBitrateAllocation adjusted_rate_allocation;
-      for (size_t si = 0; si < kMaxSpatialLayers; ++si) {
-        for (size_t ti = 0; ti < kMaxTemporalStreams; ++ti) {
-          if (rate_allocation.HasBitrate(si, ti)) {
-            adjusted_rate_allocation.SetBitrate(
-                si, ti,
-                static_cast<uint32_t>(rate_allocation.GetBitrate(si, ti) *
-                                      rate_factor_));
-          }
-        }
-      }
-      last_framerate_ = framerate;
-      return FakeEncoder::SetRateAllocation(adjusted_rate_allocation,
-                                            framerate);
-    }
-
     rtc::CriticalSection local_crit_sect_;
     bool block_next_encode_ RTC_GUARDED_BY(local_crit_sect_) = false;
     rtc::Event continue_encode_event_;
@@ -639,8 +610,6 @@ class VideoStreamEncoderTest : public ::testing::Test {
     std::vector<std::unique_ptr<Vp8TemporalLayers>> allocated_temporal_layers_
         RTC_GUARDED_BY(local_crit_sect_);
     bool force_init_encode_failed_ RTC_GUARDED_BY(local_crit_sect_) = false;
-    double rate_factor_ RTC_GUARDED_BY(local_crit_sect_) = 1.0;
-    uint32_t last_framerate_ RTC_GUARDED_BY(local_crit_sect_) = 0;
   };
 
   class TestSink : public VideoStreamEncoder::EncoderSink {
@@ -2124,8 +2093,9 @@ TEST_F(VideoStreamEncoderTest, CallsBitrateObserver) {
       DefaultVideoBitrateAllocator(fake_encoder_.codec_config())
           .GetAllocation(kLowTargetBitrateBps, kDefaultFps);
 
+  // First called on bitrate updated, then again on first frame.
   EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate))
-      .Times(1);
+      .Times(2);
   video_stream_encoder_->OnBitrateUpdated(kLowTargetBitrateBps, 0, 0);
 
   const int64_t kStartTimeMs = 1;
@@ -3186,6 +3156,9 @@ TEST_F(VideoStreamEncoderTest, DoesNotUpdateBitrateAllocationWhenSuspended) {
 
   MockBitrateObserver bitrate_observer;
   video_stream_encoder_->SetBitrateAllocationObserver(&bitrate_observer);
+
+  EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(_)).Times(1);
+  // Initial bitrate update.
   video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
   video_stream_encoder_->WaitUntilTaskQueueIsIdle();
 
@@ -3254,80 +3227,4 @@ TEST_F(VideoStreamEncoderTest,
   video_stream_encoder_->Stop();
 }
 
-TEST_F(VideoStreamEncoderTest, DropsFramesWhenEncoderOvershoots) {
-  const int kFrameWidth = 320;
-  const int kFrameHeight = 240;
-  const int kFps = 30;
-  const int kTargetBitrateBps = 120000;
-  const int kNumFramesInRun = kFps * 5;  // Runs of five seconds.
-
-  video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
-
-  int64_t timestamp_ms = fake_clock_.TimeNanos() / rtc::kNumNanosecsPerMillisec;
-  max_framerate_ = kFps;
-
-  // Insert 3 seconds of video, verify number of drops with normal bitrate.
-  fake_encoder_.SimulateOvershoot(1.0);
-  int num_dropped = 0;
-  for (int i = 0; i < kNumFramesInRun; ++i) {
-    video_source_.IncomingCapturedFrame(
-        CreateFrame(timestamp_ms, kFrameWidth, kFrameHeight));
-    // Wait up to two frame durations for a frame to arrive.
-    if (!TimedWaitForEncodedFrame(timestamp_ms, 2 * 1000 / kFps)) {
-      ++num_dropped;
-    }
-    timestamp_ms += 1000 / kFps;
-  }
-
-  // Frame drops should be less than 5%
-  EXPECT_LT(num_dropped, 5 * kNumFramesInRun / 100);
-
-  // Make encoder produce frames at double the expected bitrate during 3 seconds
-  // of video, verify number of drops. Rate needs to be slightly changed in
-  // order to force the rate to be reconfigured.
-  fake_encoder_.SimulateOvershoot(2.0);
-  video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps + 1000, 0, 0);
-  num_dropped = 0;
-  for (int i = 0; i < kNumFramesInRun; ++i) {
-    video_source_.IncomingCapturedFrame(
-        CreateFrame(timestamp_ms, kFrameWidth, kFrameHeight));
-    // Wait up to two frame durations for a frame to arrive.
-    if (!TimedWaitForEncodedFrame(timestamp_ms, 2 * 1000 / kFps)) {
-      ++num_dropped;
-    }
-    timestamp_ms += 1000 / kFps;
-  }
-
-  // Frame drops should be more than 40%.
-  EXPECT_GT(num_dropped, 40 * kNumFramesInRun / 100);
-
-  video_stream_encoder_->Stop();
-}
-
-TEST_F(VideoStreamEncoderTest, ConfiguresCorrectFrameRate) {
-  const int kFrameWidth = 320;
-  const int kFrameHeight = 240;
-  const int kActualInputFps = 24;
-  const int kTargetBitrateBps = 120000;
-
-  ASSERT_GT(max_framerate_, kActualInputFps);
-
-  int64_t timestamp_ms = fake_clock_.TimeNanos() / rtc::kNumNanosecsPerMillisec;
-  max_framerate_ = kActualInputFps;
-  video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
-
-  // Insert 3 seconds of video, with an input fps lower than configured max.
-  for (int i = 0; i < kActualInputFps * 3; ++i) {
-    video_source_.IncomingCapturedFrame(
-        CreateFrame(timestamp_ms, kFrameWidth, kFrameHeight));
-    // Wait up to two frame durations for a frame to arrive.
-    WaitForEncodedFrame(timestamp_ms);
-    timestamp_ms += 1000 / kActualInputFps;
-  }
-
-  EXPECT_NEAR(kActualInputFps, fake_encoder_.GetLastFramerate(), 1);
-
-  video_stream_encoder_->Stop();
-}
-
 }  // namespace webrtc