Modify the simulcast encoder factory adapter to allow external encoder factories that support more than one codec.

Only VP8 encoders will be wrapped in the simulcast adapter; other codec types will be created directly with the real encoder factory and cleaned up appropriately.

BUG=
R=pbos@webrtc.org, pthatcher@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/40169004

Cr-Commit-Position: refs/heads/master@{#8623}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8623 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/media/webrtc/webrtcvideoengine.cc

@@ -294,15 +294,29 @@ WebRtcSimulcastEncoderFactory::~WebRtcSimulcastEncoderFactory() {
 
 bool WebRtcSimulcastEncoderFactory::UseSimulcastEncoderFactory(
     const std::vector<WebRtcVideoEncoderFactory::VideoCodec>& codecs) {
-  return codecs.size() == 1 && codecs[0].type == webrtc::kVideoCodecVP8;
+  // If any codec is VP8, use the simulcast factory. If asked to create a
+  // non-VP8 codec, we'll just return a contained factory encoder directly.
+  for (const auto& codec: codecs) {
+    if (codec.type == webrtc::kVideoCodecVP8) {
+      return true;
+    }
+  }
+  return false;
 }
 
 webrtc::VideoEncoder* WebRtcSimulcastEncoderFactory::CreateVideoEncoder(
     webrtc::VideoCodecType type) {
-  ASSERT(type == webrtc::kVideoCodecVP8);
   ASSERT(factory_ != NULL);
-  return new webrtc::SimulcastEncoderAdapter(
-      new EncoderFactoryAdapter(factory_));
+  // If it's a codec type we can simulcast, create a wrapped encoder.
+  if (type == webrtc::kVideoCodecVP8) {
+    return new webrtc::SimulcastEncoderAdapter(
+        new EncoderFactoryAdapter(factory_));
+  }
+  webrtc::VideoEncoder* encoder = factory_->CreateVideoEncoder(type);
+  if (encoder) {
+    non_simulcast_encoders_.push_back(encoder);
+  }
+  return encoder;
 }
 
 const std::vector<WebRtcVideoEncoderFactory::VideoCodec>&
@@ -312,6 +326,17 @@ WebRtcSimulcastEncoderFactory::codecs() const {
 
 void WebRtcSimulcastEncoderFactory::DestroyVideoEncoder(
     webrtc::VideoEncoder* encoder) {
+  // Check first to see if the encoder wasn't wrapped in a
+  // SimulcastEncoderAdapter. In that case, ask the factory to destroy it.
+  if (std::remove(non_simulcast_encoders_.begin(),
+                  non_simulcast_encoders_.end(), encoder) !=
+      non_simulcast_encoders_.end()) {
+    factory_->DestroyVideoEncoder(encoder);
+    return;
+  }
+
+  // Otherwise, SimulcastEncoderAdapter can be deleted directly, and will call
+  // DestroyVideoEncoder on the factory for individual encoder instances.
   delete encoder;
 }
 

talk/media/webrtc/webrtcvideoengine.h

@@ -515,8 +515,11 @@ class WebRtcVideoMediaChannel : public rtc::MessageHandler,
   int ratio_h_;
 };
 
-// Wrap encoder factory to a simulcast encoder factory. Exposed here for code to
-// be shared with WebRtcVideoEngine2, not to be used externally.
+// An encoder factory that wraps Create requests for simulcastable codec types
+// with a webrtc::SimulcastEncoderAdapter. Non simulcastable codec type
+// requests are just passed through to the contained encoder factory.
+// Exposed here for code to be shared with WebRtcVideoEngine2, not to be used
+// externally.
 class WebRtcSimulcastEncoderFactory
     : public cricket::WebRtcVideoEncoderFactory {
  public:
@@ -535,6 +538,9 @@ class WebRtcSimulcastEncoderFactory
 
  private:
   cricket::WebRtcVideoEncoderFactory* factory_;
+  // A list of encoders that were created without being wrapped in a
+  // SimulcastEncoderAdapter.
+  std::vector<webrtc::VideoEncoder*> non_simulcast_encoders_;
 };
 
 }  // namespace cricket

talk/media/webrtc/webrtcvideoengine2_unittest.cc

@@ -683,6 +683,9 @@ TEST_F(WebRtcVideoEngine2Test, UsesSimulcastAdapterForVp8Factories) {
   }
 
   EXPECT_TRUE(channel->SetCapturer(ssrcs.front(), NULL));
+
+  channel.reset();
+  ASSERT_EQ(0u, encoder_factory.encoders().size());
 }
 
 TEST_F(WebRtcVideoEngine2Test, ChannelWithExternalH264CanChangeToInternalVp8) {
@@ -717,6 +720,66 @@ TEST_F(WebRtcVideoEngine2Test,
 
   EXPECT_TRUE(
       channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc)));
