From dd40702357dbd09e4468eee295193f9fd4a7da7b Mon Sep 17 00:00:00 2001 From: magjed Date: Thu, 1 Dec 2016 00:27:27 -0800 Subject: [PATCH] Move VideoDecoder::Create() logic to separate internal video decoder factory The goal with this CL is to move implementation details out from the webrtc root (webrtc/video_decoder.h) to simplify the dependency graph. Another goal is to streamline the creation of VideoDecoders in webrtcvideoengine2.cc; it will now have two factories of the same WebRtcVideoDecoderFactory type, one internal and one external. Specifically, this CL: * Removes webrtc::VideoDecoder::DecoderType and use webrtc::VideoCodecType instead. * Removes 'static VideoDecoder* Create(DecoderType codec_type)' and moves the create function to the internal decoder factory instead. * Removes video_decoder.cc. webrtc::VideoDecoder is now just an interface without any static functions. BUG=webrtc:6743 Review-Url: https://codereview.webrtc.org/2521203002 Cr-Commit-Position: refs/heads/master@{#15350} --- webrtc/media/BUILD.gn | 3 + webrtc/media/engine/internaldecoderfactory.cc | 90 +++++++++++++++++++ webrtc/media/engine/internaldecoderfactory.h | 34 +++++++ .../engine/internaldecoderfactory_unittest.cc | 21 +++++ .../videodecodersoftwarefallbackwrapper.cc | 29 ++---- .../videodecodersoftwarefallbackwrapper.h | 2 +- webrtc/media/engine/webrtcvideoengine2.cc | 25 ++---- webrtc/test/encoder_settings.cc | 10 ++- webrtc/video/BUILD.gn | 1 - webrtc/video/video_decoder.cc | 78 ---------------- webrtc/video_decoder.h | 32 ------- 11 files changed, 167 insertions(+), 158 deletions(-) create mode 100644 webrtc/media/engine/internaldecoderfactory.cc create mode 100644 webrtc/media/engine/internaldecoderfactory.h create mode 100644 webrtc/media/engine/internaldecoderfactory_unittest.cc delete mode 100644 webrtc/video/video_decoder.cc diff --git a/webrtc/media/BUILD.gn b/webrtc/media/BUILD.gn index 93f98a974d..f88ff495b5 100644 --- a/webrtc/media/BUILD.gn +++ b/webrtc/media/BUILD.gn @@ -136,6 +136,8 @@ rtc_static_library("rtc_media") { "base/videocommon.h", "base/videoframe.h", "base/videosourcebase.h", + "engine/internaldecoderfactory.cc", + "engine/internaldecoderfactory.h", "engine/internalencoderfactory.cc", "engine/internalencoderfactory.h", "engine/nullwebrtcvideoengine.h", @@ -348,6 +350,7 @@ if (rtc_include_tests) { "base/videocapturer_unittest.cc", "base/videocommon_unittest.cc", "base/videoengine_unittest.h", + "engine/internaldecoderfactory_unittest.cc", "engine/nullwebrtcvideoengine_unittest.cc", "engine/payload_type_mapper_unittest.cc", "engine/simulcast_unittest.cc", diff --git a/webrtc/media/engine/internaldecoderfactory.cc b/webrtc/media/engine/internaldecoderfactory.cc new file mode 100644 index 0000000000..760575df55 --- /dev/null +++ b/webrtc/media/engine/internaldecoderfactory.cc @@ -0,0 +1,90 @@ +/* + * Copyright (c) 2016 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. + */ + +#include "webrtc/media/engine/internaldecoderfactory.h" + +#include + +#include "webrtc/base/logging.h" +#include "webrtc/modules/video_coding/codecs/h264/include/h264.h" +#include "webrtc/modules/video_coding/codecs/vp8/include/vp8.h" +#include "webrtc/modules/video_coding/codecs/vp9/include/vp9.h" + +namespace cricket { + +namespace { + +// Video decoder class to be used for unknown codecs. Doesn't support decoding +// but logs messages to LS_ERROR. +class NullVideoDecoder : public webrtc::VideoDecoder { + public: + int32_t InitDecode(const webrtc::VideoCodec* codec_settings, + int32_t number_of_cores) override { + LOG(LS_ERROR) << "Can't initialize NullVideoDecoder."; + return WEBRTC_VIDEO_CODEC_OK; + } + + int32_t Decode(const webrtc::EncodedImage& input_image, + bool missing_frames, + const webrtc::RTPFragmentationHeader* fragmentation, + const webrtc::CodecSpecificInfo* codec_specific_info, + int64_t render_time_ms) override { + LOG(LS_ERROR) << "The NullVideoDecoder doesn't support decoding."; + return WEBRTC_VIDEO_CODEC_OK; + } + + int32_t RegisterDecodeCompleteCallback( + webrtc::DecodedImageCallback* callback) override { + LOG(LS_ERROR) + << "Can't register decode complete callback on NullVideoDecoder."; + return WEBRTC_VIDEO_CODEC_OK; + } + + int32_t Release() override { return WEBRTC_VIDEO_CODEC_OK; } + + const char* ImplementationName() const override { return "NullVideoDecoder"; } +}; + +} // anonymous namespace + +InternalDecoderFactory::InternalDecoderFactory() {} + +InternalDecoderFactory::~InternalDecoderFactory() {} + +// WebRtcVideoDecoderFactory implementation. +webrtc::VideoDecoder* InternalDecoderFactory::CreateVideoDecoder( + webrtc::VideoCodecType type) { + switch (type) { + case webrtc::kVideoCodecH264: + if (webrtc::H264Decoder::IsSupported()) + return webrtc::H264Decoder::Create(); + // This could happen in a software-fallback for a codec type only + // supported externally (e.g. H.264 on iOS or Android) or in current usage + // in WebRtcVideoEngine2 if the external decoder fails to be created. + LOG(LS_ERROR) << "Unable to create an H.264 decoder fallback. " + << "Decoding of this stream will be broken."; + return new NullVideoDecoder(); + case webrtc::kVideoCodecVP8: + return webrtc::VP8Decoder::Create(); + case webrtc::kVideoCodecVP9: + RTC_DCHECK(webrtc::VP9Decoder::IsSupported()); + return webrtc::VP9Decoder::Create(); + default: + LOG(LS_ERROR) << "Creating NullVideoDecoder for unsupported codec."; + return new NullVideoDecoder(); + } +} + +void InternalDecoderFactory::DestroyVideoDecoder( + webrtc::VideoDecoder* decoder) { + delete decoder; +} + +} // namespace cricket diff --git a/webrtc/media/engine/internaldecoderfactory.h b/webrtc/media/engine/internaldecoderfactory.h new file mode 100644 index 0000000000..20ceecc188 --- /dev/null +++ b/webrtc/media/engine/internaldecoderfactory.h @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2016 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_MEDIA_ENGINE_INTERNALDECODERFACTORY_H_ +#define WEBRTC_MEDIA_ENGINE_INTERNALDECODERFACTORY_H_ + +#include + +#include "webrtc/media/engine/webrtcvideodecoderfactory.h" + +namespace cricket { + +class InternalDecoderFactory : public WebRtcVideoDecoderFactory { + public: + InternalDecoderFactory(); + virtual ~InternalDecoderFactory(); + + // WebRtcVideoDecoderFactory implementation. + webrtc::VideoDecoder* CreateVideoDecoder( + webrtc::VideoCodecType type) override; + + void DestroyVideoDecoder(webrtc::VideoDecoder* decoder) override; +}; + +} // namespace cricket + +#endif // WEBRTC_MEDIA_ENGINE_INTERNALDECODERFACTORY_H_ diff --git a/webrtc/media/engine/internaldecoderfactory_unittest.cc b/webrtc/media/engine/internaldecoderfactory_unittest.cc new file mode 100644 index 0000000000..bf1ca50f24 --- /dev/null +++ b/webrtc/media/engine/internaldecoderfactory_unittest.cc @@ -0,0 +1,21 @@ +/* + * Copyright (c) 2016 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. + */ + +#include "webrtc/media/engine/internaldecoderfactory.h" + +#include "webrtc/test/gtest.h" + +TEST(InternalDecoderFactory, TestVP8) { + cricket::InternalDecoderFactory factory; + webrtc::VideoDecoder* decoder = + factory.CreateVideoDecoder(webrtc::kVideoCodecVP8); + EXPECT_TRUE(decoder); + factory.DestroyVideoDecoder(decoder); +} diff --git a/webrtc/media/engine/videodecodersoftwarefallbackwrapper.cc b/webrtc/media/engine/videodecodersoftwarefallbackwrapper.cc index 8dd0f841ea..cd6eef9b64 100644 --- a/webrtc/media/engine/videodecodersoftwarefallbackwrapper.cc +++ b/webrtc/media/engine/videodecodersoftwarefallbackwrapper.cc @@ -13,34 +13,15 @@ #include #include "webrtc/base/logging.h" +#include "webrtc/media/engine/internaldecoderfactory.h" #include "webrtc/modules/video_coding/include/video_error_codes.h" namespace webrtc { -namespace { - -VideoDecoder::DecoderType CodecTypeToDecoderType(VideoCodecType codec_type) { - switch (codec_type) { - case kVideoCodecH264: - return VideoDecoder::kH264; - case kVideoCodecVP8: - return VideoDecoder::kVp8; - case kVideoCodecVP9: - return VideoDecoder::kVp9; - default: - return VideoDecoder::kUnsupportedCodec; - } -} - -} // anonymous namespace - VideoDecoderSoftwareFallbackWrapper::VideoDecoderSoftwareFallbackWrapper( VideoCodecType codec_type, VideoDecoder* decoder) - : decoder_type_(CodecTypeToDecoderType(codec_type)), - decoder_(decoder), - callback_(nullptr) { -} + : codec_type_(codec_type), decoder_(decoder), callback_(nullptr) {} int32_t VideoDecoderSoftwareFallbackWrapper::InitDecode( const VideoCodec* codec_settings, @@ -51,10 +32,12 @@ int32_t VideoDecoderSoftwareFallbackWrapper::InitDecode( } bool VideoDecoderSoftwareFallbackWrapper::InitFallbackDecoder() { - RTC_CHECK(decoder_type_ != kUnsupportedCodec) + RTC_CHECK(codec_type_ != kVideoCodecUnknown) << "Decoder requesting fallback to codec not supported in software."; LOG(LS_WARNING) << "Decoder falling back to software decoding."; - fallback_decoder_.reset(VideoDecoder::Create(decoder_type_)); + cricket::InternalDecoderFactory internal_decoder_factory; + fallback_decoder_.reset( + internal_decoder_factory.CreateVideoDecoder(codec_type_)); if (fallback_decoder_->InitDecode(&codec_settings_, number_of_cores_) != WEBRTC_VIDEO_CODEC_OK) { LOG(LS_ERROR) << "Failed to initialize software-decoder fallback."; diff --git a/webrtc/media/engine/videodecodersoftwarefallbackwrapper.h b/webrtc/media/engine/videodecodersoftwarefallbackwrapper.h index 8e1a97d817..93c9278a8f 100644 --- a/webrtc/media/engine/videodecodersoftwarefallbackwrapper.h +++ b/webrtc/media/engine/videodecodersoftwarefallbackwrapper.h @@ -46,7 +46,7 @@ class VideoDecoderSoftwareFallbackWrapper : public webrtc::VideoDecoder { private: bool InitFallbackDecoder(); - const DecoderType decoder_type_; + const VideoCodecType codec_type_; VideoDecoder* const decoder_; VideoCodec codec_settings_; diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index fc91ca6f0f..9396fccb7d 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -25,6 +25,7 @@ #include "webrtc/common_video/h264/profile_level_id.h" #include "webrtc/media/engine/constants.h" #include "webrtc/media/engine/internalencoderfactory.h" +#include "webrtc/media/engine/internaldecoderfactory.h" #include "webrtc/media/engine/simulcast.h" #include "webrtc/media/engine/videoencodersoftwarefallbackwrapper.h" #include "webrtc/media/engine/videodecodersoftwarefallbackwrapper.h" @@ -2225,28 +2226,14 @@ WebRtcVideoChannel2::WebRtcVideoReceiveStream::CreateOrReuseVideoDecoder( external_decoder_factory_->CreateVideoDecoderWithParams( type, {stream_params_.id}); if (decoder != NULL) { - return AllocatedDecoder(decoder, type, true); + return AllocatedDecoder(decoder, type, true /* is_external */); } } - if (type == webrtc::kVideoCodecVP8) { - return AllocatedDecoder( - webrtc::VideoDecoder::Create(webrtc::VideoDecoder::kVp8), type, false); - } - - if (type == webrtc::kVideoCodecVP9) { - return AllocatedDecoder( - webrtc::VideoDecoder::Create(webrtc::VideoDecoder::kVp9), type, false); - } - - if (type == webrtc::kVideoCodecH264) { - return AllocatedDecoder( - webrtc::VideoDecoder::Create(webrtc::VideoDecoder::kH264), type, false); - } - - return AllocatedDecoder( - webrtc::VideoDecoder::Create(webrtc::VideoDecoder::kUnsupportedCodec), - webrtc::kVideoCodecUnknown, false); + InternalDecoderFactory internal_decoder_factory; + return AllocatedDecoder(internal_decoder_factory.CreateVideoDecoderWithParams( + type, {stream_params_.id}), + type, false /* is_external */); } void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ConfigureCodecs( diff --git a/webrtc/test/encoder_settings.cc b/webrtc/test/encoder_settings.cc index 80c42ef04a..061fafcb07 100644 --- a/webrtc/test/encoder_settings.cc +++ b/webrtc/test/encoder_settings.cc @@ -12,8 +12,10 @@ #include #include +#include "webrtc/modules/video_coding/codecs/h264/include/h264.h" +#include "webrtc/modules/video_coding/codecs/vp8/include/vp8.h" +#include "webrtc/modules/video_coding/codecs/vp9/include/vp9.h" #include "webrtc/test/fake_decoder.h" -#include "webrtc/video_decoder.h" namespace webrtc { namespace test { @@ -85,11 +87,11 @@ VideoReceiveStream::Decoder CreateMatchingDecoder( decoder.payload_type = encoder_settings.payload_type; decoder.payload_name = encoder_settings.payload_name; if (encoder_settings.payload_name == "H264") { - decoder.decoder = VideoDecoder::Create(VideoDecoder::kH264); + decoder.decoder = H264Decoder::Create(); } else if (encoder_settings.payload_name == "VP8") { - decoder.decoder = VideoDecoder::Create(VideoDecoder::kVp8); + decoder.decoder = VP8Decoder::Create(); } else if (encoder_settings.payload_name == "VP9") { - decoder.decoder = VideoDecoder::Create(VideoDecoder::kVp9); + decoder.decoder = VP9Decoder::Create(); } else { decoder.decoder = new FakeDecoder(); } diff --git a/webrtc/video/BUILD.gn b/webrtc/video/BUILD.gn index 8a99fbf9bd..66191efb8a 100644 --- a/webrtc/video/BUILD.gn +++ b/webrtc/video/BUILD.gn @@ -36,7 +36,6 @@ rtc_static_library("video") { "stream_synchronization.h", "transport_adapter.cc", "transport_adapter.h", - "video_decoder.cc", "video_receive_stream.cc", "video_receive_stream.h", "video_send_stream.cc", diff --git a/webrtc/video/video_decoder.cc b/webrtc/video/video_decoder.cc deleted file mode 100644 index 4ac4ff5b07..0000000000 --- a/webrtc/video/video_decoder.cc +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright (c) 2015 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. - */ - -#include "webrtc/video_decoder.h" - -#include "webrtc/base/checks.h" -#include "webrtc/base/logging.h" -#include "webrtc/modules/video_coding/codecs/h264/include/h264.h" -#include "webrtc/modules/video_coding/codecs/vp8/include/vp8.h" -#include "webrtc/modules/video_coding/codecs/vp9/include/vp9.h" - -namespace webrtc { -VideoDecoder* VideoDecoder::Create(VideoDecoder::DecoderType codec_type) { - switch (codec_type) { - case kH264: - if (!H264Decoder::IsSupported()) { - // This could happen in a software-fallback for a codec type only - // supported externally (e.g. H.264 on iOS or Android) or in current - // usage in WebRtcVideoEngine2 if the external decoder fails to be - // created. - LOG(LS_ERROR) << "Unable to create an H.264 decoder fallback. " - << "Decoding of this stream will be broken."; - return new NullVideoDecoder(); - } - return H264Decoder::Create(); - case kVp8: - return VP8Decoder::Create(); - case kVp9: - RTC_DCHECK(VP9Decoder::IsSupported()); - return VP9Decoder::Create(); - case kUnsupportedCodec: - LOG(LS_ERROR) << "Creating NullVideoDecoder for unsupported codec."; - return new NullVideoDecoder(); - } - RTC_NOTREACHED(); - return nullptr; -} - -NullVideoDecoder::NullVideoDecoder() {} - -int32_t NullVideoDecoder::InitDecode(const VideoCodec* codec_settings, - int32_t number_of_cores) { - LOG(LS_ERROR) << "Can't initialize NullVideoDecoder."; - return WEBRTC_VIDEO_CODEC_OK; -} - -int32_t NullVideoDecoder::Decode(const EncodedImage& input_image, - bool missing_frames, - const RTPFragmentationHeader* fragmentation, - const CodecSpecificInfo* codec_specific_info, - int64_t render_time_ms) { - LOG(LS_ERROR) << "The NullVideoDecoder doesn't support decoding."; - return WEBRTC_VIDEO_CODEC_OK; -} - -int32_t NullVideoDecoder::RegisterDecodeCompleteCallback( - DecodedImageCallback* callback) { - LOG(LS_ERROR) - << "Can't register decode complete callback on NullVideoDecoder."; - return WEBRTC_VIDEO_CODEC_OK; -} - -int32_t NullVideoDecoder::Release() { - return WEBRTC_VIDEO_CODEC_OK; -} - -const char* NullVideoDecoder::ImplementationName() const { - return "NullVideoDecoder"; -} - -} // namespace webrtc diff --git a/webrtc/video_decoder.h b/webrtc/video_decoder.h index 8c89ce9771..b8d2c96b1b 100644 --- a/webrtc/video_decoder.h +++ b/webrtc/video_decoder.h @@ -50,15 +50,6 @@ class DecodedImageCallback { class VideoDecoder { public: - enum DecoderType { - kH264, - kVp8, - kVp9, - kUnsupportedCodec, - }; - - static VideoDecoder* Create(DecoderType codec_type); - virtual ~VideoDecoder() {} virtual int32_t InitDecode(const VideoCodec* codec_settings, @@ -83,29 +74,6 @@ class VideoDecoder { virtual const char* ImplementationName() const { return "unknown"; } }; -// Video decoder class to be used for unknown codecs. Doesn't support decoding -// but logs messages to LS_ERROR. -class NullVideoDecoder : public VideoDecoder { - public: - NullVideoDecoder(); - - int32_t InitDecode(const VideoCodec* codec_settings, - int32_t number_of_cores) override; - - int32_t Decode(const EncodedImage& input_image, - bool missing_frames, - const RTPFragmentationHeader* fragmentation, - const CodecSpecificInfo* codec_specific_info, - int64_t render_time_ms) override; - - int32_t RegisterDecodeCompleteCallback( - DecodedImageCallback* callback) override; - - int32_t Release() override; - - const char* ImplementationName() const override; -}; - } // namespace webrtc #endif // WEBRTC_VIDEO_DECODER_H_