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
diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc
index 075a692..2f1acc1 100644
--- a/talk/media/webrtc/webrtcvideoengine.cc
+++ b/talk/media/webrtc/webrtcvideoengine.cc
@@ -294,15 +294,29 @@
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 @@
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;
}
diff --git a/talk/media/webrtc/webrtcvideoengine.h b/talk/media/webrtc/webrtcvideoengine.h
index 35b34a1..5004a38 100644
--- a/talk/media/webrtc/webrtcvideoengine.h
+++ b/talk/media/webrtc/webrtcvideoengine.h
@@ -515,8 +515,11 @@
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 @@
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
diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
index 174c3e9..637ecf3 100644
--- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc
+++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
@@ -683,6 +683,9 @@
}
EXPECT_TRUE(channel->SetCapturer(ssrcs.front(), NULL));
+
+ channel.reset();
+ ASSERT_EQ(0u, encoder_factory.encoders().size());
}
TEST_F(WebRtcVideoEngine2Test, ChannelWithExternalH264CanChangeToInternalVp8) {
@@ -717,6 +720,66 @@
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 @@
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.";
diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc
index f40bb3b..18b38b6 100644
--- a/talk/media/webrtc/webrtcvideoengine_unittest.cc
+++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc
@@ -4253,7 +4253,16 @@
}
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,
- 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