diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index e977f1b938..2da3f0e0a1 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -551,6 +551,17 @@ RTCError JsepTransportController::ApplyDescription_n( MergeEncryptedHeaderExtensionIdsForBundles(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()`. + HandleRejectedContent(content_info); + } + } + for (const cricket::ContentInfo& content_info : description->contents()) { // Don't create transports for rejected m-lines and bundled m-lines. if (content_info.rejected || @@ -569,9 +580,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()`. - HandleRejectedContent(content_info); continue; } @@ -977,7 +987,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 cfe13d04ff..6fe1ee3bf3 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 31652ac4b4..fc094161af 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/test/integration_test_helpers.h b/pc/test/integration_test_helpers.h index b0e0b34864..c7c17b7fe9 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;