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}
diff --git a/call/BUILD.gn b/call/BUILD.gn
index fc8fd34..87c0ec0 100644
--- a/call/BUILD.gn
+++ b/call/BUILD.gn
@@ -525,6 +525,8 @@
"../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",
diff --git a/call/DEPS b/call/DEPS
index 2260cea..b1b66ac 100644
--- a/call/DEPS
+++ b/call/DEPS
@@ -25,5 +25,8 @@
],
"rtp_transport_controller_send_interface\.h": [
"+common_video/frame_counts.h",
+ ],
+ "call_perf_tests\.cc": [
+ "+media/engine",
]
}
diff --git a/call/call_perf_tests.cc b/call/call_perf_tests.cc
index 9596323..b1f17c8 100644
--- a/call/call_perf_tests.cc
+++ b/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 @@
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 @@
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 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 @@
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 @@
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 @@
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 @@
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
diff --git a/common_video/framerate_controller.cc b/common_video/framerate_controller.cc
index 5f7099d..23e9c70 100644
--- a/common_video/framerate_controller.cc
+++ b/common_video/framerate_controller.cc
@@ -20,7 +20,10 @@
} // 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 @@
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 @@
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
diff --git a/common_video/framerate_controller.h b/common_video/framerate_controller.h
index 9339571..371ffd4 100644
--- a/common_video/framerate_controller.h
+++ b/common_video/framerate_controller.h
@@ -22,16 +22,20 @@
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_;
diff --git a/common_video/framerate_controller_unittest.cc b/common_video/framerate_controller_unittest.cc
index 1f4f321..690076c 100644
--- a/common_video/framerate_controller_unittest.cc
+++ b/common_video/framerate_controller_unittest.cc
@@ -143,4 +143,20 @@
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
diff --git a/media/BUILD.gn b/media/BUILD.gn
index 87d3672..7dbd917 100644
--- a/media/BUILD.gn
+++ b/media/BUILD.gn
@@ -170,6 +170,7 @@
"../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",
diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc
index 3b63725..721ec88 100644
--- a/media/engine/simulcast_encoder_adapter.cc
+++ b/media/engine/simulcast_encoder_adapter.cc
@@ -167,7 +167,7 @@
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 @@
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 @@
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 @@
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);
}
diff --git a/media/engine/simulcast_encoder_adapter.h b/media/engine/simulcast_encoder_adapter.h
index 7d15f10..e6b6bad 100644
--- a/media/engine/simulcast_encoder_adapter.h
+++ b/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 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 @@
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 @@
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_;
