Implement injectable EncoderSelectorInterface and wire it up in the VideoStreamEncoder.
The EncoderSelectorInterface is meant to replace the "WebRTC-NetworkCondition-EncoderSwitch" field trial, so the field trial will be ignored if an EncoderSelectorInterface object has been injected.
Bug: webrtc:11341
Change-Id: I5371fac9c9ad8e38223a81dd1e7bfefb2bb458cb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168193
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30490}
diff --git a/api/video/video_stream_encoder_settings.h b/api/video/video_stream_encoder_settings.h
index 4997327..743524b 100644
--- a/api/video/video_stream_encoder_settings.h
+++ b/api/video/video_stream_encoder_settings.h
@@ -34,6 +34,8 @@
// Requests that a switch to a specific encoder is performed.
virtual void RequestEncoderSwitch(const Config& conf) = 0;
+
+ virtual void RequestEncoderSwitch(const SdpVideoFormat& format) = 0;
};
struct VideoStreamEncoderSettings {
diff --git a/api/video_codecs/video_encoder_factory.h b/api/video_codecs/video_encoder_factory.h
index 1f80fa7..a84a297 100644
--- a/api/video_codecs/video_encoder_factory.h
+++ b/api/video_codecs/video_encoder_factory.h
@@ -14,6 +14,8 @@
#include <memory>
#include <vector>
+#include "absl/types/optional.h"
+#include "api/units/data_rate.h"
#include "api/video_codecs/sdp_video_format.h"
namespace webrtc {
@@ -37,6 +39,26 @@
bool has_internal_source;
};
+ // An injectable class that is continuously updated with encoding conditions
+ // and selects the best encoder given those conditions.
+ class EncoderSelectorInterface {
+ public:
+ virtual ~EncoderSelectorInterface() {}
+
+ // Informs the encoder selector about which encoder that is currently being
+ // used.
+ virtual void OnCurrentEncoder(const SdpVideoFormat& format) = 0;
+
+ // Called every time the encoding bitrate is updated. Should return a
+ // non-empty if an encoder switch should be performed.
+ virtual absl::optional<SdpVideoFormat> OnEncodingBitrate(
+ const DataRate& rate) = 0;
+
+ // Called if the currently used encoder reports itself as broken. Should
+ // return a non-empty if an encoder switch should be performed.
+ virtual absl::optional<SdpVideoFormat> OnEncoderBroken() = 0;
+ };
+
// Returns a list of supported video formats in order of preference, to use
// for signaling etc.
virtual std::vector<SdpVideoFormat> GetSupportedFormats() const = 0;
@@ -58,6 +80,10 @@
virtual std::unique_ptr<VideoEncoder> CreateVideoEncoder(
const SdpVideoFormat& format) = 0;
+ virtual std::unique_ptr<EncoderSelectorInterface> GetEncoderSelector() const {
+ return nullptr;
+ }
+
virtual ~VideoEncoderFactory() {}
};
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 029ce63..f2426ce 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -774,6 +774,31 @@
});
}
+void WebRtcVideoChannel::RequestEncoderSwitch(
+ const webrtc::SdpVideoFormat& format) {
+ invoker_.AsyncInvoke<void>(RTC_FROM_HERE, worker_thread_, [this, format] {
+ RTC_DCHECK_RUN_ON(&thread_checker_);
+
+ for (const VideoCodecSettings& codec_setting : negotiated_codecs_) {
+ if (IsSameCodec(format.name, format.parameters, codec_setting.codec.name,
+ codec_setting.codec.params)) {
+ if (send_codec_ == codec_setting) {
+ // Already using this codec, no switch required.
+ return;
+ }
+
+ ChangedSendParameters params;
+ params.send_codec = codec_setting;
+ ApplyChangedParams(params);
+ return;
+ }
+ }
+
+ RTC_LOG(LS_WARNING) << "Encoder switch failed: SdpVideoFormat "
+ << format.ToString() << " not negotiated.";
+ });
+}
+
bool WebRtcVideoChannel::ApplyChangedParams(
const ChangedSendParameters& changed_params) {
RTC_DCHECK_RUN_ON(&thread_checker_);
diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h
index d5ed95b..e4506ad 100644
--- a/media/engine/webrtc_video_engine.h
+++ b/media/engine/webrtc_video_engine.h
@@ -211,8 +211,11 @@
// Implements webrtc::EncoderSwitchRequestCallback.
void RequestEncoderFallback() override;
+
+ // TODO(bugs.webrtc.org/11341) : Remove this version of RequestEncoderSwitch.
void RequestEncoderSwitch(
const EncoderSwitchRequestCallback::Config& conf) override;
+ void RequestEncoderSwitch(const webrtc::SdpVideoFormat& format) override;
void SetRecordableEncodedFrameCallback(
uint32_t ssrc,
diff --git a/test/video_encoder_proxy_factory.h b/test/video_encoder_proxy_factory.h
index d560911..46caf8d 100644
--- a/test/video_encoder_proxy_factory.h
+++ b/test/video_encoder_proxy_factory.h
@@ -30,7 +30,12 @@
class VideoEncoderProxyFactory final : public VideoEncoderFactory {
public:
explicit VideoEncoderProxyFactory(VideoEncoder* encoder)
+ : VideoEncoderProxyFactory(encoder, nullptr) {}
+
+ explicit VideoEncoderProxyFactory(VideoEncoder* encoder,
+ EncoderSelectorInterface* encoder_selector)
: encoder_(encoder),
+ encoder_selector_(encoder_selector),
num_simultaneous_encoder_instances_(0),
max_num_simultaneous_encoder_instances_(0) {
codec_info_.is_hardware_accelerated = false;
@@ -56,6 +61,15 @@
return std::make_unique<EncoderProxy>(encoder_, this);
}
+ std::unique_ptr<EncoderSelectorInterface> GetEncoderSelector()
+ const override {
+ if (encoder_selector_ != nullptr) {
+ return std::make_unique<EncoderSelectorProxy>(encoder_selector_);
+ }
+
+ return nullptr;
+ }
+
void SetIsHardwareAccelerated(bool is_hardware_accelerated) {
codec_info_.is_hardware_accelerated = is_hardware_accelerated;
}
@@ -117,7 +131,30 @@
VideoEncoderProxyFactory* const encoder_factory_;
};
+ class EncoderSelectorProxy final : public EncoderSelectorInterface {
+ public:
+ explicit EncoderSelectorProxy(EncoderSelectorInterface* encoder_selector)
+ : encoder_selector_(encoder_selector) {}
+
+ void OnCurrentEncoder(const SdpVideoFormat& format) override {
+ encoder_selector_->OnCurrentEncoder(format);
+ }
+
+ absl::optional<SdpVideoFormat> OnEncodingBitrate(
+ const DataRate& rate) override {
+ return encoder_selector_->OnEncodingBitrate(rate);
+ }
+
+ absl::optional<SdpVideoFormat> OnEncoderBroken() override {
+ return encoder_selector_->OnEncoderBroken();
+ }
+
+ private:
+ EncoderSelectorInterface* const encoder_selector_;
+ };
+
VideoEncoder* const encoder_;
+ EncoderSelectorInterface* const encoder_selector_;
CodecInfo codec_info_;
int num_simultaneous_encoder_instances_;
diff --git a/video/BUILD.gn b/video/BUILD.gn
index 0653113..0916efc 100644
--- a/video/BUILD.gn
+++ b/video/BUILD.gn
@@ -534,6 +534,7 @@
"../api:libjingle_peerconnection_api",
"../api:mock_fec_controller_override",
"../api:mock_frame_decryptor",
+ "../api:mock_video_encoder",
"../api:rtp_headers",
"../api:rtp_parameters",
"../api:scoped_refptr",
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index 4079ab5..a276399 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -254,6 +254,7 @@
settings_(settings),
rate_control_settings_(RateControlSettings::ParseFromFieldTrials()),
quality_scaler_settings_(QualityScalerSettings::ParseFromFieldTrials()),
+ encoder_selector_(settings.encoder_factory->GetEncoderSelector()),
encoder_stats_observer_(encoder_stats_observer),
encoder_initialized_(false),
max_framerate_(-1),
@@ -435,7 +436,8 @@
void VideoStreamEncoder::ReconfigureEncoder() {
RTC_DCHECK(pending_encoder_reconfiguration_);
- if (encoder_switch_experiment_.IsPixelCountBelowThreshold(
+ if (!encoder_selector_ &&
+ encoder_switch_experiment_.IsPixelCountBelowThreshold(
last_frame_info_->width * last_frame_info_->height) &&
!encoder_switch_requested_ && settings_.encoder_switch_request_callback) {
EncoderSwitchRequestCallback::Config conf;
@@ -492,6 +494,10 @@
// or just discard incoming frames?
RTC_CHECK(encoder_);
+ if (encoder_selector_) {
+ encoder_selector_->OnCurrentEncoder(encoder_config_.video_format);
+ }
+
encoder_->SetFecControllerOverride(fec_controller_override_);
codec_info_ = settings_.encoder_factory->QueryVideoEncoder(
@@ -1283,9 +1289,17 @@
if (encode_status == WEBRTC_VIDEO_CODEC_ENCODER_FAILURE) {
RTC_LOG(LS_ERROR) << "Encoder failed, failing encoder format: "
<< encoder_config_.video_format.ToString();
+
if (settings_.encoder_switch_request_callback) {
- encoder_failed_ = true;
- settings_.encoder_switch_request_callback->RequestEncoderFallback();
+ if (encoder_selector_) {
+ if (auto encoder = encoder_selector_->OnEncoderBroken()) {
+ settings_.encoder_switch_request_callback->RequestEncoderSwitch(
+ *encoder);
+ }
+ } else {
+ encoder_failed_ = true;
+ settings_.encoder_switch_request_callback->RequestEncoderFallback();
+ }
} else {
RTC_LOG(LS_ERROR)
<< "Encoder failed but no encoder fallback callback is registered";
@@ -1548,15 +1562,23 @@
}
RTC_DCHECK_RUN_ON(&encoder_queue_);
- if (encoder_switch_experiment_.IsBitrateBelowThreshold(target_bitrate) &&
- settings_.encoder_switch_request_callback && !encoder_switch_requested_) {
- EncoderSwitchRequestCallback::Config conf;
- conf.codec_name = encoder_switch_experiment_.to_codec;
- conf.param = encoder_switch_experiment_.to_param;
- conf.value = encoder_switch_experiment_.to_value;
- settings_.encoder_switch_request_callback->RequestEncoderSwitch(conf);
+ if (settings_.encoder_switch_request_callback) {
+ if (encoder_selector_) {
+ if (auto encoder = encoder_selector_->OnEncodingBitrate(target_bitrate)) {
+ settings_.encoder_switch_request_callback->RequestEncoderSwitch(
+ *encoder);
+ }
+ } else if (encoder_switch_experiment_.IsBitrateBelowThreshold(
+ target_bitrate) &&
+ !encoder_switch_requested_) {
+ EncoderSwitchRequestCallback::Config conf;
+ conf.codec_name = encoder_switch_experiment_.to_codec;
+ conf.param = encoder_switch_experiment_.to_param;
+ conf.value = encoder_switch_experiment_.to_value;
+ settings_.encoder_switch_request_callback->RequestEncoderSwitch(conf);
- encoder_switch_requested_ = true;
+ encoder_switch_requested_ = true;
+ }
}
RTC_DCHECK(sink_) << "sink_ must be set before the encoder is active.";
diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h
index 0390d7f..f8902df 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -223,6 +223,8 @@
const RateControlSettings rate_control_settings_;
const QualityScalerSettings quality_scaler_settings_;
+ std::unique_ptr<VideoEncoderFactory::EncoderSelectorInterface> const
+ encoder_selector_;
VideoStreamEncoderObserver* const encoder_stats_observer_;
// |thread_checker_| checks that public methods that are related to lifetime
// of VideoStreamEncoder are called on the same thread.
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index d7cf579..323168a 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -18,6 +18,7 @@
#include "absl/memory/memory.h"
#include "api/task_queue/default_task_queue_factory.h"
#include "api/test/mock_fec_controller_override.h"
+#include "api/test/mock_video_encoder.h"
#include "api/video/builtin_video_bitrate_allocator_factory.h"
#include "api/video/i420_buffer.h"
#include "api/video/video_bitrate_allocation.h"
@@ -51,6 +52,9 @@
using ::testing::_;
using ::testing::AllOf;
using ::testing::Field;
+using ::testing::Matcher;
+using ::testing::NiceMock;
+using ::testing::Return;
using ::testing::StrictMock;
namespace {
@@ -385,6 +389,15 @@
MOCK_METHOD1(OnBitrateAllocationUpdated, void(const VideoBitrateAllocation&));
};
+class MockEncoderSelector
+ : public VideoEncoderFactory::EncoderSelectorInterface {
+ public:
+ MOCK_METHOD1(OnCurrentEncoder, void(const SdpVideoFormat& format));
+ MOCK_METHOD1(OnEncodingBitrate,
+ absl::optional<SdpVideoFormat>(const DataRate& rate));
+ MOCK_METHOD0(OnEncoderBroken, absl::optional<SdpVideoFormat>());
+};
+
} // namespace
class VideoStreamEncoderTest : public ::testing::Test {
@@ -5122,6 +5135,8 @@
struct MockEncoderSwitchRequestCallback : public EncoderSwitchRequestCallback {
MOCK_METHOD0(RequestEncoderFallback, void());
MOCK_METHOD1(RequestEncoderSwitch, void(const Config& conf));
+ MOCK_METHOD1(RequestEncoderSwitch,
+ void(const webrtc::SdpVideoFormat& format));
};
TEST_F(VideoStreamEncoderTest, BitrateEncoderSwitch) {
@@ -5145,10 +5160,10 @@
CreateFrame(kDontCare, kDontCare, kDontCare));
using Config = EncoderSwitchRequestCallback::Config;
- EXPECT_CALL(switch_callback,
- RequestEncoderSwitch(AllOf(Field(&Config::codec_name, "AV1"),
+ EXPECT_CALL(switch_callback, RequestEncoderSwitch(Matcher<const Config&>(
+ AllOf(Field(&Config::codec_name, "AV1"),
Field(&Config::param, "ping"),
- Field(&Config::value, "pong"))));
+ Field(&Config::value, "pong")))));
video_stream_encoder_->OnBitrateUpdated(
/*target_bitrate=*/DataRate::kbps(50),
@@ -5195,10 +5210,10 @@
WaitForEncodedFrame(1);
using Config = EncoderSwitchRequestCallback::Config;
- EXPECT_CALL(switch_callback,
- RequestEncoderSwitch(AllOf(Field(&Config::codec_name, "AV1"),
+ EXPECT_CALL(switch_callback, RequestEncoderSwitch(Matcher<const Config&>(
+ AllOf(Field(&Config::codec_name, "AV1"),
Field(&Config::param, "ping"),
- Field(&Config::value, "pong"))));
+ Field(&Config::value, "pong")))));
video_source_.IncomingCapturedFrame(CreateFrame(2, kLowRes, kLowRes));
WaitForEncodedFrame(2);
@@ -5206,6 +5221,112 @@
video_stream_encoder_->Stop();
}
+TEST_F(VideoStreamEncoderTest, EncoderSelectorCurrentEncoderIsSignaled) {
+ constexpr int kDontCare = 100;
+ StrictMock<MockEncoderSelector> encoder_selector;
+ auto encoder_factory = std::make_unique<test::VideoEncoderProxyFactory>(
+ &fake_encoder_, &encoder_selector);
+ video_send_config_.encoder_settings.encoder_factory = encoder_factory.get();
+
+ // Reset encoder for new configuration to take effect.
+ ConfigureEncoder(video_encoder_config_.Copy());
+
+ EXPECT_CALL(encoder_selector, OnCurrentEncoder(_));
+
+ video_source_.IncomingCapturedFrame(
+ CreateFrame(kDontCare, kDontCare, kDontCare));
+ video_stream_encoder_->Stop();
+
+ // The encoders produces by the VideoEncoderProxyFactory have a pointer back
+ // to it's factory, so in order for the encoder instance in the
+ // |video_stream_encoder_| to be destroyed before the |encoder_factory| we
+ // reset the |video_stream_encoder_| here.
+ video_stream_encoder_.reset();
+}
+
+TEST_F(VideoStreamEncoderTest, EncoderSelectorBitrateSwitch) {
+ constexpr int kDontCare = 100;
+
+ NiceMock<MockEncoderSelector> encoder_selector;
+ StrictMock<MockEncoderSwitchRequestCallback> switch_callback;
+ video_send_config_.encoder_settings.encoder_switch_request_callback =
+ &switch_callback;
+ auto encoder_factory = std::make_unique<test::VideoEncoderProxyFactory>(
+ &fake_encoder_, &encoder_selector);
+ video_send_config_.encoder_settings.encoder_factory = encoder_factory.get();
+
+ // Reset encoder for new configuration to take effect.
+ ConfigureEncoder(video_encoder_config_.Copy());
+
+ ON_CALL(encoder_selector, OnEncodingBitrate(_))
+ .WillByDefault(Return(SdpVideoFormat("AV1")));
+ EXPECT_CALL(switch_callback,
+ RequestEncoderSwitch(Matcher<const SdpVideoFormat&>(
+ Field(&SdpVideoFormat::name, "AV1"))));
+
+ video_stream_encoder_->OnBitrateUpdated(
+ /*target_bitrate=*/DataRate::kbps(50),
+ /*stable_target_bitrate=*/DataRate::kbps(kDontCare),
+ /*link_allocation=*/DataRate::kbps(kDontCare),
+ /*fraction_lost=*/0,
+ /*rtt_ms=*/0,
+ /*cwnd_reduce_ratio=*/0);
+
+ video_stream_encoder_->Stop();
+}
+
+TEST_F(VideoStreamEncoderTest, EncoderSelectorBrokenEncoderSwitch) {
+ constexpr int kSufficientBitrateToNotDrop = 1000;
+ constexpr int kDontCare = 100;
+
+ NiceMock<MockVideoEncoder> video_encoder;
+ NiceMock<MockEncoderSelector> encoder_selector;
+ StrictMock<MockEncoderSwitchRequestCallback> switch_callback;
+ video_send_config_.encoder_settings.encoder_switch_request_callback =
+ &switch_callback;
+ auto encoder_factory = std::make_unique<test::VideoEncoderProxyFactory>(
+ &video_encoder, &encoder_selector);
+ video_send_config_.encoder_settings.encoder_factory = encoder_factory.get();
+
+ // Reset encoder for new configuration to take effect.
+ ConfigureEncoder(video_encoder_config_.Copy());
+
+ // The VideoStreamEncoder needs some bitrate before it can start encoding,
+ // setting some bitrate so that subsequent calls to WaitForEncodedFrame does
+ // not fail.
+ video_stream_encoder_->OnBitrateUpdated(
+ /*target_bitrate=*/DataRate::kbps(kSufficientBitrateToNotDrop),
+ /*stable_target_bitrate=*/DataRate::kbps(kSufficientBitrateToNotDrop),
+ /*link_allocation=*/DataRate::kbps(kSufficientBitrateToNotDrop),
+ /*fraction_lost=*/0,
+ /*rtt_ms=*/0,
+ /*cwnd_reduce_ratio=*/0);
+
+ ON_CALL(video_encoder, Encode(_, _))
+ .WillByDefault(Return(WEBRTC_VIDEO_CODEC_ENCODER_FAILURE));
+ ON_CALL(encoder_selector, OnEncoderBroken())
+ .WillByDefault(Return(SdpVideoFormat("AV2")));
+
+ rtc::Event encode_attempted;
+ EXPECT_CALL(switch_callback,
+ RequestEncoderSwitch(Matcher<const SdpVideoFormat&>(_)))
+ .WillOnce([&encode_attempted](const SdpVideoFormat& format) {
+ EXPECT_EQ(format.name, "AV2");
+ encode_attempted.Set();
+ });
+
+ video_source_.IncomingCapturedFrame(CreateFrame(1, kDontCare, kDontCare));
+ encode_attempted.Wait(3000);
+
+ video_stream_encoder_->Stop();
+
+ // The encoders produces by the VideoEncoderProxyFactory have a pointer back
+ // to it's factory, so in order for the encoder instance in the
+ // |video_stream_encoder_| to be destroyed before the |encoder_factory| we
+ // reset the |video_stream_encoder_| here.
+ video_stream_encoder_.reset();
+}
+
TEST_F(VideoStreamEncoderTest,
AllocationPropagatedToEncoderWhenTargetRateChanged) {
const int kFrameWidth = 320;
