From 9f9bf38805e14688acef01fe6814b8ce3a98c09c Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Tue, 8 Jun 2021 04:12:37 +0000 Subject: [PATCH] Start refactoring bundle behavior into BundleManager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is step 1: Encapsulating the data. Bug: webrtc:12837 Change-Id: I15df30dc294c90136a90b072608ed4c2e8925dcb Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221602 Reviewed-by: Henrik Boström Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#34245} --- pc/BUILD.gn | 1 + pc/bundle_manager.h | 49 +++++++++++++++++++++++++++++++++ pc/jsep_transport_controller.cc | 22 +++++++-------- pc/jsep_transport_controller.h | 6 ++-- 4 files changed, 64 insertions(+), 14 deletions(-) create mode 100644 pc/bundle_manager.h diff --git a/pc/BUILD.gn b/pc/BUILD.gn index f66ec07250..f36505b153 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -41,6 +41,7 @@ rtc_library("rtc_pc_base") { visibility = [ "*" ] defines = [] sources = [ + "bundle_manager.h", "channel.cc", "channel.h", "channel_interface.h", diff --git a/pc/bundle_manager.h b/pc/bundle_manager.h new file mode 100644 index 0000000000..3cbf351bbc --- /dev/null +++ b/pc/bundle_manager.h @@ -0,0 +1,49 @@ +/* + * Copyright 2021 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef PC_BUNDLE_MANAGER_H_ +#define PC_BUNDLE_MANAGER_H_ + +#include +#include + +#include "pc/session_description.h" + +namespace webrtc { +// This class manages information about RFC 8843 BUNDLE bundles +// in SDP descriptions. + +// This is a work-in-progress. Planned steps: +// 1) Move all Bundle-related data structures from JsepTransport +// into this class. +// 2) Move all Bundle-related functions into this class. +// 3) Move remaining Bundle-related logic into this class. +// Make data members private. +// 4) Refine interface to have comprehensible semantics. +// 5) Add unit tests. +// 6) Change the logic to do what's right. +class BundleManager { + public: + const std::vector>& bundle_groups() + const { + return bundle_groups_; + } + std::vector>& bundle_groups() { + return bundle_groups_; + } + + private: + // Use unique_ptr<> to get a stable address. + std::vector> bundle_groups_; +}; + +} // namespace webrtc + +#endif // PC_BUNDLE_MANAGER_H_ diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index a12438956b..69e71d961a 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -559,7 +559,7 @@ RTCError JsepTransportController::ApplyDescription_n( // Established BUNDLE groups by MID. std::map established_bundle_groups_by_mid; - for (const auto& bundle_group : bundle_groups_) { + 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(); } @@ -567,7 +567,7 @@ RTCError JsepTransportController::ApplyDescription_n( std::map> merged_encrypted_extension_ids_by_bundle; - if (!bundle_groups_.empty()) { + if (!bundles_.bundle_groups().empty()) { merged_encrypted_extension_ids_by_bundle = MergeEncryptedHeaderExtensionIdsForBundles( established_bundle_groups_by_mid, description); @@ -593,7 +593,7 @@ RTCError JsepTransportController::ApplyDescription_n( const cricket::TransportInfo& transport_info = description->transport_infos()[i]; if (content_info.rejected) { - // This may cause groups to be removed from |bundle_groups_| and + // 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; @@ -752,7 +752,7 @@ RTCError JsepTransportController::ValidateAndMaybeUpdateBundleGroups( } } - for (const auto& bundle_group : bundle_groups_) { + for (const auto& bundle_group : bundles_.bundle_groups()) { for (const std::string& content_name : bundle_group->content_names()) { // An answer that removes m= sections from pre-negotiated BUNDLE group // without rejecting it, is invalid. @@ -778,14 +778,14 @@ RTCError JsepTransportController::ValidateAndMaybeUpdateBundleGroups( } if (ShouldUpdateBundleGroup(type, description)) { - bundle_groups_.clear(); + bundles_.bundle_groups().clear(); for (const cricket::ContentGroup* new_bundle_group : new_bundle_groups) { - bundle_groups_.push_back( + bundles_.bundle_groups().push_back( std::make_unique(*new_bundle_group)); } } - for (const auto& bundle_group : bundle_groups_) { + for (const auto& bundle_group : bundles_.bundle_groups()) { if (!bundle_group->FirstContentName()) continue; @@ -851,12 +851,12 @@ void JsepTransportController::HandleRejectedContent( } // Delete the BUNDLE group. auto bundle_group_it = std::find_if( - bundle_groups_.begin(), bundle_groups_.end(), + bundles_.bundle_groups().begin(), bundles_.bundle_groups().end(), [bundle_group](std::unique_ptr& group) { return bundle_group == group.get(); }); - RTC_DCHECK(bundle_group_it != bundle_groups_.end()); - bundle_groups_.erase(bundle_group_it); + RTC_DCHECK(bundle_group_it != bundles_.bundle_groups().end()); + bundles_.bundle_groups().erase(bundle_group_it); } else { RemoveTransportForMid(content_info.name); if (bundle_group) { @@ -990,7 +990,7 @@ JsepTransportController::MergeEncryptedHeaderExtensionIdsForBundles( const std::map& bundle_groups_by_mid, const cricket::SessionDescription* description) { RTC_DCHECK(description); - RTC_DCHECK(!bundle_groups_.empty()); + RTC_DCHECK(!bundles_.bundle_groups().empty()); std::map> merged_encrypted_extension_ids_by_bundle; // Union the encrypted header IDs in the group when bundle is enabled. diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index e3c1187fb4..2d912e5ae9 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -44,6 +44,7 @@ #include "p2p/base/port_allocator.h" #include "p2p/base/transport_description.h" #include "p2p/base/transport_info.h" +#include "pc/bundle_manager.h" #include "pc/channel.h" #include "pc/dtls_srtp_transport.h" #include "pc/dtls_transport.h" @@ -483,14 +484,13 @@ class JsepTransportController : public sigslot::has_slots<> { const cricket::SessionDescription* remote_desc_ = nullptr; absl::optional initial_offerer_; - // Use unique_ptr<> to get a stable address. - std::vector> bundle_groups_; - cricket::IceConfig ice_config_; cricket::IceRole ice_role_ = cricket::ICEROLE_CONTROLLING; uint64_t ice_tiebreaker_ = rtc::CreateRandomId64(); rtc::scoped_refptr certificate_; + BundleManager bundles_; + RTC_DISALLOW_COPY_AND_ASSIGN(JsepTransportController); };