Remove unnecessary video factory references in PeerConnectionFactory

The video codec factories should be owned by the video engine instead
of by the PeerConnectionFactory.

Bug: None
Change-Id: If63d47cef565138d51377af3fc9ea973950c9390
Reviewed-on: https://webrtc-review.googlesource.com/1601
Reviewed-by: Anders Carlsson <andersc@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Magnus Jedvert <magjed@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20002}

api/peerconnectioninterface.h

@@ -1229,8 +1229,6 @@ CreateModularPeerConnectionFactory(
     rtc::Thread* network_thread,
     rtc::Thread* worker_thread,
     rtc::Thread* signaling_thread,
-    cricket::WebRtcVideoEncoderFactory* video_encoder_factory,
-    cricket::WebRtcVideoDecoderFactory* video_decoder_factory,
     std::unique_ptr<cricket::MediaEngineInterface> media_engine,
     std::unique_ptr<CallFactoryInterface> call_factory,
     std::unique_ptr<RtcEventLogFactoryInterface> event_log_factory);

media/base/videoengine_unittest.h

@@ -27,6 +27,11 @@
 #include "rtc_base/gunit.h"
 #include "rtc_base/timeutils.h"
 
+namespace cricket {
+class WebRtcVideoEncoderFactory;
+class WebRtcVideoDecoderFactory;
+}  // namespace cricket
+
 #define EXPECT_FRAME_WAIT(c, w, h, t) \
   EXPECT_EQ_WAIT((c), renderer_.num_rendered_frames(), (t)); \
   EXPECT_EQ((w), renderer_.width()); \
@@ -65,8 +70,8 @@ class VideoMediaChannelTest : public testing::Test,
  protected:
   VideoMediaChannelTest<E, C>()
       : call_(webrtc::Call::Create(webrtc::Call::Config(&event_log_))),
-        engine_(nullptr /* external_encoder_factory */,
+        engine_(std::unique_ptr<cricket::WebRtcVideoEncoderFactory>(),
-                nullptr /* external_decoder_factory */) {}
+                std::unique_ptr<cricket::WebRtcVideoDecoderFactory>()) {}
 
   virtual cricket::VideoCodec DefaultCodec() = 0;
 

media/engine/webrtcmediaengine.cc

