Use new TransportController implementation in PeerConnection.
The TransportController will be replaced by the JsepTransportController
and JsepTransport will be replace be JsepTransport2.
The JsepTransportController will take the entire SessionDescription
and handle the RtcpMux, Sdes and BUNDLE internally.
The ownership model is also changed. The P2P layer transports are not
ref-counted and will be owned by the JsepTransport2.
In ORTC aspect, RtpTransportAdapter is now a wrapper over RtpTransport
or SrtpTransport and it implements the public and internal interface
by calling the transport underneath.
Bug: webrtc:8587
Change-Id: Ia7fa61288a566f211f8560072ea0eecaf19e48df
Reviewed-on: https://webrtc-review.googlesource.com/59586
Commit-Queue: Zhi Huang <zhihuang@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22693}diff --git a/ortc/ortcfactory.cc b/ortc/ortcfactory.cc
index 506fc86..dd3c6db 100644
--- a/ortc/ortcfactory.cc
+++ b/ortc/ortcfactory.cc
@@ -232,7 +232,7 @@
RTC_DCHECK_RUN_ON(signaling_thread_);
return RtpTransportControllerAdapter::CreateProxied(
cricket::MediaConfig(), channel_manager_.get(), null_event_log_.get(),
- signaling_thread_, worker_thread_.get());
+ signaling_thread_, worker_thread_.get(), network_thread_);
}
RTCErrorOr<std::unique_ptr<RtpTransportInterface>>
diff --git a/ortc/rtptransportadapter.cc b/ortc/rtptransportadapter.cc
index 0cf9217..acd71db 100644
--- a/ortc/rtptransportadapter.cc
+++ b/ortc/rtptransportadapter.cc
@@ -17,6 +17,7 @@
#include "api/proxy.h"
#include "rtc_base/logging.h"
+#include "rtc_base/ptr_util.h"
namespace webrtc {
@@ -115,10 +116,11 @@
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
"Must provide an RTP transport controller.");
}
- std::unique_ptr<RtpTransportAdapter> transport_adapter(
- new RtpTransportAdapter(parameters.rtcp, rtp, rtcp,
- rtp_transport_controller,
- true /*is_srtp_transport*/));
+
+ std::unique_ptr<RtpTransportAdapter> transport_adapter;
+ transport_adapter.reset(new RtpTransportAdapter(parameters.rtcp, rtp, rtcp,
+ rtp_transport_controller,
+ true /*is_srtp_transport*/));
RTCError params_result = transport_adapter->SetParameters(parameters);
if (!params_result.ok()) {
return std::move(params_result);
@@ -145,25 +147,40 @@
: rtp_packet_transport_(rtp),
rtcp_packet_transport_(rtcp),
rtp_transport_controller_(rtp_transport_controller),
- is_srtp_transport_(is_srtp_transport) {
+ network_thread_(rtp_transport_controller_->network_thread()) {
parameters_.rtcp = rtcp_params;
// CNAME should have been filled by OrtcFactory if empty.
RTC_DCHECK(!parameters_.rtcp.cname.empty());
RTC_DCHECK(rtp_transport_controller);
+
+ if (is_srtp_transport) {
+ srtp_transport_ = rtc::MakeUnique<SrtpTransport>(rtcp == nullptr);
+ transport_ = srtp_transport_.get();
+ } else {
+ unencrypted_rtp_transport_ = rtc::MakeUnique<RtpTransport>(rtcp == nullptr);
+ transport_ = unencrypted_rtp_transport_.get();
+ }
+ RTC_DCHECK(transport_);
+
+ network_thread_->Invoke<void>(RTC_FROM_HERE, [=] {
+ SetRtpPacketTransport(rtp->GetInternal());
+ if (rtcp) {
+ SetRtcpPacketTransport(rtcp->GetInternal());
+ }
+ });
+
+ transport_->SignalReadyToSend.connect(this,
+ &RtpTransportAdapter::OnReadyToSend);
+ transport_->SignalPacketReceived.connect(
+ this, &RtpTransportAdapter::OnPacketReceived);
+ transport_->SignalWritableState.connect(
+ this, &RtpTransportAdapter::OnWritableState);
}
RtpTransportAdapter::~RtpTransportAdapter() {
SignalDestroyed(this);
}
-PacketTransportInterface* RtpTransportAdapter::GetRtpPacketTransport() const {
- return rtp_packet_transport_;
-}
-
-PacketTransportInterface* RtpTransportAdapter::GetRtcpPacketTransport() const {
- return rtcp_packet_transport_;
-}
-
RTCError RtpTransportAdapter::SetParameters(
const RtpTransportParameters& parameters) {
if (!parameters.rtcp.mux && parameters_.rtcp.mux) {
@@ -194,34 +211,20 @@
RTCError RtpTransportAdapter::SetSrtpSendKey(
const cricket::CryptoParams& params) {
- if (send_key_) {
- LOG_AND_RETURN_ERROR(
- webrtc::RTCErrorType::UNSUPPORTED_OPERATION,
- "Setting the SRTP send key twice is currently unsupported.");
+ if (!network_thread_->IsCurrent()) {
+ return network_thread_->Invoke<RTCError>(
+ RTC_FROM_HERE, [&] { return SetSrtpSendKey(params); });
}
- if (receive_key_ && receive_key_->cipher_suite != params.cipher_suite) {
- LOG_AND_RETURN_ERROR(
- webrtc::RTCErrorType::UNSUPPORTED_OPERATION,
- "The send key and receive key must have the same cipher suite.");
- }
- send_key_ = params;
- return RTCError::OK();
+ return transport_->SetSrtpSendKey(params);
}
RTCError RtpTransportAdapter::SetSrtpReceiveKey(
const cricket::CryptoParams& params) {
- if (receive_key_) {
- LOG_AND_RETURN_ERROR(
- webrtc::RTCErrorType::UNSUPPORTED_OPERATION,
- "Setting the SRTP receive key twice is currently unsupported.");
+ if (!network_thread_->IsCurrent()) {
+ return network_thread_->Invoke<RTCError>(
+ RTC_FROM_HERE, [&] { return SetSrtpReceiveKey(params); });
}
- if (send_key_ && send_key_->cipher_suite != params.cipher_suite) {
- LOG_AND_RETURN_ERROR(
- webrtc::RTCErrorType::UNSUPPORTED_OPERATION,
- "The send key and receive key must have the same cipher suite.");
- }
- receive_key_ = params;
- return RTCError::OK();
+ return transport_->SetSrtpReceiveKey(params);
}
} // namespace webrtc
diff --git a/ortc/rtptransportadapter.h b/ortc/rtptransportadapter.h
index 2c98f5d..72bd3f6 100644
--- a/ortc/rtptransportadapter.h
+++ b/ortc/rtptransportadapter.h
@@ -19,19 +19,25 @@
#include "media/base/streamparams.h"
#include "ortc/rtptransportcontrolleradapter.h"
#include "pc/channel.h"
+#include "pc/rtptransport.h"
+#include "pc/rtptransportinternal.h"
+#include "pc/srtptransport.h"
#include "rtc_base/constructormagic.h"
#include "rtc_base/sigslot.h"
namespace webrtc {
-
-// Implementation of SrtpTransportInterface to be used with RtpSenderAdapter,
-// RtpReceiverAdapter, and RtpTransportControllerAdapter classes. This class
-// is used to implement both a secure and insecure RTP transport.
+// This class is a wrapper over an RtpTransport or an SrtpTransport. The base
+// class RtpTransportInternalAdapter keeps a raw pointer, |transport_|, of the
+// transport object and implements both the public SrtpTransportInterface and
+// RtpTransport internal interface by calling the |transport_| underneath.
+//
+// This adapter can be used as an unencrypted RTP transport or an SrtpTransport
+// with RtpSenderAdapter, RtpReceiverAdapter, and RtpTransportControllerAdapter.
//
// TODO(deadbeef): When BaseChannel is split apart into separate
// "RtpTransport"/"RtpTransceiver"/"RtpSender"/"RtpReceiver" objects, this
// adapter object can be removed.
-class RtpTransportAdapter : public SrtpTransportInterface {
+class RtpTransportAdapter : public RtpTransportInternalAdapter {
public:
// |rtp| can't be null. |rtcp| can if RTCP muxing is used immediately (meaning
// |rtcp_parameters.mux| is also true).
@@ -50,8 +56,12 @@
~RtpTransportAdapter() override;
// RtpTransportInterface implementation.
- PacketTransportInterface* GetRtpPacketTransport() const override;
- PacketTransportInterface* GetRtcpPacketTransport() const override;
+ PacketTransportInterface* GetRtpPacketTransport() const override {
+ return rtp_packet_transport_;
+ }
+ PacketTransportInterface* GetRtcpPacketTransport() const override {
+ return rtcp_packet_transport_;
+ }
RTCError SetParameters(const RtpTransportParameters& parameters) override;
RtpTransportParameters GetParameters() const override { return parameters_; }
@@ -70,13 +80,9 @@
// returning this transport from GetTransports().
sigslot::signal1<RtpTransportAdapter*> SignalDestroyed;
- // Used by the RtpTransportControllerAdapter to tell if an rtp sender or
- // receiver can be created.
- bool is_srtp_transport() { return is_srtp_transport_; }
- // Used by the RtpTransportControllerAdapter to set keys for senders and
- // receivers.
- rtc::Optional<cricket::CryptoParams> send_key() { return send_key_; }
- rtc::Optional<cricket::CryptoParams> receive_key() { return receive_key_; }
+ bool IsSrtpActive() const override { return transport_->IsSrtpActive(); }
+
+ bool IsSrtpTransport() const { return srtp_transport_ != nullptr; }
protected:
RtpTransportAdapter* GetInternal() override { return this; }
@@ -88,18 +94,29 @@
RtpTransportControllerAdapter* rtp_transport_controller,
bool is_srtp_transport);
- PacketTransportInterface* rtp_packet_transport_;
- PacketTransportInterface* rtcp_packet_transport_;
- RtpTransportControllerAdapter* const rtp_transport_controller_;
+ void OnReadyToSend(bool ready) { SignalReadyToSend(ready); }
+
+ void OnPacketReceived(bool rtcp,
+ rtc::CopyOnWriteBuffer* packet,
+ const rtc::PacketTime& time) {
+ SignalPacketReceived(rtcp, packet, time);
+ }
+
+ void OnWritableState(bool writable) { SignalWritableState(writable); }
+
+ PacketTransportInterface* rtp_packet_transport_ = nullptr;
+ PacketTransportInterface* rtcp_packet_transport_ = nullptr;
+ RtpTransportControllerAdapter* const rtp_transport_controller_ = nullptr;
// Non-null if this class owns the transport controller.
std::unique_ptr<RtpTransportControllerInterface>
owned_rtp_transport_controller_;
RtpTransportParameters parameters_;
- // SRTP specific members.
- rtc::Optional<cricket::CryptoParams> send_key_;
- rtc::Optional<cricket::CryptoParams> receive_key_;
- bool is_srtp_transport_;
+ // Only one of them is non-null;
+ std::unique_ptr<RtpTransport> unencrypted_rtp_transport_;
+ std::unique_ptr<SrtpTransport> srtp_transport_;
+
+ rtc::Thread* network_thread_ = nullptr;
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RtpTransportAdapter);
};
diff --git a/ortc/rtptransportcontroller_unittest.cc b/ortc/rtptransportcontroller_unittest.cc
index a663b82..4954841 100644
--- a/ortc/rtptransportcontroller_unittest.cc
+++ b/ortc/rtptransportcontroller_unittest.cc
@@ -185,6 +185,7 @@
auto video_sender_result = ortc_factory_->CreateRtpSender(
cricket::MEDIA_TYPE_VIDEO, rtp_transport2.get());
EXPECT_TRUE(video_sender_result.ok());
+
auto video_receiver_result = ortc_factory_->CreateRtpReceiver(
cricket::MEDIA_TYPE_VIDEO, rtp_transport1.get());
EXPECT_EQ(RTCErrorType::UNSUPPORTED_OPERATION,
diff --git a/ortc/rtptransportcontrolleradapter.cc b/ortc/rtptransportcontrolleradapter.cc
index 5477eff..b772ffa 100644
--- a/ortc/rtptransportcontrolleradapter.cc
+++ b/ortc/rtptransportcontrolleradapter.cc
@@ -97,10 +97,12 @@
cricket::ChannelManager* channel_manager,
webrtc::RtcEventLog* event_log,
rtc::Thread* signaling_thread,
- rtc::Thread* worker_thread) {
+ rtc::Thread* worker_thread,
+ rtc::Thread* network_thread) {
std::unique_ptr<RtpTransportControllerAdapter> wrapped(
new RtpTransportControllerAdapter(config, channel_manager, event_log,
- signaling_thread, worker_thread));
+ signaling_thread, worker_thread,
+ network_thread));
return RtpTransportControllerProxyWithInternal<
RtpTransportControllerAdapter>::Create(signaling_thread, worker_thread,
std::move(wrapped));
@@ -610,9 +612,11 @@
cricket::ChannelManager* channel_manager,
webrtc::RtcEventLog* event_log,
rtc::Thread* signaling_thread,
- rtc::Thread* worker_thread)
+ rtc::Thread* worker_thread,
+ rtc::Thread* network_thread)
: signaling_thread_(signaling_thread),
worker_thread_(worker_thread),
+ network_thread_(network_thread),
media_config_(config),
channel_manager_(channel_manager),
event_log_(event_log),
@@ -678,11 +682,7 @@
"RtpSender and RtpReceiver is not currently "
"supported.");
}
- RTCError err = MaybeSetCryptos(inner_transport, &local_audio_description_,
- &remote_audio_description_);
- if (!err.ok()) {
- return err;
- }
+
// If setting new transport, extract its RTCP parameters and create voice
// channel.
if (!inner_audio_transport_) {
@@ -713,11 +713,7 @@
"RtpSender and RtpReceiver is not currently "
"supported.");
}
- RTCError err = MaybeSetCryptos(inner_transport, &local_video_description_,
- &remote_video_description_);
- if (!err.ok()) {
- return err;
- }
+
// If setting new transport, extract its RTCP parameters and create video
// channel.
if (!inner_video_transport_) {
@@ -748,11 +744,7 @@
"RtpReceiver and RtpReceiver is not currently "
"supported.");
}
- RTCError err = MaybeSetCryptos(inner_transport, &local_audio_description_,
- &remote_audio_description_);
- if (!err.ok()) {
- return err;
- }
+
// If setting new transport, extract its RTCP parameters and create voice
// channel.
if (!inner_audio_transport_) {
@@ -783,11 +775,6 @@
"RtpReceiver and RtpReceiver is not currently "
"supported.");
}
- RTCError err = MaybeSetCryptos(inner_transport, &local_video_description_,
- &remote_video_description_);
- if (!err.ok()) {
- return err;
- }
// If setting new transport, extract its RTCP parameters and create video
// channel.
if (!inner_video_transport_) {
@@ -877,26 +864,24 @@
void RtpTransportControllerAdapter::CreateVoiceChannel() {
voice_channel_ = channel_manager_->CreateVoiceChannel(
- call_.get(), media_config_,
- inner_audio_transport_->GetRtpPacketTransport()->GetInternal(),
- inner_audio_transport_->GetRtcpPacketTransport()
- ? inner_audio_transport_->GetRtcpPacketTransport()->GetInternal()
- : nullptr,
- signaling_thread_, "audio", false, cricket::AudioOptions());
+ call_.get(), media_config_, inner_audio_transport_->GetInternal(),
+ signaling_thread_, "audio", false, rtc::CryptoOptions(),
+ cricket::AudioOptions());
RTC_DCHECK(voice_channel_);
voice_channel_->Enable(true);
+ voice_channel_->DisableEncryption(
+ !inner_audio_transport_->GetInternal()->IsSrtpTransport());
}
void RtpTransportControllerAdapter::CreateVideoChannel() {
video_channel_ = channel_manager_->CreateVideoChannel(
- call_.get(), media_config_,
- inner_video_transport_->GetRtpPacketTransport()->GetInternal(),
- inner_video_transport_->GetRtcpPacketTransport()
- ? inner_video_transport_->GetRtcpPacketTransport()->GetInternal()
- : nullptr,
- signaling_thread_, "video", false, cricket::VideoOptions());
+ call_.get(), media_config_, inner_video_transport_->GetInternal(),
+ signaling_thread_, "video", false, rtc::CryptoOptions(),
+ cricket::VideoOptions());
RTC_DCHECK(video_channel_);
video_channel_->Enable(true);
+ video_channel_->DisableEncryption(
+ !inner_video_transport_->GetInternal()->IsSrtpTransport());
}
void RtpTransportControllerAdapter::DestroyVoiceChannel() {
@@ -989,25 +974,4 @@
return result;
}
-RTCError RtpTransportControllerAdapter::MaybeSetCryptos(
- RtpTransportInterface* rtp_transport,
- cricket::MediaContentDescription* local_description,
- cricket::MediaContentDescription* remote_description) {
- if (rtp_transport->GetInternal()->is_srtp_transport()) {
- if (!rtp_transport->GetInternal()->send_key() ||
- !rtp_transport->GetInternal()->receive_key()) {
- LOG_AND_RETURN_ERROR(webrtc::RTCErrorType::UNSUPPORTED_PARAMETER,
- "The SRTP send key or receive key is not set.")
- }
- std::vector<cricket::CryptoParams> cryptos;
- cryptos.push_back(*(rtp_transport->GetInternal()->receive_key()));
- local_description->set_cryptos(cryptos);
-
- cryptos.clear();
- cryptos.push_back(*(rtp_transport->GetInternal()->send_key()));
- remote_description->set_cryptos(cryptos);
- }
- return RTCError::OK();
-}
-
} // namespace webrtc
diff --git a/ortc/rtptransportcontrolleradapter.h b/ortc/rtptransportcontrolleradapter.h
index ec30687..bdc245d 100644
--- a/ortc/rtptransportcontrolleradapter.h
+++ b/ortc/rtptransportcontrolleradapter.h
@@ -66,7 +66,8 @@
cricket::ChannelManager* channel_manager,
webrtc::RtcEventLog* event_log,
rtc::Thread* signaling_thread,
- rtc::Thread* worker_thread);
+ rtc::Thread* worker_thread,
+ rtc::Thread* network_thread);
~RtpTransportControllerAdapter() override;
@@ -100,6 +101,7 @@
// Methods used internally by other "adapter" classes.
rtc::Thread* signaling_thread() const { return signaling_thread_; }
rtc::Thread* worker_thread() const { return worker_thread_; }
+ rtc::Thread* network_thread() const { return network_thread_; }
// |parameters.keepalive| will be set for ALL RTP transports in the call.
RTCError SetRtpTransportParameters(const RtpTransportParameters& parameters,
@@ -131,7 +133,8 @@
cricket::ChannelManager* channel_manager,
webrtc::RtcEventLog* event_log,
rtc::Thread* signaling_thread,
- rtc::Thread* worker_thread);
+ rtc::Thread* worker_thread,
+ rtc::Thread* network_thread);
void Init_w();
void Close_w();
@@ -180,15 +183,9 @@
const std::string& cname,
const cricket::MediaContentDescription& description) const;
- // If the |rtp_transport| is a SrtpTransport, set the cryptos of the
- // audio/video content descriptions.
- RTCError MaybeSetCryptos(
- RtpTransportInterface* rtp_transport,
- cricket::MediaContentDescription* local_description,
- cricket::MediaContentDescription* remote_description);
-
rtc::Thread* signaling_thread_;
rtc::Thread* worker_thread_;
+ rtc::Thread* network_thread_;
// |transport_proxies_| and |inner_audio_transport_|/|inner_audio_transport_|
// are somewhat redundant, but the latter are only set when
// RtpSenders/RtpReceivers are attached to the transport.
diff --git a/ortc/srtptransport_unittest.cc b/ortc/srtptransport_unittest.cc
index 8323603..1506aa7 100644
--- a/ortc/srtptransport_unittest.cc
+++ b/ortc/srtptransport_unittest.cc
@@ -122,49 +122,4 @@
EXPECT_TRUE(receiver_result.ok());
}
-class SrtpTransportTestWithMediaType
- : public SrtpTransportTest,
- public ::testing::WithParamInterface<cricket::MediaType> {};
-
-// Tests that the senders cannot be created before setting the keys.
-TEST_P(SrtpTransportTestWithMediaType, CreateSenderBeforeSettingKeys) {
- auto sender_result =
- ortc_factory_->CreateRtpSender(GetParam(), srtp_transport_.get());
- EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, sender_result.error().type());
- EXPECT_TRUE(srtp_transport_->SetSrtpSendKey(kTestSha1CryptoParams1).ok());
- sender_result =
- ortc_factory_->CreateRtpSender(GetParam(), srtp_transport_.get());
- EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, sender_result.error().type());
- EXPECT_TRUE(srtp_transport_->SetSrtpReceiveKey(kTestSha1CryptoParams2).ok());
- // Ensure that after the keys are set, a sender can be created, despite the
- // previous errors.
- sender_result =
- ortc_factory_->CreateRtpSender(GetParam(), srtp_transport_.get());
- EXPECT_TRUE(sender_result.ok());
-}
-
-// Tests that the receivers cannot be created before setting the keys.
-TEST_P(SrtpTransportTestWithMediaType, CreateReceiverBeforeSettingKeys) {
- auto receiver_result =
- ortc_factory_->CreateRtpReceiver(GetParam(), srtp_transport_.get());
- EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
- receiver_result.error().type());
- EXPECT_TRUE(srtp_transport_->SetSrtpSendKey(kTestSha1CryptoParams1).ok());
- receiver_result =
- ortc_factory_->CreateRtpReceiver(GetParam(), srtp_transport_.get());
- EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
- receiver_result.error().type());
- EXPECT_TRUE(srtp_transport_->SetSrtpReceiveKey(kTestSha1CryptoParams2).ok());
- // Ensure that after the keys are set, a receiver can be created, despite the
- // previous errors.
- receiver_result =
- ortc_factory_->CreateRtpReceiver(GetParam(), srtp_transport_.get());
- EXPECT_TRUE(receiver_result.ok());
-}
-
-INSTANTIATE_TEST_CASE_P(SenderCreatationTest,
- SrtpTransportTestWithMediaType,
- ::testing::Values(cricket::MEDIA_TYPE_AUDIO,
- cricket::MEDIA_TYPE_VIDEO));
-
} // namespace webrtc