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}

modules/video_coding/BUILD.gn

@@ -133,8 +133,6 @@ 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,7 +22,6 @@
 #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"
@@ -46,6 +45,7 @@ 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,38 +103,34 @@ int32_t VCMGenericEncoder::Encode(const VideoFrame& frame,
   return encoder_->Encode(frame, codec_specific, &frame_types);
 }
 
-void VCMGenericEncoder::SetEncoderParameters(const EncoderParameters& params) {
+void VCMGenericEncoder::SetEncoderParameters(
+    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 =
+    rates_have_changed = target_bitrate != bitrate_allocation_ ||
-        params.target_bitrate != encoder_params_.target_bitrate ||
+                         input_frame_rate != input_frame_rate_;
-        params.input_frame_rate != encoder_params_.input_frame_rate;
+    bitrate_allocation_ = target_bitrate;
-    encoder_params_ = params;
+    input_frame_rate_ = input_frame_rate;
   }
   if (rates_have_changed) {
-    int res = encoder_->SetRateAllocation(params.target_bitrate,
+    int res = encoder_->SetRateAllocation(target_bitrate, input_frame_rate);
-                                          params.input_frame_rate);
     if (res != 0) {
       RTC_LOG(LS_WARNING) << "Error set encoder rate (total bitrate bps = "
-                          << params.target_bitrate.get_sum_bps()
+                          << target_bitrate.get_sum_bps()
-                          << ", framerate = " << params.input_frame_rate
+                          << ", framerate = " << input_frame_rate
                           << "): " << res;
     }
-    vcm_encoded_frame_callback_->OnFrameRateChanged(params.input_frame_rate);
+    vcm_encoded_frame_callback_->OnFrameRateChanged(input_frame_rate);
     for (size_t i = 0; i < streams_or_svc_num_; ++i) {
       vcm_encoded_frame_callback_->OnTargetBitrateChanged(
-          params.target_bitrate.GetSpatialLayerSum(i) / 8, i);
+          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_);
@@ -164,11 +160,9 @@ 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}),
@@ -406,20 +400,8 @@ EncodedImageCallback::Result VCMEncodedFrameCallback::OnEncodedImage(
   RTC_CHECK(videocontenttypehelpers::SetSimulcastId(
       &image_copy.content_type_, static_cast<uint8_t>(spatial_idx + 1)));
 
-  Result result = post_encode_callback_->OnEncodedImage(
+  return post_encode_callback_->OnEncodedImage(image_copy, codec_specific,
-      image_copy, codec_specific, fragmentation_header);
+                                               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,33 +23,9 @@
 
 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:
-  VCMEncodedFrameCallback(EncodedImageCallback* post_encode_callback,
+  explicit VCMEncodedFrameCallback(EncodedImageCallback* post_encode_callback);
-                          media_optimization::MediaOptimization* media_opt);
   ~VCMEncodedFrameCallback() override;
 
   // Implements EncodedImageCallback.
@@ -100,7 +76,6 @@ 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,
@@ -153,8 +128,8 @@ class VCMGenericEncoder {
                  const CodecSpecificInfo* codec_specific,
                  const std::vector<FrameType>& frame_types);
 
-  void SetEncoderParameters(const EncoderParameters& params);
+  void SetEncoderParameters(const VideoBitrateAllocation& target_bitrate,
-  EncoderParameters GetEncoderParameters() const;
+                            uint32_t input_frame_rate);
 
   int32_t RequestFrame(const std::vector<FrameType>& frame_types);
   bool InternalSource() const;
@@ -167,7 +142,8 @@ class VCMGenericEncoder {
   VCMEncodedFrameCallback* const vcm_encoded_frame_callback_;
   const bool internal_source_;
   rtc::CriticalSection params_lock_;
-  EncoderParameters encoder_params_ RTC_GUARDED_BY(params_lock_);
+  VideoBitrateAllocation bitrate_allocation_ 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, nullptr);
+  VCMEncodedFrameCallback callback(&sink);
   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, nullptr);
+  VCMEncodedFrameCallback callback(&sink);
   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, nullptr);
+  VCMEncodedFrameCallback callback(&sink);
   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, nullptr);
+  VCMEncodedFrameCallback callback(&sink);
   // 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, nullptr);
+  VCMEncodedFrameCallback callback(&sink);
   // 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

