diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 4d06d494fa..e7eff183a7 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -574,6 +574,18 @@ RTCError JsepTransportController::ApplyDescription_n( established_bundle_groups_by_mid, description); } + // Because the creation of transports depends on whether + // certain mids are present, we have to process rejection + // before we try to create transports. + for (size_t i = 0; i < description->contents().size(); ++i) { + const cricket::ContentInfo& content_info = description->contents()[i]; + if (content_info.rejected) { + // This may cause groups to be removed from |bundles_.bundle_groups()| and + // |established_bundle_groups_by_mid|. + HandleRejectedContent(content_info, established_bundle_groups_by_mid); + } + } + for (const cricket::ContentInfo& content_info : description->contents()) { // Don't create transports for rejected m-lines and bundled m-lines. if (content_info.rejected || @@ -593,10 +605,8 @@ RTCError JsepTransportController::ApplyDescription_n( const cricket::ContentInfo& content_info = description->contents()[i]; const cricket::TransportInfo& transport_info = description->transport_infos()[i]; + if (content_info.rejected) { - // This may cause groups to be removed from |bundles_.bundle_groups()| and - // |established_bundle_groups_by_mid|. - HandleRejectedContent(content_info, established_bundle_groups_by_mid); continue; } @@ -1011,7 +1021,21 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( if (transport) { return RTCError::OK(); } - + // If we have agreed to a bundle, the new mid will be added to the bundle + // according to JSEP, and the responder can't move it out of the group + // according to BUNDLE. So don't create a transport. + // The MID will be added to the bundle elsewhere in the code. + if (bundles_.bundle_groups().size() > 0) { + const auto& default_bundle_group = bundles_.bundle_groups()[0]; + if (default_bundle_group->content_names().size() > 0) { + auto bundle_transport = + GetJsepTransportByName(default_bundle_group->content_names()[0]); + if (bundle_transport) { + transports_.SetTransportForMid(content_info.name, bundle_transport); + return RTCError::OK(); + } + } + } const cricket::MediaContentDescription* content_desc = content_info.media_description(); if (certificate_ && !content_desc->cryptos().empty()) { diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index 2b261c83c8..a06f5804e4 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -2051,7 +2051,6 @@ TEST_F(JsepTransportControllerTest, ChangeTaggedMediaSectionMaxBundle) { EXPECT_TRUE(transport_controller_ ->SetLocalDescription(SdpType::kOffer, local_reoffer.get()) .ok()); - std::unique_ptr remote_reanswer( local_reoffer->Clone()); EXPECT_TRUE( diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index dfceacd777..53b674d851 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -3639,6 +3639,20 @@ TEST_P(PeerConnectionIntegrationInteropTest, ASSERT_TRUE(ExpectNewFrames(media_expectations)); } +TEST_P(PeerConnectionIntegrationTest, NewTracksDoNotCauseNewCandidates) { + ASSERT_TRUE(CreatePeerConnectionWrappers()); + ConnectFakeSignaling(); + caller()->AddAudioVideoTracks(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + ASSERT_TRUE_WAIT(DtlsConnected(), kDefaultTimeout); + caller()->ExpectCandidates(0); + callee()->ExpectCandidates(0); + caller()->AddAudioTrack(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); +} + INSTANTIATE_TEST_SUITE_P( PeerConnectionIntegrationTest, PeerConnectionIntegrationInteropTest, diff --git a/pc/rtcp_mux_filter.cc b/pc/rtcp_mux_filter.cc index a8cf717b28..62adea2243 100644 --- a/pc/rtcp_mux_filter.cc +++ b/pc/rtcp_mux_filter.cc @@ -91,7 +91,8 @@ bool RtcpMuxFilter::SetAnswer(bool answer_enable, ContentSource src) { } if (!ExpectAnswer(src)) { - RTC_LOG(LS_ERROR) << "Invalid state for RTCP mux answer"; + RTC_LOG(LS_ERROR) << "Invalid state for RTCP mux answer, state is " + << state_ << ", source is " << src; return false; } diff --git a/pc/session_description.cc b/pc/session_description.cc index 35b732d649..7b878cbf7b 100644 --- a/pc/session_description.cc +++ b/pc/session_description.cc @@ -85,6 +85,18 @@ bool ContentGroup::RemoveContentName(const std::string& content_name) { return true; } +std::string ContentGroup::ToString() const { + rtc::StringBuilder acc; + acc << semantics_ << "("; + if (!content_names_.empty()) { + for (const auto& name : content_names_) { + acc << name << " "; + } + } + acc << ")"; + return acc.Release(); +} + SessionDescription::SessionDescription() = default; SessionDescription::SessionDescription(const SessionDescription&) = default; diff --git a/pc/session_description.h b/pc/session_description.h index e05aa3ed82..a20caf624a 100644 --- a/pc/session_description.h +++ b/pc/session_description.h @@ -488,6 +488,8 @@ class ContentGroup { bool HasContentName(const std::string& content_name) const; void AddContentName(const std::string& content_name); bool RemoveContentName(const std::string& content_name); + // for debugging + std::string ToString() const; private: std::string semantics_; diff --git a/pc/test/integration_test_helpers.h b/pc/test/integration_test_helpers.h index 117f1b428b..9ec9b0e982 100644 --- a/pc/test/integration_test_helpers.h +++ b/pc/test/integration_test_helpers.h @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -704,6 +705,11 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver, audio_concealed_stat_ = *track_stats->concealed_samples; } + // Sets number of candidates expected + void ExpectCandidates(int candidate_count) { + candidates_expected_ = candidate_count; + } + private: explicit PeerConnectionIntegrationWrapper(const std::string& debug_name) : debug_name_(debug_name) {} @@ -1089,6 +1095,9 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver, } } + // Check if we expected to have a candidate. + EXPECT_GT(candidates_expected_, 1); + candidates_expected_--; std::string ice_sdp; EXPECT_TRUE(candidate->ToString(&ice_sdp)); if (signaling_message_receiver_ == nullptr || !signal_ice_candidates_) { @@ -1172,6 +1181,9 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver, peer_connection_signaling_state_history_; webrtc::FakeRtcEventLogFactory* event_log_factory_; + // Number of ICE candidates expected. The default is no limit. + int candidates_expected_ = std::numeric_limits::max(); + // Variables for tracking delay stats on an audio track int audio_packets_stat_ = 0; double audio_delay_stat_ = 0.0;