SimulcastEncoderAdapter: Use FramerateController instead of FramerateControllerDeprecated.

Results from test (CallPerfTest.TestEncodeFramerateVp8Simulcast):
Simulcast streams:
0: max_fps:20 -> StreamStats.encode_frame_rate:15 (before), 20 (after)
1: max_fps:30

Bug: webrtc:13031
Change-Id: I30e6b2dcb2746859bd3e21b098bfa7b0fb3b2dda
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/230120
Commit-Queue: Åsa Persson <asapersson@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34867}

call/BUILD.gn

@@ -525,6 +525,8 @@ if (rtc_include_tests) {
         "../api/video:builtin_video_bitrate_allocator_factory",
         "../api/video:video_bitrate_allocation",
         "../api/video_codecs:video_codecs_api",
+        "../media:rtc_internal_video_codecs",
+        "../media:rtc_simulcast_encoder_adapter",
         "../modules/audio_coding",
         "../modules/audio_device",
         "../modules/audio_device:audio_device_impl",

call/DEPS

@@ -25,5 +25,8 @@ specific_include_rules = {
   ],
   "rtp_transport_controller_send_interface\.h": [
     "+common_video/frame_counts.h",
+  ],
+  "call_perf_tests\.cc": [
+    "+media/engine",
   ]
 }

call/call_perf_tests.cc

@@ -24,6 +24,8 @@
 #include "call/call.h"
 #include "call/fake_network_pipe.h"
 #include "call/simulated_network.h"
+#include "media/engine/internal_encoder_factory.h"
+#include "media/engine/simulcast_encoder_adapter.h"
 #include "modules/audio_coding/include/audio_coding_module.h"
 #include "modules/audio_device/include/test_audio_device.h"
 #include "modules/audio_mixer/audio_mixer_impl.h"
@@ -88,6 +90,9 @@ class CallPerfTest : public test::CallTest {
                                 int min_bwe,
                                 int start_bwe,
                                 int max_bwe);
+  void TestEncodeFramerate(VideoEncoderFactory* encoder_factory,
+                           const std::string& payload_name,
+                           const std::vector<int>& max_framerates);
 };
 
 class VideoRtcpAndSyncObserver : public test::RtpRtcpObserver,
@@ -151,7 +156,7 @@ class VideoRtcpAndSyncObserver : public test::RtpRtcpObserver,
 
  private:
   Clock* const clock_;
-  std::string test_label_;
+  const std::string test_label_;
   const int64_t creation_time_ms_;
   int64_t first_time_in_sync_ = -1;
   VideoReceiveStream* receive_stream_ = nullptr;
@@ -487,9 +492,8 @@ void CallPerfTest::TestCaptureNtpTime(
     }
 
     void PerformTest() override {
-      EXPECT_TRUE(Wait()) << "Timed out while waiting for "
-                             "estimated capture NTP time to be "
-                             "within bounds.";
+      EXPECT_TRUE(Wait()) << "Timed out while waiting for estimated capture "
+                             "NTP time to be within bounds.";
       test::PrintResultList("capture_ntp_time", "", "real - estimated",
                             time_offset_ms_list_, "ms", true);
     }
