diff --git a/pc/jsep_transport_collection.cc b/pc/jsep_transport_collection.cc index 177a6ccc1c..ce068d99fc 100644 --- a/pc/jsep_transport_collection.cc +++ b/pc/jsep_transport_collection.cc @@ -27,12 +27,40 @@ void BundleManager::Update(const cricket::SessionDescription* description) { 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(); } + 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(); + } + } +} + +const cricket::ContentGroup* BundleManager::LookupGroupByMid( + const std::string& mid) const { + auto it = established_bundle_groups_by_mid_.find(mid); + return it != established_bundle_groups_by_mid_.end() ? it->second : nullptr; +} +bool BundleManager::IsFirstMidInGroup(const std::string& mid) const { + auto group = LookupGroupByMid(mid); + if (!group) { + return true; // Unbundled MIDs are considered group leaders + } + return mid == *(group->FirstContentName()); +} + +cricket::ContentGroup* BundleManager::LookupGroupByMid(const std::string& mid) { + auto it = established_bundle_groups_by_mid_.find(mid); + return it != established_bundle_groups_by_mid_.end() ? it->second : nullptr; } void BundleManager::DeleteMid(const cricket::ContentGroup* bundle_group, const std::string& mid) { RTC_DCHECK_RUN_ON(&sequence_checker_); + RTC_LOG(LS_VERBOSE) << "Deleting mid " << mid << " from bundle group " + << bundle_group->ToString(); // Remove the rejected content from the |bundle_group|. // The const pointer arg is used to identify the group, we verify // it before we use it to make a modification. @@ -43,17 +71,24 @@ void BundleManager::DeleteMid(const cricket::ContentGroup* bundle_group, }); RTC_DCHECK(bundle_group_it != bundle_groups_.end()); (*bundle_group_it)->RemoveContentName(mid); + established_bundle_groups_by_mid_.erase( + established_bundle_groups_by_mid_.find(mid)); } void BundleManager::DeleteGroup(const cricket::ContentGroup* bundle_group) { RTC_DCHECK_RUN_ON(&sequence_checker_); - // Delete the BUNDLE group. + RTC_DLOG(LS_VERBOSE) << "Deleting bundle group " << bundle_group->ToString(); + auto bundle_group_it = std::find_if( bundle_groups_.begin(), bundle_groups_.end(), [bundle_group](std::unique_ptr& group) { return bundle_group == group.get(); }); RTC_DCHECK(bundle_group_it != bundle_groups_.end()); + auto mid_list = (*bundle_group_it)->content_names(); + for (const auto& content_name : mid_list) { + DeleteMid(bundle_group, content_name); + } bundle_groups_.erase(bundle_group_it); } @@ -208,7 +243,8 @@ bool JsepTransportCollection::IsConsistent() { } const auto& lookup = mid_to_transport_.find(it.first); if (lookup->second != it.second.get()) { - RTC_LOG(LS_ERROR) << "Note: Mid " << it.first << " was registered to " + // 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; } diff --git a/pc/jsep_transport_collection.h b/pc/jsep_transport_collection.h index 0db0ca1ab4..0dd528d348 100644 --- a/pc/jsep_transport_collection.h +++ b/pc/jsep_transport_collection.h @@ -50,6 +50,12 @@ class BundleManager { RTC_DCHECK_RUN_ON(&sequence_checker_); return bundle_groups_; } + // Lookup a bundle group by a member mid name. + const cricket::ContentGroup* LookupGroupByMid(const std::string& mid) const; + cricket::ContentGroup* LookupGroupByMid(const std::string& mid); + // Returns true if the MID is the first item of a group, or if + // the MID is not a member of a group. + 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); @@ -63,6 +69,8 @@ class BundleManager { RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_; std::vector> bundle_groups_ RTC_GUARDED_BY(sequence_checker_); + std::map + established_bundle_groups_by_mid_; }; // This class keeps the mapping of MIDs to transports. diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 97a5cf5f28..95cf21587d 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -38,19 +38,6 @@ using webrtc::SdpType; namespace webrtc { -namespace { - -bool IsBundledButNotFirstMid( - const std::map& bundle_groups_by_mid, - const std::string& mid) { - auto it = bundle_groups_by_mid.find(mid); - if (it == bundle_groups_by_mid.end()) - return false; - return mid != *it->second->FirstContentName(); -} - -} // namespace - JsepTransportController::JsepTransportController( rtc::Thread* network_thread, cricket::PortAllocator* port_allocator, @@ -556,21 +543,12 @@ RTCError JsepTransportController::ApplyDescription_n( if (!error.ok()) { return error; } - // Established BUNDLE groups by MID. - std::map - established_bundle_groups_by_mid; - for (const auto& bundle_group : bundles_.bundle_groups()) { - for (const std::string& content_name : bundle_group->content_names()) { - established_bundle_groups_by_mid[content_name] = bundle_group.get(); - } - } std::map> merged_encrypted_extension_ids_by_bundle; if (!bundles_.bundle_groups().empty()) { merged_encrypted_extension_ids_by_bundle = - MergeEncryptedHeaderExtensionIdsForBundles( - established_bundle_groups_by_mid, description); + MergeEncryptedHeaderExtensionIdsForBundles(description); } // Because the creation of transports depends on whether @@ -579,17 +557,15 @@ RTCError JsepTransportController::ApplyDescription_n( 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); + // 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 || - IsBundledButNotFirstMid(established_bundle_groups_by_mid, - content_info.name)) { + !bundles_.IsFirstMidInGroup(content_info.name)) { continue; } error = MaybeCreateJsepTransport(local, content_info, *description); @@ -609,9 +585,8 @@ RTCError JsepTransportController::ApplyDescription_n( continue; } - auto it = established_bundle_groups_by_mid.find(content_info.name); const cricket::ContentGroup* established_bundle_group = - it != established_bundle_groups_by_mid.end() ? it->second : nullptr; + bundles_.LookupGroupByMid(content_info.name); // For bundle members that are not BUNDLE-tagged (not first in the group), // configure their transport to be the same as the BUNDLE-tagged transport. @@ -835,25 +810,18 @@ RTCError JsepTransportController::ValidateContent( } void JsepTransportController::HandleRejectedContent( - const cricket::ContentInfo& content_info, - std::map& - established_bundle_groups_by_mid) { + const cricket::ContentInfo& content_info) { // If the content is rejected, let the // BaseChannel/SctpTransport change the RtpTransport/DtlsTransport first, // then destroy the cricket::JsepTransport. - auto it = established_bundle_groups_by_mid.find(content_info.name); cricket::ContentGroup* bundle_group = - it != established_bundle_groups_by_mid.end() ? it->second : nullptr; + bundles_.LookupGroupByMid(content_info.name); if (bundle_group && !bundle_group->content_names().empty() && content_info.name == *bundle_group->FirstContentName()) { // Rejecting a BUNDLE group's first mid means we are rejecting the entire // group. for (const auto& content_name : bundle_group->content_names()) { transports_.RemoveTransportForMid(content_name); - // We are about to delete this BUNDLE group, erase all mappings to it. - it = established_bundle_groups_by_mid.find(content_name); - RTC_DCHECK(it != established_bundle_groups_by_mid.end()); - established_bundle_groups_by_mid.erase(it); } // Delete the BUNDLE group. bundles_.DeleteGroup(bundle_group); @@ -947,7 +915,6 @@ std::vector JsepTransportController::GetEncryptedHeaderExtensionIds( std::map> JsepTransportController::MergeEncryptedHeaderExtensionIdsForBundles( - const std::map& bundle_groups_by_mid, const cricket::SessionDescription* description) { RTC_DCHECK(description); RTC_DCHECK(!bundles_.bundle_groups().empty()); @@ -955,12 +922,12 @@ JsepTransportController::MergeEncryptedHeaderExtensionIdsForBundles( merged_encrypted_extension_ids_by_bundle; // Union the encrypted header IDs in the group when bundle is enabled. for (const cricket::ContentInfo& content_info : description->contents()) { - auto it = bundle_groups_by_mid.find(content_info.name); - if (it == bundle_groups_by_mid.end()) + auto group = bundles_.LookupGroupByMid(content_info.name); + if (!group) continue; // Get or create list of IDs for the BUNDLE group. std::vector& merged_ids = - merged_encrypted_extension_ids_by_bundle[it->second]; + merged_encrypted_extension_ids_by_bundle[group]; // Add IDs not already in the list. std::vector extension_ids = GetEncryptedHeaderExtensionIds(content_info); diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index a11bac52fd..71b01bffb2 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -330,9 +330,7 @@ class JsepTransportController : public sigslot::has_slots<> { const cricket::SessionDescription* description); RTCError ValidateContent(const cricket::ContentInfo& content_info); - void HandleRejectedContent(const cricket::ContentInfo& content_info, - std::map& - established_bundle_groups_by_mid) + void HandleRejectedContent(const cricket::ContentInfo& content_info) RTC_RUN_ON(network_thread_); bool HandleBundledContent(const cricket::ContentInfo& content_info, const cricket::ContentGroup& bundle_group) @@ -349,7 +347,6 @@ class JsepTransportController : public sigslot::has_slots<> { std::map> MergeEncryptedHeaderExtensionIdsForBundles( - const std::map& bundle_groups_by_mid, const cricket::SessionDescription* description); std::vector GetEncryptedHeaderExtensionIds( const cricket::ContentInfo& content_info);