From fa8f4eee40050e9875a4b8c38d99ea54a112c1ed Mon Sep 17 00:00:00 2001 From: Bjorn A Mellem Date: Fri, 16 Aug 2019 15:47:45 -0700 Subject: [PATCH] Only combine media transport and ICE states if used for media. Media transport (or, equivalently, datagram transport) may only be created for data channels. In this case, it's not appropriate to consider ICE not-yet-connected or failed due to the media transport's state. If the media transport disconnects or fails, it will signal data channels separately. Bug: webrtc:9719 Change-Id: Ieb7cb307116e479d01616559d8bafdfc650a78c5 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/149420 Reviewed-by: Benjamin Wright Reviewed-by: Steve Anton Commit-Queue: Bjorn Mellem Cr-Commit-Position: refs/heads/master@{#28884} --- pc/jsep_transport_controller.cc | 47 ++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 6db58dea6b..980deb690b 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -1486,28 +1486,33 @@ void JsepTransportController::UpdateAggregateStates_n() { 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; - } + // Don't indicate that the call failed or isn't connected due to media + // transport state unless the media transport is used for media. If it's only + // used for data channels, it will signal those separately. + if (config_.use_media_transport_for_media || config_.use_datagram_transport) { + 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; + // 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) {