@@ -497,10 +501,10 @@ void CallPerfTest::TestCaptureNtpTime(
     Mutex mutex_;
     const BuiltInNetworkBehaviorConfig net_config_;
     Clock* const clock_;
-    int threshold_ms_;
-    int start_time_ms_;
-    int run_time_ms_;
-    int64_t creation_time_ms_;
+    const int threshold_ms_;
+    const int start_time_ms_;
+    const int run_time_ms_;
+    const int64_t creation_time_ms_;
     test::FrameGeneratorCapturer* capturer_;
     bool rtp_start_timestamp_set_;
     uint32_t rtp_start_timestamp_;
@@ -517,7 +521,7 @@ void CallPerfTest::TestCaptureNtpTime(
 TEST_F(CallPerfTest, Real_Estimated_CaptureNtpTimeWithNetworkDelay) {
   BuiltInNetworkBehaviorConfig net_config;
   net_config.queue_delay_ms = 100;
-  // TODO(wu): lower the threshold as the calculation/estimatation becomes more
+  // TODO(wu): lower the threshold as the calculation/estimation becomes more
   // accurate.
   const int kThresholdMs = 100;
   const int kStartTimeMs = 10000;
@@ -529,7 +533,7 @@ TEST_F(CallPerfTest, Real_Estimated_CaptureNtpTimeWithNetworkJitter) {
   BuiltInNetworkBehaviorConfig net_config;
   net_config.queue_delay_ms = 100;
   net_config.delay_standard_deviation_ms = 10;
-  // TODO(wu): lower the threshold as the calculation/estimatation becomes more
+  // TODO(wu): lower the threshold as the calculation/estimation becomes more
   // accurate.
   const int kThresholdMs = 100;
   const int kStartTimeMs = 10000;
@@ -1050,4 +1054,132 @@ TEST_F(CallPerfTest, MAYBE_Min_Bitrate_VideoAndAudio) {
   TestMinAudioVideoBitrate(110, 40, -10, 10000, 70000, 200000);
 }
 
+void CallPerfTest::TestEncodeFramerate(VideoEncoderFactory* encoder_factory,
+                                       const std::string& payload_name,
+                                       const std::vector<int>& max_framerates) {
+  static constexpr double kAllowedFpsDiff = 1.5;
+  static constexpr TimeDelta kMinGetStatsInterval = TimeDelta::Millis(400);
+  static constexpr TimeDelta kMinRunTime = TimeDelta::Seconds(15);
+  static constexpr DataRate kMaxBitrate = DataRate::KilobitsPerSec(1000);
+
+  class FramerateObserver
+      : public test::EndToEndTest,
+        public test::FrameGeneratorCapturer::SinkWantsObserver {
+   public:
+    FramerateObserver(VideoEncoderFactory* encoder_factory,
+                      const std::string& payload_name,
+                      const std::vector<int>& max_framerates,
+                      TaskQueueBase* task_queue)
+        : EndToEndTest(kDefaultTimeoutMs),
+          clock_(Clock::GetRealTimeClock()),
+          encoder_factory_(encoder_factory),
+          payload_name_(payload_name),
+          max_framerates_(max_framerates),
+          task_queue_(task_queue),
+          start_time_(clock_->CurrentTime()),
+          last_getstats_time_(start_time_),
+          send_stream_(nullptr) {}
+
+    void OnFrameGeneratorCapturerCreated(
+        test::FrameGeneratorCapturer* frame_generator_capturer) override {
+      frame_generator_capturer->ChangeResolution(640, 360);
+    }
+
+    void OnSinkWantsChanged(rtc::VideoSinkInterface<VideoFrame>* sink,
+                            const rtc::VideoSinkWants& wants) override {}
+
+    void ModifySenderBitrateConfig(
+        BitrateConstraints* bitrate_config) override {
+      bitrate_config->start_bitrate_bps = kMaxBitrate.bps() / 2;
+    }
+
+    void OnVideoStreamsCreated(
+        VideoSendStream* send_stream,
+        const std::vector<VideoReceiveStream*>& receive_streams) override {
+      send_stream_ = send_stream;
+    }
+
+    size_t GetNumVideoStreams() const override {
+      return max_framerates_.size();
+    }
+
+    void ModifyVideoConfigs(
+        VideoSendStream::Config* send_config,
+        std::vector<VideoReceiveStream::Config>* receive_configs,
+        VideoEncoderConfig* encoder_config) override {
+      send_config->encoder_settings.encoder_factory = encoder_factory_;
+      send_config->rtp.payload_name = payload_name_;
+      send_config->rtp.payload_type = test::CallTest::kVideoSendPayloadType;
+      encoder_config->video_format.name = payload_name_;
+      encoder_config->codec_type = PayloadStringToCodecType(payload_name_);
+      encoder_config->max_bitrate_bps = kMaxBitrate.bps();
+      for (size_t i = 0; i < max_framerates_.size(); ++i) {
+        encoder_config->simulcast_layers[i].max_framerate = max_framerates_[i];
+        configured_framerates_[send_config->rtp.ssrcs[i]] = max_framerates_[i];
+      }
+    }
+
+    void PerformTest() override {
+      EXPECT_TRUE(Wait()) << "Timeout while waiting for framerate stats.";
+    }
+
+    void VerifyStats() const {
+      for (const auto& encode_frame_rate_list : encode_frame_rate_lists_) {
+        const std::vector<double>& values = encode_frame_rate_list.second;
+        test::PrintResultList("substream", "", "encode_frame_rate", values,
+                              "fps", false);
+        double average_fps =
+            std::accumulate(values.begin(), values.end(), 0.0) / values.size();
+        uint32_t ssrc = encode_frame_rate_list.first;
+        double expected_fps = configured_framerates_.find(ssrc)->second;
+        EXPECT_NEAR(expected_fps, average_fps, kAllowedFpsDiff);
+      }
+    }
+
+    Action OnSendRtp(const uint8_t* packet, size_t length) override {
+      const Timestamp now = clock_->CurrentTime();
+      if (now - last_getstats_time_ > kMinGetStatsInterval) {
+        last_getstats_time_ = now;
+        task_queue_->PostTask(ToQueuedTask([this, now]() {
+          VideoSendStream::Stats stats = send_stream_->GetStats();
+          for (const auto& stat : stats.substreams) {
+            encode_frame_rate_lists_[stat.first].push_back(
+                stat.second.encode_frame_rate);
+          }
+          if (now - start_time_ > kMinRunTime) {
+            VerifyStats();
+            observation_complete_.Set();
+          }
+        }));
+      }
+      return SEND_PACKET;
+    }
+
+    Clock* const clock_;
+    VideoEncoderFactory* const encoder_factory_;
+    const std::string payload_name_;
+    const std::vector<int> max_framerates_;
+    TaskQueueBase* const task_queue_;
+    const Timestamp start_time_;
+    Timestamp last_getstats_time_;
+    VideoSendStream* send_stream_;
+    std::map<uint32_t, std::vector<double>> encode_frame_rate_lists_;
+    std::map<uint32_t, double> configured_framerates_;
+  } test(encoder_factory, payload_name, max_framerates, task_queue());
+
+  RunBaseTest(&test);
+}
+
+TEST_F(CallPerfTest, TestEncodeFramerateVp8Simulcast) {
+  InternalEncoderFactory internal_encoder_factory;
+  test::FunctionVideoEncoderFactory encoder_factory(
+      [&internal_encoder_factory]() {
+        return std::make_unique<SimulcastEncoderAdapter>(
+            &internal_encoder_factory, SdpVideoFormat("VP8"));
+      });
+
+  TestEncodeFramerate(&encoder_factory, "VP8",
+                      /*max_framerates=*/{20, 30});
+}
+
 }  // namespace webrtc

common_video/framerate_controller.cc

@@ -20,7 +20,10 @@ constexpr double kMinFramerate = 0.5;
 }  // namespace
 
 FramerateController::FramerateController()
-    : max_framerate_(std::numeric_limits<double>::max()) {}
+    : FramerateController(std::numeric_limits<double>::max()) {}
+
+FramerateController::FramerateController(double max_framerate)
+    : max_framerate_(max_framerate) {}
 
 FramerateController::~FramerateController() {}
 
@@ -28,6 +31,10 @@ void FramerateController::SetMaxFramerate(double max_framerate) {
   max_framerate_ = max_framerate;
 }
 
+double FramerateController::GetMaxFramerate() const {
+  return max_framerate_;
+}
+
 bool FramerateController::ShouldDropFrame(int64_t in_timestamp_ns) {
   if (max_framerate_ < kMinFramerate)
     return true;
@@ -67,4 +74,15 @@ void FramerateController::Reset() {
   next_frame_timestamp_ns_ = absl::nullopt;
 }
 
+void FramerateController::KeepFrame(int64_t in_timestamp_ns) {
+  if (ShouldDropFrame(in_timestamp_ns)) {
+    if (max_framerate_ < kMinFramerate)
+      return;
+
+    int64_t frame_interval_ns = rtc::kNumNanosecsPerSec / max_framerate_;
+    if (next_frame_timestamp_ns_)
+      *next_frame_timestamp_ns_ += frame_interval_ns;
+  }
+}
+
 }  // namespace webrtc

common_video/framerate_controller.h

@@ -22,16 +22,20 @@ namespace webrtc {
 class FramerateController {
  public:
   FramerateController();
+  explicit FramerateController(double max_framerate);
   ~FramerateController();
 
   // Sets max framerate (default is maxdouble).
   void SetMaxFramerate(double max_framerate);
+  double GetMaxFramerate() const;
 
   // Returns true if the frame should be dropped, false otherwise.
   bool ShouldDropFrame(int64_t in_timestamp_ns);
 
   void Reset();
 
+  void KeepFrame(int64_t in_timestamp_ns);
+
  private:
   double max_framerate_;
   absl::optional<int64_t> next_frame_timestamp_ns_;

common_video/framerate_controller_unittest.cc

@@ -143,4 +143,20 @@ TEST_F(FramerateControllerTest, NoFramesDroppedAfterReset) {
     EXPECT_FALSE(controller_.ShouldDropFrame(GetNextTimestampNs()));
 }
 
+TEST_F(FramerateControllerTest, TestKeepFrame) {
+  FramerateController controller(kInputFps / 2);
+
+  EXPECT_FALSE(controller.ShouldDropFrame(GetNextTimestampNs()));
+  EXPECT_TRUE(controller.ShouldDropFrame(GetNextTimestampNs()));
+  EXPECT_FALSE(controller.ShouldDropFrame(GetNextTimestampNs()));
+  EXPECT_TRUE(controller.ShouldDropFrame(GetNextTimestampNs()));
+  EXPECT_FALSE(controller.ShouldDropFrame(GetNextTimestampNs()));
+
+  // Next frame should be dropped.
+  // Keep this frame (e.g. in case of a key frame).
+  controller.KeepFrame(GetNextTimestampNs());
+  // Expect next frame to be dropped instead.
+  EXPECT_TRUE(controller.ShouldDropFrame(GetNextTimestampNs()));
+}
+
 }  // namespace webrtc

media/BUILD.gn

@@ -170,6 +170,7 @@ rtc_library("rtc_simulcast_encoder_adapter") {
     "../api/video_codecs:rtc_software_fallback_wrappers",
     "../api/video_codecs:video_codecs_api",
     "../call:video_stream_api",
+    "../common_video",
     "../modules/video_coding:video_codec_interface",
     "../modules/video_coding:video_coding_utility",
     "../rtc_base:checks",

media/engine/simulcast_encoder_adapter.cc

@@ -167,7 +167,7 @@ void SimulcastEncoderAdapter::EncoderContext::Release() {
 SimulcastEncoderAdapter::StreamContext::StreamContext(
     SimulcastEncoderAdapter* parent,
     std::unique_ptr<EncoderContext> encoder_context,
-    std::unique_ptr<FramerateControllerDeprecated> framerate_controller,
+    std::unique_ptr<FramerateController> framerate_controller,
     int stream_idx,
     uint16_t width,
     uint16_t height,
@@ -214,7 +214,7 @@ SimulcastEncoderAdapter::StreamContext::ReleaseEncoderContext() && {
 void SimulcastEncoderAdapter::StreamContext::OnKeyframe(Timestamp timestamp) {
   is_keyframe_needed_ = false;
   if (framerate_controller_) {
-    framerate_controller_->AddFrame(timestamp.ms());
+    framerate_controller_->KeepFrame(timestamp.us() * 1000);
   }
 }
 
@@ -223,12 +223,7 @@ bool SimulcastEncoderAdapter::StreamContext::ShouldDropFrame(
   if (!framerate_controller_) {
     return false;
   }
-
-  if (framerate_controller_->DropFrame(timestamp.ms())) {
-    return true;
-  }
-  framerate_controller_->AddFrame(timestamp.ms());
-  return false;
+  return framerate_controller_->ShouldDropFrame(timestamp.us() * 1000);
 }
 
 EncodedImageCallback::Result
@@ -422,8 +417,7 @@ int SimulcastEncoderAdapter::InitEncode(
     bool is_paused = stream_start_bitrate_kbps[stream_idx] == 0;
     stream_contexts_.emplace_back(
         parent, std::move(encoder_context),
-        std::make_unique<FramerateControllerDeprecated>(
-            stream_codec.maxFramerate),
+        std::make_unique<FramerateController>(stream_codec.maxFramerate),
         stream_idx, stream_codec.width, stream_codec.height, is_paused);
   }
 

media/engine/simulcast_encoder_adapter.h

@@ -25,8 +25,8 @@
 #include "api/video_codecs/sdp_video_format.h"
 #include "api/video_codecs/video_encoder.h"
 #include "api/video_codecs/video_encoder_factory.h"
+#include "common_video/framerate_controller.h"
 #include "modules/video_coding/include/video_codec_interface.h"
-#include "modules/video_coding/utility/framerate_controller_deprecated.h"
 #include "rtc_base/atomic_ops.h"
 #include "rtc_base/experiments/encoder_info_settings.h"
 #include "rtc_base/system/no_unique_address.h"
@@ -93,14 +93,13 @@ class RTC_EXPORT SimulcastEncoderAdapter : public VideoEncoder {
 
   class StreamContext : public EncodedImageCallback {
    public:
-    StreamContext(
-        SimulcastEncoderAdapter* parent,
-        std::unique_ptr<EncoderContext> encoder_context,
-        std::unique_ptr<FramerateControllerDeprecated> framerate_controller,
-        int stream_idx,
-        uint16_t width,
-        uint16_t height,
-        bool send_stream);
+    StreamContext(SimulcastEncoderAdapter* parent,
+                  std::unique_ptr<EncoderContext> encoder_context,
+                  std::unique_ptr<FramerateController> framerate_controller,
+                  int stream_idx,
+                  uint16_t width,
+                  uint16_t height,
+                  bool send_stream);
     StreamContext(StreamContext&& rhs);
     StreamContext& operator=(StreamContext&&) = delete;
     ~StreamContext() override;
@@ -121,11 +120,11 @@ class RTC_EXPORT SimulcastEncoderAdapter : public VideoEncoder {
     void set_is_keyframe_needed() { is_keyframe_needed_ = true; }
     bool is_paused() const { return is_paused_; }
     void set_is_paused(bool is_paused) { is_paused_ = is_paused; }
-    absl::optional<float> target_fps() const {
+    absl::optional<double> target_fps() const {
       return framerate_controller_ == nullptr
                  ? absl::nullopt
-                 : absl::optional<float>(
-                       framerate_controller_->GetTargetRate());
+                 : absl::optional<double>(
+                       framerate_controller_->GetMaxFramerate());
     }
 
     std::unique_ptr<EncoderContext> ReleaseEncoderContext() &&;
@@ -135,7 +134,7 @@ class RTC_EXPORT SimulcastEncoderAdapter : public VideoEncoder {
    private:
     SimulcastEncoderAdapter* const parent_;
     std::unique_ptr<EncoderContext> encoder_context_;
-    std::unique_ptr<FramerateControllerDeprecated> framerate_controller_;
+    std::unique_ptr<FramerateController> framerate_controller_;
     const int stream_idx_;
     const uint16_t width_;
     const uint16_t height_;