@@ -40,8 +40,11 @@ MediaEngineInterface* CreateWebRtcMediaEngine(
     rtc::scoped_refptr<webrtc::AudioProcessing> audio_processing) {
 #ifdef HAVE_WEBRTC_VIDEO
   typedef WebRtcVideoEngine VideoEngine;
-  std::tuple<WebRtcVideoEncoderFactory*, WebRtcVideoDecoderFactory*> video_args(
+  std::tuple<std::unique_ptr<WebRtcVideoEncoderFactory>,
-      video_encoder_factory, video_decoder_factory);
+             std::unique_ptr<WebRtcVideoDecoderFactory>>
+      video_args(
+          (std::unique_ptr<WebRtcVideoEncoderFactory>(video_encoder_factory)),
+          (std::unique_ptr<WebRtcVideoDecoderFactory>(video_decoder_factory)));
 #else
   typedef NullWebRtcVideoEngine VideoEngine;
   std::tuple<> video_args;

media/engine/webrtcvideoengine.cc

@@ -87,22 +87,18 @@ namespace {
 std::vector<VideoCodec> AssignPayloadTypesAndAddAssociatedRtxCodecs(
     const std::vector<VideoCodec>& input_codecs);
 
-// Wraps cricket::WebRtcVideoEncoderFactory* into common EncoderFactoryAdapter
+// Wraps cricket::WebRtcVideoEncoderFactory into common EncoderFactoryAdapter
 // interface.
-// TODO(magjed): Remove once WebRtcVideoEncoderFactory* is deprecated and
+// TODO(magjed): Remove once WebRtcVideoEncoderFactory is deprecated and
 // webrtc:7925 is fixed.
 class CricketEncoderFactoryAdapter : public EncoderFactoryAdapter {
  public:
   explicit CricketEncoderFactoryAdapter(
-      WebRtcVideoEncoderFactory* external_encoder_factory)
+      std::unique_ptr<WebRtcVideoEncoderFactory> external_encoder_factory)
       : internal_encoder_factory_(new InternalEncoderFactory()),
-        external_encoder_factory_(external_encoder_factory) {}
+        external_encoder_factory_(std::move(external_encoder_factory)) {}
 
  private:
-  explicit CricketEncoderFactoryAdapter(
-      const CricketEncoderFactoryAdapter& other)
-      : CricketEncoderFactoryAdapter(other.external_encoder_factory_) {}
-
   AllocatedEncoder CreateVideoEncoder(
       const VideoCodec& codec,
       bool is_conference_mode_screenshare) const override;
@@ -110,27 +106,23 @@ class CricketEncoderFactoryAdapter : public EncoderFactoryAdapter {
   std::vector<VideoCodec> GetSupportedCodecs() const override;
 
   const std::unique_ptr<WebRtcVideoEncoderFactory> internal_encoder_factory_;
-  WebRtcVideoEncoderFactory* const external_encoder_factory_;
+  const std::unique_ptr<WebRtcVideoEncoderFactory> external_encoder_factory_;
 };
 
 class CricketDecoderFactoryAdapter : public DecoderFactoryAdapter {
  public:
   explicit CricketDecoderFactoryAdapter(
-      WebRtcVideoDecoderFactory* external_decoder_factory)
+      std::unique_ptr<WebRtcVideoDecoderFactory> external_decoder_factory)
       : internal_decoder_factory_(new InternalDecoderFactory()),
-        external_decoder_factory_(external_decoder_factory) {}
+        external_decoder_factory_(std::move(external_decoder_factory)) {}
 
  private:
-  explicit CricketDecoderFactoryAdapter(
-      const CricketDecoderFactoryAdapter& other)
-      : CricketDecoderFactoryAdapter(other.external_decoder_factory_) {}
-
   std::unique_ptr<webrtc::VideoDecoder> CreateVideoDecoder(
       const VideoCodec& codec,
       const VideoDecoderParams& decoder_params) const override;
 
   const std::unique_ptr<WebRtcVideoDecoderFactory> internal_decoder_factory_;
-  WebRtcVideoDecoderFactory* const external_decoder_factory_;
+  const std::unique_ptr<WebRtcVideoDecoderFactory> external_decoder_factory_;
 };
 
 // Wraps webrtc::VideoEncoderFactory into common EncoderFactoryAdapter
@@ -455,12 +447,12 @@ void DefaultUnsignalledSsrcHandler::SetDefaultSink(
 }
 
 WebRtcVideoEngine::WebRtcVideoEngine(
-    WebRtcVideoEncoderFactory* external_video_encoder_factory,
+    std::unique_ptr<WebRtcVideoEncoderFactory> external_video_encoder_factory,
-    WebRtcVideoDecoderFactory* external_video_decoder_factory)
+    std::unique_ptr<WebRtcVideoDecoderFactory> external_video_decoder_factory)
-    : decoder_factory_(
+    : decoder_factory_(new CricketDecoderFactoryAdapter(
-          new CricketDecoderFactoryAdapter(external_video_decoder_factory)),
+          std::move(external_video_decoder_factory))),
-      encoder_factory_(
+      encoder_factory_(new CricketEncoderFactoryAdapter(
-          new CricketEncoderFactoryAdapter(external_video_encoder_factory)) {
+          std::move(external_video_encoder_factory))) {
   LOG(LS_INFO) << "WebRtcVideoEngine::WebRtcVideoEngine()";
 }
 
@@ -1707,10 +1699,10 @@ CricketEncoderFactoryAdapter::CreateVideoEncoder(
     if (CodecNamesEq(codec.name.c_str(), kVp8CodecName)) {
       // If it's a codec type we can simulcast, create a wrapped encoder.
       external_encoder = std::unique_ptr<webrtc::VideoEncoder>(
-          new webrtc::SimulcastEncoderAdapter(external_encoder_factory_));
+          new webrtc::SimulcastEncoderAdapter(external_encoder_factory_.get()));
     } else {
       external_encoder =
-          CreateScopedVideoEncoder(external_encoder_factory_, codec);
+          CreateScopedVideoEncoder(external_encoder_factory_.get(), codec);
     }
     if (external_encoder) {
       std::unique_ptr<webrtc::VideoEncoder> internal_encoder(
@@ -2207,7 +2199,7 @@ CricketDecoderFactoryAdapter::CreateVideoDecoder(
     const VideoDecoderParams& decoder_params) const {
   if (external_decoder_factory_ != nullptr) {
     std::unique_ptr<webrtc::VideoDecoder> external_decoder =
-        CreateScopedVideoDecoder(external_decoder_factory_, codec,
+        CreateScopedVideoDecoder(external_decoder_factory_.get(), codec,
                                  decoder_params);
     if (external_decoder) {
       webrtc::VideoCodecType type =

media/engine/webrtcvideoengine.h

@@ -99,11 +99,11 @@ class DefaultUnsignalledSsrcHandler : public UnsignalledSsrcHandler {
 // WebRtcVideoEngine is used for the new native WebRTC Video API (webrtc:1667).
 class WebRtcVideoEngine {
  public:
-  // Video engine does not take ownership of the video codec factories and the
+  // Internal SW video codecs will be added on top of the external codecs.
-  // caller needs to make sure they outlive the video engine. Internal SW video
+  WebRtcVideoEngine(
-  // codecs will be added on top of the external codecs.
+      std::unique_ptr<WebRtcVideoEncoderFactory> external_video_encoder_factory,
-  WebRtcVideoEngine(WebRtcVideoEncoderFactory* external_video_encoder_factory,
+      std::unique_ptr<WebRtcVideoDecoderFactory>
-                    WebRtcVideoDecoderFactory* external_video_decoder_factory);
+          external_video_decoder_factory);
 
   // These video codec factories represents all video codecs, i.e. both software
   // and external hardware codecs.

media/engine/webrtcvideoengine_unittest.cc

@@ -151,7 +151,12 @@ class WebRtcVideoEngineTest : public ::testing::Test {
   explicit WebRtcVideoEngineTest(const char* field_trials)
       : override_field_trials_(field_trials),
         call_(webrtc::Call::Create(webrtc::Call::Config(&event_log_))),
-        engine_(&encoder_factory_, &decoder_factory_) {
+        encoder_factory_(new cricket::FakeWebRtcVideoEncoderFactory),
+        decoder_factory_(new cricket::FakeWebRtcVideoDecoderFactory),
+        engine_(std::unique_ptr<cricket::WebRtcVideoEncoderFactory>(
+                    encoder_factory_),
+                std::unique_ptr<cricket::WebRtcVideoDecoderFactory>(
+                    decoder_factory_)) {
     std::vector<VideoCodec> engine_codecs = engine_.codecs();
     RTC_DCHECK(!engine_codecs.empty());
     bool codec_set = false;
@@ -191,8 +196,9 @@ class WebRtcVideoEngineTest : public ::testing::Test {
   // TODO(magjed): Update all tests to use new video codec factories once the
   // old factories are deprecated,
   // https://bugs.chromium.org/p/webrtc/issues/detail?id=7925.
-  cricket::FakeWebRtcVideoEncoderFactory encoder_factory_;
+  // These factories are owned by the video engine.
-  cricket::FakeWebRtcVideoDecoderFactory decoder_factory_;
+  cricket::FakeWebRtcVideoEncoderFactory* encoder_factory_;
+  cricket::FakeWebRtcVideoDecoderFactory* decoder_factory_;
   WebRtcVideoEngine engine_;
   VideoCodec default_codec_;
   std::map<int, int> default_apt_rtx_types_;
@@ -280,7 +286,7 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionBeforeCapturer) {
   // dtor is called.
   cricket::FakeVideoCapturer capturer;
 
-  encoder_factory_.AddSupportedVideoCodecType("VP8");
+  encoder_factory_->AddSupportedVideoCodecType("VP8");
 
   std::unique_ptr<VideoMediaChannel> channel(SetUpForExternalEncoderFactory());
   EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(kSsrc)));
@@ -310,7 +316,7 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionBeforeAddSendStream) {
   // dtor is called.
   cricket::FakeVideoCapturer capturer;
 
-  encoder_factory_.AddSupportedVideoCodecType("VP8");
+  encoder_factory_->AddSupportedVideoCodecType("VP8");
 
   std::unique_ptr<VideoMediaChannel> channel(SetUpForExternalEncoderFactory());
   // Add CVO extension.
@@ -332,8 +338,8 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionBeforeAddSendStream) {
 TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionAfterCapturer) {
   cricket::FakeVideoCapturer capturer;
 
-  encoder_factory_.AddSupportedVideoCodecType("VP8");
+  encoder_factory_->AddSupportedVideoCodecType("VP8");
-  encoder_factory_.AddSupportedVideoCodecType("VP9");
+  encoder_factory_->AddSupportedVideoCodecType("VP9");
 
   std::unique_ptr<VideoMediaChannel> channel(SetUpForExternalEncoderFactory());
   EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(kSsrc)));
@@ -385,14 +391,14 @@ TEST_F(WebRtcVideoEngineTest, GetStatsWithoutSendCodecsSetDoesNotCrash) {
 }
 
 TEST_F(WebRtcVideoEngineTest, UseExternalFactoryForVp8WhenSupported) {
-  encoder_factory_.AddSupportedVideoCodecType("VP8");
+  encoder_factory_->AddSupportedVideoCodecType("VP8");
 
   std::unique_ptr<VideoMediaChannel> channel(SetUpForExternalEncoderFactory());
   channel->OnReadyToSend(true);
 
   EXPECT_TRUE(
       channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc)));
-  EXPECT_EQ(0, encoder_factory_.GetNumCreatedEncoders());
+  EXPECT_EQ(0, encoder_factory_->GetNumCreatedEncoders());
   EXPECT_TRUE(channel->SetSend(true));
   cricket::FakeVideoCapturer capturer;
   EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, nullptr, &capturer));
@@ -400,11 +406,11 @@ TEST_F(WebRtcVideoEngineTest, UseExternalFactoryForVp8WhenSupported) {
             capturer.Start(capturer.GetSupportedFormats()->front()));
   EXPECT_TRUE(capturer.CaptureFrame());
   // Sending one frame will have allocate the encoder.
-  ASSERT_TRUE(encoder_factory_.WaitForCreatedVideoEncoders(1));
+  ASSERT_TRUE(encoder_factory_->WaitForCreatedVideoEncoders(1));
-  EXPECT_TRUE_WAIT(encoder_factory_.encoders()[0]->GetNumEncodedFrames() > 0,
+  EXPECT_TRUE_WAIT(encoder_factory_->encoders()[0]->GetNumEncodedFrames() > 0,
                    kTimeout);
 
-  int num_created_encoders = encoder_factory_.GetNumCreatedEncoders();
+  int num_created_encoders = encoder_factory_->GetNumCreatedEncoders();
   EXPECT_EQ(num_created_encoders, 1);
 
   // Setting codecs of the same type should not reallocate any encoders
@@ -412,11 +418,11 @@ TEST_F(WebRtcVideoEngineTest, UseExternalFactoryForVp8WhenSupported) {
   cricket::VideoSendParameters parameters;
   parameters.codecs.push_back(GetEngineCodec("VP8"));
   EXPECT_TRUE(channel->SetSendParameters(parameters));
-  EXPECT_EQ(num_created_encoders, encoder_factory_.GetNumCreatedEncoders());
+  EXPECT_EQ(num_created_encoders, encoder_factory_->GetNumCreatedEncoders());
 
   // Remove stream previously added to free the external encoder instance.
   EXPECT_TRUE(channel->RemoveSendStream(kSsrc));
-  EXPECT_EQ(0u, encoder_factory_.encoders().size());
+  EXPECT_EQ(0u, encoder_factory_->encoders().size());
 }
 
 // Test that when an external encoder factory supports a codec we don't
@@ -440,9 +446,9 @@ TEST_F(WebRtcVideoEngineTest, RtxCodecAddedForExternalCodec) {
   h264_high.params[kH264FmtpProfileLevelId] = *ProfileLevelIdToString(
       ProfileLevelId(webrtc::H264::kProfileHigh, kLevel1));
 
-  encoder_factory_.AddSupportedVideoCodec(h264_constrained_baseline);
+  encoder_factory_->AddSupportedVideoCodec(h264_constrained_baseline);
-  encoder_factory_.AddSupportedVideoCodec(h264_constrained_high);
+  encoder_factory_->AddSupportedVideoCodec(h264_constrained_high);
-  encoder_factory_.AddSupportedVideoCodec(h264_high);
+  encoder_factory_->AddSupportedVideoCodec(h264_high);
 
   // First figure out what payload types the test codecs got assigned.
   const std::vector<cricket::VideoCodec> codecs = engine_.codecs();
@@ -462,7 +468,7 @@ void WebRtcVideoEngineTest::TestExtendedEncoderOveruse(
   FakeCall* fake_call = new FakeCall(webrtc::Call::Config(&event_log_));
   call_.reset(fake_call);
   if (use_external_encoder) {
-    encoder_factory_.AddSupportedVideoCodecType("VP8");
+    encoder_factory_->AddSupportedVideoCodecType("VP8");
     channel.reset(SetUpForExternalEncoderFactory());
   } else {
     channel.reset(
@@ -492,7 +498,7 @@ TEST_F(WebRtcVideoEngineTest, DisablesFullEncoderTimeForNonExternalEncoders) {
 
 #if !defined(RTC_DISABLE_VP9)
 TEST_F(WebRtcVideoEngineTest, CanConstructDecoderForVp9EncoderFactory) {
-  encoder_factory_.AddSupportedVideoCodecType("VP9");
+  encoder_factory_->AddSupportedVideoCodecType("VP9");
 
   std::unique_ptr<VideoMediaChannel> channel(SetUpForExternalEncoderFactory());
 
@@ -502,7 +508,7 @@ TEST_F(WebRtcVideoEngineTest, CanConstructDecoderForVp9EncoderFactory) {
 #endif  // !defined(RTC_DISABLE_VP9)
 
 TEST_F(WebRtcVideoEngineTest, PropagatesInputFrameTimestamp) {
-  encoder_factory_.AddSupportedVideoCodecType("VP8");
+  encoder_factory_->AddSupportedVideoCodecType("VP8");
   FakeCall* fake_call = new FakeCall(webrtc::Call::Config(&event_log_));
   call_.reset(fake_call);
   std::unique_ptr<VideoMediaChannel> channel(SetUpForExternalEncoderFactory());
@@ -571,7 +577,7 @@ VideoMediaChannel* WebRtcVideoEngineTest::SetUpForExternalEncoderFactory() {
       engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions());
   cricket::VideoSendParameters parameters;
   // We need to look up the codec in the engine to get the correct payload type.
-  for (const VideoCodec& codec : encoder_factory_.supported_codecs())
+  for (const VideoCodec& codec : encoder_factory_->supported_codecs())
     parameters.codecs.push_back(GetEngineCodec(codec.name));
 
   EXPECT_TRUE(channel->SetSendParameters(parameters));
@@ -591,7 +597,7 @@ VideoMediaChannel* WebRtcVideoEngineTest::SetUpForExternalDecoderFactory(
 }
 
 TEST_F(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8Factories) {
-  encoder_factory_.AddSupportedVideoCodecType("VP8");
+  encoder_factory_->AddSupportedVideoCodecType("VP8");
 
   std::unique_ptr<VideoMediaChannel> channel(SetUpForExternalEncoderFactory());
 
@@ -607,15 +613,15 @@ TEST_F(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8Factories) {
             capturer.Start(capturer.GetSupportedFormats()->front()));
   EXPECT_TRUE(capturer.CaptureFrame());
 
-  ASSERT_TRUE(encoder_factory_.WaitForCreatedVideoEncoders(2));
+  ASSERT_TRUE(encoder_factory_->WaitForCreatedVideoEncoders(2));
 
   // Verify that encoders are configured for simulcast through adapter
   // (increasing resolution and only configured to send one stream each).
   int prev_width = -1;
-  for (size_t i = 0; i < encoder_factory_.encoders().size(); ++i) {
+  for (size_t i = 0; i < encoder_factory_->encoders().size(); ++i) {
-    ASSERT_TRUE(encoder_factory_.encoders()[i]->WaitForInitEncode());
+    ASSERT_TRUE(encoder_factory_->encoders()[i]->WaitForInitEncode());
     webrtc::VideoCodec codec_settings =
-        encoder_factory_.encoders()[i]->GetCodecSettings();
+        encoder_factory_->encoders()[i]->GetCodecSettings();
     EXPECT_EQ(0, codec_settings.numberOfSimulcastStreams);
     EXPECT_GT(codec_settings.width, prev_width);
     prev_width = codec_settings.width;
@@ -624,27 +630,27 @@ TEST_F(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8Factories) {
   EXPECT_TRUE(channel->SetVideoSend(ssrcs.front(), true, nullptr, nullptr));
 
   channel.reset();
-  ASSERT_EQ(0u, encoder_factory_.encoders().size());
+  ASSERT_EQ(0u, encoder_factory_->encoders().size());
 }
 
 TEST_F(WebRtcVideoEngineTest, ChannelWithExternalH264CanChangeToInternalVp8) {
-  encoder_factory_.AddSupportedVideoCodecType("H264");
+  encoder_factory_->AddSupportedVideoCodecType("H264");
 
   std::unique_ptr<VideoMediaChannel> channel(SetUpForExternalEncoderFactory());
 
   EXPECT_TRUE(
       channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc)));
-  ASSERT_EQ(1u, encoder_factory_.encoders().size());
+  ASSERT_EQ(1u, encoder_factory_->encoders().size());
 
   cricket::VideoSendParameters parameters;
   parameters.codecs.push_back(GetEngineCodec("VP8"));
   EXPECT_TRUE(channel->SetSendParameters(parameters));
-  ASSERT_EQ(0u, encoder_factory_.encoders().size());
+  ASSERT_EQ(0u, encoder_factory_->encoders().size());
 }
 
 TEST_F(WebRtcVideoEngineTest,
        DontUseExternalEncoderFactoryForUnsupportedCodecs) {
-  encoder_factory_.AddSupportedVideoCodecType("H264");
+  encoder_factory_->AddSupportedVideoCodecType("H264");
 
   std::unique_ptr<VideoMediaChannel> channel(
       engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions()));
@@ -655,13 +661,13 @@ TEST_F(WebRtcVideoEngineTest,
   EXPECT_TRUE(
       channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc)));
   // Make sure DestroyVideoEncoder was called on the factory.
-  ASSERT_EQ(0u, encoder_factory_.encoders().size());
+  ASSERT_EQ(0u, encoder_factory_->encoders().size());
 }
 
 TEST_F(WebRtcVideoEngineTest,
        UsesSimulcastAdapterForVp8WithCombinedVP8AndH264Factory) {
-  encoder_factory_.AddSupportedVideoCodecType("VP8");
+  encoder_factory_->AddSupportedVideoCodecType("VP8");
-  encoder_factory_.AddSupportedVideoCodecType("H264");
+  encoder_factory_->AddSupportedVideoCodecType("H264");
 
   std::unique_ptr<VideoMediaChannel> channel(
       engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions()));
@@ -684,20 +690,20 @@ TEST_F(WebRtcVideoEngineTest,
             capturer.Start(capturer.GetSupportedFormats()->front()));
   EXPECT_TRUE(capturer.CaptureFrame());
 
-  ASSERT_TRUE(encoder_factory_.WaitForCreatedVideoEncoders(2));
+  ASSERT_TRUE(encoder_factory_->WaitForCreatedVideoEncoders(2));
-  ASSERT_TRUE(encoder_factory_.encoders()[0]->WaitForInitEncode());
+  ASSERT_TRUE(encoder_factory_->encoders()[0]->WaitForInitEncode());
   EXPECT_EQ(webrtc::kVideoCodecVP8,
-            encoder_factory_.encoders()[0]->GetCodecSettings().codecType);
+            encoder_factory_->encoders()[0]->GetCodecSettings().codecType);
 
   channel.reset();
   // Make sure DestroyVideoEncoder was called on the factory.
-  EXPECT_EQ(0u, encoder_factory_.encoders().size());
+  EXPECT_EQ(0u, encoder_factory_->encoders().size());
 }
 
 TEST_F(WebRtcVideoEngineTest,
        DestroysNonSimulcastEncoderFromCombinedVP8AndH264Factory) {
-  encoder_factory_.AddSupportedVideoCodecType("VP8");
+  encoder_factory_->AddSupportedVideoCodecType("VP8");
-  encoder_factory_.AddSupportedVideoCodecType("H264");
+  encoder_factory_->AddSupportedVideoCodecType("H264");
 
   std::unique_ptr<VideoMediaChannel> channel(
       engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions()));
@@ -707,7 +713,7 @@ TEST_F(WebRtcVideoEngineTest,
 
   EXPECT_TRUE(
       channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc)));
-  ASSERT_EQ(1u, encoder_factory_.encoders().size());
+  ASSERT_EQ(1u, encoder_factory_->encoders().size());
 
   // Send a frame of 720p. This should trigger a "real" encoder initialization.
   cricket::VideoFormat format(
@@ -716,17 +722,17 @@ TEST_F(WebRtcVideoEngineTest,
   EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, nullptr, &capturer));
   EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(format));
   EXPECT_TRUE(capturer.CaptureFrame());
-  ASSERT_TRUE(encoder_factory_.encoders()[0]->WaitForInitEncode());
+  ASSERT_TRUE(encoder_factory_->encoders()[0]->WaitForInitEncode());
   EXPECT_EQ(webrtc::kVideoCodecH264,
-            encoder_factory_.encoders()[0]->GetCodecSettings().codecType);
+            encoder_factory_->encoders()[0]->GetCodecSettings().codecType);
 
   channel.reset();
   // Make sure DestroyVideoEncoder was called on the factory.
-  ASSERT_EQ(0u, encoder_factory_.encoders().size());
+  ASSERT_EQ(0u, encoder_factory_->encoders().size());
 }
 
 TEST_F(WebRtcVideoEngineTest, SimulcastDisabledForH264) {
-  encoder_factory_.AddSupportedVideoCodecType("H264");
+  encoder_factory_->AddSupportedVideoCodecType("H264");
 
   std::unique_ptr<VideoMediaChannel> channel(SetUpForExternalEncoderFactory());
 
@@ -742,9 +748,9 @@ TEST_F(WebRtcVideoEngineTest, SimulcastDisabledForH264) {
   EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(format));
   EXPECT_TRUE(capturer.CaptureFrame());
 
-  ASSERT_EQ(1u, encoder_factory_.encoders().size());
+  ASSERT_EQ(1u, encoder_factory_->encoders().size());
-  FakeWebRtcVideoEncoder* encoder = encoder_factory_.encoders()[0];
+  FakeWebRtcVideoEncoder* encoder = encoder_factory_->encoders()[0];
-  ASSERT_TRUE(encoder_factory_.encoders()[0]->WaitForInitEncode());
+  ASSERT_TRUE(encoder_factory_->encoders()[0]->WaitForInitEncode());
   EXPECT_EQ(webrtc::kVideoCodecH264, encoder->GetCodecSettings().codecType);
   EXPECT_EQ(1u, encoder->GetCodecSettings().numberOfSimulcastStreams);
   EXPECT_TRUE(channel->SetVideoSend(ssrcs[0], true, nullptr, nullptr));
@@ -777,7 +783,7 @@ TEST_F(WebRtcVideoEngineTest,
 
 // Test that external codecs are added to the end of the supported codec list.
 TEST_F(WebRtcVideoEngineTest, ReportSupportedExternalCodecs) {
-  encoder_factory_.AddSupportedVideoCodecType("FakeExternalCodec");
+  encoder_factory_->AddSupportedVideoCodecType("FakeExternalCodec");
 
   std::vector<cricket::VideoCodec> codecs(engine_.codecs());
   ASSERT_GE(codecs.size(), 2u);
@@ -793,21 +799,21 @@ TEST_F(WebRtcVideoEngineTest, ReportSupportedExternalCodecs) {
 // does show up in the codec list after it was added.
 TEST_F(WebRtcVideoEngineTest, ReportSupportedExternalCodecsWithAddedCodec) {
   // Set up external encoder factory with first codec, and initialize engine.
-  encoder_factory_.AddSupportedVideoCodecType("FakeExternalCodec1");
+  encoder_factory_->AddSupportedVideoCodecType("FakeExternalCodec1");
 
   // The first external codec will appear last in the vector.
   std::vector<cricket::VideoCodec> codecs_before(engine_.codecs());
   EXPECT_EQ("FakeExternalCodec1", codecs_before.back().name);
 
   // Add second codec.
-  encoder_factory_.AddSupportedVideoCodecType("FakeExternalCodec2");
+  encoder_factory_->AddSupportedVideoCodecType("FakeExternalCodec2");
   std::vector<cricket::VideoCodec> codecs_after(engine_.codecs());
   EXPECT_EQ(codecs_before.size() + 1, codecs_after.size());
   EXPECT_EQ("FakeExternalCodec2", codecs_after.back().name);
 }
 
 TEST_F(WebRtcVideoEngineTest, RegisterExternalDecodersIfSupported) {
-  decoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8);
+  decoder_factory_->AddSupportedVideoCodecType(webrtc::kVideoCodecVP8);
   cricket::VideoRecvParameters parameters;
   parameters.codecs.push_back(GetEngineCodec("VP8"));
 
@@ -816,15 +822,15 @@ TEST_F(WebRtcVideoEngineTest, RegisterExternalDecodersIfSupported) {
 
   EXPECT_TRUE(
       channel->AddRecvStream(cricket::StreamParams::CreateLegacy(kSsrc)));
-  ASSERT_EQ(1u, decoder_factory_.decoders().size());
+  ASSERT_EQ(1u, decoder_factory_->decoders().size());
 
   // Setting codecs of the same type should not reallocate the decoder.
   EXPECT_TRUE(channel->SetRecvParameters(parameters));
-  EXPECT_EQ(1, decoder_factory_.GetNumCreatedDecoders());
+  EXPECT_EQ(1, decoder_factory_->GetNumCreatedDecoders());
 
   // Remove stream previously added to free the external decoder instance.
   EXPECT_TRUE(channel->RemoveRecvStream(kSsrc));
-  EXPECT_EQ(0u, decoder_factory_.decoders().size());
+  EXPECT_EQ(0u, decoder_factory_->decoders().size());
 }
 
 // Verifies that we can set up decoders that are not internally supported.
@@ -833,8 +839,8 @@ TEST_F(WebRtcVideoEngineTest, RegisterExternalH264DecoderIfSupported) {
   // can't even query the WebRtcVideoDecoderFactory for supported codecs.
   // For now we add a FakeWebRtcVideoEncoderFactory to add H264 to supported
   // codecs.
-  encoder_factory_.AddSupportedVideoCodecType("H264");
+  encoder_factory_->AddSupportedVideoCodecType("H264");
-  decoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecH264);
+  decoder_factory_->AddSupportedVideoCodecType(webrtc::kVideoCodecH264);
   std::vector<cricket::VideoCodec> codecs;
   codecs.push_back(GetEngineCodec("H264"));
 
@@ -843,7 +849,7 @@ TEST_F(WebRtcVideoEngineTest, RegisterExternalH264DecoderIfSupported) {
 
   EXPECT_TRUE(
       channel->AddRecvStream(cricket::StreamParams::CreateLegacy(kSsrc)));
-  ASSERT_EQ(1u, decoder_factory_.decoders().size());
+  ASSERT_EQ(1u, decoder_factory_->decoders().size());
 }
 
 class MockVideoEncoderFactory : public webrtc::VideoEncoderFactory {
@@ -1004,7 +1010,7 @@ class WebRtcVideoChannelBaseTest
 
 // Verifies that id given in stream params is passed to the decoder factory.
 TEST_F(WebRtcVideoEngineTest, StreamParamsIdPassedToDecoderFactory) {
-  decoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8);
+  decoder_factory_->AddSupportedVideoCodecType(webrtc::kVideoCodecVP8);
   cricket::VideoRecvParameters parameters;
   parameters.codecs.push_back(GetEngineCodec("VP8"));
 
@@ -1014,15 +1020,15 @@ TEST_F(WebRtcVideoEngineTest, StreamParamsIdPassedToDecoderFactory) {
   StreamParams sp = cricket::StreamParams::CreateLegacy(kSsrc);
   sp.id = "FakeStreamParamsId";
   EXPECT_TRUE(channel->AddRecvStream(sp));
-  EXPECT_EQ(1u, decoder_factory_.decoders().size());
+  EXPECT_EQ(1u, decoder_factory_->decoders().size());
 
-  std::vector<cricket::VideoDecoderParams> params = decoder_factory_.params();
+  std::vector<cricket::VideoDecoderParams> params = decoder_factory_->params();
   ASSERT_EQ(1u, params.size());
   EXPECT_EQ(sp.id, params[0].receive_stream_id);
 }
 
 TEST_F(WebRtcVideoEngineTest, DISABLED_RecreatesEncoderOnContentTypeChange) {
-  encoder_factory_.AddSupportedVideoCodecType("VP8");
+  encoder_factory_->AddSupportedVideoCodecType("VP8");
   std::unique_ptr<FakeCall> fake_call(
       new FakeCall(webrtc::Call::Config(&event_log_)));
   std::unique_ptr<VideoMediaChannel> channel(SetUpForExternalEncoderFactory());
@@ -1042,14 +1048,14 @@ TEST_F(WebRtcVideoEngineTest, DISABLED_RecreatesEncoderOnContentTypeChange) {
   EXPECT_EQ(cricket::CS_RUNNING,
             capturer.Start(capturer.GetSupportedFormats()->front()));
   EXPECT_TRUE(capturer.CaptureFrame());
-  ASSERT_TRUE(encoder_factory_.WaitForCreatedVideoEncoders(1));
+  ASSERT_TRUE(encoder_factory_->WaitForCreatedVideoEncoders(1));
   EXPECT_EQ(webrtc::kRealtimeVideo,
-            encoder_factory_.encoders().back()->GetCodecSettings().mode);
+            encoder_factory_->encoders().back()->GetCodecSettings().mode);
 
   EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, &options, &capturer));
   EXPECT_TRUE(capturer.CaptureFrame());
   // No change in content type, keep current encoder.
-  EXPECT_EQ(1, encoder_factory_.GetNumCreatedEncoders());
+  EXPECT_EQ(1, encoder_factory_->GetNumCreatedEncoders());
 
   options.is_screencast.emplace(true);
   EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, &options, &capturer));
@@ -1057,14 +1063,14 @@ TEST_F(WebRtcVideoEngineTest, DISABLED_RecreatesEncoderOnContentTypeChange) {
   // Change to screen content, recreate encoder. For the simulcast encoder
   // adapter case, this will result in two calls since InitEncode triggers a
   // a new instance.
-  ASSERT_TRUE(encoder_factory_.WaitForCreatedVideoEncoders(2));
+  ASSERT_TRUE(encoder_factory_->WaitForCreatedVideoEncoders(2));
   EXPECT_EQ(webrtc::kScreensharing,
-            encoder_factory_.encoders().back()->GetCodecSettings().mode);
+            encoder_factory_->encoders().back()->GetCodecSettings().mode);
 
   EXPECT_TRUE(channel->SetVideoSend(kSsrc, true, &options, &capturer));
   EXPECT_TRUE(capturer.CaptureFrame());
   // Still screen content, no need to update encoder.
-  EXPECT_EQ(2, encoder_factory_.GetNumCreatedEncoders());
+  EXPECT_EQ(2, encoder_factory_->GetNumCreatedEncoders());
 
   options.is_screencast.emplace(false);
   options.video_noise_reduction.emplace(false);
@@ -1072,13 +1078,13 @@ TEST_F(WebRtcVideoEngineTest, DISABLED_RecreatesEncoderOnContentTypeChange) {
   // Change back to regular video content, update encoder. Also change
   // a non |is_screencast| option just to verify it doesn't affect recreation.
   EXPECT_TRUE(capturer.CaptureFrame());
-  ASSERT_TRUE(encoder_factory_.WaitForCreatedVideoEncoders(3));
+  ASSERT_TRUE(encoder_factory_->WaitForCreatedVideoEncoders(3));
   EXPECT_EQ(webrtc::kRealtimeVideo,
-            encoder_factory_.encoders().back()->GetCodecSettings().mode);
+            encoder_factory_->encoders().back()->GetCodecSettings().mode);
 
   // Remove stream previously added to free the external encoder instance.
   EXPECT_TRUE(channel->RemoveSendStream(kSsrc));
-  EXPECT_EQ(0u, encoder_factory_.encoders().size());
+  EXPECT_EQ(0u, encoder_factory_->encoders().size());
 }
 
 #define WEBRTC_BASE_TEST(test) \
@@ -2059,7 +2065,7 @@ class Vp9SettingsTest : public WebRtcVideoChannelTest {
   Vp9SettingsTest() : Vp9SettingsTest("") {}
   explicit Vp9SettingsTest(const char* field_trials)
       : WebRtcVideoChannelTest(field_trials) {
-    encoder_factory_.AddSupportedVideoCodecType("VP9");
+    encoder_factory_->AddSupportedVideoCodecType("VP9");
   }
   virtual ~Vp9SettingsTest() {}
 
@@ -4583,7 +4589,12 @@ class WebRtcVideoChannelSimulcastTest : public testing::Test {
  public:
   WebRtcVideoChannelSimulcastTest()
       : fake_call_(webrtc::Call::Config(&event_log_)),
-        engine_(&encoder_factory_, &decoder_factory_),
+        encoder_factory_(new cricket::FakeWebRtcVideoEncoderFactory),
+        decoder_factory_(new cricket::FakeWebRtcVideoDecoderFactory),
+        engine_(std::unique_ptr<cricket::WebRtcVideoEncoderFactory>(
+                    encoder_factory_),
+                std::unique_ptr<cricket::WebRtcVideoDecoderFactory>(
+                    decoder_factory_)),
         last_ssrc_(0) {}
 
   void SetUp() override {
@@ -4742,8 +4753,8 @@ class WebRtcVideoChannelSimulcastTest : public testing::Test {
 
   webrtc::RtcEventLogNullImpl event_log_;
   FakeCall fake_call_;
-  cricket::FakeWebRtcVideoEncoderFactory encoder_factory_;
+  cricket::FakeWebRtcVideoEncoderFactory* encoder_factory_;
-  cricket::FakeWebRtcVideoDecoderFactory decoder_factory_;
+  cricket::FakeWebRtcVideoDecoderFactory* decoder_factory_;
   WebRtcVideoEngine engine_;
   std::unique_ptr<VideoMediaChannel> channel_;
   uint32_t last_ssrc_;

pc/createpeerconnectionfactory.cc

@@ -71,9 +71,8 @@ rtc::scoped_refptr<PeerConnectionFactoryInterface> CreatePeerConnectionFactory(
       CreateRtcEventLogFactory();
 
   return CreateModularPeerConnectionFactory(
-      network_thread, worker_thread, signaling_thread, video_encoder_factory,
+      network_thread, worker_thread, signaling_thread, std::move(media_engine),
-      video_decoder_factory, std::move(media_engine), std::move(call_factory),
+      std::move(call_factory), std::move(event_log_factory));
-      std::move(event_log_factory));
 }
 
 rtc::scoped_refptr<PeerConnectionFactoryInterface> CreatePeerConnectionFactory(
@@ -102,9 +101,7 @@ rtc::scoped_refptr<PeerConnectionFactoryInterface> CreatePeerConnectionFactory(
       CreateRtcEventLogFactory();
 
   return CreateModularPeerConnectionFactory(
-      network_thread, worker_thread, signaling_thread,
+      network_thread, worker_thread, signaling_thread, std::move(media_engine),
-      nullptr /* external_video_encoder_factory */,
-      nullptr /* external_video_decoder_factory */, std::move(media_engine),
       std::move(call_factory), std::move(event_log_factory));
 }
 

pc/peerconnectionfactory.cc

@@ -46,16 +46,14 @@ CreateModularPeerConnectionFactory(
     rtc::Thread* network_thread,
     rtc::Thread* worker_thread,
     rtc::Thread* signaling_thread,
-    cricket::WebRtcVideoEncoderFactory* video_encoder_factory,
-    cricket::WebRtcVideoDecoderFactory* video_decoder_factory,
     std::unique_ptr<cricket::MediaEngineInterface> media_engine,
     std::unique_ptr<CallFactoryInterface> call_factory,
     std::unique_ptr<RtcEventLogFactoryInterface> event_log_factory) {
   rtc::scoped_refptr<PeerConnectionFactory> pc_factory(
       new rtc::RefCountedObject<PeerConnectionFactory>(
           network_thread, worker_thread, signaling_thread,
-          video_encoder_factory, video_decoder_factory, std::move(media_engine),
+          std::move(media_engine), std::move(call_factory),
-          std::move(call_factory), std::move(event_log_factory)));
+          std::move(event_log_factory)));
 
   // Call Initialize synchronously but make sure it is executed on
   // |signaling_thread|.
@@ -74,8 +72,6 @@ PeerConnectionFactory::PeerConnectionFactory(
     rtc::Thread* network_thread,
     rtc::Thread* worker_thread,
     rtc::Thread* signaling_thread,
-    cricket::WebRtcVideoEncoderFactory* video_encoder_factory,
-    cricket::WebRtcVideoDecoderFactory* video_decoder_factory,
     std::unique_ptr<cricket::MediaEngineInterface> media_engine,
     std::unique_ptr<webrtc::CallFactoryInterface> call_factory,
     std::unique_ptr<RtcEventLogFactoryInterface> event_log_factory)
@@ -83,8 +79,6 @@ PeerConnectionFactory::PeerConnectionFactory(
       network_thread_(network_thread),
       worker_thread_(worker_thread),
       signaling_thread_(signaling_thread),
-      video_encoder_factory_(video_encoder_factory),
-      video_decoder_factory_(video_decoder_factory),
       media_engine_(std::move(media_engine)),
       call_factory_(std::move(call_factory)),
       event_log_factory_(std::move(event_log_factory)) {

pc/peerconnectionfactory.h

@@ -109,8 +109,6 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface {
       rtc::Thread* network_thread,
       rtc::Thread* worker_thread,
       rtc::Thread* signaling_thread,
-      cricket::WebRtcVideoEncoderFactory* video_encoder_factory,
-      cricket::WebRtcVideoDecoderFactory* video_decoder_factory,
       std::unique_ptr<cricket::MediaEngineInterface> media_engine,
       std::unique_ptr<webrtc::CallFactoryInterface> call_factory,
       std::unique_ptr<RtcEventLogFactoryInterface> event_log_factory);
@@ -128,12 +126,6 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface {
   std::unique_ptr<rtc::Thread> owned_worker_thread_;
   Options options_;
   std::unique_ptr<cricket::ChannelManager> channel_manager_;
-  // External Video encoder factory. This can be NULL if the client has not
-  // injected any. In that case, video engine will use the internal SW encoder.
-  std::unique_ptr<cricket::WebRtcVideoEncoderFactory> video_encoder_factory_;
-  // External Video decoder factory. This can be NULL if the client has not
-  // injected any. In that case, video engine will use the internal SW decoder.
-  std::unique_ptr<cricket::WebRtcVideoDecoderFactory> video_decoder_factory_;
   std::unique_ptr<rtc::BasicNetworkManager> default_network_manager_;
   std::unique_ptr<rtc::BasicPacketSocketFactory> default_socket_factory_;
   std::unique_ptr<cricket::MediaEngineInterface> media_engine_;

pc/peerconnectioninterface_unittest.cc

@@ -660,7 +660,7 @@ class PeerConnectionFactoryForTest : public webrtc::PeerConnectionFactory {
 
     return new rtc::RefCountedObject<PeerConnectionFactoryForTest>(
         rtc::Thread::Current(), rtc::Thread::Current(), rtc::Thread::Current(),
-        nullptr, nullptr, std::move(media_engine), std::move(call_factory),
+        std::move(media_engine), std::move(call_factory),
         std::move(event_log_factory));
   }
 
@@ -668,16 +668,12 @@ class PeerConnectionFactoryForTest : public webrtc::PeerConnectionFactory {
       rtc::Thread* network_thread,
       rtc::Thread* worker_thread,
       rtc::Thread* signaling_thread,
-      cricket::WebRtcVideoEncoderFactory* video_encoder_factory,
-      cricket::WebRtcVideoDecoderFactory* video_decoder_factory,
       std::unique_ptr<cricket::MediaEngineInterface> media_engine,
       std::unique_ptr<webrtc::CallFactoryInterface> call_factory,
       std::unique_ptr<webrtc::RtcEventLogFactoryInterface> event_log_factory)
       : webrtc::PeerConnectionFactory(network_thread,
                                       worker_thread,
                                       signaling_thread,
-                                      video_encoder_factory,
-                                      video_decoder_factory,
                                       std::move(media_engine),
                                       std::move(call_factory),
                                       std::move(event_log_factory)) {}

pc/test/mock_peerconnection.h

@@ -30,8 +30,6 @@ class FakePeerConnectionFactory
             rtc::Thread::Current(),
             rtc::Thread::Current(),
             rtc::Thread::Current(),
-            nullptr,
-            nullptr,
             std::unique_ptr<cricket::MediaEngineInterface>(),
             std::unique_ptr<webrtc::CallFactoryInterface>(),
             std::unique_ptr<RtcEventLogFactoryInterface>()) {}

sdk/android/src/jni/pc/peerconnectionfactory_jni.cc

@@ -197,8 +197,8 @@ JNI_FUNCTION_DECLARATION(
   rtc::scoped_refptr<PeerConnectionFactoryInterface> factory(
       CreateModularPeerConnectionFactory(
           network_thread.get(), worker_thread.get(), signaling_thread.get(),
-          video_encoder_factory, video_decoder_factory, std::move(media_engine),
+          std::move(media_engine), std::move(call_factory),
-          std::move(call_factory), std::move(rtc_event_log_factory)));
+          std::move(rtc_event_log_factory)));
   RTC_CHECK(factory) << "Failed to create the peer connection factory; "
                      << "WebRTC/libjingle init likely failed on this device";
   // TODO(honghaiz): Maybe put the options as the argument of

sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnectionFactory.mm

@@ -110,8 +110,6 @@
         _networkThread.get(),
         _workerThread.get(),
         _signalingThread.get(),
-        nullptr,  // video_encoder_factory
-        nullptr,  // video_decoder_factory
         std::unique_ptr<cricket::MediaEngineInterface>(),
         std::unique_ptr<webrtc::CallFactoryInterface>(),
         std::unique_ptr<webrtc::RtcEventLogFactoryInterface>());