Deprecate PeerConnectionFactory::CreatePeerConnection
Applications should use CreatePeerConnectionOrError instead.
Moved fallback implementations of CreatePeerConnection into the
api/peer_connection_interface.h file, so that we do not have to
declare these methods in the proxy.
Bug: webrtc:12238
Change-Id: I70c56336641c2a108b68446ae41f43409277a584
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217762
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33964}
diff --git a/api/BUILD.gn b/api/BUILD.gn
index 212e9cd..00542f3 100644
--- a/api/BUILD.gn
+++ b/api/BUILD.gn
@@ -197,6 +197,7 @@
"units:data_rate",
"units:timestamp",
"video:encoded_image",
+ "video:video_bitrate_allocator_factory",
"video:video_frame",
"video:video_rtp_headers",
diff --git a/api/DEPS b/api/DEPS
index 2e46029..1d3d43f 100644
--- a/api/DEPS
+++ b/api/DEPS
@@ -128,13 +128,18 @@
"peer_connection_interface\.h": [
"+media/base/media_config.h",
"+media/base/media_engine.h",
+ "+p2p/base/port.h",
"+p2p/base/port_allocator.h",
+ "+rtc_base/network.h",
+ "+rtc_base/network_constants.h",
"+rtc_base/network_monitor_factory.h",
+ "+rtc_base/ref_count.h",
"+rtc_base/rtc_certificate.h",
"+rtc_base/rtc_certificate_generator.h",
"+rtc_base/socket_address.h",
"+rtc_base/ssl_certificate.h",
"+rtc_base/ssl_stream_adapter.h",
+ "+rtc_base/thread.h",
],
"proxy\.h": [
diff --git a/api/peer_connection_factory_proxy.h b/api/peer_connection_factory_proxy.h
index 0eb2b39..de6250f 100644
--- a/api/peer_connection_factory_proxy.h
+++ b/api/peer_connection_factory_proxy.h
@@ -25,16 +25,6 @@
BEGIN_PROXY_MAP(PeerConnectionFactory)
PROXY_PRIMARY_THREAD_DESTRUCTOR()
PROXY_METHOD1(void, SetOptions, const Options&)
-PROXY_METHOD4(rtc::scoped_refptr<PeerConnectionInterface>,
- CreatePeerConnection,
- const PeerConnectionInterface::RTCConfiguration&,
- std::unique_ptr<cricket::PortAllocator>,
- std::unique_ptr<rtc::RTCCertificateGeneratorInterface>,
- PeerConnectionObserver*)
-PROXY_METHOD2(rtc::scoped_refptr<PeerConnectionInterface>,
- CreatePeerConnection,
- const PeerConnectionInterface::RTCConfiguration&,
- PeerConnectionDependencies)
PROXY_METHOD2(RTCErrorOr<rtc::scoped_refptr<PeerConnectionInterface>>,
CreatePeerConnectionOrError,
const PeerConnectionInterface::RTCConfiguration&,
diff --git a/api/peer_connection_interface.cc b/api/peer_connection_interface.cc
index e1d94dd..230731c 100644
--- a/api/peer_connection_interface.cc
+++ b/api/peer_connection_interface.cc
@@ -10,8 +10,7 @@
#include "api/peer_connection_interface.h"
-#include "api/dtls_transport_interface.h"
-#include "api/sctp_transport_interface.h"
+#include <utility>
namespace webrtc {
@@ -77,14 +76,27 @@
std::unique_ptr<cricket::PortAllocator> allocator,
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
PeerConnectionObserver* observer) {
- return nullptr;
+ PeerConnectionDependencies dependencies(observer);
+ dependencies.allocator = std::move(allocator);
+ dependencies.cert_generator = std::move(cert_generator);
+ auto result =
+ CreatePeerConnectionOrError(configuration, std::move(dependencies));
+ if (!result.ok()) {
+ return nullptr;
+ }
+ return result.MoveValue();
}
rtc::scoped_refptr<PeerConnectionInterface>
PeerConnectionFactoryInterface::CreatePeerConnection(
const PeerConnectionInterface::RTCConfiguration& configuration,
PeerConnectionDependencies dependencies) {
- return nullptr;
+ auto result =
+ CreatePeerConnectionOrError(configuration, std::move(dependencies));
+ if (!result.ok()) {
+ return nullptr;
+ }
+ return result.MoveValue();
}
RTCErrorOr<rtc::scoped_refptr<PeerConnectionInterface>>
diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h
index 800f36c..892e84e 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -67,12 +67,16 @@
#ifndef API_PEER_CONNECTION_INTERFACE_H_
#define API_PEER_CONNECTION_INTERFACE_H_
+#include <stdint.h>
#include <stdio.h>
+#include <functional>
#include <memory>
#include <string>
#include <vector>
+#include "absl/base/attributes.h"
+#include "absl/types/optional.h"
#include "api/adaptation/resource.h"
#include "api/async_dns_resolver.h"
#include "api/async_resolver_factory.h"
@@ -81,6 +85,7 @@
#include "api/audio_codecs/audio_encoder_factory.h"
#include "api/audio_options.h"
#include "api/call/call_factory_interface.h"
+#include "api/candidate.h"
#include "api/crypto/crypto_options.h"
#include "api/data_channel_interface.h"
#include "api/dtls_transport_interface.h"
@@ -88,15 +93,18 @@
#include "api/ice_transport_interface.h"
#include "api/jsep.h"
#include "api/media_stream_interface.h"
+#include "api/media_types.h"
#include "api/neteq/neteq_factory.h"
#include "api/network_state_predictor.h"
#include "api/packet_socket_factory.h"
#include "api/rtc_error.h"
#include "api/rtc_event_log/rtc_event_log_factory_interface.h"
#include "api/rtc_event_log_output.h"
+#include "api/rtp_parameters.h"
#include "api/rtp_receiver_interface.h"
#include "api/rtp_sender_interface.h"
#include "api/rtp_transceiver_interface.h"
+#include "api/scoped_refptr.h"
#include "api/sctp_transport_interface.h"
#include "api/set_local_description_observer_interface.h"
#include "api/set_remote_description_observer_interface.h"
@@ -109,19 +117,25 @@
#include "api/transport/sctp_transport_factory_interface.h"
#include "api/transport/webrtc_key_value_config.h"
#include "api/turn_customizer.h"
+#include "api/video/video_bitrate_allocator_factory.h"
#include "media/base/media_config.h"
#include "media/base/media_engine.h"
// TODO(bugs.webrtc.org/7447): We plan to provide a way to let applications
// inject a PacketSocketFactory and/or NetworkManager, and not expose
-// PortAllocator in the PeerConnection api.
+// PortAllocator in the PeerConnection api. This will let us remove nogncheck.
+#include "p2p/base/port.h" // nogncheck
#include "p2p/base/port_allocator.h" // nogncheck
+#include "rtc_base/network.h"
+#include "rtc_base/network_constants.h"
#include "rtc_base/network_monitor_factory.h"
+#include "rtc_base/ref_count.h"
#include "rtc_base/rtc_certificate.h"
#include "rtc_base/rtc_certificate_generator.h"
#include "rtc_base/socket_address.h"
#include "rtc_base/ssl_certificate.h"
#include "rtc_base/ssl_stream_adapter.h"
#include "rtc_base/system/rtc_export.h"
+#include "rtc_base/thread.h"
namespace rtc {
class Thread;
@@ -1432,6 +1446,7 @@
PeerConnectionDependencies dependencies);
// Deprecated creator - does not return an error code on error.
// TODO(bugs.webrtc.org:12238): Deprecate and remove.
+ ABSL_DEPRECATED("Use CreatePeerConnectionOrError")
virtual rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
const PeerConnectionInterface::RTCConfiguration& configuration,
PeerConnectionDependencies dependencies);
@@ -1445,6 +1460,7 @@
// responsibility of the caller to delete it. It can be safely deleted after
// Close has been called on the returned PeerConnection, which ensures no
// more observer callbacks will be invoked.
+ ABSL_DEPRECATED("Use CreatePeerConnectionOrError")
virtual rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
const PeerConnectionInterface::RTCConfiguration& configuration,
std::unique_ptr<cricket::PortAllocator> allocator,
diff --git a/examples/androidnativeapi/jni/android_call_client.cc b/examples/androidnativeapi/jni/android_call_client.cc
index f0b0606..f38de24 100644
--- a/examples/androidnativeapi/jni/android_call_client.cc
+++ b/examples/androidnativeapi/jni/android_call_client.cc
@@ -179,9 +179,9 @@
config.sdp_semantics = webrtc::SdpSemantics::kUnifiedPlan;
// DTLS SRTP has to be disabled for loopback to work.
config.enable_dtls_srtp = false;
- pc_ = pcf_->CreatePeerConnection(config, nullptr /* port_allocator */,
- nullptr /* cert_generator */,
- pc_observer_.get());
+ webrtc::PeerConnectionDependencies deps(pc_observer_.get());
+ pc_ = pcf_->CreatePeerConnectionOrError(config, std::move(deps)).MoveValue();
+
RTC_LOG(LS_INFO) << "PeerConnection created: " << pc_;
rtc::scoped_refptr<webrtc::VideoTrackInterface> local_video_track =
diff --git a/examples/objcnativeapi/objc/objc_call_client.mm b/examples/objcnativeapi/objc/objc_call_client.mm
index 5ce7eb7..419203e 100644
--- a/examples/objcnativeapi/objc/objc_call_client.mm
+++ b/examples/objcnativeapi/objc/objc_call_client.mm
@@ -144,7 +144,7 @@
// DTLS SRTP has to be disabled for loopback to work.
config.enable_dtls_srtp = false;
webrtc::PeerConnectionDependencies pc_dependencies(pc_observer_.get());
- pc_ = pcf_->CreatePeerConnection(config, std::move(pc_dependencies));
+ pc_ = pcf_->CreatePeerConnectionOrError(config, std::move(pc_dependencies)).MoveValue();
RTC_LOG(LS_INFO) << "PeerConnection created: " << pc_;
rtc::scoped_refptr<webrtc::VideoTrackInterface> local_video_track =
diff --git a/examples/unityplugin/simple_peer_connection.cc b/examples/unityplugin/simple_peer_connection.cc
index 23e4d7b..128ca76 100644
--- a/examples/unityplugin/simple_peer_connection.cc
+++ b/examples/unityplugin/simple_peer_connection.cc
@@ -192,10 +192,14 @@
config_.servers.push_back(stun_server);
config_.enable_dtls_srtp = false;
- peer_connection_ = g_peer_connection_factory->CreatePeerConnection(
- config_, nullptr, nullptr, this);
-
- return peer_connection_.get() != nullptr;
+ auto result = g_peer_connection_factory->CreatePeerConnectionOrError(
+ config_, webrtc::PeerConnectionDependencies(this));
+ if (!result.ok()) {
+ peer_connection_ = nullptr;
+ return false;
+ }
+ peer_connection_ = result.MoveValue();
+ return true;
}
void SimplePeerConnection::DeletePeerConnection() {
diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc
index 515494d..89b39e5 100644
--- a/pc/peer_connection_factory.cc
+++ b/pc/peer_connection_factory.cc
@@ -33,6 +33,7 @@
#include "p2p/base/basic_async_resolver_factory.h"
#include "p2p/base/basic_packet_socket_factory.h"
#include "p2p/base/default_ice_transport_factory.h"
+#include "p2p/base/port_allocator.h"
#include "p2p/client/basic_port_allocator.h"
#include "pc/audio_track.h"
#include "pc/local_audio_source.h"
@@ -48,6 +49,7 @@
#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_conversions.h"
#include "rtc_base/ref_counted_object.h"
+#include "rtc_base/rtc_certificate_generator.h"
#include "rtc_base/system/file_wrapper.h"
namespace webrtc {
@@ -184,33 +186,6 @@
channel_manager()->StopAecDump();
}
-rtc::scoped_refptr<PeerConnectionInterface>
-PeerConnectionFactory::CreatePeerConnection(
- const PeerConnectionInterface::RTCConfiguration& configuration,
- std::unique_ptr<cricket::PortAllocator> allocator,
- std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
- PeerConnectionObserver* observer) {
- // Convert the legacy API into the new dependency structure.
- PeerConnectionDependencies dependencies(observer);
- dependencies.allocator = std::move(allocator);
- dependencies.cert_generator = std::move(cert_generator);
- // Pass that into the new API.
- return CreatePeerConnection(configuration, std::move(dependencies));
-}
-
-rtc::scoped_refptr<PeerConnectionInterface>
-PeerConnectionFactory::CreatePeerConnection(
- const PeerConnectionInterface::RTCConfiguration& configuration,
- PeerConnectionDependencies dependencies) {
- auto result =
- CreatePeerConnectionOrError(configuration, std::move(dependencies));
- if (result.ok()) {
- return result.MoveValue();
- } else {
- return nullptr;
- }
-}
-
RTCErrorOr<rtc::scoped_refptr<PeerConnectionInterface>>
PeerConnectionFactory::CreatePeerConnectionOrError(
const PeerConnectionInterface::RTCConfiguration& configuration,
diff --git a/pc/peer_connection_factory.h b/pc/peer_connection_factory.h
index d2bac7a..bd2efe4 100644
--- a/pc/peer_connection_factory.h
+++ b/pc/peer_connection_factory.h
@@ -14,7 +14,6 @@
#include <stdint.h>
#include <stdio.h>
-
#include <memory>
#include <string>
@@ -66,16 +65,6 @@
void SetOptions(const Options& options) override;
- rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
- const PeerConnectionInterface::RTCConfiguration& configuration,
- std::unique_ptr<cricket::PortAllocator> allocator,
- std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
- PeerConnectionObserver* observer) override;
-
- rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
- const PeerConnectionInterface::RTCConfiguration& configuration,
- PeerConnectionDependencies dependencies) override;
-
RTCErrorOr<rtc::scoped_refptr<PeerConnectionInterface>>
CreatePeerConnectionOrError(
const PeerConnectionInterface::RTCConfiguration& configuration,
diff --git a/pc/peer_connection_rampup_tests.cc b/pc/peer_connection_rampup_tests.cc
index c353ae5..d50d488 100644
--- a/pc/peer_connection_rampup_tests.cc
+++ b/pc/peer_connection_rampup_tests.cc
@@ -192,14 +192,14 @@
dependencies.tls_cert_verifier =
std::make_unique<rtc::TestCertificateVerifier>();
- auto pc =
- pc_factory_->CreatePeerConnection(config, std::move(dependencies));
- if (!pc) {
+ auto result = pc_factory_->CreatePeerConnectionOrError(
+ config, std::move(dependencies));
+ if (!result.ok()) {
return nullptr;
}
return std::make_unique<PeerConnectionWrapperForRampUpTest>(
- pc_factory_, pc, std::move(observer));
+ pc_factory_, result.MoveValue(), std::move(observer));
}
void SetupOneWayCall() {
diff --git a/pc/test/peer_connection_test_wrapper.cc b/pc/test/peer_connection_test_wrapper.cc
index d88fe0d..56e81ec 100644
--- a/pc/test/peer_connection_test_wrapper.cc
+++ b/pc/test/peer_connection_test_wrapper.cc
@@ -123,10 +123,17 @@
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator(
new FakeRTCCertificateGenerator());
- peer_connection_ = peer_connection_factory_->CreatePeerConnection(
- config, std::move(port_allocator), std::move(cert_generator), this);
-
- return peer_connection_.get() != NULL;
+ webrtc::PeerConnectionDependencies deps(this);
+ deps.allocator = std::move(port_allocator);
+ deps.cert_generator = std::move(cert_generator);
+ auto result = peer_connection_factory_->CreatePeerConnectionOrError(
+ config, std::move(deps));
+ if (result.ok()) {
+ peer_connection_ = result.MoveValue();
+ return true;
+ } else {
+ return false;
+ }
}
rtc::scoped_refptr<webrtc::DataChannelInterface>
diff --git a/sdk/android/src/jni/pc/peer_connection_factory.cc b/sdk/android/src/jni/pc/peer_connection_factory.cc
index 2392db2..53e715b 100644
--- a/sdk/android/src/jni/pc/peer_connection_factory.cc
+++ b/sdk/android/src/jni/pc/peer_connection_factory.cc
@@ -471,14 +471,14 @@
jni, j_sslCertificateVerifier);
}
- rtc::scoped_refptr<PeerConnectionInterface> pc =
- PeerConnectionFactoryFromJava(factory)->CreatePeerConnection(
+ auto result =
+ PeerConnectionFactoryFromJava(factory)->CreatePeerConnectionOrError(
rtc_config, std::move(peer_connection_dependencies));
- if (!pc)
+ if (!result.ok())
return 0;
- return jlongFromPointer(
- new OwnedPeerConnection(pc, std::move(observer), std::move(constraints)));
+ return jlongFromPointer(new OwnedPeerConnection(
+ result.MoveValue(), std::move(observer), std::move(constraints)));
}
static jlong JNI_PeerConnectionFactory_CreateVideoSource(
diff --git a/sdk/objc/api/peerconnection/RTCPeerConnection.mm b/sdk/objc/api/peerconnection/RTCPeerConnection.mm
index 8a47d22..05fe581 100644
--- a/sdk/objc/api/peerconnection/RTCPeerConnection.mm
+++ b/sdk/objc/api/peerconnection/RTCPeerConnection.mm
@@ -348,11 +348,12 @@
webrtc::PeerConnectionDependencies deps = std::move(*dependencies.release());
deps.observer = _observer.get();
- _peerConnection = factory.nativeFactory->CreatePeerConnection(*config, std::move(deps));
+ auto result = factory.nativeFactory->CreatePeerConnectionOrError(*config, std::move(deps));
- if (!_peerConnection) {
+ if (!result.ok()) {
return nil;
}
+ _peerConnection = result.MoveValue();
_factory = factory;
_localStreams = [[NSMutableArray alloc] init];
_delegate = delegate;
diff --git a/test/network/network_emulation_pc_unittest.cc b/test/network/network_emulation_pc_unittest.cc
index 6420e36..bd15b5a 100644
--- a/test/network/network_emulation_pc_unittest.cc
+++ b/test/network/network_emulation_pc_unittest.cc
@@ -99,7 +99,12 @@
rtc_configuration.servers.push_back(server);
}
- return pcf->CreatePeerConnection(rtc_configuration, std::move(pc_deps));
+ auto result =
+ pcf->CreatePeerConnectionOrError(rtc_configuration, std::move(pc_deps));
+ if (!result.ok()) {
+ return nullptr;
+ }
+ return result.MoveValue();
}
} // namespace
diff --git a/test/pc/e2e/test_peer_factory.cc b/test/pc/e2e/test_peer_factory.cc
index eceec77..869b40f 100644
--- a/test/pc/e2e/test_peer_factory.cc
+++ b/test/pc/e2e/test_peer_factory.cc
@@ -348,8 +348,10 @@
PeerConnectionDependencies pc_deps = CreatePCDependencies(
observer.get(), std::move(components->pc_dependencies));
rtc::scoped_refptr<PeerConnectionInterface> peer_connection =
- peer_connection_factory->CreatePeerConnection(params->rtc_configuration,
- std::move(pc_deps));
+ peer_connection_factory
+ ->CreatePeerConnectionOrError(params->rtc_configuration,
+ std::move(pc_deps))
+ .MoveValue();
peer_connection->SetBitrate(params->bitrate_settings);
return absl::WrapUnique(new TestPeer(
diff --git a/test/peer_scenario/peer_scenario_client.cc b/test/peer_scenario/peer_scenario_client.cc
index 681a907..7f3e126 100644
--- a/test/peer_scenario/peer_scenario_client.cc
+++ b/test/peer_scenario/peer_scenario_client.cc
@@ -241,7 +241,9 @@
pc_deps.allocator->set_flags(pc_deps.allocator->flags() |
cricket::PORTALLOCATOR_DISABLE_TCP);
peer_connection_ =
- pc_factory_->CreatePeerConnection(config.rtc_config, std::move(pc_deps));
+ pc_factory_
+ ->CreatePeerConnectionOrError(config.rtc_config, std::move(pc_deps))
+ .MoveValue();
if (log_writer_factory_) {
peer_connection_->StartRtcEventLog(log_writer_factory_->Create(".rtc.dat"),
/*output_period_ms=*/1000);
diff --git a/webrtc_lib_link_test.cc b/webrtc_lib_link_test.cc
index 37e1b14..055bd96 100644
--- a/webrtc_lib_link_test.cc
+++ b/webrtc_lib_link_test.cc
@@ -65,9 +65,10 @@
auto peer_connection_factory =
webrtc::CreateModularPeerConnectionFactory(std::move(pcf_deps));
webrtc::PeerConnectionInterface::RTCConfiguration rtc_config;
- auto peer_connection = peer_connection_factory->CreatePeerConnection(
- rtc_config, nullptr, nullptr, nullptr);
- printf("peer_connection=%s\n", peer_connection == nullptr ? "nullptr" : "ok");
+ auto result = peer_connection_factory->CreatePeerConnectionOrError(
+ rtc_config, PeerConnectionDependencies(nullptr));
+ // Creation will fail because of null observer, but that's OK.
+ printf("peer_connection creation=%s\n", result.ok() ? "succeeded" : "failed");
}
void TestCase2RegularFactory() {
@@ -81,9 +82,10 @@
std::move(media_deps.video_encoder_factory),
std::move(media_deps.video_decoder_factory), nullptr, nullptr);
webrtc::PeerConnectionInterface::RTCConfiguration rtc_config;
- auto peer_connection = peer_connection_factory->CreatePeerConnection(
- rtc_config, nullptr, nullptr, nullptr);
- printf("peer_connection=%s\n", peer_connection == nullptr ? "nullptr" : "ok");
+ auto result = peer_connection_factory->CreatePeerConnectionOrError(
+ rtc_config, PeerConnectionDependencies(nullptr));
+ // Creation will fail because of null observer, but that's OK.
+ printf("peer_connection creation=%s\n", result.ok() ? "succeeded" : "failed");
}
} // namespace webrtc