@@ -1,145 +0,0 @@
-/*
- *  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

@@ -1,78 +0,0 @@
-/*
- *  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,7 +24,6 @@
 #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"
@@ -63,7 +62,6 @@ class VideoSender {
   typedef VideoCodingModule::SenderNackMode SenderNackMode;
 
   VideoSender(Clock* clock, EncodedImageCallback* post_encode_callback);
-
   ~VideoSender();
 
   // Register the send codec to be used.
@@ -75,60 +73,27 @@ class VideoSender {
   void RegisterExternalEncoder(VideoEncoder* externalEncoder,
                                bool internalSource);
 
-  // Update the channel parameters based on new rates and rtt. This will also
+  // Update the the encoder with new bitrate allocation and framerate.
-  // cause an immediate call to VideoEncoder::SetRateAllocation().
+  int32_t SetChannelParameters(const VideoBitrateAllocation& bitrate_allocation,
-  int32_t SetChannelParameters(
+                               uint32_t framerate_fps);
-      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,7 +25,6 @@
 #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"
@@ -39,20 +38,11 @@
 namespace webrtc {
 namespace vcm {
 
-namespace {
-
-constexpr char kFrameDropperFieldTrial[] = "WebRTC-FrameDropper";
-
-}  // namespace
-
 VideoSender::VideoSender(Clock* clock,
                          EncodedImageCallback* post_encode_callback)
     : _encoder(nullptr),
-      _mediaOpt(clock),
+      _encodedFrameCallback(post_encode_callback),
-      _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) {
@@ -93,27 +83,6 @@ 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();
@@ -128,9 +97,6 @@ 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;
 }
 
@@ -156,149 +122,58 @@ 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(
-    uint32_t target_bitrate_bps,
+    const VideoBitrateAllocation& bitrate_allocation,
-    VideoBitrateAllocator* bitrate_allocator,
+    uint32_t framerate_fps) {
-    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) {
-      SetEncoderParameters(encoder_params, encoder_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 (!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_bitrate_kbps * 1000, rate_allocator_.get(),
+  sender_->SetChannelParameters(new_rate_allocation, settings_.maxFramerate);
-                                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, rate_allocator_.get(), nullptr);
+  sender_->SetChannelParameters(VideoBitrateAllocation(),
+                                settings_.maxFramerate);
   AddFrame();
 }
 
@@ -363,8 +363,7 @@ TEST_F(TestVideoSenderWithMockEncoder, TestEncoderParametersForInternalSource) {
   EXPECT_CALL(encoder_, SetRateAllocation(new_rate_allocation, _))
       .Times(1)
       .WillOnce(Return(0));
-  sender_->SetChannelParameters(new_bitrate_kbps * 1000, rate_allocator_.get(),
+  sender_->SetChannelParameters(new_rate_allocation, settings_.maxFramerate);
-                                nullptr);
 }
 
 TEST_F(TestVideoSenderWithMockEncoder,
@@ -375,8 +374,9 @@ 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(settings_.startBitrate * 1000,
+  sender_->SetChannelParameters(
-                                rate_allocator_.get(), nullptr);
+      rate_allocator_->GetAllocation(settings_.startBitrate * 1000, kInputFps),
+      kInputFps);
   while (clock_.TimeInMilliseconds() < start_time + kRateStatsWindowMs) {
     AddFrame();
     clock_.AdvanceTimeMilliseconds(1000 / kInputFps);
@@ -390,8 +390,7 @@ TEST_F(TestVideoSenderWithMockEncoder,
   EXPECT_CALL(encoder_, SetRateAllocation(new_rate_allocation, kInputFps))
       .Times(1)
       .WillOnce(Return(0));
-  sender_->SetChannelParameters(new_bitrate_bps, rate_allocator_.get(),
+  sender_->SetChannelParameters(new_rate_allocation, kInputFps);
-                                nullptr);
   AddFrame();
 }
 
@@ -439,12 +438,13 @@ class TestVideoSenderWithVp8 : public TestVideoSender {
       AddFrame();
       // SetChannelParameters needs to be called frequently to propagate
       // framerate from the media optimization into the encoder.
-      // Note: SetChannelParameters fails if less than 2 frames are in the
+      const VideoBitrateAllocation bitrate_allocation =
-      // buffer since it will fail to calculate the framerate.
+          rate_allocator_->GetAllocation(available_bitrate_kbps_ * 1000,
+                                         static_cast<uint32_t>(framerate));
       if (i != 0) {
-        EXPECT_EQ(VCM_OK, sender_->SetChannelParameters(
+        EXPECT_EQ(VCM_OK,
-                              available_bitrate_kbps_ * 1000,
+                  sender_->SetChannelParameters(
-                              rate_allocator_.get(), nullptr));
+                      bitrate_allocation, static_cast<uint32_t>(framerate)));
       }
     }
   }

video/BUILD.gn

@@ -204,6 +204,7 @@ rtc_source_set("video_stream_encoder_impl") {
     "../rtc_base/experiments:quality_scaling_experiment",
     "../rtc_base/system:fallthrough",
     "../system_wrappers:field_trial",
+    "//third_party/abseil-cpp/absl/memory:memory",
     "//third_party/abseil-cpp/absl/types:optional",
   ]
 }

video/video_send_stream_tests.cc

@@ -2822,11 +2822,21 @@ TEST_P(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) {
     }
 
     void WaitForSetRates(uint32_t expected_bitrate) {
-      EXPECT_TRUE(
+      // Wait for the expected rate to be set. In some cases there can be
-          bitrate_changed_event_.Wait(VideoSendStreamTest::kDefaultTimeoutMs))
+      // more than one update pending, in which case we keep waiting
-          << "Timed out while waiting encoder rate to be set.";
+      // until the correct value has been observed.
+      const int64_t start_time = rtc::TimeMillis();
+      do {
         rtc::CritScope lock(&crit_);
-      EXPECT_EQ(expected_bitrate, target_bitrate_);
+        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.";
     }
 
     void ModifySenderBitrateConfig(

video/video_stream_encoder.cc

@@ -15,12 +15,14 @@
 #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"
@@ -45,6 +47,7 @@ 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.
@@ -53,6 +56,10 @@ 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) {
@@ -380,6 +387,9 @@ 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_);
@@ -545,7 +555,6 @@ 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) {
@@ -605,8 +614,39 @@ void VideoStreamEncoder::ReconfigureEncoder() {
     rate_allocator_.reset();
   }
 
-  video_sender_.UpdateChannelParameters(rate_allocator_.get(),
+  int num_layers;
-                                        bitrate_observer_);
+  if (codec.codecType == kVideoCodecVP8) {
+    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);
 
@@ -776,6 +816,31 @@ 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_);
@@ -798,6 +863,8 @@ 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();
@@ -805,8 +872,10 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame,
   } else if (!last_parameters_update_ms_ ||
              now_ms - *last_parameters_update_ms_ >=
                  vcm::VCMProcessTimer::kDefaultProcessIntervalMs) {
-    video_sender_.UpdateChannelParameters(rate_allocator_.get(),
+    video_sender_.SetChannelParameters(
-                                          bitrate_observer_);
+        GetBitrateAllocationAndNotifyObserver(last_observed_bitrate_bps_,
+                                              framerate_fps),
+        framerate_fps);
     last_parameters_update_ms_.emplace(now_ms);
   }
 
@@ -848,6 +917,23 @@ 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);
 }
 
@@ -896,6 +982,7 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame,
   }
   encoder_info_ = info;
 
+  input_framerate_.Update(1u, clock_->TimeInMilliseconds());
   video_sender_.AddVideoFrame(out_frame, nullptr, encoder_info_);
 }
 
@@ -937,14 +1024,25 @@ EncodedImageCallback::Result VideoStreamEncoder::OnEncodedImage(
          encoded_image.timing_.encode_start_ms));
   }
 
-  encoder_queue_.PostTask(
+  // Run post encode tasks, such as overuse detection and frame rate/drop
-      [this, timestamp, time_sent_us, qp, capture_time_us, encode_duration_us] {
+  // stats for internal encoders.
-        RTC_DCHECK_RUN_ON(&encoder_queue_);
+  const size_t frame_size = encoded_image.size();
-        overuse_detector_->FrameSent(timestamp, time_sent_us, capture_time_us,
+  const bool keyframe = encoded_image._frameType == FrameType::kVideoFrameKey;
-                                     encode_duration_us);
+  RunPostEncode(timestamp, time_sent_us, capture_time_us, encode_duration_us,
-        if (quality_scaler_ && qp >= 0)
+                qp, frame_size, keyframe);
-          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;
 }
@@ -1002,8 +1100,12 @@ void VideoStreamEncoder::OnBitrateUpdated(uint32_t bitrate_bps,
     has_seen_first_significant_bwe_change_ = true;
   }
 
-  video_sender_.SetChannelParameters(bitrate_bps, rate_allocator_.get(),
+  uint32_t framerate_fps = GetInputFramerateFps();
-                                     bitrate_observer_);
+  frame_dropper_.SetRates((bitrate_bps + 500) / 1000, framerate_fps);
+
+  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_;
@@ -1262,6 +1364,44 @@ 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,10 +24,12 @@
 #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"
@@ -131,6 +133,11 @@ 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:
@@ -173,6 +180,13 @@ 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_;
 
@@ -267,6 +281,19 @@ 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,6 +553,16 @@ 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,
@@ -599,6 +609,25 @@ 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_;
@@ -610,6 +639,8 @@ 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 {
@@ -2093,9 +2124,8 @@ 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(2);
+      .Times(1);
   video_stream_encoder_->OnBitrateUpdated(kLowTargetBitrateBps, 0, 0);
 
   const int64_t kStartTimeMs = 1;
@@ -3156,9 +3186,6 @@ 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();
 
@@ -3227,4 +3254,80 @@ 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