admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Change PeerConnection creation to use a static "Create" method

Browse Source 
This allows making more members (including IsUnifiedPlan) const in a future CL.

Also revises the test for ReportUsageHistogram to use a configuration member
variable rather than a hook function in PeerConnectionFactory.

Bug: webrtc:12079
Change-Id: I6f1af7d6164c8a0d8466f76378a925d72d57d685
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/190280
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32485}
This commit is contained in:
Harald Alvestrand 2020-10-26 08:30:41 +00:00 committed by Commit Bot
parent 7c85d395d7
commit 6216693363
6 changed files with 147 additions and 156 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
api/peer_connection_interface.h
View File

@ -640,6 +640,9 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface {
// Whether network condition based codec switching is allowed.
absl::optional<bool> allow_codec_switching;
// The delay before doing a usage histogram report for long-lived
// PeerConnections. Used for testing only.
absl::optional<int> report_usage_pattern_delay_ms;
//
// Don't forget to update operator== if adding something.
//

188
pc/peer_connection.cc
View File

@ -209,6 +209,61 @@ bool NeedIceRestart(bool surface_ice_candidates_on_ice_transport_type_changed,
return (current_filter & modified_filter) != current_filter;
}
cricket::IceConfig ParseIceConfig(
const PeerConnectionInterface::RTCConfiguration& config) {
cricket::ContinualGatheringPolicy gathering_policy;
switch (config.continual_gathering_policy) {
case PeerConnectionInterface::GATHER_ONCE:
gathering_policy = cricket::GATHER_ONCE;
break;
case PeerConnectionInterface::GATHER_CONTINUALLY:
gathering_policy = cricket::GATHER_CONTINUALLY;
break;
default:
RTC_NOTREACHED();
gathering_policy = cricket::GATHER_ONCE;
}
cricket::IceConfig ice_config;
ice_config.receiving_timeout = RTCConfigurationToIceConfigOptionalInt(
config.ice_connection_receiving_timeout);
ice_config.prioritize_most_likely_candidate_pairs =
config.prioritize_most_likely_ice_candidate_pairs;
ice_config.backup_connection_ping_interval =
RTCConfigurationToIceConfigOptionalInt(
config.ice_backup_candidate_pair_ping_interval);
ice_config.continual_gathering_policy = gathering_policy;
ice_config.presume_writable_when_fully_relayed =
config.presume_writable_when_fully_relayed;
ice_config.surface_ice_candidates_on_ice_transport_type_changed =
config.surface_ice_candidates_on_ice_transport_type_changed;
ice_config.ice_check_interval_strong_connectivity =
config.ice_check_interval_strong_connectivity;
ice_config.ice_check_interval_weak_connectivity =
config.ice_check_interval_weak_connectivity;
ice_config.ice_check_min_interval = config.ice_check_min_interval;
ice_config.ice_unwritable_timeout = config.ice_unwritable_timeout;
ice_config.ice_unwritable_min_checks = config.ice_unwritable_min_checks;
ice_config.ice_inactive_timeout = config.ice_inactive_timeout;
ice_config.stun_keepalive_interval = config.stun_candidate_keepalive_interval;
ice_config.network_preference = config.network_preference;
return ice_config;
}
// Ensures the configuration doesn't have any parameters with invalid values,
// or values that conflict with other parameters.
//
// Returns RTCError::OK() if there are no issues.
RTCError ValidateConfiguration(
const PeerConnectionInterface::RTCConfiguration& config) {
return cricket::P2PTransportChannel::ValidateIceConfig(
ParseIceConfig(config));
}
bool HasRtcpMuxEnabled(const cricket::ContentInfo* content) {
return content->media_description()->rtcp_mux();
}
} // namespace
bool PeerConnectionInterface::RTCConfiguration::operator==(
@ -263,6 +318,7 @@ bool PeerConnectionInterface::RTCConfiguration::operator==(
std::string turn_logging_id;
bool enable_implicit_rollback;
absl::optional<bool> allow_codec_switching;
absl::optional<int> report_usage_pattern_delay_ms;
};
static_assert(sizeof(stuff_being_tested_for_equality) == sizeof(*this),
"Did you add something to RTCConfiguration and forget to "
@ -322,7 +378,8 @@ bool PeerConnectionInterface::RTCConfiguration::operator==(
offer_extmap_allow_mixed == o.offer_extmap_allow_mixed &&
turn_logging_id == o.turn_logging_id &&
enable_implicit_rollback == o.enable_implicit_rollback &&
allow_codec_switching == o.allow_codec_switching;
allow_codec_switching == o.allow_codec_switching &&
report_usage_pattern_delay_ms == o.report_usage_pattern_delay_ms;
}
bool PeerConnectionInterface::RTCConfiguration::operator!=(
@ -330,10 +387,50 @@ bool PeerConnectionInterface::RTCConfiguration::operator!=(
return !(*this == o);
}
rtc::scoped_refptr<PeerConnection> PeerConnection::Create(
rtc::scoped_refptr<ConnectionContext> context,
std::unique_ptr<RtcEventLog> event_log,
std::unique_ptr<Call> call,
const PeerConnectionInterface::RTCConfiguration& configuration,
PeerConnectionDependencies dependencies) {
RTCError config_error = cricket::P2PTransportChannel::ValidateIceConfig(
ParseIceConfig(configuration));
if (!config_error.ok()) {
RTC_LOG(LS_ERROR) << "Invalid configuration: " << config_error.message();
return nullptr;
}
if (!dependencies.allocator) {
RTC_LOG(LS_ERROR)
<< "PeerConnection initialized without a PortAllocator? "
"This shouldn't happen if using PeerConnectionFactory.";
return nullptr;
}
if (!dependencies.observer) {
// TODO(deadbeef): Why do we do this?
RTC_LOG(LS_ERROR) << "PeerConnection initialized without a "
"PeerConnectionObserver";
return nullptr;
}
bool is_unified_plan =
configuration.sdp_semantics == SdpSemantics::kUnifiedPlan;
rtc::scoped_refptr<PeerConnection> pc(
new rtc::RefCountedObject<PeerConnection>(
context, is_unified_plan, std::move(event_log), std::move(call)));
if (!pc->Initialize(configuration, std::move(dependencies))) {
return nullptr;
}
return pc;
}
PeerConnection::PeerConnection(rtc::scoped_refptr<ConnectionContext> context,
bool is_unified_plan,
std::unique_ptr<RtcEventLog> event_log,
std::unique_ptr<Call> call)
: context_(context),
is_unified_plan_(is_unified_plan),
event_log_(std::move(event_log)),
event_log_ptr_(event_log_.get()),
call_(std::move(call)),
@ -393,26 +490,6 @@ bool PeerConnection::Initialize(
RTC_DCHECK_RUN_ON(signaling_thread());
TRACE_EVENT0("webrtc", "PeerConnection::Initialize");
RTCError config_error = ValidateConfiguration(configuration);
if (!config_error.ok()) {
RTC_LOG(LS_ERROR) << "Invalid configuration: " << config_error.message();
return false;
}
if (!dependencies.allocator) {
RTC_LOG(LS_ERROR)
<< "PeerConnection initialized without a PortAllocator? "
"This shouldn't happen if using PeerConnectionFactory.";
return false;
}
if (!dependencies.observer) {
// TODO(deadbeef): Why do we do this?
RTC_LOG(LS_ERROR) << "PeerConnection initialized without a "
"PeerConnectionObserver";
return false;
}
observer_ = dependencies.observer;
async_resolver_factory_ = std::move(dependencies.async_resolver_factory);
port_allocator_ = std::move(dependencies.allocator);
@ -487,20 +564,12 @@ bool PeerConnection::Initialize(
#endif
config.active_reset_srtp_params = configuration.active_reset_srtp_params;
// Obtain a certificate from RTCConfiguration if any were provided (optional).
rtc::scoped_refptr<rtc::RTCCertificate> certificate;
if (!configuration.certificates.empty()) {
// TODO(hbos,torbjorng): Decide on certificate-selection strategy instead of
// just picking the first one. The decision should be made based on the DTLS
// handshake. The DTLS negotiations need to know about all certificates.
certificate = configuration.certificates[0];
}
if (options.disable_encryption) {
dtls_enabled_ = false;
} else {
// Enable DTLS by default if we have an identity store or a certificate.
dtls_enabled_ = (dependencies.cert_generator || certificate);
dtls_enabled_ =
(dependencies.cert_generator || !configuration.certificates.empty());
// |configuration| can override the default |dtls_enabled_| value.
if (configuration.enable_dtls_srtp) {
dtls_enabled_ = *(configuration.enable_dtls_srtp);
@ -571,11 +640,11 @@ bool PeerConnection::Initialize(
RtpTransceiverProxyWithInternal<RtpTransceiver>::Create(
signaling_thread(), new RtpTransceiver(cricket::MEDIA_TYPE_VIDEO)));
}
int delay_ms =
return_histogram_very_quickly_ ? 0 : REPORT_USAGE_PATTERN_DELAY_MS;
sdp_handler_.Initialize(configuration, &dependencies);
int delay_ms = configuration.report_usage_pattern_delay_ms
? *configuration.report_usage_pattern_delay_ms
: REPORT_USAGE_PATTERN_DELAY_MS;
message_handler_.RequestUsagePatternReport(
[this]() {
RTC_DCHECK_RUN_ON(signaling_thread());
@ -586,12 +655,6 @@ bool PeerConnection::Initialize(
return true;
}
RTCError PeerConnection::ValidateConfiguration(
const RTCConfiguration& config) const {
return cricket::P2PTransportChannel::ValidateIceConfig(
ParseIceConfig(config));
}
rtc::scoped_refptr<StreamCollectionInterface> PeerConnection::local_streams() {
RTC_DCHECK_RUN_ON(signaling_thread());
RTC_CHECK(!IsUnifiedPlan()) << "local_streams is not available with Unified "
@ -1912,47 +1975,6 @@ bool PeerConnection::GetTransportDescription(
return true;
}
cricket::IceConfig PeerConnection::ParseIceConfig(
const PeerConnectionInterface::RTCConfiguration& config) const {
cricket::ContinualGatheringPolicy gathering_policy;
switch (config.continual_gathering_policy) {
case PeerConnectionInterface::GATHER_ONCE:
gathering_policy = cricket::GATHER_ONCE;
break;
case PeerConnectionInterface::GATHER_CONTINUALLY:
gathering_policy = cricket::GATHER_CONTINUALLY;
break;
default:
RTC_NOTREACHED();
gathering_policy = cricket::GATHER_ONCE;
}
cricket::IceConfig ice_config;
ice_config.receiving_timeout = RTCConfigurationToIceConfigOptionalInt(
config.ice_connection_receiving_timeout);
ice_config.prioritize_most_likely_candidate_pairs =
config.prioritize_most_likely_ice_candidate_pairs;
ice_config.backup_connection_ping_interval =
RTCConfigurationToIceConfigOptionalInt(
config.ice_backup_candidate_pair_ping_interval);
ice_config.continual_gathering_policy = gathering_policy;
ice_config.presume_writable_when_fully_relayed =
config.presume_writable_when_fully_relayed;
ice_config.surface_ice_candidates_on_ice_transport_type_changed =
config.surface_ice_candidates_on_ice_transport_type_changed;
ice_config.ice_check_interval_strong_connectivity =
config.ice_check_interval_strong_connectivity;
ice_config.ice_check_interval_weak_connectivity =
config.ice_check_interval_weak_connectivity;
ice_config.ice_check_min_interval = config.ice_check_min_interval;
ice_config.ice_unwritable_timeout = config.ice_unwritable_timeout;
ice_config.ice_unwritable_min_checks = config.ice_unwritable_min_checks;
ice_config.ice_inactive_timeout = config.ice_inactive_timeout;
ice_config.stun_keepalive_interval = config.stun_candidate_keepalive_interval;
ice_config.network_preference = config.network_preference;
return ice_config;
}
std::vector<DataChannelStats> PeerConnection::GetDataChannelStats() const {
RTC_DCHECK_RUN_ON(signaling_thread());
return data_channel_controller_.GetDataChannelStats();
@ -2250,10 +2272,6 @@ bool PeerConnection::ValidateBundleSettings(const SessionDescription* desc) {
return true;
}
bool PeerConnection::HasRtcpMuxEnabled(const cricket::ContentInfo* content) {
return content->media_description()->rtcp_mux();
}
void PeerConnection::ReportSdpFormatReceived(
const SessionDescriptionInterface& remote_offer) {
int num_audio_mlines = 0;

51
pc/peer_connection.h
View File

@ -118,11 +118,16 @@ class PeerConnection : public PeerConnectionInternal,
public JsepTransportController::Observer,
public sigslot::has_slots<> {
public:
explicit PeerConnection(rtc::scoped_refptr<ConnectionContext> context,
std::unique_ptr<RtcEventLog> event_log,
std::unique_ptr<Call> call);
bool Initialize(
// Creates a PeerConnection and initializes it with the given values.
// If the initialization fails, the function releases the PeerConnection
// and returns nullptr.
//
// Note that the function takes ownership of dependencies, and will
// either use them or release them, whether it succeeds or fails.
static rtc::scoped_refptr<PeerConnection> Create(
rtc::scoped_refptr<ConnectionContext> context,
std::unique_ptr<RtcEventLog> event_log,
std::unique_ptr<Call> call,
const PeerConnectionInterface::RTCConfiguration& configuration,
PeerConnectionDependencies dependencies);
@ -396,7 +401,7 @@ class PeerConnection : public PeerConnectionInternal,
// sufficient time has passed.
bool IsUnifiedPlan() const {
RTC_DCHECK_RUN_ON(signaling_thread());
return configuration_.sdp_semantics == SdpSemantics::kUnifiedPlan;
return is_unified_plan_;
}
bool ValidateBundleSettings(const cricket::SessionDescription* desc);
@ -454,9 +459,19 @@ class PeerConnection : public PeerConnectionInternal,
void RequestUsagePatternReportForTesting();
protected:
// Available for rtc::scoped_refptr creation
explicit PeerConnection(rtc::scoped_refptr<ConnectionContext> context,
bool is_unified_plan,
std::unique_ptr<RtcEventLog> event_log,
std::unique_ptr<Call> call);
~PeerConnection() override;
private:
bool Initialize(
const PeerConnectionInterface::RTCConfiguration& configuration,
PeerConnectionDependencies dependencies);
rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>
FindTransceiverBySender(rtc::scoped_refptr<RtpSenderInterface> sender)
RTC_RUN_ON(signaling_thread());
@ -527,16 +542,6 @@ class PeerConnection : public PeerConnectionInternal,
// This function should only be called from the worker thread.
void StopRtcEventLog_w();
// Ensures the configuration doesn't have any parameters with invalid values,
// or values that conflict with other parameters.
//
// Returns RTCError::OK() if there are no issues.
RTCError ValidateConfiguration(const RTCConfiguration& config) const;
cricket::IceConfig ParseIceConfig(
const PeerConnectionInterface::RTCConfiguration& config) const;
// Returns true and the TransportInfo of the given |content_name|
// from |description|. Returns false if it's not available.
static bool GetTransportDescription(
@ -551,12 +556,6 @@ class PeerConnection : public PeerConnectionInternal,
int* sdp_mline_index)
RTC_RUN_ON(signaling_thread());
bool HasRtcpMuxEnabled(const cricket::ContentInfo* content);
// Verifies a=setup attribute as per RFC 5763.
bool ValidateDtlsSetupAttribute(const cricket::SessionDescription* desc,
SdpType type);
// JsepTransportController signal handlers.
void OnTransportControllerConnectionState(cricket::IceConnectionState state)
RTC_RUN_ON(signaling_thread());
@ -608,16 +607,12 @@ class PeerConnection : public PeerConnectionInternal,
int64_t packet_time_us)>
InitializeRtcpCallback();
// Storing the factory as a scoped reference pointer ensures that the memory
// in the PeerConnectionFactoryImpl remains available as long as the
// PeerConnection is running. It is passed to PeerConnection as a raw pointer.
// However, since the reference counting is done in the
// PeerConnectionFactoryInterface all instances created using the raw pointer
// will refer to the same reference count.
const rtc::scoped_refptr<ConnectionContext> context_;
PeerConnectionObserver* observer_ RTC_GUARDED_BY(signaling_thread()) =
nullptr;
const bool is_unified_plan_;
// The EventLog needs to outlive |call_| (and any other object that uses it).
std::unique_ptr<RtcEventLog> event_log_ RTC_GUARDED_BY(worker_thread());

9
pc/peer_connection_factory.cc
View File

@ -251,11 +251,10 @@ PeerConnectionFactory::CreatePeerConnection(
RTC_FROM_HERE,
rtc::Bind(&PeerConnectionFactory::CreateCall_w, this, event_log.get()));
rtc::scoped_refptr<PeerConnection> pc(
new rtc::RefCountedObject<PeerConnection>(context_, std::move(event_log),
std::move(call)));
ActionsBeforeInitializeForTesting(pc);
if (!pc->Initialize(configuration, std::move(dependencies))) {
rtc::scoped_refptr<PeerConnection> pc =
PeerConnection::Create(context_, std::move(event_log), std::move(call),
configuration, std::move(dependencies));
if (!pc) {
return nullptr;
}
return PeerConnectionProxy::Create(signaling_thread(), pc);

4
pc/peer_connection_factory.h
View File

@ -121,10 +121,6 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface {
explicit PeerConnectionFactory(
PeerConnectionFactoryDependencies dependencies);
// Hook to let testing framework insert actions between
// "new RTCPeerConnection" and "pc.Initialize"
virtual void ActionsBeforeInitializeForTesting(PeerConnectionInterface*) {}
virtual ~PeerConnectionFactory();
private:

48
pc/peer_connection_histogram_unittest.cc
View File

@ -85,18 +85,6 @@ class PeerConnectionFactoryForUsageHistogramTest
dependencies.call_factory = CreateCallFactory();
return dependencies;
}()) {}
void ActionsBeforeInitializeForTesting(PeerConnectionInterface* pc) override {
PeerConnection* internal_pc = static_cast<PeerConnection*>(pc);
if (return_histogram_very_quickly_) {
internal_pc->ReturnHistogramVeryQuicklyForTesting();
}
}
void ReturnHistogramVeryQuickly() { return_histogram_very_quickly_ = true; }
private:
bool return_histogram_very_quickly_ = false;
};
class PeerConnectionWrapperForUsageHistogramTest;
@ -255,14 +243,13 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
}
WrapperPtr CreatePeerConnection() {
return CreatePeerConnection(RTCConfiguration(),
PeerConnectionFactoryInterface::Options(),
nullptr, false);
return CreatePeerConnection(
RTCConfiguration(), PeerConnectionFactoryInterface::Options(), nullptr);
}
WrapperPtr CreatePeerConnection(const RTCConfiguration& config) {
return CreatePeerConnection(
config, PeerConnectionFactoryInterface::Options(), nullptr, false);
config, PeerConnectionFactoryInterface::Options(), nullptr);
}
WrapperPtr CreatePeerConnectionWithMdns(const RTCConfiguration& config) {
@ -282,15 +269,15 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
deps.async_resolver_factory = std::move(resolver_factory);
deps.allocator = std::move(port_allocator);
return CreatePeerConnection(config,
PeerConnectionFactoryInterface::Options(),
std::move(deps), false);
return CreatePeerConnection(
config, PeerConnectionFactoryInterface::Options(), std::move(deps));
}
WrapperPtr CreatePeerConnectionWithImmediateReport() {
return CreatePeerConnection(RTCConfiguration(),
PeerConnectionFactoryInterface::Options(),
nullptr, true);
RTCConfiguration configuration;
configuration.report_usage_pattern_delay_ms = 0;
return CreatePeerConnection(
configuration, PeerConnectionFactoryInterface::Options(), nullptr);
}
WrapperPtr CreatePeerConnectionWithPrivateLocalAddresses() {
@ -300,10 +287,9 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
auto port_allocator =
std::make_unique<cricket::BasicPortAllocator>(fake_network);
return CreatePeerConnection(RTCConfiguration(),
PeerConnectionFactoryInterface::Options(),
std::move(port_allocator), false);
std::move(port_allocator));
}
WrapperPtr CreatePeerConnectionWithPrivateIpv6LocalAddresses() {
@ -316,32 +302,26 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
return CreatePeerConnection(RTCConfiguration(),
PeerConnectionFactoryInterface::Options(),
std::move(port_allocator), false);
std::move(port_allocator));
}
WrapperPtr CreatePeerConnection(
const RTCConfiguration& config,
const PeerConnectionFactoryInterface::Options factory_options,
std::unique_ptr<cricket::PortAllocator> allocator,
bool immediate_report) {
std::unique_ptr<cricket::PortAllocator> allocator) {
PeerConnectionDependencies deps(nullptr);
deps.allocator = std::move(allocator);
return CreatePeerConnection(config, factory_options, std::move(deps),
immediate_report);
return CreatePeerConnection(config, factory_options, std::move(deps));
}
WrapperPtr CreatePeerConnection(
const RTCConfiguration& config,
const PeerConnectionFactoryInterface::Options factory_options,
PeerConnectionDependencies deps,
bool immediate_report) {
PeerConnectionDependencies deps) {
rtc::scoped_refptr<PeerConnectionFactoryForUsageHistogramTest> pc_factory(
new PeerConnectionFactoryForUsageHistogramTest());
pc_factory->SetOptions(factory_options);
if (immediate_report) {
pc_factory->ReturnHistogramVeryQuickly();
}
// If no allocator is provided, one will be created using a network manager
// that uses the host network. This doesn't work on all trybots.