From 5f2bb62f717de894d78b4322281c11b474e6e340 Mon Sep 17 00:00:00 2001 From: Anders Carlsson Date: Mon, 14 May 2018 09:48:06 +0200 Subject: [PATCH] Remove dependency in FakeWebRtcVideoCodecFactories. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, constructing a PeerConnection or WebRtcVideoEngine with fake encoder/decoder factories would result in the real, built-in factories also being used. In https://webrtc-review.googlesource.com/c/src/+/71162, this changed, so to temporarily allow tests to continue working exactly the same as before, the fake factories started encapsulating the real factories. This CL removes that behavior and updates the tests accordingly. Bug: webrtc:9228 Change-Id: Ida14a1e3f5f5a0e2f03100b7895b3b1bdf0a0a42 Reviewed-on: https://webrtc-review.googlesource.com/75260 Reviewed-by: Patrik Höglund Reviewed-by: Magnus Jedvert Reviewed-by: Taylor Brandstetter Commit-Queue: Anders Carlsson Cr-Commit-Position: refs/heads/master@{#23209} --- BUILD.gn | 1 + api/video_codecs/test/BUILD.gn | 27 +++ .../builtin_video_encoder_factory_unittest.cc | 37 ++++ media/BUILD.gn | 7 + media/engine/fakewebrtcvideoengine.cc | 47 +---- media/engine/fakewebrtcvideoengine.h | 23 +-- media/engine/webrtcvideoengine_unittest.cc | 191 +++++++++--------- pc/peerconnection_integrationtest.cc | 90 +-------- 8 files changed, 198 insertions(+), 225 deletions(-) create mode 100644 api/video_codecs/test/BUILD.gn create mode 100644 api/video_codecs/test/builtin_video_encoder_factory_unittest.cc diff --git a/BUILD.gn b/BUILD.gn index 4f019c1b6d..9291b626e2 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -425,6 +425,7 @@ if (rtc_include_tests) { "api:rtc_api_unittests", "api/audio/test:audio_api_unittests", "api/audio_codecs/test:audio_codecs_api_unittests", + "api/video_codecs/test:builtin_video_codec_factory_unittests", "p2p:libstunprober_unittests", "p2p:rtc_p2p_unittests", "rtc_base:rtc_base_approved_unittests", diff --git a/api/video_codecs/test/BUILD.gn b/api/video_codecs/test/BUILD.gn new file mode 100644 index 0000000000..aa68f939ac --- /dev/null +++ b/api/video_codecs/test/BUILD.gn @@ -0,0 +1,27 @@ +# Copyright (c) 2018 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. + +import("../../../webrtc.gni") + +if (rtc_include_tests) { + rtc_source_set("builtin_video_codec_factory_unittests") { + testonly = true + sources = [ + "builtin_video_encoder_factory_unittest.cc", + ] + + deps = [ + "..:builtin_video_encoder_factory", + "..:video_codecs_api", + "../../../system_wrappers:metrics_default", + "../../../test:field_trial", + "../../../test:test_support", + "//testing/gtest", + ] + } +} diff --git a/api/video_codecs/test/builtin_video_encoder_factory_unittest.cc b/api/video_codecs/test/builtin_video_encoder_factory_unittest.cc new file mode 100644 index 0000000000..73957c999f --- /dev/null +++ b/api/video_codecs/test/builtin_video_encoder_factory_unittest.cc @@ -0,0 +1,37 @@ +/* + * Copyright (c) 2018 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 "api/video_codecs/builtin_video_encoder_factory.h" + +#include + +#include "api/video_codecs/sdp_video_format.h" +#include "test/gmock.h" + +namespace webrtc { + +TEST(BuiltinVideoEncoderFactoryTest, AnnouncesVp9AccordingToBuildFlags) { + std::unique_ptr factory = + CreateBuiltinVideoEncoderFactory(); + bool claims_vp9_support = false; + for (const SdpVideoFormat& format : factory->GetSupportedFormats()) { + if (format.name == "VP9") { + claims_vp9_support = true; + break; + } + } +#if defined(RTC_DISABLE_VP9) + EXPECT_FALSE(claims_vp9_support); +#else + EXPECT_TRUE(claims_vp9_support); +#endif // defined(RTC_DISABLE_VP9) +} + +} // namespace webrtc diff --git a/media/BUILD.gn b/media/BUILD.gn index 2808191e2b..402fcfbb44 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -448,6 +448,7 @@ if (rtc_include_tests) { rtc_source_set("rtc_media_tests_utils") { testonly = true + defines = [] include_dirs = [] deps = [ ":rtc_audio_video", @@ -493,6 +494,10 @@ if (rtc_include_tests) { suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] } + if (rtc_use_h264) { + defines += [ "WEBRTC_USE_H264" ] + } + deps += [ ":rtc_internal_video_codecs", ":rtc_media", @@ -654,6 +659,8 @@ if (rtc_include_tests) { "../api/audio_codecs:builtin_audio_decoder_factory", "../api/audio_codecs:builtin_audio_encoder_factory", "../api/video:video_frame", + "../api/video_codecs:builtin_video_decoder_factory", + "../api/video_codecs:builtin_video_encoder_factory", "../api/video_codecs:video_codecs_api", "../audio", "../call:call_interfaces", diff --git a/media/engine/fakewebrtcvideoengine.cc b/media/engine/fakewebrtcvideoengine.cc index 6f1f166025..96ab1d1dd1 100644 --- a/media/engine/fakewebrtcvideoengine.cc +++ b/media/engine/fakewebrtcvideoengine.cc @@ -12,7 +12,6 @@ #include "media/base/codec.h" #include "media/engine/simulcast_encoder_adapter.h" -#include "media/engine/vp8_encoder_simulcast_proxy.h" #include "media/engine/webrtcvideodecoderfactory.h" #include "media/engine/webrtcvideoencoderfactory.h" #include "modules/video_coding/include/video_error_codes.h" @@ -78,19 +77,12 @@ int FakeWebRtcVideoDecoder::GetNumFramesReceived() const { // Decoder factory. FakeWebRtcVideoDecoderFactory::FakeWebRtcVideoDecoderFactory() - : num_created_decoders_(0), - internal_decoder_factory_(new webrtc::InternalDecoderFactory()) {} - -FakeWebRtcVideoDecoderFactory::~FakeWebRtcVideoDecoderFactory() { - delete internal_decoder_factory_; -} + : num_created_decoders_(0) {} std::vector FakeWebRtcVideoDecoderFactory::GetSupportedFormats() const { - std::vector formats = - internal_decoder_factory_->GetSupportedFormats(); + std::vector formats; - // Add external codecs. for (const webrtc::SdpVideoFormat& format : supported_codec_formats_) { // Don't add same codec twice. if (!IsFormatSupported(formats, format)) @@ -109,9 +101,9 @@ FakeWebRtcVideoDecoderFactory::CreateVideoDecoder( rtc::MakeUnique(this); decoders_.push_back(decoder.get()); return decoder; - } else { - return internal_decoder_factory_->CreateVideoDecoder(format); } + + return nullptr; } void FakeWebRtcVideoDecoderFactory::DecoderDestroyed( @@ -206,19 +198,12 @@ FakeWebRtcVideoEncoderFactory::FakeWebRtcVideoEncoderFactory() : created_video_encoder_event_(false, false), num_created_encoders_(0), encoders_have_internal_sources_(false), - internal_encoder_factory_(new webrtc::InternalEncoderFactory()), vp8_factory_mode_(false) {} -FakeWebRtcVideoEncoderFactory::~FakeWebRtcVideoEncoderFactory() { - delete internal_encoder_factory_; -} - std::vector FakeWebRtcVideoEncoderFactory::GetSupportedFormats() const { - std::vector formats = - internal_encoder_factory_->GetSupportedFormats(); + std::vector formats; - // Add external codecs. for (const webrtc::SdpVideoFormat& format : formats_) { // Don't add same codec twice. if (!IsFormatSupported(formats, format)) @@ -247,16 +232,6 @@ FakeWebRtcVideoEncoderFactory::CreateVideoEncoder( encoder = rtc::MakeUnique(this); encoders_.push_back(static_cast(encoder.get())); } - } else { - RTC_LOG(LS_INFO) << "FakeWebRtcVideoEncoderFactory: no match for " - << format.name; - for (auto elem : format.parameters) { - RTC_LOG(LS_INFO) << elem.first << " " << elem.second; - } - encoder = CodecNamesEq(format.name.c_str(), kVp8CodecName) - ? rtc::MakeUnique( - internal_encoder_factory_) - : internal_encoder_factory_->CreateVideoEncoder(format); } return encoder; } @@ -264,14 +239,10 @@ FakeWebRtcVideoEncoderFactory::CreateVideoEncoder( webrtc::VideoEncoderFactory::CodecInfo FakeWebRtcVideoEncoderFactory::QueryVideoEncoder( const webrtc::SdpVideoFormat& format) const { - if (IsFormatSupported(formats_, format)) { - webrtc::VideoEncoderFactory::CodecInfo info; - info.has_internal_source = encoders_have_internal_sources_; - info.is_hardware_accelerated = true; - return info; - } else { - return internal_encoder_factory_->QueryVideoEncoder(format); - } + webrtc::VideoEncoderFactory::CodecInfo info; + info.has_internal_source = encoders_have_internal_sources_; + info.is_hardware_accelerated = true; + return info; } bool FakeWebRtcVideoEncoderFactory::WaitForCreatedVideoEncoders( diff --git a/media/engine/fakewebrtcvideoengine.h b/media/engine/fakewebrtcvideoengine.h index c23e35f2e3..8ebabb1a04 100644 --- a/media/engine/fakewebrtcvideoengine.h +++ b/media/engine/fakewebrtcvideoengine.h @@ -19,8 +19,6 @@ #include "api/video_codecs/video_decoder_factory.h" #include "api/video_codecs/video_encoder.h" #include "api/video_codecs/video_encoder_factory.h" -#include "media/engine/internaldecoderfactory.h" -#include "media/engine/internalencoderfactory.h" #include "rtc_base/criticalsection.h" #include "rtc_base/event.h" #include "rtc_base/ptr_util.h" @@ -37,13 +35,14 @@ class FakeWebRtcVideoDecoder : public webrtc::VideoDecoder { explicit FakeWebRtcVideoDecoder(FakeWebRtcVideoDecoderFactory* factory); ~FakeWebRtcVideoDecoder(); - virtual int32_t InitDecode(const webrtc::VideoCodec*, int32_t); - virtual int32_t Decode(const webrtc::EncodedImage&, - bool, - const webrtc::CodecSpecificInfo*, - int64_t); - virtual int32_t RegisterDecodeCompleteCallback(webrtc::DecodedImageCallback*); - virtual int32_t Release(); + int32_t InitDecode(const webrtc::VideoCodec*, int32_t) override; + int32_t Decode(const webrtc::EncodedImage&, + bool, + const webrtc::CodecSpecificInfo*, + int64_t) override; + int32_t RegisterDecodeCompleteCallback( + webrtc::DecodedImageCallback*) override; + int32_t Release() override; int GetNumFramesReceived() const; @@ -53,11 +52,9 @@ class FakeWebRtcVideoDecoder : public webrtc::VideoDecoder { }; // Fake class for mocking out webrtc::VideoDecoderFactory. -// TODO(bugs.webrtc.org/9228): Remove internal_decoder_factory_. class FakeWebRtcVideoDecoderFactory : public webrtc::VideoDecoderFactory { public: FakeWebRtcVideoDecoderFactory(); - ~FakeWebRtcVideoDecoderFactory(); std::vector GetSupportedFormats() const override; std::unique_ptr CreateVideoDecoder( @@ -72,7 +69,6 @@ class FakeWebRtcVideoDecoderFactory : public webrtc::VideoDecoderFactory { std::vector supported_codec_formats_; std::vector decoders_; int num_created_decoders_; - webrtc::InternalDecoderFactory* internal_decoder_factory_; }; // Fake class for mocking out webrtc::VideoEnoder @@ -107,11 +103,9 @@ class FakeWebRtcVideoEncoder : public webrtc::VideoEncoder { }; // Fake class for mocking out webrtc::VideoEncoderFactory. -// TODO(bugs.webrtc.org/9228): Remove internal_encoder_factory_. class FakeWebRtcVideoEncoderFactory : public webrtc::VideoEncoderFactory { public: FakeWebRtcVideoEncoderFactory(); - ~FakeWebRtcVideoEncoderFactory(); std::vector GetSupportedFormats() const override; std::unique_ptr CreateVideoEncoder( @@ -134,7 +128,6 @@ class FakeWebRtcVideoEncoderFactory : public webrtc::VideoEncoderFactory { std::vector encoders_ RTC_GUARDED_BY(crit_); int num_created_encoders_ RTC_GUARDED_BY(crit_); bool encoders_have_internal_sources_; - webrtc::InternalEncoderFactory* internal_encoder_factory_; bool vp8_factory_mode_; }; diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index 31f56568e7..6d6e61f0bd 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -17,6 +17,8 @@ #include "api/rtpparameters.h" #include "api/test/mock_video_decoder_factory.h" #include "api/test/mock_video_encoder_factory.h" +#include "api/video_codecs/builtin_video_decoder_factory.h" +#include "api/video_codecs/builtin_video_encoder_factory.h" #include "api/video_codecs/sdp_video_format.h" #include "api/video_codecs/video_decoder_factory.h" #include "api/video_codecs/video_encoder.h" @@ -33,7 +35,6 @@ #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" @@ -186,27 +187,12 @@ class WebRtcVideoEngineTest : public ::testing::Test { engine_(std::unique_ptr( encoder_factory_), std::unique_ptr( - decoder_factory_)) { - std::vector engine_codecs = engine_.codecs(); - RTC_DCHECK(!engine_codecs.empty()); - bool codec_set = false; - for (const cricket::VideoCodec& codec : engine_codecs) { - if (codec.name == "rtx") { - int associated_payload_type; - if (codec.GetParam(kCodecParamAssociatedPayloadType, - &associated_payload_type)) { - default_apt_rtx_types_[associated_payload_type] = codec.id; - } - } else if (!codec_set && codec.name != "red" && codec.name != "ulpfec") { - default_codec_ = codec; - codec_set = true; - } - } - - RTC_DCHECK(codec_set); - } + decoder_factory_)) {} protected: + void AssignDefaultAptRtxTypes(); + void AssignDefaultCodec(); + // Find the index of the codec in the engine with the given name. The codec // must be present. size_t GetEngineCodecIndex(const std::string& name) const; @@ -215,9 +201,9 @@ class WebRtcVideoEngineTest : public ::testing::Test { // present. cricket::VideoCodec GetEngineCodec(const std::string& name) const; - VideoMediaChannel* SetUpForExternalEncoderFactory(); + VideoMediaChannel* SetSendParamsWithAllSupportedCodecs(); - VideoMediaChannel* SetUpForExternalDecoderFactory( + VideoMediaChannel* SetRecvParamsWithSupportedCodecs( const std::vector& codecs); void TestExtendedEncoderOveruse(bool use_external_encoder); @@ -234,22 +220,10 @@ class WebRtcVideoEngineTest : public ::testing::Test { std::map default_apt_rtx_types_; }; -TEST_F(WebRtcVideoEngineTest, AnnouncesVp9AccordingToBuildFlags) { - bool claims_vp9_support = false; - for (const cricket::VideoCodec& codec : engine_.codecs()) { - if (codec.name == "VP9") { - claims_vp9_support = true; - break; - } - } -#if defined(RTC_DISABLE_VP9) - EXPECT_FALSE(claims_vp9_support); -#else - EXPECT_TRUE(claims_vp9_support); -#endif // defined(RTC_DISABLE_VP9) -} - TEST_F(WebRtcVideoEngineTest, DefaultRtxCodecHasAssociatedPayloadTypeSet) { + encoder_factory_->AddSupportedVideoCodecType("VP8"); + AssignDefaultCodec(); + std::vector engine_codecs = engine_.codecs(); for (size_t i = 0; i < engine_codecs.size(); ++i) { if (engine_codecs[i].name != kRtxCodecName) @@ -318,7 +292,8 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionBeforeCapturer) { encoder_factory_->AddSupportedVideoCodecType("VP8"); - std::unique_ptr channel(SetUpForExternalEncoderFactory()); + std::unique_ptr channel( + SetSendParamsWithAllSupportedCodecs()); EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(kSsrc))); // Add CVO extension. @@ -348,7 +323,8 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionBeforeAddSendStream) { encoder_factory_->AddSupportedVideoCodecType("VP8"); - std::unique_ptr channel(SetUpForExternalEncoderFactory()); + std::unique_ptr channel( + SetSendParamsWithAllSupportedCodecs()); // Add CVO extension. const int id = 1; cricket::VideoSendParameters parameters; @@ -371,7 +347,8 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionAfterCapturer) { encoder_factory_->AddSupportedVideoCodecType("VP8"); encoder_factory_->AddSupportedVideoCodecType("VP9"); - std::unique_ptr channel(SetUpForExternalEncoderFactory()); + std::unique_ptr channel( + SetSendParamsWithAllSupportedCodecs()); EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(kSsrc))); // Set capturer. @@ -401,6 +378,8 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionAfterCapturer) { } TEST_F(WebRtcVideoEngineTest, SetSendFailsBeforeSettingCodecs) { + encoder_factory_->AddSupportedVideoCodecType("VP8"); + std::unique_ptr channel( engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions())); @@ -413,6 +392,8 @@ TEST_F(WebRtcVideoEngineTest, SetSendFailsBeforeSettingCodecs) { } TEST_F(WebRtcVideoEngineTest, GetStatsWithoutSendCodecsSetDoesNotCrash) { + encoder_factory_->AddSupportedVideoCodecType("VP8"); + std::unique_ptr channel( engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions())); EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(123))); @@ -420,10 +401,11 @@ TEST_F(WebRtcVideoEngineTest, GetStatsWithoutSendCodecsSetDoesNotCrash) { channel->GetStats(&info); } -TEST_F(WebRtcVideoEngineTest, UseExternalFactoryForVp8WhenSupported) { +TEST_F(WebRtcVideoEngineTest, UseFactoryForVp8WhenSupported) { encoder_factory_->AddSupportedVideoCodecType("VP8"); - std::unique_ptr channel(SetUpForExternalEncoderFactory()); + std::unique_ptr channel( + SetSendParamsWithAllSupportedCodecs()); channel->OnReadyToSend(true); EXPECT_TRUE( @@ -455,12 +437,11 @@ TEST_F(WebRtcVideoEngineTest, UseExternalFactoryForVp8WhenSupported) { EXPECT_EQ(0u, encoder_factory_->encoders().size()); } -// Test that when an external encoder factory supports a codec we don't -// internally support, we still add an RTX codec for it. -// TODO(deadbeef): Currently this test is only effective if WebRTC is -// built with no internal H264 support. This test should be updated -// if/when we start adding RTX codecs for unrecognized codec names. -TEST_F(WebRtcVideoEngineTest, RtxCodecAddedForExternalCodec) { +// Test that when an encoder factory supports H264, we add an RTX +// codec for it. +// TODO(deadbeef): This test should be updated if/when we start +// adding RTX codecs for unrecognized codec names. +TEST_F(WebRtcVideoEngineTest, RtxCodecAddedForH264Codec) { using webrtc::H264::ProfileLevelIdToString; using webrtc::H264::ProfileLevelId; using webrtc::H264::kLevel1; @@ -500,7 +481,8 @@ TEST_F(WebRtcVideoEngineTest, RtxCodecAddedForExternalCodec) { TEST_F(WebRtcVideoEngineTest, CanConstructDecoderForVp9EncoderFactory) { encoder_factory_->AddSupportedVideoCodecType("VP9"); - std::unique_ptr channel(SetUpForExternalEncoderFactory()); + std::unique_ptr channel( + SetSendParamsWithAllSupportedCodecs()); EXPECT_TRUE( channel->AddRecvStream(cricket::StreamParams::CreateLegacy(kSsrc))); @@ -511,7 +493,8 @@ TEST_F(WebRtcVideoEngineTest, PropagatesInputFrameTimestamp) { encoder_factory_->AddSupportedVideoCodecType("VP8"); FakeCall* fake_call = new FakeCall(); call_.reset(fake_call); - std::unique_ptr channel(SetUpForExternalEncoderFactory()); + std::unique_ptr channel( + SetSendParamsWithAllSupportedCodecs()); EXPECT_TRUE( channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc))); @@ -561,6 +544,35 @@ TEST_F(WebRtcVideoEngineTest, PropagatesInputFrameTimestamp) { EXPECT_TRUE(channel->RemoveSendStream(kSsrc)); } +void WebRtcVideoEngineTest::AssignDefaultAptRtxTypes() { + std::vector engine_codecs = engine_.codecs(); + RTC_DCHECK(!engine_codecs.empty()); + for (const cricket::VideoCodec& codec : engine_codecs) { + if (codec.name == "rtx") { + int associated_payload_type; + if (codec.GetParam(kCodecParamAssociatedPayloadType, + &associated_payload_type)) { + default_apt_rtx_types_[associated_payload_type] = codec.id; + } + } + } +} + +void WebRtcVideoEngineTest::AssignDefaultCodec() { + std::vector engine_codecs = engine_.codecs(); + RTC_DCHECK(!engine_codecs.empty()); + bool codec_set = false; + for (const cricket::VideoCodec& codec : engine_codecs) { + if (!codec_set && codec.name != "rtx" && codec.name != "red" && + codec.name != "ulpfec") { + default_codec_ = codec; + codec_set = true; + } + } + + RTC_DCHECK(codec_set); +} + size_t WebRtcVideoEngineTest::GetEngineCodecIndex( const std::string& name) const { const std::vector codecs = engine_.codecs(); @@ -590,7 +602,8 @@ cricket::VideoCodec WebRtcVideoEngineTest::GetEngineCodec( return engine_.codecs()[GetEngineCodecIndex(name)]; } -VideoMediaChannel* WebRtcVideoEngineTest::SetUpForExternalEncoderFactory() { +VideoMediaChannel* +WebRtcVideoEngineTest::SetSendParamsWithAllSupportedCodecs() { VideoMediaChannel* channel = engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions()); cricket::VideoSendParameters parameters; @@ -609,7 +622,7 @@ VideoMediaChannel* WebRtcVideoEngineTest::SetUpForExternalEncoderFactory() { return channel; } -VideoMediaChannel* WebRtcVideoEngineTest::SetUpForExternalDecoderFactory( +VideoMediaChannel* WebRtcVideoEngineTest::SetRecvParamsWithSupportedCodecs( const std::vector& codecs) { VideoMediaChannel* channel = engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions()); @@ -623,7 +636,8 @@ VideoMediaChannel* WebRtcVideoEngineTest::SetUpForExternalDecoderFactory( TEST_F(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8Factories) { encoder_factory_->AddSupportedVideoCodecType("VP8"); - std::unique_ptr channel(SetUpForExternalEncoderFactory()); + std::unique_ptr channel( + SetSendParamsWithAllSupportedCodecs()); std::vector ssrcs = MAKE_VECTOR(kSsrcs3); @@ -657,7 +671,8 @@ TEST_F(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8Factories) { ASSERT_EQ(0u, encoder_factory_->encoders().size()); } -TEST_F(WebRtcVideoEngineTest, ChannelWithExternalH264CanChangeToInternalVp8) { +TEST_F(WebRtcVideoEngineTest, ChannelWithH264CanChangeToVp8) { + encoder_factory_->AddSupportedVideoCodecType("VP8"); encoder_factory_->AddSupportedVideoCodecType("H264"); // Set capturer. @@ -689,22 +704,6 @@ TEST_F(WebRtcVideoEngineTest, ChannelWithExternalH264CanChangeToInternalVp8) { EXPECT_EQ_WAIT(0u, encoder_factory_->encoders().size(), kTimeout); } -TEST_F(WebRtcVideoEngineTest, - DontUseExternalEncoderFactoryForUnsupportedCodecs) { - encoder_factory_->AddSupportedVideoCodecType("H264"); - - std::unique_ptr channel( - engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions())); - cricket::VideoSendParameters parameters; - parameters.codecs.push_back(GetEngineCodec("VP8")); - EXPECT_TRUE(channel->SetSendParameters(parameters)); - - 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(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8WithCombinedVP8AndH264Factory) { encoder_factory_->AddSupportedVideoCodecType("VP8"); @@ -809,6 +808,8 @@ TEST_F(WebRtcVideoEngineTest, SimulcastDisabledForH264) { // TODO(brandtr): Remove this test, when the FlexFEC field trial is gone. TEST_F(WebRtcVideoEngineTest, Flexfec03SupportedAsInternalCodecBehindFieldTrial) { + encoder_factory_->AddSupportedVideoCodecType("VP8"); + auto is_flexfec = [](const VideoCodec& codec) { if (codec.name == "flexfec-03") return true; @@ -828,21 +829,21 @@ TEST_F(WebRtcVideoEngineTest, std::find_if(codecs_after.begin(), codecs_after.end(), is_flexfec)); } -// Test that external codecs are added after internal SW codecs. -TEST_F(WebRtcVideoEngineTest, ReportSupportedExternalCodecs) { - const char* kFakeExternalCodecName = "FakeExternalCodec"; - encoder_factory_->AddSupportedVideoCodecType(kFakeExternalCodecName); +// Test that codecs are added in the order they are reported from the factory. +TEST_F(WebRtcVideoEngineTest, ReportSupportedCodecs) { + encoder_factory_->AddSupportedVideoCodecType("VP8"); + const char* kFakeCodecName = "FakeCodec"; + encoder_factory_->AddSupportedVideoCodecType(kFakeCodecName); - // The external codec should appear after the internal codec in the vector. + // The last reported codec should appear after the first codec in the vector. const size_t vp8_index = GetEngineCodecIndex("VP8"); - const size_t fake_external_codec_index = - GetEngineCodecIndex(kFakeExternalCodecName); - EXPECT_LT(vp8_index, fake_external_codec_index); + const size_t fake_codec_index = GetEngineCodecIndex(kFakeCodecName); + EXPECT_LT(vp8_index, fake_codec_index); } -// Test that an external codec that was added after the engine was initialized +// Test that a codec that was added after the engine was initialized // does show up in the codec list after it was added. -TEST_F(WebRtcVideoEngineTest, ReportSupportedExternalCodecsWithAddedCodec) { +TEST_F(WebRtcVideoEngineTest, ReportSupportedAddedCodec) { const char* kFakeExternalCodecName1 = "FakeExternalCodec1"; const char* kFakeExternalCodecName2 = "FakeExternalCodec2"; @@ -863,13 +864,14 @@ TEST_F(WebRtcVideoEngineTest, ReportSupportedExternalCodecsWithAddedCodec) { EXPECT_LT(fake_codec_index1, fake_codec_index2); } -TEST_F(WebRtcVideoEngineTest, RegisterExternalDecodersIfSupported) { +TEST_F(WebRtcVideoEngineTest, RegisterDecodersIfSupported) { + encoder_factory_->AddSupportedVideoCodecType("VP8"); decoder_factory_->AddSupportedVideoCodecType(webrtc::SdpVideoFormat("VP8")); cricket::VideoRecvParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); std::unique_ptr channel( - SetUpForExternalDecoderFactory(parameters.codecs)); + SetRecvParamsWithSupportedCodecs(parameters.codecs)); EXPECT_TRUE( channel->AddRecvStream(cricket::StreamParams::CreateLegacy(kSsrc))); @@ -884,8 +886,8 @@ TEST_F(WebRtcVideoEngineTest, RegisterExternalDecodersIfSupported) { EXPECT_EQ(0u, decoder_factory_->decoders().size()); } -// Verifies that we can set up decoders that are not internally supported. -TEST_F(WebRtcVideoEngineTest, RegisterExternalH264DecoderIfSupported) { +// Verifies that we can set up decoders. +TEST_F(WebRtcVideoEngineTest, RegisterH264DecoderIfSupported) { // TODO(pbos): Do not assume that encoder/decoder support is symmetric. We // can't even query the WebRtcVideoDecoderFactory for supported codecs. // For now we add a FakeWebRtcVideoEncoderFactory to add H264 to supported @@ -896,7 +898,7 @@ TEST_F(WebRtcVideoEngineTest, RegisterExternalH264DecoderIfSupported) { codecs.push_back(GetEngineCodec("H264")); std::unique_ptr channel( - SetUpForExternalDecoderFactory(codecs)); + SetRecvParamsWithSupportedCodecs(codecs)); EXPECT_TRUE( channel->AddRecvStream(cricket::StreamParams::CreateLegacy(kSsrc))); @@ -1079,7 +1081,8 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, NullDecoder) { TEST_F(WebRtcVideoEngineTest, DISABLED_RecreatesEncoderOnContentTypeChange) { encoder_factory_->AddSupportedVideoCodecType("VP8"); std::unique_ptr fake_call(new FakeCall()); - std::unique_ptr channel(SetUpForExternalEncoderFactory()); + std::unique_ptr channel( + SetSendParamsWithAllSupportedCodecs()); ASSERT_TRUE( channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc))); cricket::VideoCodec codec = GetEngineCodec("VP8"); @@ -1139,10 +1142,8 @@ class WebRtcVideoChannelBaseTest : public testing::Test { protected: WebRtcVideoChannelBaseTest() : call_(webrtc::Call::Create(webrtc::Call::Config(&event_log_))), - engine_(std::unique_ptr( - new cricket::FakeWebRtcVideoEncoderFactory()), - std::unique_ptr( - new cricket::FakeWebRtcVideoDecoderFactory())) {} + engine_(webrtc::CreateBuiltinVideoEncoderFactory(), + webrtc::CreateBuiltinVideoDecoderFactory()) {} virtual void SetUp() { cricket::MediaConfig media_config; @@ -1975,6 +1976,12 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { explicit WebRtcVideoChannelTest(const char* field_trials) : WebRtcVideoEngineTest(field_trials), last_ssrc_(0) {} void SetUp() override { + encoder_factory_->AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("VP9"); +#if defined(WEBRTC_USE_H264) + encoder_factory_->AddSupportedVideoCodecType("H264"); +#endif + fake_call_.reset(new FakeCall()); channel_.reset(engine_.CreateChannel(fake_call_.get(), GetMediaConfig(), VideoOptions())); @@ -2570,6 +2577,7 @@ TEST_F(WebRtcVideoChannelTest, TransportCcCanBeEnabledAndDisabled) { } TEST_F(WebRtcVideoChannelTest, NackIsEnabledByDefault) { + AssignDefaultCodec(); VerifyCodecHasDefaultFeedbackParams(default_codec_); cricket::VideoSendParameters parameters; @@ -3439,6 +3447,7 @@ TEST_F(WebRtcVideoChannelTest, EstimatesNtpStartTimeCorrectly) { } TEST_F(WebRtcVideoChannelTest, SetDefaultSendCodecs) { + AssignDefaultAptRtxTypes(); ASSERT_TRUE(channel_->SetSendParameters(send_parameters_)); VideoCodec codec; @@ -5010,6 +5019,7 @@ TEST_F(WebRtcVideoChannelTest, Vp9PacketCreatesUnsignalledStream) { } TEST_F(WebRtcVideoChannelTest, RtxPacketDoesntCreateUnsignalledStream) { + AssignDefaultAptRtxTypes(); const cricket::VideoCodec vp8 = GetEngineCodec("VP8"); const int rtx_vp8_payload_type = default_apt_rtx_types_[vp8.id]; TestReceiveUnsignaledSsrcPacket(rtx_vp8_payload_type, @@ -5754,6 +5764,7 @@ class WebRtcVideoChannelSimulcastTest : public testing::Test { last_ssrc_(0) {} void SetUp() override { + encoder_factory_->AddSupportedVideoCodecType("VP8"); channel_.reset( engine_.CreateChannel(&fake_call_, GetMediaConfig(), VideoOptions())); channel_->OnReadyToSend(true); diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index e5f3ad424a..e697e3aef9 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -420,48 +420,14 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, // frames. int min_video_frames_received_per_track() const { int min_frames = INT_MAX; - if (video_decoder_factory_enabled_) { - const std::vector& decoders = - fake_video_decoder_factory_->decoders(); - if (decoders.empty()) { - return 0; - } - for (FakeWebRtcVideoDecoder* decoder : decoders) { - min_frames = std::min(min_frames, decoder->GetNumFramesReceived()); - } - return min_frames; - } else { - if (fake_video_renderers_.empty()) { - return 0; - } - - for (const auto& pair : fake_video_renderers_) { - min_frames = std::min(min_frames, pair.second->num_rendered_frames()); - } - return min_frames; + if (fake_video_renderers_.empty()) { + return 0; } - } - // In contrast to the above, sums the video frames received for all tracks. - // Can be used to verify that no video frames were received, or that the - // counts didn't increase. - int total_video_frames_received() const { - int total = 0; - if (video_decoder_factory_enabled_) { - const std::vector& decoders = - fake_video_decoder_factory_->decoders(); - for (const FakeWebRtcVideoDecoder* decoder : decoders) { - total += decoder->GetNumFramesReceived(); - } - } else { - for (const auto& pair : fake_video_renderers_) { - total += pair.second->num_rendered_frames(); - } - for (const auto& renderer : removed_fake_video_renderers_) { - total += renderer->num_rendered_frames(); - } + for (const auto& pair : fake_video_renderers_) { + min_frames = std::min(min_frames, pair.second->num_rendered_frames()); } - return total; + return min_frames; } // Returns a MockStatsObserver in a state after stats gathering finished, @@ -619,10 +585,6 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, if (!fake_audio_capture_module_) { return false; } - // Note that these factories don't end up getting used unless supported - // codecs are added to them. - fake_video_decoder_factory_ = new FakeWebRtcVideoDecoderFactory(); - fake_video_encoder_factory_ = new FakeWebRtcVideoEncoderFactory(); rtc::Thread* const signaling_thread = rtc::Thread::Current(); peer_connection_factory_ = webrtc::CreatePeerConnectionFactory( network_thread, worker_thread, signaling_thread, @@ -630,11 +592,9 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, fake_audio_capture_module_), webrtc::CreateBuiltinAudioEncoderFactory(), webrtc::CreateBuiltinAudioDecoderFactory(), - std::unique_ptr( - fake_video_encoder_factory_), - std::unique_ptr( - fake_video_decoder_factory_), - nullptr /* audio_mixer */, nullptr /* audio_processing */); + webrtc::CreateBuiltinVideoEncoderFactory(), + webrtc::CreateBuiltinVideoDecoderFactory(), nullptr /* audio_mixer */, + nullptr /* audio_processing */); if (!peer_connection_factory_) { return false; } @@ -689,12 +649,6 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, signal_ice_candidates_ = signal; } - void EnableVideoDecoderFactory() { - video_decoder_factory_enabled_ = true; - fake_video_decoder_factory_->AddSupportedVideoCodecType( - webrtc::SdpVideoFormat("VP8")); - } - rtc::scoped_refptr CreateLocalVideoTrackInternal( webrtc::FakePeriodicVideoSource::Config config) { // Set max frame rate to 10fps to reduce the risk of test flakiness. @@ -978,11 +932,6 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, // Needed to ensure frames aren't received for removed tracks. std::vector> removed_fake_video_renderers_; - // Needed to keep track of number of frames received when external decoder - // used. - FakeWebRtcVideoDecoderFactory* fake_video_decoder_factory_ = nullptr; - FakeWebRtcVideoEncoderFactory* fake_video_encoder_factory_ = nullptr; - bool video_decoder_factory_enabled_ = false; // For remote peer communication. SignalingMessageReceiver* signaling_message_receiver_ = nullptr; @@ -1317,11 +1266,6 @@ class PeerConnectionIntegrationBaseTest : public testing::Test { callee_->set_signal_ice_candidates(signal); } - void EnableVideoDecoderFactory() { - caller_->EnableVideoDecoderFactory(); - callee_->EnableVideoDecoderFactory(); - } - // Messages may get lost on the unreliable DataChannel, so we send multiple // times to avoid test flakiness. void SendRtpDataWithRetries(webrtc::DataChannelInterface* dc, @@ -3675,24 +3619,6 @@ TEST_P(PeerConnectionIntegrationTest, ASSERT_TRUE(ExpectNewFrames(media_expectations)); } -// This test sets up a Jsep call between two parties with external -// VideoDecoderFactory. -// TODO(holmer): Disabled due to sometimes crashing on buildbots. -// See issue webrtc/2378. -TEST_P(PeerConnectionIntegrationTest, - DISABLED_EndToEndCallWithVideoDecoderFactory) { - ASSERT_TRUE(CreatePeerConnectionWrappers()); - EnableVideoDecoderFactory(); - ConnectFakeSignaling(); - caller()->AddAudioVideoTracks(); - callee()->AddAudioVideoTracks(); - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - MediaExpectations media_expectations; - media_expectations.ExpectBidirectionalAudioAndVideo(); - ASSERT_TRUE(ExpectNewFrames(media_expectations)); -} - // This tests that if we negotiate after calling CreateSender but before we // have a track, then set a track later, frames from the newly-set track are // received end-to-end.