diff --git a/pc/jsep_transport_collection.cc b/pc/jsep_transport_collection.cc index 98b8cd2a3d..ce068d99fc 100644 --- a/pc/jsep_transport_collection.cc +++ b/pc/jsep_transport_collection.cc @@ -20,53 +20,21 @@ namespace webrtc { -void BundleManager::Update(const cricket::SessionDescription* description, - SdpType type) { +void BundleManager::Update(const cricket::SessionDescription* description) { RTC_DCHECK_RUN_ON(&sequence_checker_); - // Rollbacks should call Rollback, not Update. - RTC_DCHECK(type != SdpType::kRollback); - bool bundle_groups_changed = false; - // TODO(bugs.webrtc.org/3349): Do this for kPrAnswer as well. To make this - // work, we also need to make sure PRANSWERs don't call - // MaybeDestroyJsepTransport, because the final answer may need the destroyed - // transport if it changes the BUNDLE group. - if (bundle_policy_ == PeerConnectionInterface::kBundlePolicyMaxBundle || - type == SdpType::kAnswer) { - // If our policy is "max-bundle" or this is an answer, update all bundle - // groups. - bundle_groups_changed = true; - bundle_groups_.clear(); - for (const cricket::ContentGroup* new_bundle_group : - description->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE)) { - bundle_groups_.push_back( - std::make_unique(*new_bundle_group)); - RTC_DLOG(LS_VERBOSE) << "Establishing bundle group " - << new_bundle_group->ToString(); - } - } else if (type == SdpType::kOffer) { - // If this is an offer, update existing bundle groups. - // We do this because as per RFC 8843, section 7.3.2, the answerer cannot - // remove an m= section from an existing BUNDLE group without rejecting it. - // Thus any m= sections added to a BUNDLE group in this offer can - // preemptively start using the bundled transport, as there is no possible - // non-bundled fallback. - for (const cricket::ContentGroup* new_bundle_group : - description->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE)) { - // Attempt to find a matching existing group. - for (const std::string& mid : new_bundle_group->content_names()) { - auto it = established_bundle_groups_by_mid_.find(mid); - if (it != established_bundle_groups_by_mid_.end()) { - *it->second = *new_bundle_group; - bundle_groups_changed = true; - RTC_DLOG(LS_VERBOSE) - << "Establishing bundle group " << new_bundle_group->ToString(); - break; - } - } - } + bundle_groups_.clear(); + for (const cricket::ContentGroup* new_bundle_group : + description->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE)) { + bundle_groups_.push_back( + std::make_unique(*new_bundle_group)); + RTC_DLOG(LS_VERBOSE) << "Establishing bundle group " + << new_bundle_group->ToString(); } - if (bundle_groups_changed) { - RefreshEstablishedBundleGroupsByMid(); + established_bundle_groups_by_mid_.clear(); + for (const auto& bundle_group : bundle_groups_) { + for (const std::string& content_name : bundle_group->content_names()) { + established_bundle_groups_by_mid_[content_name] = bundle_group.get(); + } } } @@ -124,34 +92,6 @@ void BundleManager::DeleteGroup(const cricket::ContentGroup* bundle_group) { bundle_groups_.erase(bundle_group_it); } -void BundleManager::Rollback() { - RTC_DCHECK_RUN_ON(&sequence_checker_); - bundle_groups_.clear(); - for (const auto& bundle_group : stable_bundle_groups_) { - bundle_groups_.push_back( - std::make_unique(*bundle_group)); - } - RefreshEstablishedBundleGroupsByMid(); -} - -void BundleManager::Commit() { - RTC_DCHECK_RUN_ON(&sequence_checker_); - stable_bundle_groups_.clear(); - for (const auto& bundle_group : bundle_groups_) { - stable_bundle_groups_.push_back( - std::make_unique(*bundle_group)); - } -} - -void BundleManager::RefreshEstablishedBundleGroupsByMid() { - established_bundle_groups_by_mid_.clear(); - for (const auto& bundle_group : bundle_groups_) { - for (const std::string& content_name : bundle_group->content_names()) { - established_bundle_groups_by_mid_[content_name] = bundle_group.get(); - } - } -} - void JsepTransportCollection::RegisterTransport( const std::string& mid, std::unique_ptr transport) { @@ -217,6 +157,8 @@ bool JsepTransportCollection::SetTransportForMid( if (it != mid_to_transport_.end() && it->second == jsep_transport) return true; + pending_mids_.push_back(mid); + // The map_change_callback must be called before destroying the // transport, because it removes references to the transport // in the RTP demuxer. @@ -249,33 +191,17 @@ void JsepTransportCollection::RemoveTransportForMid(const std::string& mid) { RTC_DCHECK(IsConsistent()); } -bool JsepTransportCollection::RollbackTransports() { +void JsepTransportCollection::RollbackTransports() { RTC_DCHECK_RUN_ON(&sequence_checker_); - bool ret = true; - // First, remove any new mid->transport mappings. - for (const auto& kv : mid_to_transport_) { - if (stable_mid_to_transport_.count(kv.first) == 0) { - ret = ret && map_change_callback_(kv.first, nullptr); - } + for (auto&& mid : pending_mids_) { + RemoveTransportForMid(mid); } - // Next, restore old mappings. - for (const auto& kv : stable_mid_to_transport_) { - auto it = mid_to_transport_.find(kv.first); - if (it == mid_to_transport_.end() || it->second != kv.second) { - ret = ret && map_change_callback_(kv.first, kv.second); - } - } - mid_to_transport_ = stable_mid_to_transport_; - DestroyUnusedTransports(); - RTC_DCHECK(IsConsistent()); - return ret; + pending_mids_.clear(); } void JsepTransportCollection::CommitTransports() { RTC_DCHECK_RUN_ON(&sequence_checker_); - stable_mid_to_transport_ = mid_to_transport_; - DestroyUnusedTransports(); - RTC_DCHECK(IsConsistent()); + pending_mids_.clear(); } bool JsepTransportCollection::TransportInUse( @@ -286,11 +212,6 @@ bool JsepTransportCollection::TransportInUse( return true; } } - for (const auto& kv : stable_mid_to_transport_) { - if (kv.second == jsep_transport) { - return true; - } - } return false; } @@ -298,7 +219,7 @@ void JsepTransportCollection::MaybeDestroyJsepTransport( cricket::JsepTransport* transport) { RTC_DCHECK_RUN_ON(&sequence_checker_); // Don't destroy the JsepTransport if there are still media sections referring - // to it, or if it will be needed in case of rollback. + // to it. if (TransportInUse(transport)) { return; } @@ -312,23 +233,6 @@ void JsepTransportCollection::MaybeDestroyJsepTransport( RTC_DCHECK(IsConsistent()); } -void JsepTransportCollection::DestroyUnusedTransports() { - RTC_DCHECK_RUN_ON(&sequence_checker_); - bool need_state_change_callback = false; - auto it = jsep_transports_by_name_.begin(); - while (it != jsep_transports_by_name_.end()) { - if (TransportInUse(it->second.get())) { - ++it; - } else { - it = jsep_transports_by_name_.erase(it); - need_state_change_callback = true; - } - } - if (need_state_change_callback) { - state_change_callback_(); - } -} - bool JsepTransportCollection::IsConsistent() { RTC_DCHECK_RUN_ON(&sequence_checker_); for (const auto& it : jsep_transports_by_name_) { @@ -337,6 +241,13 @@ bool JsepTransportCollection::IsConsistent() { << " is not in use, transport " << it.second.get(); return false; } + const auto& lookup = mid_to_transport_.find(it.first); + if (lookup->second != it.second.get()) { + // Not an error, but unusual. + RTC_DLOG(LS_INFO) << "Note: Mid " << it.first << " was registered to " + << it.second.get() << " but currently maps to " + << lookup->second; + } } return true; } diff --git a/pc/jsep_transport_collection.h b/pc/jsep_transport_collection.h index 93570379ac..0dd528d348 100644 --- a/pc/jsep_transport_collection.h +++ b/pc/jsep_transport_collection.h @@ -18,7 +18,6 @@ #include #include -#include "api/peer_connection_interface.h" #include "api/sequence_checker.h" #include "pc/jsep_transport.h" #include "pc/session_description.h" @@ -42,8 +41,7 @@ namespace webrtc { // 6) Change the logic to do what's right. class BundleManager { public: - explicit BundleManager(PeerConnectionInterface::BundlePolicy bundle_policy) - : bundle_policy_(bundle_policy) { + BundleManager() { // Allow constructor to be called on a different thread. sequence_checker_.Detach(); } @@ -60,27 +58,17 @@ class BundleManager { bool IsFirstMidInGroup(const std::string& mid) const; // Update the groups description. This completely replaces the group // description with the one from the SessionDescription. - void Update(const cricket::SessionDescription* description, SdpType type); + void Update(const cricket::SessionDescription* description); // Delete a MID from the group that contains it. void DeleteMid(const cricket::ContentGroup* bundle_group, const std::string& mid); // Delete a group. void DeleteGroup(const cricket::ContentGroup* bundle_group); - // Roll back to previous stable state. - void Rollback(); - // Commit current bundle groups. - void Commit(); private: - // Recalculate established_bundle_groups_by_mid_ from bundle_groups_. - void RefreshEstablishedBundleGroupsByMid() RTC_RUN_ON(sequence_checker_); - RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_; - PeerConnectionInterface::BundlePolicy bundle_policy_; std::vector> bundle_groups_ RTC_GUARDED_BY(sequence_checker_); - std::vector> stable_bundle_groups_ - RTC_GUARDED_BY(sequence_checker_); std::map established_bundle_groups_by_mid_; }; @@ -120,25 +108,17 @@ class JsepTransportCollection { // Remove a transport for a MID. This may destroy a transport if it is // no longer in use. void RemoveTransportForMid(const std::string& mid); - // Roll back to previous stable mid-to-transport mappings. - bool RollbackTransports(); - // Commit pending mid-transport mappings (rollback is no longer possible), - // and destroy unused transports because we know now we'll never need them - // again. + // Roll back pending mid-to-transport mappings. + void RollbackTransports(); + // Commit pending mid-transport mappings (rollback is no longer possible). void CommitTransports(); - - private: - // Returns true if any mid currently maps to this transport, in either the - // pending or stable mapping. + // Returns true if any mid currently maps to this transport. bool TransportInUse(cricket::JsepTransport* jsep_transport) const; - // Destroy a transport if it's no longer in use. This includes whether it - // will be needed in case of rollback. + private: + // Destroy a transport if it's no longer in use. void MaybeDestroyJsepTransport(cricket::JsepTransport* transport); - // Destroys all transports that are no longer in use. - void DestroyUnusedTransports(); - bool IsConsistent(); // For testing only: Verify internal structure. RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_; @@ -150,10 +130,8 @@ class JsepTransportCollection { // (BaseChannel/SctpTransport) and the JsepTransport underneath. std::map mid_to_transport_ RTC_GUARDED_BY(sequence_checker_); - // A snapshot of mid_to_transport_ at the last stable state. Used for - // rollback. - std::map stable_mid_to_transport_ - RTC_GUARDED_BY(sequence_checker_); + // Keep track of mids that have been mapped to transports. Used for rollback. + std::vector pending_mids_ RTC_GUARDED_BY(sequence_checker_); // Callback used to inform subscribers of altered transports. const std::function diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 8b9596b64e..47fddd4e3e 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -55,8 +55,7 @@ JsepTransportController::JsepTransportController( UpdateAggregateStates_n(); }), config_(config), - active_reset_srtp_params_(config.active_reset_srtp_params), - bundles_(config.bundle_policy) { + active_reset_srtp_params_(config.active_reset_srtp_params) { // The |transport_observer| is assumed to be non-null. RTC_DCHECK(config_.transport_observer); RTC_DCHECK(config_.rtcp_handler); @@ -375,18 +374,13 @@ void JsepTransportController::SetActiveResetSrtpParams( } } -RTCError JsepTransportController::RollbackTransports() { +void JsepTransportController::RollbackTransports() { if (!network_thread_->IsCurrent()) { - return network_thread_->Invoke( - RTC_FROM_HERE, [=] { return RollbackTransports(); }); + network_thread_->Invoke(RTC_FROM_HERE, [=] { RollbackTransports(); }); + return; } RTC_DCHECK_RUN_ON(network_thread_); - bundles_.Rollback(); - if (!transports_.RollbackTransports()) { - LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR, - "Failed to roll back transport state."); - } - return RTCError::OK(); + transports_.RollbackTransports(); } rtc::scoped_refptr @@ -557,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 || @@ -577,8 +582,6 @@ RTCError JsepTransportController::ApplyDescription_n( description->transport_infos()[i]; if (content_info.rejected) { - // This may cause groups to be removed from |bundles_.bundle_groups()|. - HandleRejectedContent(content_info); continue; } @@ -644,7 +647,6 @@ RTCError JsepTransportController::ApplyDescription_n( } if (type == SdpType::kAnswer) { transports_.CommitTransports(); - bundles_.Commit(); } return RTCError::OK(); } @@ -680,44 +682,7 @@ RTCError JsepTransportController::ValidateAndMaybeUpdateBundleGroups( } } - if (type == SdpType::kOffer) { - // For an offer, we need to verify that there is not a conflicting mapping - // between existing and new bundle groups. For example, if the existing - // groups are [[1,2],[3,4]] and new are [[1,3],[2,4]] or [[1,2,3,4]], or - // vice versa. Switching things around like this requires a separate offer - // that removes the relevant sections from their group, as per RFC 8843, - // section 7.5.2. - std::map - new_bundle_groups_by_existing_bundle_groups; - std::map - existing_bundle_groups_by_new_bundle_groups; - for (const cricket::ContentGroup* new_bundle_group : new_bundle_groups) { - for (const std::string& mid : new_bundle_group->content_names()) { - cricket::ContentGroup* existing_bundle_group = - bundles_.LookupGroupByMid(mid); - if (!existing_bundle_group) { - continue; - } - auto it = new_bundle_groups_by_existing_bundle_groups.find( - existing_bundle_group); - if (it != new_bundle_groups_by_existing_bundle_groups.end() && - it->second != new_bundle_group) { - return RTCError(RTCErrorType::INVALID_PARAMETER, - "MID " + mid + " in the offer has changed group."); - } - new_bundle_groups_by_existing_bundle_groups.insert( - std::make_pair(existing_bundle_group, new_bundle_group)); - it = existing_bundle_groups_by_new_bundle_groups.find(new_bundle_group); - if (it != existing_bundle_groups_by_new_bundle_groups.end() && - it->second != existing_bundle_group) { - return RTCError(RTCErrorType::INVALID_PARAMETER, - "MID " + mid + " in the offer has changed group."); - } - existing_bundle_groups_by_new_bundle_groups.insert( - std::make_pair(new_bundle_group, existing_bundle_group)); - } - } - } else if (type == SdpType::kAnswer) { + if (type == SdpType::kAnswer) { std::vector offered_bundle_groups = local ? remote_desc_->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE) : local_desc_->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE); @@ -797,7 +762,9 @@ RTCError JsepTransportController::ValidateAndMaybeUpdateBundleGroups( "max-bundle is used but no bundle group found."); } - bundles_.Update(description, type); + if (ShouldUpdateBundleGroup(type, description)) { + bundles_.Update(description); + } for (const auto& bundle_group : bundles_.bundle_groups()) { if (!bundle_group->FirstContentName()) @@ -905,6 +872,26 @@ JsepTransportController::CreateJsepTransportDescription( rtp_abs_sendtime_extn_id, transport_info.description); } +bool JsepTransportController::ShouldUpdateBundleGroup( + SdpType type, + const cricket::SessionDescription* description) { + if (config_.bundle_policy == + PeerConnectionInterface::kBundlePolicyMaxBundle) { + return true; + } + + if (type != SdpType::kAnswer) { + return false; + } + + RTC_DCHECK(local_desc_ && remote_desc_); + std::vector local_bundles = + local_desc_->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE); + std::vector remote_bundles = + remote_desc_->GetGroupsByName(cricket::GROUP_TYPE_BUNDLE); + return !local_bundles.empty() && !remote_bundles.empty(); +} + std::vector JsepTransportController::GetEncryptedHeaderExtensionIds( const cricket::ContentInfo& content_info) { const cricket::MediaContentDescription* content_desc = @@ -1000,6 +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.h b/pc/jsep_transport_controller.h index 3b20bbb5a9..71b01bffb2 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -224,7 +224,9 @@ class JsepTransportController : public sigslot::has_slots<> { void SetActiveResetSrtpParams(bool active_reset_srtp_params); - RTCError RollbackTransports(); + // For now the rollback only removes mid to transport mappings + // and deletes unused transports, but doesn't consider anything more complex. + void RollbackTransports(); // F: void(const std::string&, const std::vector&) template @@ -340,6 +342,9 @@ class JsepTransportController : public sigslot::has_slots<> { const std::vector& encrypted_extension_ids, int rtp_abs_sendtime_extn_id); + bool ShouldUpdateBundleGroup(SdpType type, + const cricket::SessionDescription* description); + std::map> MergeEncryptedHeaderExtensionIdsForBundles( const cricket::SessionDescription* description); diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index bc7cfebdb9..a06f5804e4 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -1608,356 +1608,6 @@ TEST_F(JsepTransportControllerTest, MultipleBundleGroupsChangeFirstMid) { EXPECT_EQ(mid5_transport, mid6_transport); } -TEST_F(JsepTransportControllerTest, - MultipleBundleGroupsSectionsAddedInSubsequentOffer) { - static const char kMid1Audio[] = "1_audio"; - static const char kMid2Audio[] = "2_audio"; - static const char kMid3Audio[] = "3_audio"; - static const char kMid4Video[] = "4_video"; - static const char kMid5Video[] = "5_video"; - static const char kMid6Video[] = "6_video"; - - CreateJsepTransportController(JsepTransportController::Config()); - // Start by grouping (kMid1Audio,kMid2Audio) and (kMid4Video,kMid4f5Video). - cricket::ContentGroup bundle_group1(cricket::GROUP_TYPE_BUNDLE); - bundle_group1.AddContentName(kMid1Audio); - bundle_group1.AddContentName(kMid2Audio); - cricket::ContentGroup bundle_group2(cricket::GROUP_TYPE_BUNDLE); - bundle_group2.AddContentName(kMid4Video); - bundle_group2.AddContentName(kMid5Video); - - auto local_offer = std::make_unique(); - AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(local_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(local_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(local_offer.get(), kMid5Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - local_offer->AddGroup(bundle_group1); - local_offer->AddGroup(bundle_group2); - - auto remote_answer = std::make_unique(); - AddAudioSection(remote_answer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(remote_answer.get(), kMid2Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(remote_answer.get(), kMid4Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(remote_answer.get(), kMid5Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - remote_answer->AddGroup(bundle_group1); - remote_answer->AddGroup(bundle_group2); - - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, local_offer.get()) - .ok()); - EXPECT_TRUE(transport_controller_ - ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get()) - .ok()); - - // Add kMid3Audio and kMid6Video to the respective audio/video bundle groups. - cricket::ContentGroup new_bundle_group1(cricket::GROUP_TYPE_BUNDLE); - bundle_group1.AddContentName(kMid3Audio); - cricket::ContentGroup new_bundle_group2(cricket::GROUP_TYPE_BUNDLE); - bundle_group2.AddContentName(kMid6Video); - - auto subsequent_offer = std::make_unique(); - AddAudioSection(subsequent_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(subsequent_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(subsequent_offer.get(), kMid3Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer.get(), kMid5Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer.get(), kMid6Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - subsequent_offer->AddGroup(bundle_group1); - subsequent_offer->AddGroup(bundle_group2); - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, subsequent_offer.get()) - .ok()); - auto mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio); - auto mid2_transport = transport_controller_->GetRtpTransport(kMid2Audio); - auto mid3_transport = transport_controller_->GetRtpTransport(kMid3Audio); - auto mid4_transport = transport_controller_->GetRtpTransport(kMid4Video); - auto mid5_transport = transport_controller_->GetRtpTransport(kMid5Video); - auto mid6_transport = transport_controller_->GetRtpTransport(kMid6Video); - EXPECT_NE(mid1_transport, mid4_transport); - EXPECT_EQ(mid1_transport, mid2_transport); - EXPECT_EQ(mid2_transport, mid3_transport); - EXPECT_EQ(mid4_transport, mid5_transport); - EXPECT_EQ(mid5_transport, mid6_transport); -} - -TEST_F(JsepTransportControllerTest, - MultipleBundleGroupsCombinedInSubsequentOffer) { - static const char kMid1Audio[] = "1_audio"; - static const char kMid2Audio[] = "2_audio"; - static const char kMid3Video[] = "3_video"; - static const char kMid4Video[] = "4_video"; - - CreateJsepTransportController(JsepTransportController::Config()); - // Start by grouping (kMid1Audio,kMid2Audio) and (kMid3Video,kMid4Video). - cricket::ContentGroup bundle_group1(cricket::GROUP_TYPE_BUNDLE); - bundle_group1.AddContentName(kMid1Audio); - bundle_group1.AddContentName(kMid2Audio); - cricket::ContentGroup bundle_group2(cricket::GROUP_TYPE_BUNDLE); - bundle_group2.AddContentName(kMid3Video); - bundle_group2.AddContentName(kMid4Video); - - auto local_offer = std::make_unique(); - AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(local_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(local_offer.get(), kMid3Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(local_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - local_offer->AddGroup(bundle_group1); - local_offer->AddGroup(bundle_group2); - - auto remote_answer = std::make_unique(); - AddAudioSection(remote_answer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(remote_answer.get(), kMid2Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(remote_answer.get(), kMid3Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(remote_answer.get(), kMid4Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - remote_answer->AddGroup(bundle_group1); - remote_answer->AddGroup(bundle_group2); - - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, local_offer.get()) - .ok()); - EXPECT_TRUE(transport_controller_ - ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get()) - .ok()); - - // Switch to grouping (kMid1Audio,kMid2Audio,kMid3Video,kMid4Video). - // This is a illegal without first removing m= sections from their groups. - cricket::ContentGroup new_bundle_group(cricket::GROUP_TYPE_BUNDLE); - new_bundle_group.AddContentName(kMid1Audio); - new_bundle_group.AddContentName(kMid2Audio); - new_bundle_group.AddContentName(kMid3Video); - new_bundle_group.AddContentName(kMid4Video); - - auto subsequent_offer = std::make_unique(); - AddAudioSection(subsequent_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(subsequent_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer.get(), kMid3Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - subsequent_offer->AddGroup(new_bundle_group); - EXPECT_FALSE( - transport_controller_ - ->SetLocalDescription(SdpType::kOffer, subsequent_offer.get()) - .ok()); -} - -TEST_F(JsepTransportControllerTest, - MultipleBundleGroupsSplitInSubsequentOffer) { - static const char kMid1Audio[] = "1_audio"; - static const char kMid2Audio[] = "2_audio"; - static const char kMid3Video[] = "3_video"; - static const char kMid4Video[] = "4_video"; - - CreateJsepTransportController(JsepTransportController::Config()); - // Start by grouping (kMid1Audio,kMid2Audio,kMid3Video,kMid4Video). - cricket::ContentGroup bundle_group(cricket::GROUP_TYPE_BUNDLE); - bundle_group.AddContentName(kMid1Audio); - bundle_group.AddContentName(kMid2Audio); - bundle_group.AddContentName(kMid3Video); - bundle_group.AddContentName(kMid4Video); - - auto local_offer = std::make_unique(); - AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(local_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(local_offer.get(), kMid3Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(local_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - local_offer->AddGroup(bundle_group); - - auto remote_answer = std::make_unique(); - AddAudioSection(remote_answer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(remote_answer.get(), kMid2Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(remote_answer.get(), kMid3Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(remote_answer.get(), kMid4Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - remote_answer->AddGroup(bundle_group); - - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, local_offer.get()) - .ok()); - EXPECT_TRUE(transport_controller_ - ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get()) - .ok()); - - // Switch to grouping (kMid1Audio,kMid2Audio) and (kMid3Video,kMid4Video). - // This is a illegal without first removing m= sections from their groups. - cricket::ContentGroup new_bundle_group1(cricket::GROUP_TYPE_BUNDLE); - new_bundle_group1.AddContentName(kMid1Audio); - new_bundle_group1.AddContentName(kMid2Audio); - cricket::ContentGroup new_bundle_group2(cricket::GROUP_TYPE_BUNDLE); - new_bundle_group2.AddContentName(kMid3Video); - new_bundle_group2.AddContentName(kMid4Video); - - auto subsequent_offer = std::make_unique(); - AddAudioSection(subsequent_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(subsequent_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer.get(), kMid3Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - subsequent_offer->AddGroup(new_bundle_group1); - subsequent_offer->AddGroup(new_bundle_group2); - EXPECT_FALSE( - transport_controller_ - ->SetLocalDescription(SdpType::kOffer, subsequent_offer.get()) - .ok()); -} - -TEST_F(JsepTransportControllerTest, - MultipleBundleGroupsShuffledInSubsequentOffer) { - static const char kMid1Audio[] = "1_audio"; - static const char kMid2Audio[] = "2_audio"; - static const char kMid3Video[] = "3_video"; - static const char kMid4Video[] = "4_video"; - - CreateJsepTransportController(JsepTransportController::Config()); - // Start by grouping (kMid1Audio,kMid2Audio) and (kMid3Video,kMid4Video). - cricket::ContentGroup bundle_group1(cricket::GROUP_TYPE_BUNDLE); - bundle_group1.AddContentName(kMid1Audio); - bundle_group1.AddContentName(kMid2Audio); - cricket::ContentGroup bundle_group2(cricket::GROUP_TYPE_BUNDLE); - bundle_group2.AddContentName(kMid3Video); - bundle_group2.AddContentName(kMid4Video); - - auto local_offer = std::make_unique(); - AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(local_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(local_offer.get(), kMid3Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(local_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - local_offer->AddGroup(bundle_group1); - local_offer->AddGroup(bundle_group2); - - auto remote_answer = std::make_unique(); - AddAudioSection(remote_answer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(remote_answer.get(), kMid2Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(remote_answer.get(), kMid3Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(remote_answer.get(), kMid4Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - remote_answer->AddGroup(bundle_group1); - remote_answer->AddGroup(bundle_group2); - - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, local_offer.get()) - .ok()); - EXPECT_TRUE(transport_controller_ - ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get()) - .ok()); - - // Switch to grouping (kMid1Audio,kMid3Video) and (kMid2Audio,kMid3Video). - // This is a illegal without first removing m= sections from their groups. - cricket::ContentGroup new_bundle_group1(cricket::GROUP_TYPE_BUNDLE); - new_bundle_group1.AddContentName(kMid1Audio); - new_bundle_group1.AddContentName(kMid3Video); - cricket::ContentGroup new_bundle_group2(cricket::GROUP_TYPE_BUNDLE); - new_bundle_group2.AddContentName(kMid2Audio); - new_bundle_group2.AddContentName(kMid4Video); - - auto subsequent_offer = std::make_unique(); - AddAudioSection(subsequent_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(subsequent_offer.get(), kMid2Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer.get(), kMid3Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer.get(), kMid4Video, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - subsequent_offer->AddGroup(new_bundle_group1); - subsequent_offer->AddGroup(new_bundle_group2); - EXPECT_FALSE( - transport_controller_ - ->SetLocalDescription(SdpType::kOffer, subsequent_offer.get()) - .ok()); -} - // Tests that only a subset of all the m= sections are bundled. TEST_F(JsepTransportControllerTest, BundleSubsetOfMediaSections) { CreateJsepTransportController(JsepTransportController::Config()); @@ -2409,220 +2059,4 @@ TEST_F(JsepTransportControllerTest, ChangeTaggedMediaSectionMaxBundle) { .ok()); } -TEST_F(JsepTransportControllerTest, RollbackRestoresRejectedTransport) { - static const char kMid1Audio[] = "1_audio"; - - // Perform initial offer/answer. - CreateJsepTransportController(JsepTransportController::Config()); - auto local_offer = std::make_unique(); - AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - std::unique_ptr remote_answer( - local_offer->Clone()); - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, local_offer.get()) - .ok()); - EXPECT_TRUE(transport_controller_ - ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get()) - .ok()); - - auto mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio); - - // Apply a reoffer which rejects the m= section, causing the transport to be - // set to null. - auto local_reoffer = std::make_unique(); - AddAudioSection(local_reoffer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - local_reoffer->contents()[0].rejected = true; - - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, local_reoffer.get()) - .ok()); - auto old_mid1_transport = mid1_transport; - mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio); - EXPECT_EQ(nullptr, mid1_transport); - - // Rolling back shouldn't just create a new transport for MID 1, it should - // restore the old transport. - EXPECT_TRUE(transport_controller_->RollbackTransports().ok()); - mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio); - EXPECT_EQ(old_mid1_transport, mid1_transport); -} - -// If an offer with a modified BUNDLE group causes a MID->transport mapping to -// change, rollback should restore the previous mapping. -TEST_F(JsepTransportControllerTest, RollbackRestoresPreviousTransportMapping) { - static const char kMid1Audio[] = "1_audio"; - static const char kMid2Audio[] = "2_audio"; - static const char kMid3Audio[] = "3_audio"; - - // Perform an initial offer/answer to establish a (kMid1Audio,kMid2Audio) - // group. - CreateJsepTransportController(JsepTransportController::Config()); - cricket::ContentGroup bundle_group(cricket::GROUP_TYPE_BUNDLE); - bundle_group.AddContentName(kMid1Audio); - bundle_group.AddContentName(kMid2Audio); - - auto local_offer = std::make_unique(); - AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(local_offer.get(), kMid2Audio, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(local_offer.get(), kMid3Audio, kIceUfrag3, kIcePwd3, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - local_offer->AddGroup(bundle_group); - - std::unique_ptr remote_answer( - local_offer->Clone()); - - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, local_offer.get()) - .ok()); - EXPECT_TRUE(transport_controller_ - ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get()) - .ok()); - - auto mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio); - auto mid2_transport = transport_controller_->GetRtpTransport(kMid2Audio); - auto mid3_transport = transport_controller_->GetRtpTransport(kMid3Audio); - EXPECT_EQ(mid1_transport, mid2_transport); - EXPECT_NE(mid1_transport, mid3_transport); - - // Apply a reoffer adding kMid3Audio to the group; transport mapping should - // change, even without an answer, since this is an existing group. - bundle_group.AddContentName(kMid3Audio); - auto local_reoffer = std::make_unique(); - AddAudioSection(local_reoffer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(local_reoffer.get(), kMid2Audio, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddAudioSection(local_reoffer.get(), kMid3Audio, kIceUfrag3, kIcePwd3, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - local_reoffer->AddGroup(bundle_group); - - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, local_reoffer.get()) - .ok()); - - // Store the old transport pointer and verify that the offer actually changed - // transports. - auto old_mid3_transport = mid3_transport; - mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio); - mid2_transport = transport_controller_->GetRtpTransport(kMid2Audio); - mid3_transport = transport_controller_->GetRtpTransport(kMid3Audio); - EXPECT_EQ(mid1_transport, mid2_transport); - EXPECT_EQ(mid1_transport, mid3_transport); - - // Rolling back shouldn't just create a new transport for MID 3, it should - // restore the old transport. - EXPECT_TRUE(transport_controller_->RollbackTransports().ok()); - mid3_transport = transport_controller_->GetRtpTransport(kMid3Audio); - EXPECT_EQ(old_mid3_transport, mid3_transport); -} - -// Test that if an offer adds a MID to a specific BUNDLE group and is then -// rolled back, it can be added to a different BUNDLE group in a new offer. -// This is effectively testing that rollback resets the BundleManager state. -TEST_F(JsepTransportControllerTest, RollbackAndAddToDifferentBundleGroup) { - static const char kMid1Audio[] = "1_audio"; - static const char kMid2Audio[] = "2_audio"; - static const char kMid3Audio[] = "3_audio"; - - // Perform an initial offer/answer to establish two bundle groups, each with - // one MID. - CreateJsepTransportController(JsepTransportController::Config()); - cricket::ContentGroup bundle_group1(cricket::GROUP_TYPE_BUNDLE); - bundle_group1.AddContentName(kMid1Audio); - cricket::ContentGroup bundle_group2(cricket::GROUP_TYPE_BUNDLE); - bundle_group2.AddContentName(kMid2Audio); - - auto local_offer = std::make_unique(); - AddAudioSection(local_offer.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(local_offer.get(), kMid2Audio, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - local_offer->AddGroup(bundle_group1); - local_offer->AddGroup(bundle_group2); - - std::unique_ptr remote_answer( - local_offer->Clone()); - - EXPECT_TRUE(transport_controller_ - ->SetLocalDescription(SdpType::kOffer, local_offer.get()) - .ok()); - EXPECT_TRUE(transport_controller_ - ->SetRemoteDescription(SdpType::kAnswer, remote_answer.get()) - .ok()); - - // Apply an offer that adds kMid3Audio to the first BUNDLE group., - cricket::ContentGroup modified_bundle_group1(cricket::GROUP_TYPE_BUNDLE); - modified_bundle_group1.AddContentName(kMid1Audio); - modified_bundle_group1.AddContentName(kMid3Audio); - auto subsequent_offer_1 = std::make_unique(); - AddAudioSection(subsequent_offer_1.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer_1.get(), kMid2Audio, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer_1.get(), kMid3Audio, kIceUfrag3, kIcePwd3, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - subsequent_offer_1->AddGroup(modified_bundle_group1); - subsequent_offer_1->AddGroup(bundle_group2); - - EXPECT_TRUE( - transport_controller_ - ->SetLocalDescription(SdpType::kOffer, subsequent_offer_1.get()) - .ok()); - - auto mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio); - auto mid2_transport = transport_controller_->GetRtpTransport(kMid2Audio); - auto mid3_transport = transport_controller_->GetRtpTransport(kMid3Audio); - EXPECT_NE(mid1_transport, mid2_transport); - EXPECT_EQ(mid1_transport, mid3_transport); - - // Rollback and expect the transport to be reset. - EXPECT_TRUE(transport_controller_->RollbackTransports().ok()); - EXPECT_EQ(nullptr, transport_controller_->GetRtpTransport(kMid3Audio)); - - // Apply an offer that adds kMid3Audio to the second BUNDLE group., - cricket::ContentGroup modified_bundle_group2(cricket::GROUP_TYPE_BUNDLE); - modified_bundle_group2.AddContentName(kMid2Audio); - modified_bundle_group2.AddContentName(kMid3Audio); - auto subsequent_offer_2 = std::make_unique(); - AddAudioSection(subsequent_offer_2.get(), kMid1Audio, kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer_2.get(), kMid2Audio, kIceUfrag2, kIcePwd2, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - AddVideoSection(subsequent_offer_2.get(), kMid3Audio, kIceUfrag3, kIcePwd3, - cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS, - nullptr); - subsequent_offer_2->AddGroup(bundle_group1); - subsequent_offer_2->AddGroup(modified_bundle_group2); - - EXPECT_TRUE( - transport_controller_ - ->SetLocalDescription(SdpType::kOffer, subsequent_offer_2.get()) - .ok()); - - mid1_transport = transport_controller_->GetRtpTransport(kMid1Audio); - mid2_transport = transport_controller_->GetRtpTransport(kMid2Audio); - mid3_transport = transport_controller_->GetRtpTransport(kMid3Audio); - EXPECT_NE(mid1_transport, mid2_transport); - EXPECT_EQ(mid2_transport, mid3_transport); -} - } // namespace webrtc diff --git a/pc/peer_connection_bundle_unittest.cc b/pc/peer_connection_bundle_unittest.cc index be05af920c..08754c6820 100644 --- a/pc/peer_connection_bundle_unittest.cc +++ b/pc/peer_connection_bundle_unittest.cc @@ -909,11 +909,11 @@ TEST_F(PeerConnectionBundleTestUnifiedPlan, MultipleBundleGroups) { EXPECT_TRUE( caller->SetLocalDescription(CloneSessionDescription(offer.get()))); - EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer))); + callee->SetRemoteDescription(std::move(offer)); auto answer = callee->CreateAnswer(); EXPECT_TRUE( callee->SetLocalDescription(CloneSessionDescription(answer.get()))); - EXPECT_TRUE(caller->SetRemoteDescription(std::move(answer))); + caller->SetRemoteDescription(std::move(answer)); // Verify bundling on sender side. auto senders = caller->pc()->GetSenders(); @@ -938,59 +938,4 @@ TEST_F(PeerConnectionBundleTestUnifiedPlan, MultipleBundleGroups) { EXPECT_NE(receiver0_transport, receiver2_transport); } -// Test that, with the "max-compat" bundle policy, it's possible to add an m= -// section that's not part of an existing bundle group. -TEST_F(PeerConnectionBundleTestUnifiedPlan, AddNonBundledSection) { - RTCConfiguration config; - config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxCompat; - auto caller = CreatePeerConnection(config); - caller->AddAudioTrack("0_audio"); - caller->AddAudioTrack("1_audio"); - auto callee = CreatePeerConnection(config); - - // Establish an existing BUNDLE group. - auto offer = caller->CreateOffer(RTCOfferAnswerOptions()); - EXPECT_TRUE( - caller->SetLocalDescription(CloneSessionDescription(offer.get()))); - EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer))); - auto answer = callee->CreateAnswer(); - EXPECT_TRUE( - callee->SetLocalDescription(CloneSessionDescription(answer.get()))); - EXPECT_TRUE(caller->SetRemoteDescription(std::move(answer))); - - // Add a track but munge SDP so it's not part of the bundle group. - caller->AddAudioTrack("3_audio"); - offer = caller->CreateOffer(RTCOfferAnswerOptions()); - offer->description()->RemoveGroupByName(cricket::GROUP_TYPE_BUNDLE); - cricket::ContentGroup bundle_group(cricket::GROUP_TYPE_BUNDLE); - bundle_group.AddContentName("0"); - bundle_group.AddContentName("1"); - offer->description()->AddGroup(bundle_group); - EXPECT_TRUE( - caller->SetLocalDescription(CloneSessionDescription(offer.get()))); - EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer))); - answer = callee->CreateAnswer(); - EXPECT_TRUE( - callee->SetLocalDescription(CloneSessionDescription(answer.get()))); - EXPECT_TRUE(caller->SetRemoteDescription(std::move(answer))); - - // Verify bundling on the sender side. - auto senders = caller->pc()->GetSenders(); - ASSERT_EQ(senders.size(), 3u); - auto sender0_transport = senders[0]->dtls_transport(); - auto sender1_transport = senders[1]->dtls_transport(); - auto sender2_transport = senders[2]->dtls_transport(); - EXPECT_EQ(sender0_transport, sender1_transport); - EXPECT_NE(sender0_transport, sender2_transport); - - // Verify bundling on receiver side. - auto receivers = callee->pc()->GetReceivers(); - ASSERT_EQ(receivers.size(), 3u); - auto receiver0_transport = receivers[0]->dtls_transport(); - auto receiver1_transport = receivers[1]->dtls_transport(); - auto receiver2_transport = receivers[2]->dtls_transport(); - EXPECT_EQ(receiver0_transport, receiver1_transport); - EXPECT_NE(receiver0_transport, receiver2_transport); -} - } // namespace webrtc diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 929736e8c6..533bd84dbe 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -2737,10 +2737,7 @@ RTCError SdpOfferAnswerHandler::Rollback(SdpType desc_type) { transceiver->internal()->set_mid(state.mid()); transceiver->internal()->set_mline_index(state.mline_index()); } - RTCError e = transport_controller()->RollbackTransports(); - if (!e.ok()) { - return e; - } + transport_controller()->RollbackTransports(); transceivers()->DiscardStableStates(); pending_local_description_.reset(); pending_remote_description_.reset();