libvpx-vp8: Add settings struct to constructor.

Migrate the injectable Vp8FrameBufferControllerFactory
into a settings struct, allowing for straight-forward
future extensions.

Bug: webrtc:11436
Change-Id: I53e555eb6ef88cf5b10ee8a43abd6ef9c930d100
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/170635
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30839}

modules/video_coding/BUILD.gn

@@ -435,6 +435,7 @@ rtc_library("webrtc_vp8") {
     "../../api/video_codecs:vp8_temporal_layers_factory",
     "../../common_video",
     "../../rtc_base:checks",
+    "../../rtc_base:deprecation",
     "../../rtc_base:rtc_base_approved",
     "../../rtc_base:rtc_numerics",
     "../../rtc_base/experiments:cpu_speed_experiment",

modules/video_coding/codecs/vp8/include/vp8.h

@@ -6,8 +6,6 @@
  *  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.
- *
- *  WEBRTC VP8 wrapper interface
  */
 
 #ifndef MODULES_VIDEO_CODING_CODECS_VP8_INCLUDE_VP8_H_
@@ -17,22 +15,36 @@
 
 #include "api/video_codecs/vp8_frame_buffer_controller.h"
 #include "modules/video_coding/include/video_codec_interface.h"
+#include "rtc_base/deprecation.h"
 
 namespace webrtc {
 
+// TODO(brandtr): Move these interfaces to the api/ folder.
 class VP8Encoder {
  public:
-  static std::unique_ptr<VideoEncoder> Create();
+  struct Settings {
+    // Allows for overriding the Vp8FrameBufferController used by the encoder.
+    // If unset, a default Vp8FrameBufferController will be instantiated
+    // internally.
+    std::unique_ptr<Vp8FrameBufferControllerFactory>
+        frame_buffer_controller_factory = nullptr;
 
-  static std::unique_ptr<VideoEncoder> Create(
+    // TODO(https://bugs.webrtc.org/11436): Add resolution_bitrate_limits.
+  };
+
+  static std::unique_ptr<VideoEncoder> Create();
+  static std::unique_ptr<VideoEncoder> Create(Settings settings);
+
+  RTC_DEPRECATED static std::unique_ptr<VideoEncoder> Create(
       std::unique_ptr<Vp8FrameBufferControllerFactory>
           frame_buffer_controller_factory);
-};  // end of VP8Encoder class
+};
 
 class VP8Decoder {
  public:
   static std::unique_ptr<VideoDecoder> Create();
-};  // end of VP8Decoder class
+};
+
 }  // namespace webrtc
 
 #endif  // MODULES_VIDEO_CODING_CODECS_VP8_INCLUDE_VP8_H_

modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc

@@ -221,14 +221,24 @@ void ApplyVp8EncoderConfigToVpxConfig(const Vp8EncoderConfig& encoder_config,
 }  // namespace
 
 std::unique_ptr<VideoEncoder> VP8Encoder::Create() {
-  return VP8Encoder::Create(nullptr);
+  return std::make_unique<LibvpxVp8Encoder>(LibvpxInterface::CreateEncoder(),
+                                            VP8Encoder::Settings());
+}
+
+std::unique_ptr<VideoEncoder> VP8Encoder::Create(
+    VP8Encoder::Settings settings) {
+  return std::make_unique<LibvpxVp8Encoder>(LibvpxInterface::CreateEncoder(),
+                                            std::move(settings));
 }
 
 std::unique_ptr<VideoEncoder> VP8Encoder::Create(
     std::unique_ptr<Vp8FrameBufferControllerFactory>
         frame_buffer_controller_factory) {
-  return std::make_unique<LibvpxVp8Encoder>(
+  VP8Encoder::Settings settings;
-      std::move(frame_buffer_controller_factory));
+  settings.frame_buffer_controller_factory =
+      std::move(frame_buffer_controller_factory);
+  return std::make_unique<LibvpxVp8Encoder>(LibvpxInterface::CreateEncoder(),
+                                            std::move(settings));
 }
 
 vpx_enc_frame_flags_t LibvpxVp8Encoder::EncodeFlags(
@@ -260,29 +270,15 @@ vpx_enc_frame_flags_t LibvpxVp8Encoder::EncodeFlags(
   return flags;
 }
 
-LibvpxVp8Encoder::LibvpxVp8Encoder()
+LibvpxVp8Encoder::LibvpxVp8Encoder(std::unique_ptr<LibvpxInterface> interface,
-    : LibvpxVp8Encoder(nullptr, LibvpxInterface::CreateEncoder()) {}
+                                   VP8Encoder::Settings settings)
-
-LibvpxVp8Encoder::LibvpxVp8Encoder(
-    std::unique_ptr<Vp8FrameBufferControllerFactory>
-        frame_buffer_controller_factory)
-    : LibvpxVp8Encoder(std::move(frame_buffer_controller_factory),
-                       LibvpxInterface::CreateEncoder()) {}
-
-LibvpxVp8Encoder::LibvpxVp8Encoder(std::unique_ptr<LibvpxInterface> interface)
-    : LibvpxVp8Encoder(nullptr, std::move(interface)) {}
-
-LibvpxVp8Encoder::LibvpxVp8Encoder(
-    std::unique_ptr<Vp8FrameBufferControllerFactory>
-        frame_buffer_controller_factory,
-    std::unique_ptr<LibvpxInterface> interface)
     : libvpx_(std::move(interface)),
       experimental_cpu_speed_config_arm_(CpuSpeedExperiment::GetConfigs()),
       rate_control_settings_(RateControlSettings::ParseFromFieldTrials()),
       screenshare_max_qp_(
           ExperimentalScreenshareSettings::ParseFromFieldTrials().MaxQp()),
       frame_buffer_controller_factory_(
-          std::move(frame_buffer_controller_factory)),
+          std::move(settings.frame_buffer_controller_factory)),
       key_frame_request_(kMaxSimulcastStreams, false),
       variable_framerate_experiment_(ParseVariableFramerateConfig(
           "WebRTC-VP8VariableFramerateScreenshare")),

modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h

@@ -34,13 +34,8 @@ namespace webrtc {
 
 class LibvpxVp8Encoder : public VideoEncoder {
  public:
-  LibvpxVp8Encoder();
+  LibvpxVp8Encoder(std::unique_ptr<LibvpxInterface> interface,
-  explicit LibvpxVp8Encoder(std::unique_ptr<Vp8FrameBufferControllerFactory>
+                   VP8Encoder::Settings settings);
-                                frame_buffer_controller_factory);
-  explicit LibvpxVp8Encoder(std::unique_ptr<LibvpxInterface> interface);
-  LibvpxVp8Encoder(std::unique_ptr<Vp8FrameBufferControllerFactory>
-                       frame_buffer_controller_factory,
-                   std::unique_ptr<LibvpxInterface> interface);
   ~LibvpxVp8Encoder() override;
 
   int Release() override;

modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc

@@ -124,7 +124,8 @@ TEST_F(TestVp8Impl, ErrorResilienceDisabledForNoTemporalLayers) {
   codec_settings_.simulcastStream[0].numberOfTemporalLayers = 1;
 
   auto* const vpx = new NiceMock<MockLibvpxVp8Interface>();
-  LibvpxVp8Encoder encoder((std::unique_ptr<LibvpxInterface>(vpx)));
+  LibvpxVp8Encoder encoder((std::unique_ptr<LibvpxInterface>(vpx)),
+                           VP8Encoder::Settings());
   EXPECT_CALL(*vpx,
               codec_enc_init(
                   _, _, Field(&vpx_codec_enc_cfg_t::g_error_resilient, 0), _));
@@ -137,7 +138,8 @@ TEST_F(TestVp8Impl, DefaultErrorResilienceEnabledForTemporalLayers) {
   codec_settings_.VP8()->numberOfTemporalLayers = 2;
 
   auto* const vpx = new NiceMock<MockLibvpxVp8Interface>();
-  LibvpxVp8Encoder encoder((std::unique_ptr<LibvpxInterface>(vpx)));
+  LibvpxVp8Encoder encoder((std::unique_ptr<LibvpxInterface>(vpx)),
+                           VP8Encoder::Settings());
   EXPECT_CALL(*vpx,
               codec_enc_init(_, _,
                              Field(&vpx_codec_enc_cfg_t::g_error_resilient,
@@ -155,7 +157,8 @@ TEST_F(TestVp8Impl,
   codec_settings_.VP8()->numberOfTemporalLayers = 2;
 
   auto* const vpx = new NiceMock<MockLibvpxVp8Interface>();
-  LibvpxVp8Encoder encoder((std::unique_ptr<LibvpxInterface>(vpx)));
+  LibvpxVp8Encoder encoder((std::unique_ptr<LibvpxInterface>(vpx)),
+                           VP8Encoder::Settings());
   EXPECT_CALL(*vpx,
               codec_enc_init(_, _,
                              Field(&vpx_codec_enc_cfg_t::g_error_resilient,
@@ -167,7 +170,8 @@ TEST_F(TestVp8Impl,
 
 TEST_F(TestVp8Impl, SetRates) {
   auto* const vpx = new NiceMock<MockLibvpxVp8Interface>();
-  LibvpxVp8Encoder encoder((std::unique_ptr<LibvpxInterface>(vpx)));
+  LibvpxVp8Encoder encoder((std::unique_ptr<LibvpxInterface>(vpx)),
+                           VP8Encoder::Settings());
   EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK,
             encoder.InitEncode(&codec_settings_,
                                VideoEncoder::Settings(kCapabilities, 1, 1000)));
@@ -194,7 +198,8 @@ TEST_F(TestVp8Impl, DynamicSetRates) {
   test::ScopedFieldTrials field_trials(
       "WebRTC-VideoRateControl/vp8_dynamic_rate:true/");
   auto* const vpx = new NiceMock<MockLibvpxVp8Interface>();
-  LibvpxVp8Encoder encoder((std::unique_ptr<LibvpxInterface>(vpx)));
+  LibvpxVp8Encoder encoder((std::unique_ptr<LibvpxInterface>(vpx)),
+                           VP8Encoder::Settings());
   EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK,
             encoder.InitEncode(&codec_settings_,
                                VideoEncoder::Settings(kCapabilities, 1, 1000)));
@@ -514,7 +519,8 @@ TEST_F(TestVp8Impl, DontDropKeyframes) {
 
 TEST_F(TestVp8Impl, KeepsTimestampOnReencode) {
   auto* const vpx = new NiceMock<MockLibvpxVp8Interface>();
-  LibvpxVp8Encoder encoder((std::unique_ptr<LibvpxInterface>(vpx)));
+  LibvpxVp8Encoder encoder((std::unique_ptr<LibvpxInterface>(vpx)),
+                           VP8Encoder::Settings());
 
   // Settings needed to trigger ScreenshareLayers usage, which is required for
   // overshoot-drop-reencode logic.

sdk/android/src/jni/vp8_codec.cc

@@ -19,7 +19,7 @@ namespace webrtc {
 namespace jni {
 
 static jlong JNI_LibvpxVp8Encoder_CreateEncoder(JNIEnv* jni) {
-  return jlongFromPointer(VP8Encoder::Create(nullptr).release());
+  return jlongFromPointer(VP8Encoder::Create().release());
 }
 
 static jlong JNI_LibvpxVp8Decoder_CreateDecoder(JNIEnv* jni) {