Plug-in media transport state listener
IceConnected state (transport state) now includes the state of the
MediaTransport.
This is a first change of two. Second change will add state change
signals to the PeerConnectionInterface informing separately about
ice+media transport vs ice+dtls.
Bug: webrtc:9719
Change-Id: I5731530073e8f26dfc8b188778d268b815da7052
Reviewed-on: https://webrtc-review.googlesource.com/c/108901
Commit-Queue: Peter Slatala <psla@webrtc.org>
Reviewed-by: Bjorn Mellem <mellem@webrtc.org>
Reviewed-by: Anton Sukhanov <sukhanov@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25473}diff --git a/api/test/fake_media_transport.h b/api/test/fake_media_transport.h
index 1ef7c1f..730d497 100644
--- a/api/test/fake_media_transport.h
+++ b/api/test/fake_media_transport.h
@@ -57,9 +57,6 @@
void SetTargetTransferRateObserver(
webrtc::TargetTransferRateObserver* observer) override {}
- void SetMediaTransportStateCallback(
- MediaTransportStateCallback* callback) override {}
-
RTCError SendData(int channel_id,
const SendDataParams& params,
const rtc::CopyOnWriteBuffer& buffer) override {
@@ -70,8 +67,20 @@
void SetDataSink(DataChannelSink* sink) override {}
+ void SetMediaTransportStateCallback(
+ MediaTransportStateCallback* callback) override {
+ state_callback_ = callback;
+ }
+
+ void SetState(webrtc::MediaTransportState state) {
+ if (state_callback_) {
+ state_callback_->OnStateChanged(state);
+ }
+ }
+
private:
const MediaTransportSettings settings_;
+ MediaTransportStateCallback* state_callback_;
};
// Fake media transport factory creates fake media transport.
diff --git a/pc/jseptransport.cc b/pc/jseptransport.cc
index 368b32f..88d20a4 100644
--- a/pc/jseptransport.cc
+++ b/pc/jseptransport.cc
@@ -115,9 +115,17 @@
RTC_DCHECK(!sdes_transport);
dtls_srtp_transport_ = std::move(dtls_srtp_transport);
}
+
+ if (media_transport_) {
+ media_transport_->SetMediaTransportStateCallback(this);
+ }
}
-JsepTransport::~JsepTransport() {}
+JsepTransport::~JsepTransport() {
+ if (media_transport_) {
+ media_transport_->SetMediaTransportStateCallback(nullptr);
+ }
+}
webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription(
const JsepTransportDescription& jsep_description,
@@ -636,4 +644,12 @@
return true;
}
+void JsepTransport::OnStateChanged(webrtc::MediaTransportState state) {
+ // TODO(bugs.webrtc.org/9719) This method currently fires on the network
+ // thread, but media transport does not make such guarantees. We need to make
+ // sure this callback is guaranteed to be executed on the network thread.
+ media_transport_state_ = state;
+ SignalMediaTransportStateChanged();
+}
+
} // namespace cricket
diff --git a/pc/jseptransport.h b/pc/jseptransport.h
index 8883218..952f2cc 100644
--- a/pc/jseptransport.h
+++ b/pc/jseptransport.h
@@ -71,7 +71,8 @@
//
// On Threading: JsepTransport performs work solely on the network thread, and
// so its methods should only be called on the network thread.
-class JsepTransport : public sigslot::has_slots<> {
+class JsepTransport : public sigslot::has_slots<>,
+ public webrtc::MediaTransportStateCallback {
public:
// |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
@@ -171,11 +172,19 @@
return media_transport_.get();
}
+ // Returns the latest media transport state.
+ webrtc::MediaTransportState media_transport_state() const {
+ return media_transport_state_;
+ }
+
// 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.
sigslot::signal<> SignalRtcpMuxActive;
+ // This is signaled for changes in |media_transport_| state.
+ sigslot::signal<> SignalMediaTransportStateChanged;
+
// TODO(deadbeef): The methods below are only public for testing. Should make
// them utility functions or objects so they can be tested independently from
// this class.
@@ -231,6 +240,9 @@
bool GetTransportStats(DtlsTransportInternal* dtls_transport,
TransportStats* stats);
+ // Invoked whenever the state of the media transport changes.
+ void OnStateChanged(webrtc::MediaTransportState state) override;
+
const std::string mid_;
// needs-ice-restart bit as described in JSEP.
bool needs_ice_restart_ = false;
@@ -257,6 +269,11 @@
// Optional media transport (experimental).
std::unique_ptr<webrtc::MediaTransportInterface> media_transport_;
+ // If |media_transport_| is provided, this variable represents the state of
+ // media transport.
+ webrtc::MediaTransportState media_transport_state_ =
+ webrtc::MediaTransportState::kPending;
+
RTC_DISALLOW_COPY_AND_ASSIGN(JsepTransport);
};
diff --git a/pc/jseptransportcontroller.cc b/pc/jseptransportcontroller.cc
index 19b5025..1613e1e 100644
--- a/pc/jseptransportcontroller.cc
+++ b/pc/jseptransportcontroller.cc
@@ -1041,6 +1041,8 @@
std::move(media_transport));
jsep_transport->SignalRtcpMuxActive.connect(
this, &JsepTransportController::UpdateAggregateStates_n);
+ jsep_transport->SignalMediaTransportStateChanged.connect(
+ this, &JsepTransportController::UpdateAggregateStates_n);
SetTransportForMid(content_info.name, jsep_transport.get());
jsep_transports_by_name_[content_info.name] = std::move(jsep_transport);
@@ -1234,6 +1236,10 @@
PeerConnectionInterface::PeerConnectionState::kNew;
cricket::IceGatheringState new_gathering_state = cricket::kIceGatheringNew;
bool any_failed = false;
+
+ // TODO(http://bugs.webrtc.org/9719) If(when) media_transport disables
+ // dtls_transports entirely, the below line will have to be changed to account
+ // for the fact that dtls transports might be absent.
bool all_connected = !dtls_transports.empty();
bool all_completed = !dtls_transports.empty();
bool any_gathering = false;
@@ -1262,6 +1268,31 @@
dtls_state_counts[dtls->dtls_state()]++;
ice_state_counts[dtls->ice_transport()->GetIceTransportState()]++;
}
+
+ for (auto it = jsep_transports_by_name_.begin();
+ it != jsep_transports_by_name_.end(); ++it) {
+ auto jsep_transport = it->second.get();
+ if (!jsep_transport->media_transport()) {
+ continue;
+ }
+
+ // There is no 'kIceConnectionDisconnected', so we only need to handle
+ // connected and completed.
+ // We treat kClosed as failed, because if it happens before shutting down
+ // media transports it means that there was a failure.
+ // MediaTransportInterface allows to flip back and forth between kWritable
+ // and kPending, but there does not exist an implementation that does that,
+ // and the contract of jsep transport controller doesn't quite expect that.
+ // When this happens, we would go from connected to connecting state, but
+ // this may change in future.
+ any_failed |= jsep_transport->media_transport_state() ==
+ webrtc::MediaTransportState::kClosed;
+ all_completed &= jsep_transport->media_transport_state() ==
+ webrtc::MediaTransportState::kWritable;
+ all_connected &= jsep_transport->media_transport_state() ==
+ webrtc::MediaTransportState::kWritable;
+ }
+
if (any_failed) {
new_connection_state = cricket::kIceConnectionFailed;
} else if (all_completed) {
diff --git a/pc/jseptransportcontroller_unittest.cc b/pc/jseptransportcontroller_unittest.cc
index 08b1f9b..1ab5d29 100644
--- a/pc/jseptransportcontroller_unittest.cc
+++ b/pc/jseptransportcontroller_unittest.cc
@@ -730,7 +730,8 @@
EXPECT_EQ(1, combined_connection_state_signal_count_);
}
-TEST_F(JsepTransportControllerTest, SignalConnectionStateConnected) {
+TEST_F(JsepTransportControllerTest,
+ SignalConnectionStateConnectedNoMediaTransport) {
CreateJsepTransportController(JsepTransportController::Config());
auto description = CreateSessionDescriptionWithoutBundle();
EXPECT_TRUE(transport_controller_
@@ -778,6 +779,93 @@
EXPECT_EQ(2, combined_connection_state_signal_count_);
}
+TEST_F(JsepTransportControllerTest,
+ SignalConnectionStateConnectedWithMediaTransport) {
+ FakeMediaTransportFactory fake_media_transport_factory;
+ JsepTransportController::Config config;
+ config.media_transport_factory = &fake_media_transport_factory;
+ CreateJsepTransportController(config);
+ auto description = CreateSessionDescriptionWithoutBundle();
+ 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));
+ fake_audio_dtls->SetWritable(true);
+ fake_video_dtls->SetWritable(true);
+ // Decreasing connection count from 2 to 1 triggers connection state event.
+ fake_audio_dtls->fake_ice_transport()->SetConnectionCount(2);
+ fake_audio_dtls->fake_ice_transport()->SetConnectionCount(1);
+ fake_video_dtls->fake_ice_transport()->SetConnectionCount(2);
+ fake_video_dtls->fake_ice_transport()->SetConnectionCount(1);
+ fake_audio_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED);
+ fake_video_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED);
+
+ // 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;
+ CreateJsepTransportController(config);
+ auto description = CreateSessionDescriptionWithoutBundle();
+ 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));
+ fake_audio_dtls->SetWritable(true);
+ fake_video_dtls->SetWritable(true);
+ // Decreasing connection count from 2 to 1 triggers connection state event.
+ fake_audio_dtls->fake_ice_transport()->SetConnectionCount(2);
+ fake_audio_dtls->fake_ice_transport()->SetConnectionCount(1);
+ fake_video_dtls->fake_ice_transport()->SetConnectionCount(2);
+ fake_video_dtls->fake_ice_transport()->SetConnectionCount(1);
+ fake_audio_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED);
+ fake_video_dtls->SetDtlsState(cricket::DTLS_TRANSPORT_CONNECTED);
+
+ FakeMediaTransport* media_transport = static_cast<FakeMediaTransport*>(
+ transport_controller_->GetMediaTransport(kAudioMid1));
+
+ media_transport->SetState(webrtc::MediaTransportState::kWritable);
+
+ media_transport = static_cast<FakeMediaTransport*>(
+ transport_controller_->GetMediaTransport(kVideoMid1));
+
+ media_transport->SetState(webrtc::MediaTransportState::kWritable);
+
+ EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout);
+
+ media_transport->SetState(webrtc::MediaTransportState::kClosed);
+ EXPECT_EQ_WAIT(cricket::kIceConnectionFailed, connection_state_, kTimeout);
+}
+
TEST_F(JsepTransportControllerTest, SignalConnectionStateComplete) {
CreateJsepTransportController(JsepTransportController::Config());
auto description = CreateSessionDescriptionWithoutBundle();
