From a3aa9bd75b094893160de8957592d7937a3b50da Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Wed, 17 Apr 2019 07:38:40 +0200 Subject: [PATCH] Make VideoBitrateAllocatorFactory injectable. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch makes VideoBitrateAllocatorFactory injectable by adding to PeerConnectionDependencies instead of allowing it to be overridden using MediaEngine (on PeerConnectionFactory). With this patch VideoBitrateAllocatorFactory is owned by the PeerConnection. WANT_LGTM (examples) : sakal@ WANT_LGTM (api/pc) : steveanton@ Bug: webrtc:10547 Change-Id: I768d400a621f2b7a98795eb7f410adb48651bfd6 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132706 Commit-Queue: Jonas Oreland Reviewed-by: Sami Kalliomäki Reviewed-by: Steve Anton Reviewed-by: Rasmus Brandt Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#27654} --- api/peer_connection_interface.h | 2 + api/video/BUILD.gn | 1 + .../jni/android_call_client.cc | 1 - media/BUILD.gn | 1 + media/base/fake_media_engine.cc | 3 +- media/base/fake_media_engine.h | 4 +- media/base/media_engine.h | 5 +- media/engine/null_webrtc_video_engine.h | 4 +- media/engine/webrtc_media_engine.cc | 23 +--- media/engine/webrtc_media_engine.h | 13 -- media/engine/webrtc_media_engine_defaults.cc | 4 - media/engine/webrtc_media_engine_unittest.cc | 11 -- media/engine/webrtc_video_engine.cc | 12 +- media/engine/webrtc_video_engine.h | 8 +- media/engine/webrtc_video_engine_unittest.cc | 125 ++++++++++-------- pc/BUILD.gn | 5 + pc/channel_manager.cc | 13 +- pc/channel_manager.h | 3 +- pc/channel_manager_unittest.cc | 8 +- pc/peer_connection.cc | 11 +- pc/peer_connection.h | 9 ++ pc/rtp_sender_receiver_unittest.cc | 7 +- 22 files changed, 144 insertions(+), 129 deletions(-) diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 36f2bcd6fa..623b869f17 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -1219,6 +1219,8 @@ struct PeerConnectionDependencies final { std::unique_ptr async_resolver_factory; std::unique_ptr cert_generator; std::unique_ptr tls_cert_verifier; + std::unique_ptr + video_bitrate_allocator_factory; }; // PeerConnectionFactoryDependencies holds all of the PeerConnectionFactory diff --git a/api/video/BUILD.gn b/api/video/BUILD.gn index 59b6a4b982..dfa5467f15 100644 --- a/api/video/BUILD.gn +++ b/api/video/BUILD.gn @@ -152,6 +152,7 @@ rtc_source_set("video_bitrate_allocator_factory") { ] deps = [ ":video_bitrate_allocator", + "../../rtc_base:rtc_base_approved", "../video_codecs:video_codecs_api", ] } diff --git a/examples/androidnativeapi/jni/android_call_client.cc b/examples/androidnativeapi/jni/android_call_client.cc index 69afc89d2e..50afe65c21 100644 --- a/examples/androidnativeapi/jni/android_call_client.cc +++ b/examples/androidnativeapi/jni/android_call_client.cc @@ -162,7 +162,6 @@ void AndroidCallClient::CreatePeerConnectionFactory() { webrtc::CreateBuiltinAudioDecoderFactory(), absl::make_unique(), absl::make_unique(), - webrtc::CreateBuiltinVideoBitrateAllocatorFactory(), nullptr /* audio_mixer */, webrtc::AudioProcessingBuilder().Create()); RTC_LOG(LS_INFO) << "Media engine created: " << media_engine.get(); diff --git a/media/BUILD.gn b/media/BUILD.gn index 88119e3272..fbed41daee 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -78,6 +78,7 @@ rtc_static_library("rtc_media_base") { "../api:scoped_refptr", "../api/audio_codecs:audio_codecs_api", "../api/video:video_bitrate_allocation", + "../api/video:video_bitrate_allocator_factory", "../api/video:video_frame", "../api/video:video_frame_i420", "../api/video_codecs:video_codecs_api", diff --git a/media/base/fake_media_engine.cc b/media/base/fake_media_engine.cc index e4e94caa7a..9724423e0b 100644 --- a/media/base/fake_media_engine.cc +++ b/media/base/fake_media_engine.cc @@ -563,7 +563,8 @@ VideoMediaChannel* FakeVideoEngine::CreateMediaChannel( webrtc::Call* call, const MediaConfig& config, const VideoOptions& options, - const webrtc::CryptoOptions& crypto_options) { + const webrtc::CryptoOptions& crypto_options, + webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory) { if (fail_create_channel_) { return nullptr; } diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index c26095d3a7..f1192c51ff 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h @@ -546,7 +546,9 @@ class FakeVideoEngine : public VideoEngineInterface { webrtc::Call* call, const MediaConfig& config, const VideoOptions& options, - const webrtc::CryptoOptions& crypto_options) override; + const webrtc::CryptoOptions& crypto_options, + webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory) + override; FakeVideoMediaChannel* GetChannel(size_t index); void UnregisterChannel(VideoMediaChannel* channel); std::vector codecs() const override; diff --git a/media/base/media_engine.h b/media/base/media_engine.h index 5867c9e181..9bc49f95f1 100644 --- a/media/base/media_engine.h +++ b/media/base/media_engine.h @@ -23,6 +23,7 @@ #include "api/audio_codecs/audio_encoder_factory.h" #include "api/crypto/crypto_options.h" #include "api/rtp_parameters.h" +#include "api/video/video_bitrate_allocator_factory.h" #include "call/audio_state.h" #include "media/base/codec.h" #include "media/base/media_channel.h" @@ -97,7 +98,9 @@ class VideoEngineInterface { webrtc::Call* call, const MediaConfig& config, const VideoOptions& options, - const webrtc::CryptoOptions& crypto_options) = 0; + const webrtc::CryptoOptions& crypto_options, + webrtc::VideoBitrateAllocatorFactory* + video_bitrate_allocator_factory) = 0; virtual std::vector codecs() const = 0; virtual RtpCapabilities GetCapabilities() const = 0; diff --git a/media/engine/null_webrtc_video_engine.h b/media/engine/null_webrtc_video_engine.h index 29b9ba5b4b..590f0b0be7 100644 --- a/media/engine/null_webrtc_video_engine.h +++ b/media/engine/null_webrtc_video_engine.h @@ -40,7 +40,9 @@ class NullWebRtcVideoEngine : public VideoEngineInterface { webrtc::Call* call, const MediaConfig& config, const VideoOptions& options, - const webrtc::CryptoOptions& crypto_options) override { + const webrtc::CryptoOptions& crypto_options, + webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory) + override { return nullptr; } }; diff --git a/media/engine/webrtc_media_engine.cc b/media/engine/webrtc_media_engine.cc index e2ead01979..637db06a18 100644 --- a/media/engine/webrtc_media_engine.cc +++ b/media/engine/webrtc_media_engine.cc @@ -40,8 +40,7 @@ std::unique_ptr CreateMediaEngine( #ifdef HAVE_WEBRTC_VIDEO auto video_engine = absl::make_unique( std::move(dependencies.video_encoder_factory), - std::move(dependencies.video_decoder_factory), - std::move(dependencies.video_bitrate_allocator_factory)); + std::move(dependencies.video_decoder_factory)); #else auto video_engine = absl::make_unique(); #endif @@ -57,27 +56,9 @@ std::unique_ptr WebRtcMediaEngineFactory::Create( std::unique_ptr video_decoder_factory, rtc::scoped_refptr audio_mixer, rtc::scoped_refptr audio_processing) { - return WebRtcMediaEngineFactory::Create( - adm, audio_encoder_factory, audio_decoder_factory, - std::move(video_encoder_factory), std::move(video_decoder_factory), - webrtc::CreateBuiltinVideoBitrateAllocatorFactory(), audio_mixer, - audio_processing); -} - -std::unique_ptr WebRtcMediaEngineFactory::Create( - rtc::scoped_refptr adm, - rtc::scoped_refptr audio_encoder_factory, - rtc::scoped_refptr audio_decoder_factory, - std::unique_ptr video_encoder_factory, - std::unique_ptr video_decoder_factory, - std::unique_ptr - video_bitrate_allocator_factory, - rtc::scoped_refptr audio_mixer, - rtc::scoped_refptr audio_processing) { #ifdef HAVE_WEBRTC_VIDEO auto video_engine = absl::make_unique( - std::move(video_encoder_factory), std::move(video_decoder_factory), - std::move(video_bitrate_allocator_factory)); + std::move(video_encoder_factory), std::move(video_decoder_factory)); #else auto video_engine = absl::make_unique(); #endif diff --git a/media/engine/webrtc_media_engine.h b/media/engine/webrtc_media_engine.h index 70f3ba3fa9..b13cebefcb 100644 --- a/media/engine/webrtc_media_engine.h +++ b/media/engine/webrtc_media_engine.h @@ -48,8 +48,6 @@ struct MediaEngineDependencies { std::unique_ptr video_encoder_factory; std::unique_ptr video_decoder_factory; - std::unique_ptr - video_bitrate_allocator_factory; }; std::unique_ptr CreateMediaEngine( @@ -72,17 +70,6 @@ class WebRtcMediaEngineFactory { std::unique_ptr video_decoder_factory, rtc::scoped_refptr audio_mixer, rtc::scoped_refptr audio_processing); - - static std::unique_ptr Create( - rtc::scoped_refptr adm, - rtc::scoped_refptr audio_encoder_factory, - rtc::scoped_refptr audio_decoder_factory, - std::unique_ptr video_encoder_factory, - std::unique_ptr video_decoder_factory, - std::unique_ptr - video_bitrate_allocator_factory, - rtc::scoped_refptr audio_mixer, - rtc::scoped_refptr audio_processing); }; // Verify that extension IDs are within 1-byte extension range and are not diff --git a/media/engine/webrtc_media_engine_defaults.cc b/media/engine/webrtc_media_engine_defaults.cc index 038cd4e23c..1660873e8b 100644 --- a/media/engine/webrtc_media_engine_defaults.cc +++ b/media/engine/webrtc_media_engine_defaults.cc @@ -38,10 +38,6 @@ void SetMediaEngineDefaults(cricket::MediaEngineDependencies* deps) { deps->video_encoder_factory = CreateBuiltinVideoEncoderFactory(); if (deps->video_decoder_factory == nullptr) deps->video_decoder_factory = CreateBuiltinVideoDecoderFactory(); - if (deps->video_bitrate_allocator_factory == nullptr) { - deps->video_bitrate_allocator_factory = - CreateBuiltinVideoBitrateAllocatorFactory(); - } } } // namespace webrtc diff --git a/media/engine/webrtc_media_engine_unittest.cc b/media/engine/webrtc_media_engine_unittest.cc index 722b227703..0849a65cb4 100644 --- a/media/engine/webrtc_media_engine_unittest.cc +++ b/media/engine/webrtc_media_engine_unittest.cc @@ -297,17 +297,6 @@ TEST(WebRtcMediaEngineFactoryTest, CreateWithBuiltinDecoders) { EXPECT_TRUE(engine); } -TEST(WebRtcMediaEngineFactoryTest, CreateWithVideoBitrateFactory) { - std::unique_ptr engine(WebRtcMediaEngineFactory::Create( - nullptr /* adm */, webrtc::CreateBuiltinAudioEncoderFactory(), - webrtc::CreateBuiltinAudioDecoderFactory(), - webrtc::CreateBuiltinVideoEncoderFactory(), - webrtc::CreateBuiltinVideoDecoderFactory(), - webrtc::CreateBuiltinVideoBitrateAllocatorFactory(), - nullptr /* audio_mixer */, webrtc::AudioProcessingBuilder().Create())); - EXPECT_TRUE(engine); -} - TEST(WebRtcMediaEngineFactoryTest, Create) { MediaEngineDependencies deps; webrtc::SetMediaEngineDefaults(&deps); diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 57ff3723d0..5240b2874b 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -464,12 +464,9 @@ void DefaultUnsignalledSsrcHandler::SetDefaultSink( WebRtcVideoEngine::WebRtcVideoEngine( std::unique_ptr video_encoder_factory, - std::unique_ptr video_decoder_factory, - std::unique_ptr - video_bitrate_allocator_factory) + std::unique_ptr video_decoder_factory) : decoder_factory_(std::move(video_decoder_factory)), - encoder_factory_(std::move(video_encoder_factory)), - bitrate_allocator_factory_(std::move(video_bitrate_allocator_factory)) { + encoder_factory_(std::move(video_encoder_factory)) { RTC_LOG(LS_INFO) << "WebRtcVideoEngine::WebRtcVideoEngine()"; } @@ -481,11 +478,12 @@ VideoMediaChannel* WebRtcVideoEngine::CreateMediaChannel( webrtc::Call* call, const MediaConfig& config, const VideoOptions& options, - const webrtc::CryptoOptions& crypto_options) { + const webrtc::CryptoOptions& crypto_options, + webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory) { RTC_LOG(LS_INFO) << "CreateMediaChannel. Options: " << options.ToString(); return new WebRtcVideoChannel(call, config, options, crypto_options, encoder_factory_.get(), decoder_factory_.get(), - bitrate_allocator_factory_.get()); + video_bitrate_allocator_factory); } std::vector WebRtcVideoEngine::codecs() const { return AssignPayloadTypesAndDefaultCodecs(encoder_factory_.get()); diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index 0b209873c8..44d06131c9 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -84,9 +84,7 @@ class WebRtcVideoEngine : public VideoEngineInterface { // and external hardware codecs. WebRtcVideoEngine( std::unique_ptr video_encoder_factory, - std::unique_ptr video_decoder_factory, - std::unique_ptr - video_bitrate_allocator_factory); + std::unique_ptr video_decoder_factory); ~WebRtcVideoEngine() override; @@ -94,7 +92,9 @@ class WebRtcVideoEngine : public VideoEngineInterface { webrtc::Call* call, const MediaConfig& config, const VideoOptions& options, - const webrtc::CryptoOptions& crypto_options) override; + const webrtc::CryptoOptions& crypto_options, + webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory) + override; std::vector codecs() const override; RtpCapabilities GetCapabilities() const override; diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 54e76e53e4..2b5a01dd21 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -217,11 +217,12 @@ class WebRtcVideoEngineTest : public ::testing::Test { call_(webrtc::Call::Create(webrtc::Call::Config(&event_log_))), encoder_factory_(new cricket::FakeWebRtcVideoEncoderFactory), decoder_factory_(new cricket::FakeWebRtcVideoDecoderFactory), + video_bitrate_allocator_factory_( + webrtc::CreateBuiltinVideoBitrateAllocatorFactory()), engine_(std::unique_ptr( encoder_factory_), std::unique_ptr( - decoder_factory_), - webrtc::CreateBuiltinVideoBitrateAllocatorFactory()) { + decoder_factory_)) { // Ensure fake clock doesn't return 0, which will cause some initializations // fail inside RTP senders. fake_clock_.AdvanceTimeMicros(1); @@ -256,6 +257,8 @@ class WebRtcVideoEngineTest : public ::testing::Test { std::unique_ptr call_; cricket::FakeWebRtcVideoEncoderFactory* encoder_factory_; cricket::FakeWebRtcVideoDecoderFactory* decoder_factory_; + std::unique_ptr + video_bitrate_allocator_factory_; WebRtcVideoEngine engine_; VideoCodec default_codec_; std::map default_apt_rtx_types_; @@ -455,7 +458,8 @@ TEST_F(WebRtcVideoEngineTest, SetSendFailsBeforeSettingCodecs) { encoder_factory_->AddSupportedVideoCodecType("VP8"); std::unique_ptr channel(engine_.CreateMediaChannel( - call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions())); + call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get())); EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(123))); @@ -469,7 +473,8 @@ TEST_F(WebRtcVideoEngineTest, GetStatsWithoutSendCodecsSetDoesNotCrash) { encoder_factory_->AddSupportedVideoCodecType("VP8"); std::unique_ptr channel(engine_.CreateMediaChannel( - call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions())); + call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get())); EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(123))); VideoMediaInfo info; channel->GetStats(&info); @@ -678,7 +683,8 @@ cricket::VideoCodec WebRtcVideoEngineTest::GetEngineCodec( VideoMediaChannel* WebRtcVideoEngineTest::SetSendParamsWithAllSupportedCodecs() { VideoMediaChannel* channel = engine_.CreateMediaChannel( - call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions()); + call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get()); cricket::VideoSendParameters parameters; // We need to look up the codec in the engine to get the correct payload type. for (const webrtc::SdpVideoFormat& format : @@ -697,7 +703,8 @@ WebRtcVideoEngineTest::SetSendParamsWithAllSupportedCodecs() { VideoMediaChannel* WebRtcVideoEngineTest::SetRecvParamsWithSupportedCodecs( const std::vector& codecs) { VideoMediaChannel* channel = engine_.CreateMediaChannel( - call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions()); + call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get()); cricket::VideoRecvParameters parameters; parameters.codecs = codecs; EXPECT_TRUE(channel->SetRecvParameters(parameters)); @@ -765,7 +772,8 @@ TEST_F(WebRtcVideoEngineTest, ChannelWithH264CanChangeToVp8) { rtc::kNumMicrosecsPerSec / 30); std::unique_ptr channel(engine_.CreateMediaChannel( - call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions())); + call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get())); cricket::VideoSendParameters parameters; parameters.codecs.push_back(GetEngineCodec("H264")); EXPECT_TRUE(channel->SetSendParameters(parameters)); @@ -794,7 +802,8 @@ TEST_F(WebRtcVideoEngineTest, encoder_factory_->AddSupportedVideoCodecType("H264"); std::unique_ptr channel(engine_.CreateMediaChannel( - call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions())); + call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get())); cricket::VideoSendParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); EXPECT_TRUE(channel->SetSendParameters(parameters)); @@ -829,7 +838,8 @@ TEST_F(WebRtcVideoEngineTest, encoder_factory_->AddSupportedVideoCodecType("H264"); std::unique_ptr channel(engine_.CreateMediaChannel( - call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions())); + call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get())); cricket::VideoSendParameters parameters; parameters.codecs.push_back(GetEngineCodec("H264")); EXPECT_TRUE(channel->SetSendParameters(parameters)); @@ -860,7 +870,8 @@ TEST_F(WebRtcVideoEngineTest, SimulcastEnabledForH264BehindFieldTrial) { encoder_factory_->AddSupportedVideoCodecType("H264"); std::unique_ptr channel(engine_.CreateMediaChannel( - call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions())); + call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get())); cricket::VideoSendParameters parameters; parameters.codecs.push_back(GetEngineCodec("H264")); EXPECT_TRUE(channel->SetSendParameters(parameters)); @@ -1011,10 +1022,8 @@ TEST_F(WebRtcVideoEngineTest, GetSourcesWithNonExistingSsrc) { TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, NullFactories) { std::unique_ptr encoder_factory; std::unique_ptr decoder_factory; - std::unique_ptr rate_allocator_factory; WebRtcVideoEngine engine(std::move(encoder_factory), - std::move(decoder_factory), - std::move(rate_allocator_factory)); + std::move(decoder_factory)); EXPECT_EQ(0u, engine.codecs().size()); } @@ -1024,18 +1033,13 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, EmptyFactories) { new webrtc::MockVideoEncoderFactory(); webrtc::MockVideoDecoderFactory* decoder_factory = new webrtc::MockVideoDecoderFactory(); - webrtc::MockVideoBitrateAllocatorFactory* rate_allocator_factory = - new webrtc::MockVideoBitrateAllocatorFactory(); WebRtcVideoEngine engine( (std::unique_ptr(encoder_factory)), - (std::unique_ptr(decoder_factory)), - (std::unique_ptr( - rate_allocator_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()); - EXPECT_CALL(*rate_allocator_factory, Die()); } // Test full behavior in the video engine when video codec factories of the new @@ -1048,17 +1052,16 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) { new webrtc::MockVideoEncoderFactory(); webrtc::MockVideoDecoderFactory* decoder_factory = new webrtc::MockVideoDecoderFactory(); - webrtc::MockVideoBitrateAllocatorFactory* rate_allocator_factory = - new webrtc::MockVideoBitrateAllocatorFactory(); + std::unique_ptr + rate_allocator_factory = + absl::make_unique(); EXPECT_CALL(*rate_allocator_factory, CreateVideoBitrateAllocatorProxy(Field( &webrtc::VideoCodec::codecType, webrtc::kVideoCodecVP8))) .WillOnce(::testing::Return(new webrtc::MockVideoBitrateAllocator())); WebRtcVideoEngine engine( (std::unique_ptr(encoder_factory)), - (std::unique_ptr(decoder_factory)), - (std::unique_ptr( - rate_allocator_factory))); + (std::unique_ptr(decoder_factory))); const webrtc::SdpVideoFormat vp8_format("VP8"); const std::vector supported_formats = {vp8_format}; EXPECT_CALL(*encoder_factory, GetSupportedFormats()) @@ -1123,7 +1126,8 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) { // Create send channel. const int send_ssrc = 123; std::unique_ptr send_channel(engine.CreateMediaChannel( - call.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions())); + call.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), + rate_allocator_factory.get())); cricket::VideoSendParameters send_parameters; send_parameters.codecs.push_back(engine_codecs.at(0)); EXPECT_TRUE(send_channel->SetSendParameters(send_parameters)); @@ -1144,7 +1148,8 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) { // Create recv channel. const int recv_ssrc = 321; std::unique_ptr recv_channel(engine.CreateMediaChannel( - call.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions())); + call.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), + rate_allocator_factory.get())); cricket::VideoRecvParameters recv_parameters; recv_parameters.codecs.push_back(engine_codecs.at(0)); EXPECT_TRUE(recv_channel->SetRecvParameters(recv_parameters)); @@ -1166,13 +1171,12 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, NullDecoder) { new webrtc::MockVideoEncoderFactory(); webrtc::MockVideoDecoderFactory* decoder_factory = new webrtc::MockVideoDecoderFactory(); - webrtc::MockVideoBitrateAllocatorFactory* rate_allocator_factory = - new webrtc::MockVideoBitrateAllocatorFactory(); + std::unique_ptr + rate_allocator_factory = + absl::make_unique(); WebRtcVideoEngine engine( (std::unique_ptr(encoder_factory)), - (std::unique_ptr(decoder_factory)), - (std::unique_ptr( - rate_allocator_factory))); + (std::unique_ptr(decoder_factory))); const webrtc::SdpVideoFormat vp8_format("VP8"); const std::vector supported_formats = {vp8_format}; EXPECT_CALL(*encoder_factory, GetSupportedFormats()) @@ -1190,7 +1194,8 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, NullDecoder) { // Create recv channel. const int recv_ssrc = 321; std::unique_ptr recv_channel(engine.CreateMediaChannel( - call.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions())); + call.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), + rate_allocator_factory.get())); cricket::VideoRecvParameters recv_parameters; recv_parameters.codecs.push_back(engine.codecs().front()); EXPECT_TRUE(recv_channel->SetRecvParameters(recv_parameters)); @@ -1264,9 +1269,10 @@ TEST_F(WebRtcVideoEngineTest, DISABLED_RecreatesEncoderOnContentTypeChange) { class WebRtcVideoChannelBaseTest : public ::testing::Test { protected: WebRtcVideoChannelBaseTest() - : engine_(webrtc::CreateBuiltinVideoEncoderFactory(), - webrtc::CreateBuiltinVideoDecoderFactory(), - webrtc::CreateBuiltinVideoBitrateAllocatorFactory()) {} + : video_bitrate_allocator_factory_( + webrtc::CreateBuiltinVideoBitrateAllocatorFactory()), + engine_(webrtc::CreateBuiltinVideoEncoderFactory(), + webrtc::CreateBuiltinVideoDecoderFactory()) {} virtual void SetUp() { // One testcase calls SetUp in a loop, only create call_ once. @@ -1282,7 +1288,7 @@ class WebRtcVideoChannelBaseTest : public ::testing::Test { channel_.reset( static_cast(engine_.CreateMediaChannel( call_.get(), media_config, cricket::VideoOptions(), - webrtc::CryptoOptions()))); + webrtc::CryptoOptions(), video_bitrate_allocator_factory_.get()))); channel_->OnReadyToSend(true); EXPECT_TRUE(channel_.get() != NULL); network_interface_.SetDestination(channel_.get()); @@ -1453,6 +1459,8 @@ class WebRtcVideoChannelBaseTest : public ::testing::Test { webrtc::RtcEventLogNullImpl event_log_; std::unique_ptr call_; + std::unique_ptr + video_bitrate_allocator_factory_; WebRtcVideoEngine engine_; std::unique_ptr frame_source_; @@ -2073,9 +2081,9 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { #endif fake_call_.reset(new FakeCall()); - channel_.reset(engine_.CreateMediaChannel(fake_call_.get(), - GetMediaConfig(), VideoOptions(), - webrtc::CryptoOptions())); + channel_.reset(engine_.CreateMediaChannel( + fake_call_.get(), GetMediaConfig(), VideoOptions(), + webrtc::CryptoOptions(), video_bitrate_allocator_factory_.get())); channel_->OnReadyToSend(true); last_ssrc_ = 123; send_parameters_.codecs = engine_.codecs(); @@ -2909,7 +2917,8 @@ TEST_F(WebRtcVideoChannelTest, SetMediaConfigSuspendBelowMinBitrate) { media_config.video.suspend_below_min_bitrate = true; channel_.reset(engine_.CreateMediaChannel( - fake_call_.get(), media_config, VideoOptions(), webrtc::CryptoOptions())); + fake_call_.get(), media_config, VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get())); channel_->OnReadyToSend(true); channel_->SetSendParameters(send_parameters_); @@ -2919,7 +2928,8 @@ TEST_F(WebRtcVideoChannelTest, SetMediaConfigSuspendBelowMinBitrate) { media_config.video.suspend_below_min_bitrate = false; channel_.reset(engine_.CreateMediaChannel( - fake_call_.get(), media_config, VideoOptions(), webrtc::CryptoOptions())); + fake_call_.get(), media_config, VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get())); channel_->OnReadyToSend(true); channel_->SetSendParameters(send_parameters_); @@ -3322,7 +3332,8 @@ TEST_F(WebRtcVideoChannelTest, PreviousAdaptationDoesNotApplyToScreenshare) { MediaConfig media_config = GetMediaConfig(); media_config.video.enable_cpu_adaptation = true; channel_.reset(engine_.CreateMediaChannel( - fake_call_.get(), media_config, VideoOptions(), webrtc::CryptoOptions())); + fake_call_.get(), media_config, VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get())); channel_->OnReadyToSend(true); ASSERT_TRUE(channel_->SetSendParameters(parameters)); @@ -3371,7 +3382,8 @@ void WebRtcVideoChannelTest::TestDegradationPreference( MediaConfig media_config = GetMediaConfig(); media_config.video.enable_cpu_adaptation = true; channel_.reset(engine_.CreateMediaChannel( - fake_call_.get(), media_config, VideoOptions(), webrtc::CryptoOptions())); + fake_call_.get(), media_config, VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get())); channel_->OnReadyToSend(true); EXPECT_TRUE(channel_->SetSendParameters(parameters)); @@ -3403,7 +3415,8 @@ void WebRtcVideoChannelTest::TestCpuAdaptation(bool enable_overuse, media_config.video.enable_cpu_adaptation = true; } channel_.reset(engine_.CreateMediaChannel( - fake_call_.get(), media_config, VideoOptions(), webrtc::CryptoOptions())); + fake_call_.get(), media_config, VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get())); channel_->OnReadyToSend(true); EXPECT_TRUE(channel_->SetSendParameters(parameters)); @@ -4597,7 +4610,8 @@ TEST_F(WebRtcVideoChannelTest, TestSetDscpOptions) { channel.reset( static_cast(engine_.CreateMediaChannel( - call_.get(), config, VideoOptions(), webrtc::CryptoOptions()))); + call_.get(), config, VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get()))); channel->SetInterface(network_interface.get(), /*media_transport=*/nullptr); // Default value when DSCP is disabled should be DSCP_DEFAULT. EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface->dscp()); @@ -4607,7 +4621,8 @@ TEST_F(WebRtcVideoChannelTest, TestSetDscpOptions) { config.enable_dscp = true; channel.reset( static_cast(engine_.CreateMediaChannel( - call_.get(), config, VideoOptions(), webrtc::CryptoOptions()))); + call_.get(), config, VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get()))); channel->SetInterface(network_interface.get(), /*media_transport=*/nullptr); EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface->dscp()); @@ -4640,7 +4655,8 @@ TEST_F(WebRtcVideoChannelTest, TestSetDscpOptions) { config.enable_dscp = false; channel.reset( static_cast(engine_.CreateMediaChannel( - call_.get(), config, VideoOptions(), webrtc::CryptoOptions()))); + call_.get(), config, VideoOptions(), webrtc::CryptoOptions(), + video_bitrate_allocator_factory_.get()))); channel->SetInterface(network_interface.get(), /*media_transport=*/nullptr); EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface->dscp()); } @@ -7120,20 +7136,18 @@ class WebRtcVideoChannelSimulcastTest : public ::testing::Test { encoder_factory_(new cricket::FakeWebRtcVideoEncoderFactory), decoder_factory_(new cricket::FakeWebRtcVideoDecoderFactory), mock_rate_allocator_factory_( - new webrtc::MockVideoBitrateAllocatorFactory), + absl::make_unique()), engine_(std::unique_ptr( encoder_factory_), std::unique_ptr( - decoder_factory_), - std::unique_ptr( - mock_rate_allocator_factory_)), + decoder_factory_)), last_ssrc_(0) {} void SetUp() override { encoder_factory_->AddSupportedVideoCodecType("VP8"); - channel_.reset(engine_.CreateMediaChannel(&fake_call_, GetMediaConfig(), - VideoOptions(), - webrtc::CryptoOptions())); + channel_.reset(engine_.CreateMediaChannel( + &fake_call_, GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), + mock_rate_allocator_factory_.get())); channel_->OnReadyToSend(true); last_ssrc_ = 123; } @@ -7284,7 +7298,8 @@ class WebRtcVideoChannelSimulcastTest : public ::testing::Test { FakeCall fake_call_; cricket::FakeWebRtcVideoEncoderFactory* encoder_factory_; cricket::FakeWebRtcVideoDecoderFactory* decoder_factory_; - webrtc::MockVideoBitrateAllocatorFactory* mock_rate_allocator_factory_; + std::unique_ptr + mock_rate_allocator_factory_; WebRtcVideoEngine engine_; std::unique_ptr channel_; uint32_t last_ssrc_; diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 732640961c..5ff4b16a23 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -79,6 +79,7 @@ rtc_static_library("rtc_pc_base") { "../api:ortc_api", "../api:rtp_headers", "../api:scoped_refptr", + "../api/video:builtin_video_bitrate_allocator_factory", "../api/video:video_frame", "../call:call_interfaces", "../call:rtp_interfaces", @@ -200,6 +201,7 @@ rtc_static_library("peerconnection") { "../api:rtc_stats_api", "../api:scoped_refptr", "../api/task_queue", + "../api/video:builtin_video_bitrate_allocator_factory", "../api/video:video_frame", "../api/video_codecs:video_codecs_api", "../call:call_interfaces", @@ -280,6 +282,7 @@ if (rtc_include_tests) { "../api:libjingle_peerconnection_api", "../api:loopback_media_transport", "../api:rtp_headers", + "../api/video:builtin_video_bitrate_allocator_factory", "../call:rtp_interfaces", "../call:rtp_receiver", "../logging:rtc_event_log_api", @@ -407,6 +410,7 @@ if (rtc_include_tests) { "../api:scoped_refptr", "../api/audio:audio_mixer_api", "../api/audio_codecs:audio_codecs_api", + "../api/video:builtin_video_bitrate_allocator_factory", "../api/video:video_frame", "../api/video_codecs:builtin_video_decoder_factory", "../api/video_codecs:builtin_video_encoder_factory", @@ -501,6 +505,7 @@ if (rtc_include_tests) { "../api:scoped_refptr", "../api/audio:audio_mixer_api", "../api/units:time_delta", + "../api/video:builtin_video_bitrate_allocator_factory", "../logging:fake_rtc_event_log", "../media:rtc_media_config", "../modules/audio_device:audio_device_api", diff --git a/pc/channel_manager.cc b/pc/channel_manager.cc index 0a1c8b4876..525dc52517 100644 --- a/pc/channel_manager.cc +++ b/pc/channel_manager.cc @@ -233,12 +233,14 @@ VideoChannel* ChannelManager::CreateVideoChannel( bool srtp_required, const webrtc::CryptoOptions& crypto_options, rtc::UniqueRandomIdGenerator* ssrc_generator, - const VideoOptions& options) { + const VideoOptions& options, + webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory) { if (!worker_thread_->IsCurrent()) { return worker_thread_->Invoke(RTC_FROM_HERE, [&] { - return CreateVideoChannel( - call, media_config, rtp_transport, media_transport, signaling_thread, - content_name, srtp_required, crypto_options, ssrc_generator, options); + return CreateVideoChannel(call, media_config, rtp_transport, + media_transport, signaling_thread, content_name, + srtp_required, crypto_options, ssrc_generator, + options, video_bitrate_allocator_factory); }); } @@ -250,7 +252,8 @@ VideoChannel* ChannelManager::CreateVideoChannel( } VideoMediaChannel* media_channel = media_engine_->video().CreateMediaChannel( - call, media_config, options, crypto_options); + call, media_config, options, crypto_options, + video_bitrate_allocator_factory); if (!media_channel) { return nullptr; } diff --git a/pc/channel_manager.h b/pc/channel_manager.h index b70ed2e3cd..a749b7f928 100644 --- a/pc/channel_manager.h +++ b/pc/channel_manager.h @@ -118,7 +118,8 @@ class ChannelManager final { bool srtp_required, const webrtc::CryptoOptions& crypto_options, rtc::UniqueRandomIdGenerator* ssrc_generator, - const VideoOptions& options); + const VideoOptions& options, + webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory); // Destroys a video channel created by CreateVideoChannel. void DestroyVideoChannel(VideoChannel* video_channel); diff --git a/pc/channel_manager_unittest.cc b/pc/channel_manager_unittest.cc index 8a20f40ff3..6410319acc 100644 --- a/pc/channel_manager_unittest.cc +++ b/pc/channel_manager_unittest.cc @@ -13,6 +13,7 @@ #include "absl/memory/memory.h" #include "api/rtc_error.h" #include "api/test/fake_media_transport.h" +#include "api/video/builtin_video_bitrate_allocator_factory.h" #include "media/base/fake_media_engine.h" #include "media/base/test_utils.h" #include "media/engine/fake_webrtc_call.h" @@ -45,6 +46,8 @@ class ChannelManagerTest : public ::testing::Test { ChannelManagerTest() : network_(rtc::Thread::CreateWithSocketServer()), worker_(rtc::Thread::Create()), + video_bitrate_allocator_factory_( + webrtc::CreateBuiltinVideoBitrateAllocatorFactory()), fme_(new cricket::FakeMediaEngine()), fdme_(new cricket::FakeDataEngine()), cm_(new cricket::ChannelManager( @@ -90,7 +93,8 @@ class ChannelManagerTest : public ::testing::Test { cricket::VideoChannel* video_channel = cm_->CreateVideoChannel( &fake_call_, cricket::MediaConfig(), rtp_transport, media_transport, rtc::Thread::Current(), cricket::CN_VIDEO, kDefaultSrtpRequired, - webrtc::CryptoOptions(), &ssrc_generator_, VideoOptions()); + webrtc::CryptoOptions(), &ssrc_generator_, VideoOptions(), + video_bitrate_allocator_factory_.get()); EXPECT_TRUE(video_channel != nullptr); cricket::RtpDataChannel* rtp_data_channel = cm_->CreateRtpDataChannel( cricket::MediaConfig(), rtp_transport, rtc::Thread::Current(), @@ -106,6 +110,8 @@ class ChannelManagerTest : public ::testing::Test { std::unique_ptr rtp_dtls_transport_; std::unique_ptr network_; std::unique_ptr worker_; + std::unique_ptr + video_bitrate_allocator_factory_; // |fme_| and |fdme_| are actually owned by |cm_|. cricket::FakeMediaEngine* fme_; cricket::FakeDataEngine* fdme_; diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 8ac1e83543..129eed6654 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -27,6 +27,7 @@ #include "api/rtc_error.h" #include "api/rtp_parameters.h" #include "api/uma_metrics.h" +#include "api/video/builtin_video_bitrate_allocator_factory.h" #include "call/call.h" #include "logging/rtc_event_log/ice_logger.h" #include "logging/rtc_event_log/output/rtc_event_log_output_file.h" @@ -1199,6 +1200,14 @@ bool PeerConnection::Initialize( return_histogram_very_quickly_ ? 0 : REPORT_USAGE_PATTERN_DELAY_MS; signaling_thread()->PostDelayed(RTC_FROM_HERE, delay_ms, this, MSG_REPORT_USAGE_PATTERN, nullptr); + + if (dependencies.video_bitrate_allocator_factory) { + video_bitrate_allocator_factory_ = + std::move(dependencies.video_bitrate_allocator_factory); + } else { + video_bitrate_allocator_factory_ = + CreateBuiltinVideoBitrateAllocatorFactory(); + } return true; } @@ -6311,7 +6320,7 @@ cricket::VideoChannel* PeerConnection::CreateVideoChannel( cricket::VideoChannel* video_channel = channel_manager()->CreateVideoChannel( call_ptr_, configuration_.media_config, rtp_transport, media_transport, signaling_thread(), mid, SrtpRequired(), GetCryptoOptions(), - &ssrc_generator_, video_options_); + &ssrc_generator_, video_options_, video_bitrate_allocator_factory_.get()); if (!video_channel) { return nullptr; } diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 2cf3662b65..c21261cc11 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -1332,6 +1332,15 @@ class PeerConnection : public PeerConnectionInternal, // channel manager and the session description factory. rtc::UniqueRandomIdGenerator ssrc_generator_ RTC_GUARDED_BY(signaling_thread()); + + // A video bitrate allocator factory. + // This can injected using the PeerConnectionDependencies, + // or else the CreateBuiltinVideoBitrateAllocatorFactory() will be called. + // Note that one can still choose to override this in a MediaEngine + // if one wants too. + std::unique_ptr + video_bitrate_allocator_factory_; + bool is_negotiation_needed_ RTC_GUARDED_BY(signaling_thread()) = false; }; diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc index 38f414cb3b..67cbe2ca4f 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc @@ -29,6 +29,7 @@ #include "api/scoped_refptr.h" #include "api/test/fake_frame_decryptor.h" #include "api/test/fake_frame_encryptor.h" +#include "api/video/builtin_video_bitrate_allocator_factory.h" #include "logging/rtc_event_log/rtc_event_log.h" #include "media/base/codec.h" #include "media/base/fake_media_engine.h" @@ -94,6 +95,8 @@ class RtpSenderReceiverTest RtpSenderReceiverTest() : network_thread_(rtc::Thread::Current()), worker_thread_(rtc::Thread::Current()), + video_bitrate_allocator_factory_( + webrtc::CreateBuiltinVideoBitrateAllocatorFactory()), // Create fake media engine/etc. so we can create channels to use to // test RtpSenders/RtpReceivers. media_engine_(new cricket::FakeMediaEngine()), @@ -119,7 +122,7 @@ class RtpSenderReceiverTest &fake_call_, cricket::MediaConfig(), rtp_transport_.get(), /*media_transport=*/nullptr, rtc::Thread::Current(), cricket::CN_VIDEO, srtp_required, webrtc::CryptoOptions(), &ssrc_generator_, - cricket::VideoOptions()); + cricket::VideoOptions(), video_bitrate_allocator_factory_.get()); voice_channel_->Enable(true); video_channel_->Enable(true); voice_media_channel_ = media_engine_->GetVoiceChannel(0); @@ -510,6 +513,8 @@ class RtpSenderReceiverTest // the |channel_manager|. std::unique_ptr rtp_dtls_transport_; std::unique_ptr rtp_transport_; + std::unique_ptr + video_bitrate_allocator_factory_; // |media_engine_| is actually owned by |channel_manager_|. cricket::FakeMediaEngine* media_engine_; cricket::ChannelManager channel_manager_;