Make VideoBitrateAllocatorFactory injectable.

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 <jonaso@webrtc.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27654}
This commit is contained in:
Jonas Oreland
2019-04-17 07:38:40 +02:00
committed by Commit Bot
parent a1a93fba36
commit a3aa9bd75b
22 changed files with 144 additions and 129 deletions

View File

@ -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",

View File

@ -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<VideoChannel*>(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;
}

View File

@ -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);

View File

@ -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<DtlsTransportInternal> rtp_dtls_transport_;
std::unique_ptr<rtc::Thread> network_;
std::unique_ptr<rtc::Thread> worker_;
std::unique_ptr<webrtc::VideoBitrateAllocatorFactory>
video_bitrate_allocator_factory_;
// |fme_| and |fdme_| are actually owned by |cm_|.
cricket::FakeMediaEngine* fme_;
cricket::FakeDataEngine* fdme_;

View File

@ -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;
}

View File

@ -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<webrtc::VideoBitrateAllocatorFactory>
video_bitrate_allocator_factory_;
bool is_negotiation_needed_ RTC_GUARDED_BY(signaling_thread()) = false;
};

View File

@ -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<cricket::DtlsTransportInternal> rtp_dtls_transport_;
std::unique_ptr<webrtc::RtpTransportInternal> rtp_transport_;
std::unique_ptr<webrtc::VideoBitrateAllocatorFactory>
video_bitrate_allocator_factory_;
// |media_engine_| is actually owned by |channel_manager_|.
cricket::FakeMediaEngine* media_engine_;
cricket::ChannelManager channel_manager_;