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

Only create no-op DTLS if media transport is used for both media and data

Browse Source 
Currently it's possible that no-op DTLS is created if media transport is only used for data channels.
Changing it so that no-op DTLS is only created when both media & data will flow through media transport.

Bug: webrtc:9719
Change-Id: I87f27fc778ea21b12f2904bad1452d893f66b541
Reviewed-on: https://webrtc-review.googlesource.com/c/119909
Commit-Queue: Peter Slatala <psla@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Anton Sukhanov <sukhanov@webrtc.org>
Reviewed-by: Bjorn Mellem <mellem@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26416}
This commit is contained in:
Piotr (Peter) Slatala 2019-01-25 13:31:15 -08:00 committed by Commit Bot
parent 9058e076c0
commit 55b91b988f
6 changed files with 185 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
api/peer_connection_interface.h
View File

@ -602,9 +602,9 @@ class PeerConnectionInterface : public rtc::RefCountInterface {
bool active_reset_srtp_params = false;
// If MediaTransportFactory is provided in PeerConnectionFactory, this flag
// informs PeerConnection that it should use the MediaTransportInterface.
// It's invalid to set it to |true| if the MediaTransportFactory wasn't
// provided.
// informs PeerConnection that it should use the MediaTransportInterface for
// media (audio/video). It's invalid to set it to |true| if the
// MediaTransportFactory wasn't provided.
bool use_media_transport = false;
// If MediaTransportFactory is provided in PeerConnectionFactory, this flag

35
pc/jsep_transport_controller.cc
View File

@ -402,13 +402,24 @@ void JsepTransportController::SetActiveResetSrtpParams(
}
}
void JsepTransportController::SetMediaTransportFactory(
MediaTransportFactory* media_transport_factory) {
RTC_DCHECK(media_transport_factory == config_.media_transport_factory ||
void JsepTransportController::SetMediaTransportSettings(
bool use_media_transport_for_media,
bool use_media_transport_for_data_channels) {
RTC_DCHECK(use_media_transport_for_media ==
config_.use_media_transport_for_media ||
jsep_transports_by_name_.empty())
<< "You can only call SetMediaTransportFactory before "
"JsepTransportController created its first transport.";
config_.media_transport_factory = media_transport_factory;
<< "You can only change media transport configuration before creating "
"the first transport.";
RTC_DCHECK(use_media_transport_for_data_channels ==
config_.use_media_transport_for_data_channels ||
jsep_transports_by_name_.empty())
<< "You can only change media transport configuration before creating "
"the first transport.";
config_.use_media_transport_for_media = use_media_transport_for_media;
config_.use_media_transport_for_data_channels =
use_media_transport_for_data_channels;
}
std::unique_ptr<cricket::IceTransportInternal>
@ -433,7 +444,12 @@ JsepTransportController::CreateDtlsTransport(
RTC_DCHECK(network_thread_->IsCurrent());
std::unique_ptr<cricket::DtlsTransportInternal> dtls;
if (is_media_transport_factory_enabled_ && config_.media_transport_factory) {
// If media transport is used for both media and data channels,
// then we don't need to create DTLS.
// Otherwise, DTLS is still created.
if (is_media_transport_factory_enabled_ && config_.media_transport_factory &&
config_.use_media_transport_for_media &&
config_.use_media_transport_for_data_channels) {
dtls = absl::make_unique<cricket::NoOpDtlsTransport>(
std::move(ice), config_.crypto_options);
} else if (config_.external_transport_factory) {
@ -954,6 +970,11 @@ JsepTransportController::MaybeCreateMediaTransport(
return nullptr;
}
if (!config_.use_media_transport_for_media &&
!config_.use_media_transport_for_data_channels) {
return nullptr;
}
absl::optional<cricket::CryptoParams> selected_crypto_for_media_transport;
if (content_info.media_description() &&
!content_info.media_description()->cryptos().empty()) {

24
pc/jsep_transport_controller.h
View File

@ -83,10 +83,19 @@ class JsepTransportController : public sigslot::has_slots<> {
bool active_reset_srtp_params = false;
RtcEventLog* event_log = nullptr;
// Whether media transport is used for media.
bool use_media_transport_for_media = false;
// Whether media transport is used for data channels.
bool use_media_transport_for_data_channels = false;
// 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
// used to create media_transport (as long as either
// |use_media_transport_for_media| or
// |use_media_transport_for_data_channels| is set to true). However, whether
// it will be used to send / receive audio and video frames instead of RTP
// is determined by |use_media_transport_for_media|. Note that currently
// media_transport co-exists with RTP / RTCP transports and may use the same
// underlying ICE transport.
MediaTransportFactory* media_transport_factory = nullptr;
};
@ -173,10 +182,11 @@ class JsepTransportController : public sigslot::has_slots<> {
void SetActiveResetSrtpParams(bool active_reset_srtp_params);
// Allows to overwrite the settings from config. You may set or reset the
// media transport factory on the jsep transport controller, as long as you
// did not call 'GetMediaTransport' or 'MaybeCreateJsepTransport'. Once Jsep
// transport is created, you can't change this setting.
void SetMediaTransportFactory(MediaTransportFactory* media_transport_factory);
// media transport configuration on the jsep transport controller, as long as
// you did not call 'GetMediaTransport' or 'MaybeCreateJsepTransport'. Once
// Jsep transport is created, you can't change this setting.
void SetMediaTransportSettings(bool use_media_transport_for_media,
bool use_media_transport_for_data_channels);
// All of these signals are fired on the signaling thread.

105
pc/jsep_transport_controller_unittest.cc
View File

@ -415,12 +415,48 @@ TEST_F(JsepTransportControllerTest, GetDtlsTransportWithRtcpMux) {
EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kAudioMid1));
}
TEST_F(JsepTransportControllerTest,
DtlsIsStillCreatedIfMediaTransportIsOnlyUsedForDataChannels) {
FakeMediaTransportFactory fake_media_transport_factory;
JsepTransportController::Config config;
config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire;
config.media_transport_factory = &fake_media_transport_factory;
config.use_media_transport_for_data_channels = true;
CreateJsepTransportController(config);
auto description = CreateSessionDescriptionWithBundleGroup();
AddCryptoSettings(description.get());
EXPECT_TRUE(transport_controller_
->SetLocalDescription(SdpType::kOffer, description.get())
.ok());
FakeMediaTransport* media_transport = static_cast<FakeMediaTransport*>(
transport_controller_->GetMediaTransport(kAudioMid1));
ASSERT_NE(nullptr, media_transport);
// After SetLocalDescription, media transport should be created as caller.
EXPECT_TRUE(media_transport->is_caller());
EXPECT_TRUE(media_transport->pre_shared_key().has_value());
// Return nullptr for non-existing mids.
EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kVideoMid2));
EXPECT_EQ(cricket::ICE_CANDIDATE_COMPONENT_RTP,
transport_controller_->GetDtlsTransport(kAudioMid1)->component())
<< "Media transport for media was not enabled, and so DTLS transport "
"should be created.";
}
TEST_F(JsepTransportControllerTest, GetMediaTransportInCaller) {
FakeMediaTransportFactory fake_media_transport_factory;
JsepTransportController::Config config;
config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire;
config.media_transport_factory = &fake_media_transport_factory;
config.use_media_transport_for_data_channels = true;
config.use_media_transport_for_media = true;
CreateJsepTransportController(config);
auto description = CreateSessionDescriptionWithBundleGroup();
AddCryptoSettings(description.get());
@ -452,6 +488,8 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportInCallee) {
config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire;
config.media_transport_factory = &fake_media_transport_factory;
config.use_media_transport_for_data_channels = true;
config.use_media_transport_for_media = true;
CreateJsepTransportController(config);
auto description = CreateSessionDescriptionWithBundleGroup();
AddCryptoSettings(description.get());
@ -482,6 +520,7 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportIsNotSetIfNoSdes) {
config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyNegotiate;
config.media_transport_factory = &fake_media_transport_factory;
config.use_media_transport_for_media = true;
CreateJsepTransportController(config);
auto description = CreateSessionDescriptionWithoutBundle();
EXPECT_TRUE(transport_controller_
@ -512,6 +551,7 @@ TEST_F(JsepTransportControllerTest,
config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire;
config.media_transport_factory = &fake_media_transport_factory;
config.use_media_transport_for_media = true;
CreateJsepTransportController(config);
auto description = CreateSessionDescriptionWithoutBundle();
AddCryptoSettings(description.get());
@ -821,10 +861,12 @@ TEST_F(JsepTransportControllerTest,
}
TEST_F(JsepTransportControllerTest,
SignalConnectionStateConnectedWithMediaTransport) {
SignalConnectionStateConnectedWithMediaTransportAndNoDtls) {
FakeMediaTransportFactory fake_media_transport_factory;
JsepTransportController::Config config;
config.media_transport_factory = &fake_media_transport_factory;
config.use_media_transport_for_data_channels = true;
config.use_media_transport_for_media = true;
CreateJsepTransportController(config);
// Media Transport is only used with bundle.
@ -864,11 +906,64 @@ TEST_F(JsepTransportControllerTest,
EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout);
}
TEST_F(JsepTransportControllerTest,
SignalConnectionStateConnectedWithMediaTransport) {
FakeMediaTransportFactory fake_media_transport_factory;
JsepTransportController::Config config;
config.media_transport_factory = &fake_media_transport_factory;
config.use_media_transport_for_media = true;
CreateJsepTransportController(config);
// Media Transport is only used with bundle.
auto description = CreateSessionDescriptionWithBundleGroup();
AddCryptoSettings(description.get());
EXPECT_TRUE(transport_controller_
->SetLocalDescription(SdpType::kOffer, description.get())
.ok());
auto fake_audio_dtls = static_cast<FakeDtlsTransport*>(
transport_controller_->GetDtlsTransport(kAudioMid1));
auto fake_video_dtls = static_cast<FakeDtlsTransport*>(
transport_controller_->GetDtlsTransport(kVideoMid1));
auto fake_audio_ice = static_cast<cricket::FakeIceTransport*>(
transport_controller_->GetDtlsTransport(kAudioMid1)->ice_transport());
auto fake_video_ice = static_cast<cricket::FakeIceTransport*>(
transport_controller_->GetDtlsTransport(kVideoMid1)->ice_transport());
fake_audio_ice->SetConnectionCount(2);
fake_audio_ice->SetConnectionCount(1);
fake_video_ice->SetConnectionCount(2);
fake_video_ice->SetConnectionCount(1);
fake_audio_ice->SetWritable(true);
fake_video_ice->SetWritable(true);
fake_audio_dtls->SetWritable(true);
fake_video_dtls->SetWritable(true);
// Still not connected, because we are waiting for media transport.
EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, connection_state_,
kTimeout);
FakeMediaTransport* media_transport = static_cast<FakeMediaTransport*>(
transport_controller_->GetMediaTransport(kAudioMid1));
media_transport->SetState(webrtc::MediaTransportState::kWritable);
EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, connection_state_,
kTimeout);
// Still waiting for the second media transport.
media_transport = static_cast<FakeMediaTransport*>(
transport_controller_->GetMediaTransport(kVideoMid1));
media_transport->SetState(webrtc::MediaTransportState::kWritable);
EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout);
}
TEST_F(JsepTransportControllerTest,
SignalConnectionStateFailedWhenMediaTransportClosed) {
FakeMediaTransportFactory fake_media_transport_factory;
JsepTransportController::Config config;
config.media_transport_factory = &fake_media_transport_factory;
config.use_media_transport_for_media = true;
CreateJsepTransportController(config);
auto description = CreateSessionDescriptionWithoutBundle();
AddCryptoSettings(description.get());
@ -876,6 +971,11 @@ TEST_F(JsepTransportControllerTest,
->SetLocalDescription(SdpType::kOffer, description.get())
.ok());
auto fake_audio_dtls = static_cast<FakeDtlsTransport*>(
transport_controller_->GetDtlsTransport(kAudioMid1));
auto fake_video_dtls = static_cast<FakeDtlsTransport*>(
transport_controller_->GetDtlsTransport(kVideoMid1));
auto fake_audio_ice = static_cast<cricket::FakeIceTransport*>(
transport_controller_->GetDtlsTransport(kAudioMid1)->ice_transport());
auto fake_video_ice = static_cast<cricket::FakeIceTransport*>(
@ -888,6 +988,9 @@ TEST_F(JsepTransportControllerTest,
fake_video_ice->SetConnectionCount(2);
fake_video_ice->SetConnectionCount(1);
fake_audio_dtls->SetWritable(true);
fake_video_dtls->SetWritable(true);
FakeMediaTransport* media_transport = static_cast<FakeMediaTransport*>(
transport_controller_->GetMediaTransport(kAudioMid1));

17
pc/peer_connection.cc
View File

@ -989,6 +989,9 @@ bool PeerConnection::Initialize(
return false;
}
config.use_media_transport_for_media = configuration.use_media_transport;
config.use_media_transport_for_data_channels =
configuration.use_media_transport_for_data_channels;
config.media_transport_factory = factory_->media_transport_factory();
}
@ -3207,11 +3210,9 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration,
}
transport_controller_->SetIceConfig(ParseIceConfig(modified_config));
transport_controller_->SetMediaTransportFactory(
modified_config.use_media_transport ||
modified_config.use_media_transport_for_data_channels
? factory_->media_transport_factory()
: nullptr);
transport_controller_->SetMediaTransportSettings(
modified_config.use_media_transport,
modified_config.use_media_transport_for_data_channels);
if (configuration_.active_reset_srtp_params !=
modified_config.active_reset_srtp_params) {
@ -6748,7 +6749,11 @@ bool PeerConnection::OnTransportChanged(
sctp_transport_->SetDtlsTransport(dtls_transport);
}
call_->MediaTransportChange(media_transport);
if (configuration_.use_media_transport) {
// Only pass media transport to call object if media transport is used
// for media (and not data channel).
call_->MediaTransportChange(media_transport);
}
return ret;
}

22
pc/peer_connection_integrationtest.cc
View File

@ -3638,6 +3638,28 @@ TEST_P(PeerConnectionIntegrationTest, MediaTransportBidirectionalVideo) {
EXPECT_GE(first_stats.sent_video_frames, second_stats.received_video_frames);
}
TEST_P(PeerConnectionIntegrationTest,
MediaTransportDataChannelUsesRtpBidirectionalVideo) {
PeerConnectionInterface::RTCConfiguration rtc_config;
rtc_config.use_media_transport = false;
rtc_config.use_media_transport_for_data_channels = true;
rtc_config.enable_dtls_srtp = false; // SDES is required for media transport.
ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory(
rtc_config, rtc_config, loopback_media_transports()->first_factory(),
loopback_media_transports()->second_factory()));
ConnectFakeSignaling();
caller()->AddVideoTrack();
callee()->AddVideoTrack();
// Start offer/answer exchange and wait for it to complete.
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
MediaExpectations media_expectations;
media_expectations.ExpectBidirectionalVideo();
ASSERT_TRUE(ExpectNewFrames(media_expectations));
}
// Test that the ICE connection and gathering states eventually reach
// "complete".
TEST_P(PeerConnectionIntegrationTest, IceStatesReachCompletion) {