From d4b0c0562323e24d3075b6831bafa62ea8bf32bd Mon Sep 17 00:00:00 2001 From: Magnus Jedvert Date: Thu, 14 Sep 2017 10:24:54 +0200 Subject: [PATCH] Add new video codec factories This CL adds interfaces for the new video codec factories and wires them up in WebRtcVideoEngine. The default behavior is unmodified however, and the new code is currently unused except for the tests. A follow-up CL will be uploaded for exposing them in the PeerConnectionFactory API: https://codereview.webrtc.org/3004353002/. BUG=webrtc:7925 R=andersc@webrtc.org, stefan@webrtc.org Review-Url: https://codereview.webrtc.org/3007073002 . Cr-Commit-Position: refs/heads/master@{#19828} --- webrtc/api/video_codecs/BUILD.gn | 2 + .../api/video_codecs/video_decoder_factory.h | 39 +++++ .../api/video_codecs/video_encoder_factory.h | 57 +++++++ .../media/engine/webrtcvideodecoderfactory.h | 2 + .../media/engine/webrtcvideoencoderfactory.h | 6 +- webrtc/media/engine/webrtcvideoengine.cc | 77 +++++++++- webrtc/media/engine/webrtcvideoengine.h | 9 ++ .../engine/webrtcvideoengine_unittest.cc | 145 ++++++++++++++++++ 8 files changed, 332 insertions(+), 5 deletions(-) create mode 100644 webrtc/api/video_codecs/video_decoder_factory.h create mode 100644 webrtc/api/video_codecs/video_encoder_factory.h diff --git a/webrtc/api/video_codecs/BUILD.gn b/webrtc/api/video_codecs/BUILD.gn index c8bde01496..4a050ab5cc 100644 --- a/webrtc/api/video_codecs/BUILD.gn +++ b/webrtc/api/video_codecs/BUILD.gn @@ -16,8 +16,10 @@ rtc_source_set("video_codecs_api") { sources = [ "sdp_video_format.h", "video_decoder.h", + "video_decoder_factory.h", "video_encoder.cc", "video_encoder.h", + "video_encoder_factory.h", ] deps = [ diff --git a/webrtc/api/video_codecs/video_decoder_factory.h b/webrtc/api/video_codecs/video_decoder_factory.h new file mode 100644 index 0000000000..2a442075a6 --- /dev/null +++ b/webrtc/api/video_codecs/video_decoder_factory.h @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * 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. + */ + +#ifndef WEBRTC_API_VIDEO_CODECS_VIDEO_DECODER_FACTORY_H_ +#define WEBRTC_API_VIDEO_CODECS_VIDEO_DECODER_FACTORY_H_ + +#include +#include + +namespace webrtc { + +class VideoDecoder; +struct SdpVideoFormat; + +// A factory that creates VideoDecoders. +// NOTE: This class is still under development and may change without notice. +class VideoDecoderFactory { + public: + // Returns a list of supported video formats in order of preference, to use + // for signaling etc. + virtual std::vector GetSupportedFormats() const = 0; + + // Creates a VideoDecoder for the specified format. + virtual std::unique_ptr CreateVideoDecoder( + const SdpVideoFormat& format) = 0; + + virtual ~VideoDecoderFactory() {} +}; + +} // namespace webrtc + +#endif // WEBRTC_API_VIDEO_CODECS_VIDEO_DECODER_FACTORY_H_ diff --git a/webrtc/api/video_codecs/video_encoder_factory.h b/webrtc/api/video_codecs/video_encoder_factory.h new file mode 100644 index 0000000000..4e77392c8e --- /dev/null +++ b/webrtc/api/video_codecs/video_encoder_factory.h @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * 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. + */ + +#ifndef WEBRTC_API_VIDEO_CODECS_VIDEO_ENCODER_FACTORY_H_ +#define WEBRTC_API_VIDEO_CODECS_VIDEO_ENCODER_FACTORY_H_ + +#include +#include + +namespace webrtc { + +class VideoEncoder; +struct SdpVideoFormat; + +// A factory that creates VideoEncoders. +// NOTE: This class is still under development and may change without notice. +class VideoEncoderFactory { + public: + // TODO(magjed): Try to get rid of this struct. + struct CodecInfo { + // |is_hardware_accelerated| is true if the encoders created by this factory + // of the given codec will use hardware support. + bool is_hardware_accelerated; + // |has_internal_source| is true if encoders created by this factory of the + // given codec will use internal camera sources, meaning that they don't + // require/expect frames to be delivered via webrtc::VideoEncoder::Encode. + // This flag is used as the internal_source parameter to + // webrtc::ViEExternalCodec::RegisterExternalSendCodec. + bool has_internal_source; + }; + + // Returns a list of supported video formats in order of preference, to use + // for signaling etc. + virtual std::vector GetSupportedFormats() const = 0; + + // Returns information about how this format will be encoded. The specified + // format must be one of the supported formats by this factory. + // TODO(magjed): Try to get rid of this method. + virtual CodecInfo QueryVideoEncoder(const SdpVideoFormat& format) const = 0; + + // Creates a VideoEncoder for the specified format. + virtual std::unique_ptr CreateVideoEncoder( + const SdpVideoFormat& format) = 0; + + virtual ~VideoEncoderFactory() {} +}; + +} // namespace webrtc + +#endif // WEBRTC_API_VIDEO_CODECS_VIDEO_ENCODER_FACTORY_H_ diff --git a/webrtc/media/engine/webrtcvideodecoderfactory.h b/webrtc/media/engine/webrtcvideodecoderfactory.h index d0dc3a500e..e4d1c98407 100644 --- a/webrtc/media/engine/webrtcvideodecoderfactory.h +++ b/webrtc/media/engine/webrtcvideodecoderfactory.h @@ -25,6 +25,8 @@ struct VideoDecoderParams { std::string receive_stream_id; }; +// Deprecated. Use webrtc::VideoDecoderFactory instead. +// https://bugs.chromium.org/p/webrtc/issues/detail?id=7925 class WebRtcVideoDecoderFactory { public: // Caller takes the ownership of the returned object and it should be released diff --git a/webrtc/media/engine/webrtcvideoencoderfactory.h b/webrtc/media/engine/webrtcvideoencoderfactory.h index 208ab8b16b..284f1ef133 100644 --- a/webrtc/media/engine/webrtcvideoencoderfactory.h +++ b/webrtc/media/engine/webrtcvideoencoderfactory.h @@ -22,6 +22,8 @@ class VideoEncoder; namespace cricket { +// Deprecated. Use webrtc::VideoEncoderFactory instead. +// https://bugs.chromium.org/p/webrtc/issues/detail?id=7925 class WebRtcVideoEncoderFactory { public: virtual ~WebRtcVideoEncoderFactory() {} @@ -36,9 +38,7 @@ class WebRtcVideoEncoderFactory { // Returns true if encoders created by this factory of the given codec type // will use internal camera sources, meaning that they don't require/expect - // frames to be delivered via webrtc::VideoEncoder::Encode. This flag is used - // as the internal_source parameter to - // webrtc::ViEExternalCodec::RegisterExternalSendCodec. + // frames to be delivered via webrtc::VideoEncoder::Encode. virtual bool EncoderTypeHasInternalSource(webrtc::VideoCodecType type) const { return false; } diff --git a/webrtc/media/engine/webrtcvideoengine.cc b/webrtc/media/engine/webrtcvideoengine.cc index a21f63359d..26b9397f3b 100644 --- a/webrtc/media/engine/webrtcvideoengine.cc +++ b/webrtc/media/engine/webrtcvideoengine.cc @@ -17,8 +17,11 @@ #include #include "webrtc/api/video/i420_buffer.h" +#include "webrtc/api/video_codecs/sdp_video_format.h" #include "webrtc/api/video_codecs/video_decoder.h" +#include "webrtc/api/video_codecs/video_decoder_factory.h" #include "webrtc/api/video_codecs/video_encoder.h" +#include "webrtc/api/video_codecs/video_encoder_factory.h" #include "webrtc/call/call.h" #include "webrtc/common_video/h264/profile_level_id.h" #include "webrtc/media/engine/constants.h" @@ -86,8 +89,8 @@ std::vector AssignPayloadTypesAndAddAssociatedRtxCodecs( // Wraps cricket::WebRtcVideoEncoderFactory* into common EncoderFactoryAdapter // interface. -// TODO(magjed): Add wrapper class for future webrtc::VideoEncoderFactory -// interface, https://bugs.chromium.org/p/webrtc/issues/detail?id=7925. +// TODO(magjed): Remove once WebRtcVideoEncoderFactory* is deprecated and +// webrtc:7925 is fixed. class CricketEncoderFactoryAdapter : public EncoderFactoryAdapter { public: explicit CricketEncoderFactoryAdapter( @@ -130,6 +133,66 @@ class CricketDecoderFactoryAdapter : public DecoderFactoryAdapter { WebRtcVideoDecoderFactory* const external_decoder_factory_; }; +// Wraps webrtc::VideoEncoderFactory into common EncoderFactoryAdapter +// interface. +class WebRtcEncoderFactoryAdapter : public EncoderFactoryAdapter { + public: + explicit WebRtcEncoderFactoryAdapter( + std::unique_ptr encoder_factory) + : encoder_factory_(std::move(encoder_factory)) {} + + private: + AllocatedEncoder CreateVideoEncoder( + const VideoCodec& codec, + bool is_conference_mode_screenshare) const override { + if (!encoder_factory_) + return AllocatedEncoder(); + const webrtc::SdpVideoFormat format(codec.name, codec.params); + const webrtc::VideoEncoderFactory::CodecInfo info = + encoder_factory_->QueryVideoEncoder(format); + return AllocatedEncoder(encoder_factory_->CreateVideoEncoder(format), + info.is_hardware_accelerated, + info.has_internal_source); + } + + std::vector GetSupportedCodecs() const override { + if (!encoder_factory_) + return std::vector(); + std::vector codecs; + for (const webrtc::SdpVideoFormat& format : + encoder_factory_->GetSupportedFormats()) { + VideoCodec codec; + codec.name = format.name; + codec.params = format.parameters; + codecs.push_back(codec); + } + return AssignPayloadTypesAndAddAssociatedRtxCodecs(codecs); + } + + std::unique_ptr encoder_factory_; +}; + +// Wraps webrtc::VideoDecoderFactory into common DecoderFactoryAdapter +// interface. +class WebRtcDecoderFactoryAdapter : public DecoderFactoryAdapter { + public: + explicit WebRtcDecoderFactoryAdapter( + std::unique_ptr decoder_factory) + : decoder_factory_(std::move(decoder_factory)) {} + + private: + std::unique_ptr CreateVideoDecoder( + const VideoCodec& codec, + const VideoDecoderParams& decoder_params) const override { + return decoder_factory_ + ? decoder_factory_->CreateVideoDecoder( + webrtc::SdpVideoFormat(codec.name, codec.params)) + : nullptr; + } + + std::unique_ptr decoder_factory_; +}; + // If this field trial is enabled, we will enable sending FlexFEC and disable // sending ULPFEC whenever the former has been negotiated in the SDPs. bool IsFlexfecFieldTrialEnabled() { @@ -401,6 +464,16 @@ WebRtcVideoEngine::WebRtcVideoEngine( LOG(LS_INFO) << "WebRtcVideoEngine::WebRtcVideoEngine()"; } +WebRtcVideoEngine::WebRtcVideoEngine( + std::unique_ptr video_encoder_factory, + std::unique_ptr video_decoder_factory) + : decoder_factory_( + new WebRtcDecoderFactoryAdapter(std::move(video_decoder_factory))), + encoder_factory_( + new WebRtcEncoderFactoryAdapter(std::move(video_encoder_factory))) { + LOG(LS_INFO) << "WebRtcVideoEngine::WebRtcVideoEngine()"; +} + WebRtcVideoEngine::~WebRtcVideoEngine() { LOG(LS_INFO) << "WebRtcVideoEngine::~WebRtcVideoEngine"; } diff --git a/webrtc/media/engine/webrtcvideoengine.h b/webrtc/media/engine/webrtcvideoengine.h index 063d80bf53..ff5505915b 100644 --- a/webrtc/media/engine/webrtcvideoengine.h +++ b/webrtc/media/engine/webrtcvideoengine.h @@ -38,7 +38,9 @@ namespace webrtc { class VideoDecoder; +class VideoDecoderFactory; class VideoEncoder; +class VideoEncoderFactory; struct MediaConfig; } @@ -102,6 +104,13 @@ class WebRtcVideoEngine { // codecs will be added on top of the external codecs. WebRtcVideoEngine(WebRtcVideoEncoderFactory* external_video_encoder_factory, WebRtcVideoDecoderFactory* external_video_decoder_factory); + + // These video codec factories represents all video codecs, i.e. both software + // and external hardware codecs. + WebRtcVideoEngine( + std::unique_ptr video_encoder_factory, + std::unique_ptr video_decoder_factory); + virtual ~WebRtcVideoEngine(); WebRtcVideoChannel* CreateChannel(webrtc::Call* call, diff --git a/webrtc/media/engine/webrtcvideoengine_unittest.cc b/webrtc/media/engine/webrtcvideoengine_unittest.cc index 683ccd5747..195f757726 100644 --- a/webrtc/media/engine/webrtcvideoengine_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine_unittest.cc @@ -13,7 +13,10 @@ #include #include +#include "webrtc/api/video_codecs/sdp_video_format.h" +#include "webrtc/api/video_codecs/video_decoder_factory.h" #include "webrtc/api/video_codecs/video_encoder.h" +#include "webrtc/api/video_codecs/video_encoder_factory.h" #include "webrtc/call/flexfec_receive_stream.h" #include "webrtc/common_video/h264/profile_level_id.h" #include "webrtc/logging/rtc_event_log/rtc_event_log.h" @@ -31,6 +34,7 @@ #include "webrtc/rtc_base/gunit.h" #include "webrtc/rtc_base/stringutils.h" #include "webrtc/test/field_trial.h" +#include "webrtc/test/gmock.h" using webrtc::RtpExtension; @@ -184,6 +188,9 @@ class WebRtcVideoEngineTest : public ::testing::Test { // Used in WebRtcVideoEngineVoiceTest, but defined here so it's properly // initialized when the constructor is called. std::unique_ptr call_; + // 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_; cricket::FakeWebRtcVideoDecoderFactory decoder_factory_; WebRtcVideoEngine engine_; @@ -839,6 +846,144 @@ TEST_F(WebRtcVideoEngineTest, RegisterExternalH264DecoderIfSupported) { ASSERT_EQ(1u, decoder_factory_.decoders().size()); } +class MockVideoEncoderFactory : public webrtc::VideoEncoderFactory { + public: + MOCK_CONST_METHOD0(GetSupportedFormats, + std::vector()); + MOCK_CONST_METHOD1(QueryVideoEncoder, + CodecInfo(const webrtc::SdpVideoFormat&)); + + // We need to proxy to a return type that is copyable. + std::unique_ptr CreateVideoEncoder( + const webrtc::SdpVideoFormat& format) { + return std::unique_ptr( + CreateVideoEncoderProxy(format)); + } + MOCK_METHOD1(CreateVideoEncoderProxy, + webrtc::VideoEncoder*(const webrtc::SdpVideoFormat&)); + + MOCK_METHOD0(Die, void()); + ~MockVideoEncoderFactory() { Die(); } +}; + +class MockVideoDecoderFactory : public webrtc::VideoDecoderFactory { + public: + MOCK_CONST_METHOD0(GetSupportedFormats, + std::vector()); + + // We need to proxy to a return type that is copyable. + std::unique_ptr CreateVideoDecoder( + const webrtc::SdpVideoFormat& format) { + return std::unique_ptr( + CreateVideoDecoderProxy(format)); + } + MOCK_METHOD1(CreateVideoDecoderProxy, + webrtc::VideoDecoder*(const webrtc::SdpVideoFormat&)); + + MOCK_METHOD0(Die, void()); + ~MockVideoDecoderFactory() { Die(); } +}; + +TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, NullFactories) { + std::unique_ptr encoder_factory; + std::unique_ptr decoder_factory; + WebRtcVideoEngine engine(std::move(encoder_factory), + std::move(decoder_factory)); + EXPECT_EQ(0u, engine.codecs().size()); +} + +TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, EmptyFactories) { + // |engine| take ownership of the factories. + MockVideoEncoderFactory* encoder_factory = new MockVideoEncoderFactory(); + MockVideoDecoderFactory* decoder_factory = new MockVideoDecoderFactory(); + WebRtcVideoEngine engine( + (std::unique_ptr(encoder_factory)), + (std::unique_ptr(decoder_factory))); + EXPECT_CALL(*encoder_factory, GetSupportedFormats()); + EXPECT_EQ(0u, engine.codecs().size()); + EXPECT_CALL(*encoder_factory, Die()); + EXPECT_CALL(*decoder_factory, Die()); +} + +// Test full behavior in the video engine when video codec factories of the new +// type are injected supporting the single codec Vp8. Check the returned codecs +// from the engine and that we will create a Vp8 encoder and decoder using the +// new factories. +TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) { + // |engine| take ownership of the factories. + MockVideoEncoderFactory* encoder_factory = new MockVideoEncoderFactory(); + MockVideoDecoderFactory* decoder_factory = new MockVideoDecoderFactory(); + WebRtcVideoEngine engine( + (std::unique_ptr(encoder_factory)), + (std::unique_ptr(decoder_factory))); + const webrtc::SdpVideoFormat vp8_format("VP8"); + const std::vector supported_formats = {vp8_format}; + EXPECT_CALL(*encoder_factory, GetSupportedFormats()) + .WillRepeatedly(testing::Return(supported_formats)); + + // Verify the codecs from the engine. + const std::vector engine_codecs = engine.codecs(); + // Verify an RTX codec has been added correctly. + EXPECT_EQ(2u, engine_codecs.size()); + EXPECT_EQ("VP8", engine_codecs.at(0).name); + EXPECT_EQ("rtx", engine_codecs.at(1).name); + int associated_payload_type; + EXPECT_TRUE(engine_codecs.at(1).GetParam( + cricket::kCodecParamAssociatedPayloadType, &associated_payload_type)); + EXPECT_EQ(engine_codecs.at(0).id, associated_payload_type); + // Verify default parameters has been added to the VP8 codec. + VerifyCodecHasDefaultFeedbackParams(engine_codecs.at(0)); + + // Mock encoder creation. |engine| take ownership of the encoder. + webrtc::VideoEncoderFactory::CodecInfo codec_info; + codec_info.is_hardware_accelerated = false; + codec_info.has_internal_source = false; + const webrtc::SdpVideoFormat format("VP8"); + EXPECT_CALL(*encoder_factory, QueryVideoEncoder(format)) + .WillRepeatedly(testing::Return(codec_info)); + FakeWebRtcVideoEncoder* const encoder = new FakeWebRtcVideoEncoder(); + EXPECT_CALL(*encoder_factory, CreateVideoEncoderProxy(format)) + .WillOnce(testing::Return(encoder)); + + // Mock decoder creation. |engine| take ownership of the decoder. + FakeWebRtcVideoDecoder* const decoder = new FakeWebRtcVideoDecoder(); + EXPECT_CALL(*decoder_factory, CreateVideoDecoderProxy(format)) + .WillOnce(testing::Return(decoder)); + + // Create a call. + webrtc::RtcEventLogNullImpl event_log; + std::unique_ptr call( + webrtc::Call::Create(webrtc::Call::Config(&event_log))); + + // Create send channel. + const int send_ssrc = 123; + std::unique_ptr send_channel( + engine.CreateChannel(call.get(), GetMediaConfig(), VideoOptions())); + cricket::VideoSendParameters send_parameters; + send_parameters.codecs.push_back(engine_codecs.at(0)); + EXPECT_TRUE(send_channel->SetSendParameters(send_parameters)); + send_channel->OnReadyToSend(true); + EXPECT_TRUE( + send_channel->AddSendStream(StreamParams::CreateLegacy(send_ssrc))); + EXPECT_TRUE(send_channel->SetSend(true)); + + // Create recv channel. + const int recv_ssrc = 321; + std::unique_ptr recv_channel( + engine.CreateChannel(call.get(), GetMediaConfig(), VideoOptions())); + cricket::VideoRecvParameters recv_parameters; + recv_parameters.codecs.push_back(engine_codecs.at(0)); + EXPECT_TRUE(recv_channel->SetRecvParameters(recv_parameters)); + EXPECT_TRUE(recv_channel->AddRecvStream( + cricket::StreamParams::CreateLegacy(recv_ssrc))); + + // Remove streams previously added to free the encoder and decoder instance. + EXPECT_CALL(*encoder_factory, Die()); + EXPECT_CALL(*decoder_factory, Die()); + EXPECT_TRUE(send_channel->RemoveSendStream(send_ssrc)); + EXPECT_TRUE(recv_channel->RemoveRecvStream(recv_ssrc)); +} + class WebRtcVideoChannelBaseTest : public VideoMediaChannelTest { protected: