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}

api/video/video_stream_encoder_settings.h

@@ -34,6 +34,8 @@ class EncoderSwitchRequestCallback {
 
   // 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 {

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 @@ class VideoEncoderFactory {
     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 @@ class VideoEncoderFactory {
   virtual std::unique_ptr<VideoEncoder> CreateVideoEncoder(
       const SdpVideoFormat& format) = 0;
 
+  virtual std::unique_ptr<EncoderSelectorInterface> GetEncoderSelector() const {
+    return nullptr;
+  }
+
   virtual ~VideoEncoderFactory() {}
 };
 

media/engine/webrtc_video_engine.cc

@@ -774,6 +774,31 @@ void WebRtcVideoChannel::RequestEncoderSwitch(
   });
 }
 
+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_);

media/engine/webrtc_video_engine.h

@@ -211,8 +211,11 @@ class WebRtcVideoChannel : public VideoMediaChannel,
 
   // 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,

test/video_encoder_proxy_factory.h

@@ -30,7 +30,12 @@ const VideoEncoder::Capabilities kCapabilities(false);
 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 @@ class VideoEncoderProxyFactory final : public VideoEncoderFactory {
     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 @@ class VideoEncoderProxyFactory final : public VideoEncoderFactory {
     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_;

video/BUILD.gn

@@ -534,6 +534,7 @@ if (rtc_include_tests) {
       "../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",

video/video_stream_encoder.cc

@@ -254,6 +254,7 @@ VideoStreamEncoder::VideoStreamEncoder(
       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::ConfigureEncoder(VideoEncoderConfig config,
 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 @@ void VideoStreamEncoder::ReconfigureEncoder() {
     // 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 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame,
     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) {
+        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,8 +1562,15 @@ void VideoStreamEncoder::OnBitrateUpdated(DataRate target_bitrate,
   }
   RTC_DCHECK_RUN_ON(&encoder_queue_);
 
-  if (encoder_switch_experiment_.IsBitrateBelowThreshold(target_bitrate) &&
+  if (settings_.encoder_switch_request_callback) {
-      settings_.encoder_switch_request_callback && !encoder_switch_requested_) {
+    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;
@@ -1558,6 +1579,7 @@ void VideoStreamEncoder::OnBitrateUpdated(DataRate target_bitrate,
 
       encoder_switch_requested_ = true;
     }
+  }
 
   RTC_DCHECK(sink_) << "sink_ must be set before the encoder is active.";
 

video/video_stream_encoder.h

@@ -223,6 +223,8 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
   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.

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 ScaleReason = AdaptationObserverInterface::AdaptReason;
 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 @@ class MockBitrateObserver : public VideoBitrateAllocationObserver {
   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 @@ TEST_F(VideoStreamEncoderTest, EncoderRatesPropagatedOnReconfigure) {
 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 @@ TEST_F(VideoStreamEncoderTest, BitrateEncoderSwitch) {
       CreateFrame(kDontCare, kDontCare, kDontCare));
 
   using Config = EncoderSwitchRequestCallback::Config;
-  EXPECT_CALL(switch_callback,
+  EXPECT_CALL(switch_callback, RequestEncoderSwitch(Matcher<const Config&>(
-              RequestEncoderSwitch(AllOf(Field(&Config::codec_name, "AV1"),
+                                   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 @@ TEST_F(VideoStreamEncoderTest, ResolutionEncoderSwitch) {
   WaitForEncodedFrame(1);
 
   using Config = EncoderSwitchRequestCallback::Config;
-  EXPECT_CALL(switch_callback,
+  EXPECT_CALL(switch_callback, RequestEncoderSwitch(Matcher<const Config&>(
-              RequestEncoderSwitch(AllOf(Field(&Config::codec_name, "AV1"),
+                                   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 @@ TEST_F(VideoStreamEncoderTest, ResolutionEncoderSwitch) {
   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;