Pass the x-mt line from SDP to the media transport
If x-mt line is present (one or more), and the first line is dedicated
for the media transport that we support, pass the config down to this
media transport.
In the future we will do 3 changes:
1) Add MediaTransportFactory::IsSupported(config) to let the
implementation decide whether the current factory can support a given
setting
2) Add support for multiple x-mt lines. Right now the support is
minimal: we only look at the first line (because we only allow single
media transport factory). In the future, when RtpMediaTransport is
introduced, this may and will change.
3) Allow multiple MediaTransportFactories and add fallback to RTP if
media transport is not supported.
Current solution provides backward compatibility for the 2 above
extensions.
Bug: webrtc:9719
Change-Id: I82a469fecda57effc95d7d8191f4a9e4a01d199c
Reviewed-on: https://webrtc-review.googlesource.com/c/124800
Commit-Queue: Peter Slatala <psla@webrtc.org>
Reviewed-by: Bjorn Mellem <mellem@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26882}diff --git a/api/test/fake_media_transport.h b/api/test/fake_media_transport.h
index 16cd1ba..6f87dfc 100644
--- a/api/test/fake_media_transport.h
+++ b/api/test/fake_media_transport.h
@@ -98,6 +98,9 @@
int target_rate_observers_size() { return target_rate_observers_.size(); }
+ // Settings that were passed down to fake media transport.
+ const MediaTransportSettings& settings() { return settings_; }
+
private:
const MediaTransportSettings settings_;
MediaTransportStateCallback* state_callback_;
@@ -110,6 +113,8 @@
FakeMediaTransportFactory() = default;
~FakeMediaTransportFactory() = default;
+ std::string GetTransportName() const override { return "fake"; }
+
RTCErrorOr<std::unique_ptr<MediaTransportInterface>> CreateMediaTransport(
rtc::PacketTransportInternal* packet_transport,
rtc::Thread* network_thread,
diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc
index eb78f34..c1b8e4e 100644
--- a/pc/jsep_transport_controller.cc
+++ b/pc/jsep_transport_controller.cc
@@ -596,7 +596,7 @@
(IsBundled(content_info.name) && content_info.name != *bundled_mid())) {
continue;
}
- error = MaybeCreateJsepTransport(local, content_info);
+ error = MaybeCreateJsepTransport(local, content_info, *description);
if (!error.ok()) {
return error;
}
@@ -958,6 +958,7 @@
std::unique_ptr<webrtc::MediaTransportInterface>
JsepTransportController::MaybeCreateMediaTransport(
const cricket::ContentInfo& content_info,
+ const cricket::SessionDescription& description,
bool local,
cricket::IceTransportInternal* ice_transport) {
if (!is_media_transport_factory_enabled_) {
@@ -998,6 +999,7 @@
return nullptr;
}
+ // TODO(psla): This code will be removed in favor of media transport settings.
// Note that we ignore here lifetime and length.
// In fact we take those bits (inline, lifetime and length) and keep it as
// part of key derivation.
@@ -1031,6 +1033,7 @@
rtc::ArrayView<const uint8_t>(
reinterpret_cast<const uint8_t*>(label.data()), label.size()),
kDerivedKeyByteSize);
+ // TODO(psla): End of the code to be removed.
// We want to crash the app if we don't have a key, and not silently fall
// back to the unsecure communication.
@@ -1042,6 +1045,15 @@
if (config_.use_media_transport_for_media) {
settings.event_log = config_.event_log;
}
+
+ // Assume there is only one media transport (or if more, use the first one).
+ if (!local && !description.MediaTransportSettings().empty() &&
+ config_.media_transport_factory->GetTransportName() ==
+ description.MediaTransportSettings()[0].transport_name) {
+ settings.remote_transport_parameters =
+ description.MediaTransportSettings()[0].transport_setting;
+ }
+
auto media_transport_result =
config_.media_transport_factory->CreateMediaTransport(
ice_transport, network_thread_, settings);
@@ -1054,7 +1066,8 @@
RTCError JsepTransportController::MaybeCreateJsepTransport(
bool local,
- const cricket::ContentInfo& content_info) {
+ const cricket::ContentInfo& content_info,
+ const cricket::SessionDescription& description) {
RTC_DCHECK(network_thread_->IsCurrent());
cricket::JsepTransport* transport = GetJsepTransportByName(content_info.name);
if (transport) {
@@ -1073,7 +1086,7 @@
CreateIceTransport(content_info.name, /*rtcp=*/false);
std::unique_ptr<MediaTransportInterface> media_transport =
- MaybeCreateMediaTransport(content_info, local, ice.get());
+ MaybeCreateMediaTransport(content_info, description, local, ice.get());
std::unique_ptr<cricket::DtlsTransportInternal> rtp_dtls_transport =
CreateDtlsTransport(std::move(ice));
diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h
index f28c10c..1363907 100644
--- a/pc/jsep_transport_controller.h
+++ b/pc/jsep_transport_controller.h
@@ -283,14 +283,17 @@
// 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);
+ RTCError MaybeCreateJsepTransport(
+ bool local,
+ const cricket::ContentInfo& content_info,
+ const cricket::SessionDescription& description);
// Creates media transport if config wants to use it, and pre-shared key is
// provided in content info. It modifies the config to disable media transport
// if pre-shared key is not provided.
std::unique_ptr<webrtc::MediaTransportInterface> MaybeCreateMediaTransport(
const cricket::ContentInfo& content_info,
+ const cricket::SessionDescription& description,
bool local,
cricket::IceTransportInternal* ice_transport);
void MaybeDestroyJsepTransport(const std::string& mid);
diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc
index 43ba4ad..590f57d 100644
--- a/pc/jsep_transport_controller_unittest.cc
+++ b/pc/jsep_transport_controller_unittest.cc
@@ -514,6 +514,87 @@
<< "Because media transport is used, expected no-op DTLS transport.";
}
+TEST_F(JsepTransportControllerTest, GetMediaTransportInCalleePassesSdp) {
+ 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());
+ description->AddMediaTransportSetting("fake", "this-is-a-test-setting");
+ EXPECT_TRUE(transport_controller_
+ ->SetRemoteDescription(SdpType::kOffer, description.get())
+ .ok());
+
+ FakeMediaTransport* media_transport = static_cast<FakeMediaTransport*>(
+ transport_controller_->GetMediaTransport(kAudioMid1));
+
+ ASSERT_NE(nullptr, media_transport);
+
+ EXPECT_EQ("this-is-a-test-setting",
+ media_transport->settings().remote_transport_parameters);
+}
+
+// Caller ignores its own outgoing parameters.
+TEST_F(JsepTransportControllerTest,
+ GetMediaTransportInCallerIgnoresXmtSection) {
+ 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());
+ description->AddMediaTransportSetting("fake", "this-is-a-test-setting");
+ 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);
+
+ // Remote parameters are nullopt, because we are the offerer (we don't)
+ // have the remote transport parameters, only ours.
+ EXPECT_EQ(absl::nullopt,
+ media_transport->settings().remote_transport_parameters);
+}
+
+TEST_F(JsepTransportControllerTest,
+ GetMediaTransportInCalleeIgnoresDifferentTransport) {
+ 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());
+ description->AddMediaTransportSetting("not-a-fake-transport",
+ "this-is-a-test-setting");
+ EXPECT_TRUE(transport_controller_
+ ->SetRemoteDescription(SdpType::kOffer, description.get())
+ .ok());
+
+ FakeMediaTransport* media_transport = static_cast<FakeMediaTransport*>(
+ transport_controller_->GetMediaTransport(kAudioMid1));
+
+ ASSERT_NE(nullptr, media_transport);
+
+ EXPECT_EQ(absl::nullopt,
+ media_transport->settings().remote_transport_parameters);
+}
+
TEST_F(JsepTransportControllerTest, GetMediaTransportIsNotSetIfNoSdes) {
FakeMediaTransportFactory fake_media_transport_factory;
JsepTransportController::Config config;
