From 7501b1c3d11097cba92e72c89b0267a617eec4de Mon Sep 17 00:00:00 2001 From: Magnus Jedvert Date: Thu, 9 Nov 2017 13:43:42 +0100 Subject: [PATCH] Reland "Update internal video decoder factory to new interface" This reverts commit 267d84baf0597f89a3d1f66d323db754bc5d9239. Reason for reland: Fix the bug; decoder is not allowed to ever be null and we need to use a NullVideoDecoder that ignores calls instead. Original change's description: > Revert "Update internal video decoder factory to new interface" > > This reverts commit b2fc9b1b104240e68047901309deaee3e8b94bea. > > Reason for revert: Suspected to cause failures on Android bots on webrtc.fyi, see https://build.chromium.org/p/chromium.webrtc.fyi/builders/Android%20Tests%20%28dbg%29%20%28K%20Nexus5%29/builds/21051 > > Original change's description: > > Update internal video decoder factory to new interface > > > > We want to move away from cricket::WebRtcVideoDecoderFactory and this CL > > updates the internal factory. Also, VideoDecoderSoftwareFallbackWrapper > > is updated to take a VideoDecoder as argument instead of a factory so it > > can be used with external SW decoders. > > > > Bug: webrtc:7925 > > Change-Id: Ie6dc6c24f8610a2129620c6e2b42e3cebb2ddef7 > > Reviewed-on: https://webrtc-review.googlesource.com/7301 > > Reviewed-by: Rasmus Brandt > > Reviewed-by: Anders Carlsson > > Commit-Queue: Magnus Jedvert > > Cr-Commit-Position: refs/heads/master@{#20597} > > TBR=brandtr@webrtc.org,magjed@webrtc.org,andersc@webrtc.org > > Change-Id: I0a12c98fdc30f00d58c85ee7e088f50160d39724 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: webrtc:7925 > Reviewed-on: https://webrtc-review.googlesource.com/21420 > Reviewed-by: Christian Fremerey > Commit-Queue: Christian Fremerey > Cr-Commit-Position: refs/heads/master@{#20605} TBR=brandtr@webrtc.org,magjed@webrtc.org,andersc@webrtc.org,chfremer@webrtc.org,chfremer@google.com Change-Id: I6cf5794dc3fadfa86809a94da80b69dbb4c56f52 No-Try: true Bug: webrtc:7925 Reviewed-on: https://webrtc-review.googlesource.com/21541 Commit-Queue: Magnus Jedvert Reviewed-by: Magnus Jedvert Cr-Commit-Position: refs/heads/master@{#20623} --- media/engine/convert_legacy_video_factory.cc | 54 +++++++++-- media/engine/internaldecoderfactory.cc | 96 ++++++------------- media/engine/internaldecoderfactory.h | 20 ++-- .../engine/internaldecoderfactory_unittest.cc | 14 ++- .../videodecodersoftwarefallbackwrapper.cc | 77 +++++++-------- .../videodecodersoftwarefallbackwrapper.h | 16 ++-- ...decodersoftwarefallbackwrapper_unittest.cc | 3 +- media/engine/webrtcvideoengine.cc | 39 ++++++++ media/engine/webrtcvideoengine_unittest.cc | 39 ++++++++ .../test/videoprocessor_integrationtest.cc | 23 ++++- 10 files changed, 235 insertions(+), 146 deletions(-) diff --git a/media/engine/convert_legacy_video_factory.cc b/media/engine/convert_legacy_video_factory.cc index 6495e277c4..d40f8699dd 100644 --- a/media/engine/convert_legacy_video_factory.cc +++ b/media/engine/convert_legacy_video_factory.cc @@ -15,6 +15,7 @@ #include "api/video_codecs/video_decoder_factory.h" #include "api/video_codecs/video_encoder_factory.h" +#include "media/base/h264_profile_level_id.h" #include "media/engine/internaldecoderfactory.h" #include "media/engine/internalencoderfactory.h" #include "media/engine/scopedvideodecoder.h" @@ -31,6 +32,34 @@ namespace cricket { namespace { +bool IsSameFormat(const webrtc::SdpVideoFormat& format1, + const webrtc::SdpVideoFormat& format2) { + // If different names (case insensitive), then not same formats. + if (!CodecNamesEq(format1.name, format2.name)) + return false; + // For every format besides H264, comparing names is enough. + if (!CodecNamesEq(format1.name.c_str(), kH264CodecName)) + return true; + // Compare H264 profiles. + const rtc::Optional profile_level_id = + webrtc::H264::ParseSdpProfileLevelId(format1.parameters); + const rtc::Optional other_profile_level_id = + webrtc::H264::ParseSdpProfileLevelId(format2.parameters); + // Compare H264 profiles, but not levels. + return profile_level_id && other_profile_level_id && + profile_level_id->profile == other_profile_level_id->profile; +} + +bool IsFormatSupported( + const std::vector& supported_formats, + const webrtc::SdpVideoFormat& format) { + for (const webrtc::SdpVideoFormat& supported_format : supported_formats) { + if (IsSameFormat(format, supported_format)) + return true; + } + return false; +} + class EncoderAdapter : public webrtc::VideoEncoderFactory { public: explicit EncoderAdapter( @@ -138,11 +167,17 @@ class DecoderAdapter : public webrtc::VideoDecoderFactory { public: explicit DecoderAdapter( std::unique_ptr external_decoder_factory) - : internal_decoder_factory_(new InternalDecoderFactory()), - external_decoder_factory_(std::move(external_decoder_factory)) {} + : external_decoder_factory_(std::move(external_decoder_factory)) {} std::unique_ptr CreateVideoDecoder( const webrtc::SdpVideoFormat& format) override { + std::unique_ptr internal_decoder; + webrtc::InternalDecoderFactory internal_decoder_factory; + if (IsFormatSupported(internal_decoder_factory.GetSupportedFormats(), + format)) { + internal_decoder = internal_decoder_factory.CreateVideoDecoder(format); + } + const VideoCodec codec(format); const VideoDecoderParams params = {}; if (external_decoder_factory_ != nullptr) { @@ -150,16 +185,16 @@ class DecoderAdapter : public webrtc::VideoDecoderFactory { CreateScopedVideoDecoder(external_decoder_factory_.get(), codec, params); if (external_decoder) { - webrtc::VideoCodecType type = - webrtc::PayloadStringToCodecType(codec.name); - std::unique_ptr internal_decoder( + if (!internal_decoder) + return external_decoder; + // Both external and internal decoder available - create fallback + // wrapper. + return std::unique_ptr( new webrtc::VideoDecoderSoftwareFallbackWrapper( - type, std::move(external_decoder))); - return internal_decoder; + std::move(internal_decoder), std::move(external_decoder))); } } - std::unique_ptr internal_decoder( - internal_decoder_factory_->CreateVideoDecoderWithParams(codec, params)); + return internal_decoder; } @@ -170,7 +205,6 @@ class DecoderAdapter : public webrtc::VideoDecoderFactory { } private: - const std::unique_ptr internal_decoder_factory_; const std::unique_ptr external_decoder_factory_; }; diff --git a/media/engine/internaldecoderfactory.cc b/media/engine/internaldecoderfactory.cc index 33119b138e..86b4ed163c 100644 --- a/media/engine/internaldecoderfactory.cc +++ b/media/engine/internaldecoderfactory.cc @@ -10,81 +10,41 @@ #include "media/engine/internaldecoderfactory.h" -#include - +#include "media/base/mediaconstants.h" #include "modules/video_coding/codecs/h264/include/h264.h" #include "modules/video_coding/codecs/vp8/include/vp8.h" #include "modules/video_coding/codecs/vp9/include/vp9.h" +#include "rtc_base/checks.h" #include "rtc_base/logging.h" -namespace cricket { +namespace webrtc { -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 { - RTC_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 { - RTC_LOG(LS_ERROR) << "The NullVideoDecoder doesn't support decoding."; - return WEBRTC_VIDEO_CODEC_OK; - } - - int32_t RegisterDecodeCompleteCallback( - webrtc::DecodedImageCallback* callback) override { - RTC_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 WebRtcVideoEngine if the external decoder fails to be created. - RTC_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: - RTC_LOG(LS_ERROR) << "Creating NullVideoDecoder for unsupported codec."; - return new NullVideoDecoder(); - } +std::vector InternalDecoderFactory::GetSupportedFormats() + const { + std::vector formats; + formats.push_back(SdpVideoFormat(cricket::kVp8CodecName)); + if (VP9Decoder::IsSupported()) + formats.push_back(SdpVideoFormat(cricket::kVp9CodecName)); + for (const SdpVideoFormat& h264_format : SupportedH264Codecs()) + formats.push_back(h264_format); + return formats; } -void InternalDecoderFactory::DestroyVideoDecoder( - webrtc::VideoDecoder* decoder) { - delete decoder; +std::unique_ptr InternalDecoderFactory::CreateVideoDecoder( + const SdpVideoFormat& format) { + if (cricket::CodecNamesEq(format.name, cricket::kVp8CodecName)) + return std::unique_ptr(VP8Decoder::Create()); + + if (cricket::CodecNamesEq(format.name, cricket::kVp9CodecName)) { + RTC_DCHECK(VP9Decoder::IsSupported()); + return std::unique_ptr(VP9Decoder::Create()); + } + + if (cricket::CodecNamesEq(format.name, cricket::kH264CodecName)) + return std::unique_ptr(H264Decoder::Create()); + + RTC_LOG(LS_ERROR) << "Trying to create decoder for unsupported format"; + return nullptr; } -} // namespace cricket +} // namespace webrtc diff --git a/media/engine/internaldecoderfactory.h b/media/engine/internaldecoderfactory.h index 283d103f7b..7420129600 100644 --- a/media/engine/internaldecoderfactory.h +++ b/media/engine/internaldecoderfactory.h @@ -11,24 +11,20 @@ #ifndef MEDIA_ENGINE_INTERNALDECODERFACTORY_H_ #define MEDIA_ENGINE_INTERNALDECODERFACTORY_H_ +#include #include -#include "media/engine/webrtcvideodecoderfactory.h" +#include "api/video_codecs/video_decoder_factory.h" -namespace cricket { +namespace webrtc { -class InternalDecoderFactory : public WebRtcVideoDecoderFactory { +class InternalDecoderFactory : public VideoDecoderFactory { public: - InternalDecoderFactory(); - virtual ~InternalDecoderFactory(); - - // WebRtcVideoDecoderFactory implementation. - webrtc::VideoDecoder* CreateVideoDecoder( - webrtc::VideoCodecType type) override; - - void DestroyVideoDecoder(webrtc::VideoDecoder* decoder) override; + std::vector GetSupportedFormats() const override; + std::unique_ptr CreateVideoDecoder( + const SdpVideoFormat& format) override; }; -} // namespace cricket +} // namespace webrtc #endif // MEDIA_ENGINE_INTERNALDECODERFACTORY_H_ diff --git a/media/engine/internaldecoderfactory_unittest.cc b/media/engine/internaldecoderfactory_unittest.cc index cbf39dfd03..147293dcf7 100644 --- a/media/engine/internaldecoderfactory_unittest.cc +++ b/media/engine/internaldecoderfactory_unittest.cc @@ -10,12 +10,18 @@ #include "media/engine/internaldecoderfactory.h" +#include "api/video_codecs/sdp_video_format.h" +#include "api/video_codecs/video_decoder.h" +#include "media/base/mediaconstants.h" #include "test/gtest.h" +namespace webrtc { + TEST(InternalDecoderFactory, TestVP8) { - cricket::InternalDecoderFactory factory; - webrtc::VideoDecoder* decoder = - factory.CreateVideoDecoder(webrtc::kVideoCodecVP8); + InternalDecoderFactory factory; + std::unique_ptr decoder = + factory.CreateVideoDecoder(SdpVideoFormat(cricket::kVp8CodecName)); EXPECT_TRUE(decoder); - factory.DestroyVideoDecoder(decoder); } + +} // namespace webrtc diff --git a/media/engine/videodecodersoftwarefallbackwrapper.cc b/media/engine/videodecodersoftwarefallbackwrapper.cc index 101b44417f..87fb62239a 100644 --- a/media/engine/videodecodersoftwarefallbackwrapper.cc +++ b/media/engine/videodecodersoftwarefallbackwrapper.cc @@ -22,52 +22,49 @@ namespace webrtc { VideoDecoderSoftwareFallbackWrapper::VideoDecoderSoftwareFallbackWrapper( - VideoCodecType codec_type, - std::unique_ptr decoder) - : codec_type_(codec_type), - decoder_(std::move(decoder)), - decoder_initialized_(false), + std::unique_ptr sw_fallback_decoder, + std::unique_ptr hw_decoder) + : use_hw_decoder_(true), + hw_decoder_(std::move(hw_decoder)), + hw_decoder_initialized_(false), + fallback_decoder_(std::move(sw_fallback_decoder)), + fallback_implementation_name_( + std::string(fallback_decoder_->ImplementationName()) + + " (fallback from: " + hw_decoder_->ImplementationName() + ")"), callback_(nullptr) {} int32_t VideoDecoderSoftwareFallbackWrapper::InitDecode( const VideoCodec* codec_settings, int32_t number_of_cores) { - RTC_DCHECK(!fallback_decoder_) << "Fallback decoder should never be " - "initialized here, it should've been " - "released."; + // Always try to use the HW decoder in this state. + use_hw_decoder_ = true; codec_settings_ = *codec_settings; number_of_cores_ = number_of_cores; - int32_t ret = decoder_->InitDecode(codec_settings, number_of_cores); + int32_t ret = hw_decoder_->InitDecode(codec_settings, number_of_cores); if (ret == WEBRTC_VIDEO_CODEC_OK) { - decoder_initialized_ = true; + hw_decoder_initialized_ = true; return ret; } - decoder_initialized_ = false; + hw_decoder_initialized_ = false; // Try to initialize fallback decoder. if (InitFallbackDecoder()) return WEBRTC_VIDEO_CODEC_OK; + return ret; } bool VideoDecoderSoftwareFallbackWrapper::InitFallbackDecoder() { - RTC_CHECK(codec_type_ != kVideoCodecUnknown) - << "Decoder requesting fallback to codec not supported in software."; RTC_LOG(LS_WARNING) << "Decoder falling back to software decoding."; - 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) { RTC_LOG(LS_ERROR) << "Failed to initialize software-decoder fallback."; - fallback_decoder_.reset(); + use_hw_decoder_ = true; return false; } if (callback_) fallback_decoder_->RegisterDecodeCompleteCallback(callback_); - fallback_implementation_name_ = - std::string(fallback_decoder_->ImplementationName()) + - " (fallback from: " + decoder_->ImplementationName() + ")"; + use_hw_decoder_ = false; return true; } @@ -80,31 +77,30 @@ int32_t VideoDecoderSoftwareFallbackWrapper::Decode( TRACE_EVENT0("webrtc", "VideoDecoderSoftwareFallbackWrapper::Decode"); // Try initializing and decoding with the provided decoder on every keyframe // or when there's no fallback decoder. This is the normal case. - if (!fallback_decoder_ || input_image._frameType == kVideoFrameKey) { + if (use_hw_decoder_ || input_image._frameType == kVideoFrameKey) { int32_t ret = WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE; // Try reinitializing the decoder if it had failed before. - if (!decoder_initialized_) { - decoder_initialized_ = - decoder_->InitDecode(&codec_settings_, number_of_cores_) == + if (!hw_decoder_initialized_) { + hw_decoder_initialized_ = + hw_decoder_->InitDecode(&codec_settings_, number_of_cores_) == WEBRTC_VIDEO_CODEC_OK; } - if (decoder_initialized_) { - ret = decoder_->Decode(input_image, missing_frames, fragmentation, + if (hw_decoder_initialized_) { + ret = hw_decoder_->Decode(input_image, missing_frames, fragmentation, codec_specific_info, render_time_ms); } if (ret == WEBRTC_VIDEO_CODEC_OK) { - if (fallback_decoder_) { + if (!use_hw_decoder_) { // Decode OK -> stop using fallback decoder. RTC_LOG(LS_WARNING) << "Decode OK, no longer using the software fallback decoder."; - fallback_decoder_->Release(); - fallback_decoder_.reset(); + use_hw_decoder_ = true; return WEBRTC_VIDEO_CODEC_OK; } } if (ret != WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE) return ret; - if (!fallback_decoder_) { + if (use_hw_decoder_) { // Try to initialize fallback decoder. if (!InitFallbackDecoder()) return ret; @@ -117,32 +113,29 @@ int32_t VideoDecoderSoftwareFallbackWrapper::Decode( int32_t VideoDecoderSoftwareFallbackWrapper::RegisterDecodeCompleteCallback( DecodedImageCallback* callback) { callback_ = callback; - int32_t ret = decoder_->RegisterDecodeCompleteCallback(callback); - if (fallback_decoder_) + int32_t ret = hw_decoder_->RegisterDecodeCompleteCallback(callback); + if (!use_hw_decoder_) return fallback_decoder_->RegisterDecodeCompleteCallback(callback); return ret; } int32_t VideoDecoderSoftwareFallbackWrapper::Release() { - if (fallback_decoder_) { + if (!use_hw_decoder_) { RTC_LOG(LS_INFO) << "Releasing software fallback decoder."; fallback_decoder_->Release(); - fallback_decoder_.reset(); } - decoder_initialized_ = false; - return decoder_->Release(); + hw_decoder_initialized_ = false; + return hw_decoder_->Release(); } bool VideoDecoderSoftwareFallbackWrapper::PrefersLateDecoding() const { - if (fallback_decoder_) - return fallback_decoder_->PrefersLateDecoding(); - return decoder_->PrefersLateDecoding(); + return use_hw_decoder_ ? hw_decoder_->PrefersLateDecoding() + : fallback_decoder_->PrefersLateDecoding(); } const char* VideoDecoderSoftwareFallbackWrapper::ImplementationName() const { - if (fallback_decoder_) - return fallback_implementation_name_.c_str(); - return decoder_->ImplementationName(); + return use_hw_decoder_ ? hw_decoder_->ImplementationName() + : fallback_implementation_name_.c_str(); } } // namespace webrtc diff --git a/media/engine/videodecodersoftwarefallbackwrapper.h b/media/engine/videodecodersoftwarefallbackwrapper.h index 84a86ca59c..97953ddc82 100644 --- a/media/engine/videodecodersoftwarefallbackwrapper.h +++ b/media/engine/videodecodersoftwarefallbackwrapper.h @@ -23,8 +23,9 @@ namespace webrtc { // hardware restrictions, such as max resolution. class VideoDecoderSoftwareFallbackWrapper : public VideoDecoder { public: - VideoDecoderSoftwareFallbackWrapper(VideoCodecType codec_type, - std::unique_ptr decoder); + VideoDecoderSoftwareFallbackWrapper( + std::unique_ptr sw_fallback_decoder, + std::unique_ptr hw_decoder); int32_t InitDecode(const VideoCodec* codec_settings, int32_t number_of_cores) override; @@ -46,14 +47,15 @@ class VideoDecoderSoftwareFallbackWrapper : public VideoDecoder { private: bool InitFallbackDecoder(); - const VideoCodecType codec_type_; - std::unique_ptr decoder_; - bool decoder_initialized_; + // Determines if we are trying to use the HW or SW decoder. + bool use_hw_decoder_; + std::unique_ptr hw_decoder_; + bool hw_decoder_initialized_; VideoCodec codec_settings_; int32_t number_of_cores_; - std::string fallback_implementation_name_; - std::unique_ptr fallback_decoder_; + const std::unique_ptr fallback_decoder_; + const std::string fallback_implementation_name_; DecodedImageCallback* callback_; }; diff --git a/media/engine/videodecodersoftwarefallbackwrapper_unittest.cc b/media/engine/videodecodersoftwarefallbackwrapper_unittest.cc index dfd388be13..367527eff7 100644 --- a/media/engine/videodecodersoftwarefallbackwrapper_unittest.cc +++ b/media/engine/videodecodersoftwarefallbackwrapper_unittest.cc @@ -10,6 +10,7 @@ #include "media/engine/videodecodersoftwarefallbackwrapper.h" #include "api/video_codecs/video_decoder.h" +#include "modules/video_coding/codecs/vp8/include/vp8.h" #include "modules/video_coding/include/video_error_codes.h" #include "rtc_base/checks.h" #include "test/gtest.h" @@ -20,7 +21,7 @@ class VideoDecoderSoftwareFallbackWrapperTest : public ::testing::Test { protected: VideoDecoderSoftwareFallbackWrapperTest() : fake_decoder_(new CountingFakeDecoder()), - fallback_wrapper_(kVideoCodecVP8, + fallback_wrapper_(std::unique_ptr(VP8Decoder::Create()), std::unique_ptr(fake_decoder_)) {} class CountingFakeDecoder : public VideoDecoder { diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index 8c4ffe820e..ed7d67e7aa 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -29,6 +29,7 @@ #include "media/engine/simulcast.h" #include "media/engine/webrtcmediaengine.h" #include "media/engine/webrtcvoiceengine.h" +#include "modules/video_coding/include/video_error_codes.h" #include "rtc_base/copyonwritebuffer.h" #include "rtc_base/logging.h" #include "rtc_base/stringutils.h" @@ -123,6 +124,37 @@ class DecoderFactoryAdapter { 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"; } +}; + std::vector AssignPayloadTypesAndAddAssociatedRtxCodecs( const std::vector& input_formats); @@ -2101,6 +2133,13 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::ConfigureCodecs( recv_codec.codec.name, recv_codec.codec.params)); } + // If we still have no valid decoder, we have to create a "Null" decoder + // that ignores all calls. The reason we can get into this state is that + // the old decoder factory interface doesn't have a way to query supported + // codecs. + if (!new_decoder) + new_decoder.reset(new NullVideoDecoder()); + webrtc::VideoReceiveStream::Decoder decoder; decoder.decoder = new_decoder.get(); decoder.payload_type = recv_codec.codec.id; diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index e9632cb804..ecde12997a 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -30,6 +30,7 @@ #include "media/engine/constants.h" #include "media/engine/fakewebrtccall.h" #include "media/engine/fakewebrtcvideoengine.h" +#include "media/engine/internalencoderfactory.h" #include "media/engine/simulcast.h" #include "media/engine/webrtcvideoengine.h" #include "media/engine/webrtcvoiceengine.h" @@ -970,6 +971,44 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) { EXPECT_TRUE(recv_channel->RemoveRecvStream(recv_ssrc)); } +// Test behavior when decoder factory fails to create a decoder (returns null). +TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, NullDecoder) { + // |engine| take ownership of the factories. + webrtc::MockVideoEncoderFactory* encoder_factory = + new testing::StrictMock(); + webrtc::MockVideoDecoderFactory* decoder_factory = + new testing::StrictMock(); + 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)); + + // Decoder creation fails. + EXPECT_CALL(*decoder_factory, CreateVideoDecoderProxy(testing::_)) + .WillOnce(testing::Return(nullptr)); + + // Create a call. + webrtc::RtcEventLogNullImpl event_log; + std::unique_ptr call( + webrtc::Call::Create(webrtc::Call::Config(&event_log))); + + // 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().front()); + 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_TRUE(recv_channel->RemoveRecvStream(recv_ssrc)); +} + class WebRtcVideoChannelBaseTest : public VideoMediaChannelTest { protected: diff --git a/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc b/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc index 84a77b7185..33bb4c703d 100644 --- a/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc +++ b/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc @@ -80,6 +80,23 @@ bool RunEncodeInRealTime(const TestConfig& config) { #endif } +// An internal decoder factory in the old WebRtcVideoDecoderFactory format. +// TODO(magjed): Update these tests to use new webrtc::VideoDecoderFactory +// instead. +class LegacyInternalDecoderFactory : public cricket::WebRtcVideoDecoderFactory { + public: + // WebRtcVideoDecoderFactory implementation. + VideoDecoder* CreateVideoDecoderWithParams( + const cricket::VideoCodec& codec, + cricket::VideoDecoderParams params) override { + return InternalDecoderFactory() + .CreateVideoDecoder(SdpVideoFormat(codec.name, codec.params)) + .release(); + } + + void DestroyVideoDecoder(VideoDecoder* decoder) override { delete decoder; } +}; + } // namespace void VideoProcessorIntegrationTest::H264KeyframeChecker::CheckEncodedFrame( @@ -311,7 +328,7 @@ void VideoProcessorIntegrationTest::CreateEncoderAndDecoder() { RTC_NOTREACHED() << "Only support HW decoder on Android and iOS."; #endif } else { - decoder_factory.reset(new cricket::InternalDecoderFactory()); + decoder_factory.reset(new LegacyInternalDecoderFactory()); } cricket::VideoCodec codec; @@ -369,7 +386,9 @@ void VideoProcessorIntegrationTest::CreateEncoderAndDecoder() { } if (config_.sw_fallback_decoder) { decoder_ = rtc::MakeUnique( - config_.codec_settings.codecType, std::move(decoder_)); + InternalDecoderFactory().CreateVideoDecoder( + SdpVideoFormat(codec.name, codec.params)), + std::move(decoder_)); } EXPECT_TRUE(encoder_) << "Encoder not successfully created.";