From 7940da0f2ef535ef2684606d816c763c2df46981 Mon Sep 17 00:00:00 2001 From: Anton Sukhanov Date: Wed, 10 Oct 2018 10:34:49 -0700 Subject: [PATCH] Integration of media_transport in JSepTransportController Basic integration of media_transport in JSepTransportController. - Creates media_transport if media transport factory provided in jsep transport controller configuration. - Unittest that makes sure media_transport is created with correct caller or callee setting. - Added fake_media_transport, for now simple implementation which only stores caller/callee, but in the future fake media transport will be expanded to pass frames between two fake media_transports, which will enable audio / video integration tests. NEXT STEPS: Once integration of media_transport with PeerConnection (https://webrtc-review.googlesource.com/c/src/+/103860) lands, we can start passing media transport factory from peer connection to jsep transport controller. NOTE: Includes missing include change from https://webrtc-review.googlesource.com/c/src/+/103540 (otherwise this change will not compile) Bug: webrtc:9719 Change-Id: I1e8a521beab445aa9f7ea93c8d7a537dc137d11c Reviewed-on: https://webrtc-review.googlesource.com/c/104400 Commit-Queue: Anton Sukhanov Reviewed-by: Peter Slatala Reviewed-by: Steve Anton Reviewed-by: Niels Moller Reviewed-by: Stefan Holmer Cr-Commit-Position: refs/heads/master@{#25096} --- api/BUILD.gn | 13 +++++ api/test/fake_media_transport.h | 77 ++++++++++++++++++++++++++ pc/BUILD.gn | 1 + pc/jseptransport.cc | 7 ++- pc/jseptransport.h | 18 +++++- pc/jseptransport_unittest.cc | 7 ++- pc/jseptransportcontroller.cc | 39 +++++++++++-- pc/jseptransportcontroller.h | 17 +++++- pc/jseptransportcontroller_unittest.cc | 51 ++++++++++++++++- 9 files changed, 217 insertions(+), 13 deletions(-) create mode 100644 api/test/fake_media_transport.h diff --git a/api/BUILD.gn b/api/BUILD.gn index 5fdebc9140..60eb7514c2 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -542,4 +542,17 @@ if (rtc_include_tests) { "units:units_unittests", ] } + + rtc_source_set("fake_media_transport") { + testonly = true + + sources = [ + "test/fake_media_transport.h", + ] + + deps = [ + ":libjingle_peerconnection_api", + "../rtc_base:checks", + ] + } } diff --git a/api/test/fake_media_transport.h b/api/test/fake_media_transport.h new file mode 100644 index 0000000000..48970a5fac --- /dev/null +++ b/api/test/fake_media_transport.h @@ -0,0 +1,77 @@ +/* + * Copyright 2018 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef API_TEST_FAKE_MEDIA_TRANSPORT_H_ +#define API_TEST_FAKE_MEDIA_TRANSPORT_H_ + +#include +#include + +#include "api/media_transport_interface.h" + +namespace webrtc { + +// TODO(sukhanov): For now fake media transport does nothing and is used only +// in jsepcontroller unittests. In the future we should implement fake media +// transport, which forwards frames to another fake media transport, so we +// could unit test audio / video integration. +class FakeMediaTransport : public MediaTransportInterface { + public: + explicit FakeMediaTransport(bool is_caller) : is_caller_(is_caller) {} + ~FakeMediaTransport() = default; + + RTCError SendAudioFrame(uint64_t channel_id, + MediaTransportEncodedAudioFrame frame) override { + return RTCError::OK(); + } + + RTCError SendVideoFrame( + uint64_t channel_id, + const MediaTransportEncodedVideoFrame& frame) override { + return RTCError::OK(); + } + + RTCError RequestKeyFrame(uint64_t channel_id) override { + return RTCError::OK(); + }; + + void SetReceiveAudioSink(MediaTransportAudioSinkInterface* sink) override {} + void SetReceiveVideoSink(MediaTransportVideoSinkInterface* sink) override {} + + // Returns true if fake media trasport was created as a caller. + bool is_caller() const { return is_caller_; } + + private: + const bool is_caller_; +}; + +// Fake media transport factory creates fake media transport. +class FakeMediaTransportFactory : public MediaTransportFactory { + public: + FakeMediaTransportFactory() = default; + ~FakeMediaTransportFactory() = default; + + RTCErrorOr> CreateMediaTransport( + rtc::PacketTransportInternal* packet_transport, + rtc::Thread* network_thread, + bool is_caller) override { + RTC_CHECK(network_thread != nullptr); + RTC_CHECK(packet_transport != nullptr); + + std::unique_ptr media_transport = + absl::make_unique(is_caller); + + return std::move(media_transport); + } +}; + +} // namespace webrtc + +#endif // API_TEST_FAKE_MEDIA_TRANSPORT_H_ diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 4e6e55810a..b6e9c66fcb 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -316,6 +316,7 @@ if (rtc_include_tests) { ":rtc_pc", ":rtc_pc_base", "../api:array_view", + "../api:fake_media_transport", "../api:libjingle_peerconnection_api", "../call:rtp_interfaces", "../logging:rtc_event_log_api", diff --git a/pc/jseptransport.cc b/pc/jseptransport.cc index c3dfd32464..894dce122d 100644 --- a/pc/jseptransport.cc +++ b/pc/jseptransport.cc @@ -13,7 +13,6 @@ #include #include // for std::pair -#include "absl/memory/memory.h" #include "api/candidate.h" #include "p2p/base/p2pconstants.h" #include "p2p/base/p2ptransportchannel.h" @@ -94,11 +93,13 @@ JsepTransport::JsepTransport( std::unique_ptr sdes_transport, std::unique_ptr dtls_srtp_transport, std::unique_ptr rtp_dtls_transport, - std::unique_ptr rtcp_dtls_transport) + std::unique_ptr rtcp_dtls_transport, + std::unique_ptr media_transport) : mid_(mid), local_certificate_(local_certificate), rtp_dtls_transport_(std::move(rtp_dtls_transport)), - rtcp_dtls_transport_(std::move(rtcp_dtls_transport)) { + rtcp_dtls_transport_(std::move(rtcp_dtls_transport)), + media_transport_(std::move(media_transport)) { RTC_DCHECK(rtp_dtls_transport_); if (unencrypted_rtp_transport) { RTC_DCHECK(!sdes_transport); diff --git a/pc/jseptransport.h b/pc/jseptransport.h index 8e898533ef..8883218742 100644 --- a/pc/jseptransport.h +++ b/pc/jseptransport.h @@ -19,6 +19,7 @@ #include "absl/types/optional.h" #include "api/candidate.h" #include "api/jsep.h" +#include "api/media_transport_interface.h" #include "p2p/base/dtlstransport.h" #include "p2p/base/p2pconstants.h" #include "p2p/base/transportinfo.h" @@ -75,6 +76,10 @@ class JsepTransport : public sigslot::has_slots<> { // |mid| is just used for log statements in order to identify the Transport. // Note that |local_certificate| is allowed to be null since a remote // description may be set before a local certificate is generated. + // + // |media_trasport| is optional (experimental). If available it will be used + // to send / receive encoded audio and video frames instead of RTP. + // Currently |media_transport| can co-exist with RTP / RTCP transports. JsepTransport( const std::string& mid, const rtc::scoped_refptr& local_certificate, @@ -82,7 +87,8 @@ class JsepTransport : public sigslot::has_slots<> { std::unique_ptr sdes_transport, std::unique_ptr dtls_srtp_transport, std::unique_ptr rtp_dtls_transport, - std::unique_ptr rtcp_dtls_transport); + std::unique_ptr rtcp_dtls_transport, + std::unique_ptr media_transport); ~JsepTransport() override; @@ -158,6 +164,13 @@ class JsepTransport : public sigslot::has_slots<> { return rtcp_dtls_transport_.get(); } + // Returns media transport, if available. + // Note that media transport is owned by jseptransport and the pointer + // to media transport will becomes invalid after destruction of jseptransport. + webrtc::MediaTransportInterface* media_transport() const { + return media_transport_.get(); + } + // This is signaled when RTCP-mux becomes active and // |rtcp_dtls_transport_| is destroyed. The JsepTransportController will // handle the signal and update the aggregate transport states. @@ -241,6 +254,9 @@ class JsepTransport : public sigslot::has_slots<> { absl::optional> send_extension_ids_; absl::optional> recv_extension_ids_; + // Optional media transport (experimental). + std::unique_ptr media_transport_; + RTC_DISALLOW_COPY_AND_ASSIGN(JsepTransport); }; diff --git a/pc/jseptransport_unittest.cc b/pc/jseptransport_unittest.cc index 56a742c338..1b42578c73 100644 --- a/pc/jseptransport_unittest.cc +++ b/pc/jseptransport_unittest.cc @@ -98,11 +98,16 @@ class JsepTransport2Test : public testing::Test, public sigslot::has_slots<> { RTC_NOTREACHED(); } + // TODO(sukhanov): Currently there is no media_transport specific + // logic in jseptransport, so jseptransport unittests are created with + // media_transport = nullptr. In the future we will probably add + // more logic that require unit tests. Note that creation of media_transport + // is covered in jseptransportcontroller_unittest. auto jsep_transport = absl::make_unique( kTransportName, /*local_certificate=*/nullptr, std::move(unencrypted_rtp_transport), std::move(sdes_transport), std::move(dtls_srtp_transport), std::move(rtp_dtls_transport), - std::move(rtcp_dtls_transport)); + std::move(rtcp_dtls_transport), /*media_transport=*/nullptr); signal_rtcp_mux_active_received_ = false; jsep_transport->SignalRtcpMuxActive.connect( diff --git a/pc/jseptransportcontroller.cc b/pc/jseptransportcontroller.cc index 4c69a6a07e..1847d95c94 100644 --- a/pc/jseptransportcontroller.cc +++ b/pc/jseptransportcontroller.cc @@ -14,7 +14,6 @@ #include #include -#include "absl/memory/memory.h" #include "p2p/base/port.h" #include "rtc_base/bind.h" #include "rtc_base/checks.h" @@ -133,6 +132,15 @@ RtpTransportInternal* JsepTransportController::GetRtpTransport( return jsep_transport->rtp_transport(); } +MediaTransportInterface* JsepTransportController::GetMediaTransport( + const std::string& mid) const { + auto jsep_transport = GetJsepTransportForMid(mid); + if (!jsep_transport) { + return nullptr; + } + return jsep_transport->media_transport(); +} + cricket::DtlsTransportInternal* JsepTransportController::GetDtlsTransport( const std::string& mid) const { auto jsep_transport = GetJsepTransportForMid(mid); @@ -523,7 +531,7 @@ RTCError JsepTransportController::ApplyDescription_n( (IsBundled(content_info.name) && content_info.name != *bundled_mid())) { continue; } - error = MaybeCreateJsepTransport(content_info); + error = MaybeCreateJsepTransport(local, content_info); if (!error.ok()) { return error; } @@ -889,6 +897,7 @@ cricket::JsepTransport* JsepTransportController::GetJsepTransportByName( } RTCError JsepTransportController::MaybeCreateJsepTransport( + bool local, const cricket::ContentInfo& content_info) { RTC_DCHECK(network_thread_->IsCurrent()); cricket::JsepTransport* transport = GetJsepTransportByName(content_info.name); @@ -906,7 +915,13 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( std::unique_ptr rtp_dtls_transport = CreateDtlsTransport(content_info.name, /*rtcp =*/false); + std::unique_ptr rtcp_dtls_transport; + std::unique_ptr unencrypted_rtp_transport; + std::unique_ptr sdes_transport; + std::unique_ptr dtls_srtp_transport; + std::unique_ptr media_transport; + if (config_.rtcp_mux_policy != PeerConnectionInterface::kRtcpMuxPolicyRequire && content_info.type == cricket::MediaProtocolType::kRtp) { @@ -914,9 +929,20 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( CreateDtlsTransport(content_info.name, /*rtcp =*/true); } - std::unique_ptr unencrypted_rtp_transport; - std::unique_ptr sdes_transport; - std::unique_ptr dtls_srtp_transport; + if (config_.media_transport_factory != nullptr) { + auto media_transport_result = + config_.media_transport_factory->CreateMediaTransport( + rtp_dtls_transport->ice_transport(), network_thread_, + /*is_caller=*/local); + + // TODO(sukhanov): Proper error handling. + RTC_CHECK(media_transport_result.ok()); + + media_transport = std::move(media_transport_result.value()); + } + + // TODO(sukhanov): Do not create RTP/RTCP transports if media transport is + // used. if (config_.disable_encryption) { unencrypted_rtp_transport = CreateUnencryptedRtpTransport( content_info.name, rtp_dtls_transport.get(), rtcp_dtls_transport.get()); @@ -932,7 +958,8 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( absl::make_unique( content_info.name, certificate_, std::move(unencrypted_rtp_transport), std::move(sdes_transport), std::move(dtls_srtp_transport), - std::move(rtp_dtls_transport), std::move(rtcp_dtls_transport)); + std::move(rtp_dtls_transport), std::move(rtcp_dtls_transport), + std::move(media_transport)); jsep_transport->SignalRtcpMuxActive.connect( this, &JsepTransportController::UpdateAggregateStates_n); SetTransportForMid(content_info.name, jsep_transport.get()); diff --git a/pc/jseptransportcontroller.h b/pc/jseptransportcontroller.h index d9340cf357..bca4481624 100644 --- a/pc/jseptransportcontroller.h +++ b/pc/jseptransportcontroller.h @@ -18,6 +18,7 @@ #include #include "api/candidate.h" +#include "api/media_transport_interface.h" #include "api/peerconnectioninterface.h" #include "logging/rtc_event_log/rtc_event_log.h" #include "media/sctp/sctptransportinternal.h" @@ -79,6 +80,13 @@ class JsepTransportController : public sigslot::has_slots<> { Observer* transport_observer = nullptr; bool active_reset_srtp_params = false; RtcEventLog* event_log = nullptr; + + // Optional media transport factory (experimental). If provided it will be + // used to create media_transport and will be used to send / receive + // audio and video frames instead of RTP. Note that currently + // media_transport co-exists with RTP / RTCP transports and uses the same + // underlying ICE transport. + MediaTransportFactory* media_transport_factory = nullptr; }; // The ICE related events are signaled on the |signaling_thread|. @@ -108,6 +116,8 @@ class JsepTransportController : public sigslot::has_slots<> { cricket::DtlsTransportInternal* GetRtcpDtlsTransport( const std::string& mid) const; + MediaTransportInterface* GetMediaTransport(const std::string& mid) const; + /********************* * ICE-related methods ********************/ @@ -241,7 +251,12 @@ class JsepTransportController : public sigslot::has_slots<> { cricket::JsepTransport* GetJsepTransportByName( const std::string& transport_name); - RTCError MaybeCreateJsepTransport(const cricket::ContentInfo& content_info); + // Creates jsep transport. Noop if transport is already created. + // Transport is created either during SetLocalDescription (|local| == true) or + // during SetRemoteDescription (|local| == false). Passing |local| helps to + // differentiate initiator (caller) from answerer (callee). + RTCError MaybeCreateJsepTransport(bool local, + const cricket::ContentInfo& content_info); void MaybeDestroyJsepTransport(const std::string& mid); void DestroyAllJsepTransports_n(); diff --git a/pc/jseptransportcontroller_unittest.cc b/pc/jseptransportcontroller_unittest.cc index 6f8693bea7..ce431c38ae 100644 --- a/pc/jseptransportcontroller_unittest.cc +++ b/pc/jseptransportcontroller_unittest.cc @@ -11,7 +11,7 @@ #include #include -#include "absl/memory/memory.h" +#include "api/test/fake_media_transport.h" #include "p2p/base/fakedtlstransport.h" #include "p2p/base/fakeicetransport.h" #include "p2p/base/transportfactoryinterface.h" @@ -341,6 +341,55 @@ TEST_F(JsepTransportControllerTest, GetDtlsTransportWithRtcpMux) { EXPECT_EQ(nullptr, transport_controller_->GetRtcpDtlsTransport(kAudioMid1)); EXPECT_NE(nullptr, transport_controller_->GetDtlsTransport(kVideoMid1)); EXPECT_EQ(nullptr, transport_controller_->GetRtcpDtlsTransport(kVideoMid1)); + EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kAudioMid1)); +} + +TEST_F(JsepTransportControllerTest, GetMediaTransportInCaller) { + FakeMediaTransportFactory fake_media_transport_factory; + JsepTransportController::Config config; + + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyNegotiate; + config.media_transport_factory = &fake_media_transport_factory; + CreateJsepTransportController(config); + auto description = CreateSessionDescriptionWithoutBundle(); + EXPECT_TRUE(transport_controller_ + ->SetLocalDescription(SdpType::kOffer, description.get()) + .ok()); + + FakeMediaTransport* media_transport = static_cast( + transport_controller_->GetMediaTransport(kAudioMid1)); + + ASSERT_NE(nullptr, media_transport); + + // After SetLocalDescription, media transport should be created as caller. + EXPECT_TRUE(media_transport->is_caller()); + + // Return nullptr for non-existing mids. + EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kVideoMid2)); +} + +TEST_F(JsepTransportControllerTest, GetMediaTransportInCallee) { + FakeMediaTransportFactory fake_media_transport_factory; + JsepTransportController::Config config; + + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyNegotiate; + config.media_transport_factory = &fake_media_transport_factory; + CreateJsepTransportController(config); + auto description = CreateSessionDescriptionWithoutBundle(); + EXPECT_TRUE(transport_controller_ + ->SetRemoteDescription(SdpType::kOffer, description.get()) + .ok()); + + FakeMediaTransport* media_transport = static_cast( + transport_controller_->GetMediaTransport(kAudioMid1)); + + ASSERT_NE(nullptr, media_transport); + + // After SetRemoteDescription, media transport should be created as callee. + EXPECT_FALSE(media_transport->is_caller()); + + // Return nullptr for non-existing mids. + EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kVideoMid2)); } TEST_F(JsepTransportControllerTest, SetIceConfig) {