+  // Make sure DestroyVideoEncoder was called on the factory.
+  ASSERT_EQ(0u, encoder_factory.encoders().size());
+}
+
+TEST_F(WebRtcVideoEngine2Test,
+       UsesSimulcastAdapterForVp8WithCombinedVP8AndH264Factory) {
+  cricket::FakeWebRtcVideoEncoderFactory encoder_factory;
+  encoder_factory.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8, "VP8");
+  encoder_factory.AddSupportedVideoCodecType(webrtc::kVideoCodecH264, "H264");
+
+  std::vector<cricket::VideoCodec> codecs;
+  codecs.push_back(kVp8Codec);
+
+  rtc::scoped_ptr<VideoMediaChannel> channel(
+      SetUpForExternalEncoderFactory(&encoder_factory, codecs));
+
+  std::vector<uint32> ssrcs = MAKE_VECTOR(kSsrcs3);
+
+  EXPECT_TRUE(
+      channel->AddSendStream(CreateSimStreamParams("cname", ssrcs)));
+  EXPECT_TRUE(channel->SetSend(true));
+
+  // Send a fake frame, or else the media engine will configure the simulcast
+  // encoder adapter at a low-enough size that it'll only create a single
+  // encoder layer.
+  cricket::FakeVideoCapturer capturer;
+  EXPECT_TRUE(channel->SetCapturer(ssrcs.front(), &capturer));
+  EXPECT_EQ(cricket::CS_RUNNING,
+            capturer.Start(capturer.GetSupportedFormats()->front()));
+  EXPECT_TRUE(capturer.CaptureFrame());
+
+  ASSERT_GT(encoder_factory.encoders().size(), 1u);
+  EXPECT_EQ(webrtc::kVideoCodecVP8,
+            encoder_factory.encoders()[0]->GetCodecSettings().codecType);
+
+  channel.reset();
+  // Make sure DestroyVideoEncoder was called on the factory.
+  EXPECT_EQ(0u, encoder_factory.encoders().size());
+}
+
+TEST_F(WebRtcVideoEngine2Test,
+       DestroysNonSimulcastEncoderFromCombinedVP8AndH264Factory) {
+  cricket::FakeWebRtcVideoEncoderFactory encoder_factory;
+  encoder_factory.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8, "VP8");
+  encoder_factory.AddSupportedVideoCodecType(webrtc::kVideoCodecH264, "H264");
+
+  std::vector<cricket::VideoCodec> codecs;
+  codecs.push_back(kH264Codec);
+
+  rtc::scoped_ptr<VideoMediaChannel> channel(
+      SetUpForExternalEncoderFactory(&encoder_factory, codecs));
+
+  EXPECT_TRUE(
+      channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc)));
+  ASSERT_EQ(1u, encoder_factory.encoders().size());
+  EXPECT_EQ(webrtc::kVideoCodecH264,
+            encoder_factory.encoders()[0]->GetCodecSettings().codecType);
+
+  channel.reset();
+  // Make sure DestroyVideoEncoder was called on the factory.
   ASSERT_EQ(0u, encoder_factory.encoders().size());
 }
 
@@ -2656,12 +2719,6 @@ TEST_F(WebRtcVideoEngine2SimulcastTest,
   FAIL() << "Not implemented.";
 }
 
-TEST_F(WebRtcVideoEngine2SimulcastTest,
-       DISABLED_DontUseSimulcastAdapterOnMultipleCodecsFactory) {
-  // TODO(pbos): Implement.
-  FAIL() << "Not implemented.";
-}
-
 TEST_F(WebRtcVideoChannel2SimulcastTest, DISABLED_SimulcastSend_1280x800) {
   // TODO(pbos): Implement.
   FAIL() << "Not implemented.";

talk/media/webrtc/webrtcvideoengine_unittest.cc

@@ -4253,7 +4253,16 @@ TEST_F(WebRtcVideoEngineSimulcastTestFake,
 }
 
 TEST_F(WebRtcVideoEngineSimulcastTestFake,
-       DontUseSimulcastAdapterOnNoneVp8Factory) {
+       UsesSimulcastAdapterForVp8WithCombinedVP8AndH264Factory) {
+  encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8, "VP8");
+  encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecGeneric,
+                                              "H264");
+  TestSimulcastAdapter(kVP8Codec, true);
+}
+
+TEST_F(WebRtcVideoEngineSimulcastTestFake,
+       DontUseSimulcastAdapterForH264WithCombinedVP8AndH264Factory) {
+  encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8, "VP8");
   encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecGeneric,
                                               "H264");
   static const cricket::VideoCodec kH264Codec(100, "H264", 640, 400, 30, 0);
@@ -4261,11 +4270,11 @@ TEST_F(WebRtcVideoEngineSimulcastTestFake,
 }
 
 TEST_F(WebRtcVideoEngineSimulcastTestFake,
-    DontUseSimulcastAdapterOnMultipleCodecsFactory) {
-  encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecVP8, "VP8");
+       DontUseSimulcastAdapterOnNonVp8Factory) {
   encoder_factory_.AddSupportedVideoCodecType(webrtc::kVideoCodecGeneric,
                                               "H264");
-  TestSimulcastAdapter(kVP8Codec, false);
+  static const cricket::VideoCodec kH264Codec(100, "H264", 640, 400, 30, 0);
+  TestSimulcastAdapter(kH264Codec, false);
 }
 
 // Flaky on Windows and tsan. https://code.google.com/p/webrtc/issues/detail?id=4135