diff --git a/pc/congestion_control_integrationtest.cc b/pc/congestion_control_integrationtest.cc index 55d880fda9..d93a31924f 100644 --- a/pc/congestion_control_integrationtest.cc +++ b/pc/congestion_control_integrationtest.cc @@ -53,19 +53,24 @@ TEST_F(PeerConnectionCongestionControlTest, ReceiveOfferSetsCcfbFlag) { caller()->CreateAndSetAndSignalOffer(); ASSERT_THAT(WaitUntil([&] { return SignalingStateStable(); }, IsTrue()), IsRtcOk()); - // Check that the callee parsed it. - auto parsed_contents = - callee()->pc()->remote_description()->description()->contents(); - EXPECT_FALSE(parsed_contents.empty()); - for (const auto& content : parsed_contents) { - EXPECT_TRUE(content.media_description()->rtcp_fb_ack_ccfb()); + { + // Check that the callee parsed it. + auto parsed_contents = + callee()->pc()->remote_description()->description()->contents(); + EXPECT_FALSE(parsed_contents.empty()); + for (const auto& content : parsed_contents) { + EXPECT_TRUE(content.media_description()->rtcp_fb_ack_ccfb()); + } } - // Check that the caller also parsed it. - parsed_contents = - caller()->pc()->remote_description()->description()->contents(); - EXPECT_FALSE(parsed_contents.empty()); - for (const auto& content : parsed_contents) { - EXPECT_TRUE(content.media_description()->rtcp_fb_ack_ccfb()); + + { + // Check that the caller also parsed it. + auto parsed_contents = + caller()->pc()->remote_description()->description()->contents(); + EXPECT_FALSE(parsed_contents.empty()); + for (const auto& content : parsed_contents) { + EXPECT_TRUE(content.media_description()->rtcp_fb_ack_ccfb()); + } } // Check that the answer does not contain transport-cc std::string answer_str = absl::StrCat(*caller()->pc()->remote_description()); diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 4daf03c336..d4367c0581 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -687,7 +687,7 @@ RTCError JsepTransportController::ApplyDescription_n( for (const cricket::ContentInfo& content_info : description->contents()) { // Don't create transports for rejected m-lines and bundled m-lines. if (content_info.rejected || - !bundles_.IsFirstMidInGroup(content_info.name)) { + !bundles_.IsFirstMidInGroup(content_info.mid())) { continue; } error = MaybeCreateJsepTransport(local, content_info, *description); @@ -710,17 +710,17 @@ RTCError JsepTransportController::ApplyDescription_n( } const cricket::ContentGroup* established_bundle_group = - bundles_.LookupGroupByMid(content_info.name); + bundles_.LookupGroupByMid(content_info.mid()); // 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. if (established_bundle_group && - content_info.name != *established_bundle_group->FirstContentName()) { + content_info.mid() != *established_bundle_group->FirstContentName()) { if (!HandleBundledContent(content_info, *established_bundle_group)) { return RTCError(RTCErrorType::INVALID_PARAMETER, "Failed to process the bundled m= section with " "mid='" + - content_info.name + "'."); + content_info.mid() + "'."); } continue; } @@ -733,7 +733,7 @@ RTCError JsepTransportController::ApplyDescription_n( std::vector extension_ids; // Is BUNDLE-tagged (first in the group)? if (established_bundle_group && - content_info.name == *established_bundle_group->FirstContentName()) { + content_info.mid() == *established_bundle_group->FirstContentName()) { auto it = merged_encrypted_extension_ids_by_bundle.find( established_bundle_group); RTC_DCHECK(it != merged_encrypted_extension_ids_by_bundle.end()); @@ -746,12 +746,12 @@ RTCError JsepTransportController::ApplyDescription_n( GetRtpAbsSendTimeHeaderExtensionId(content_info); cricket::JsepTransport* transport = - GetJsepTransportForMid(content_info.name); + GetJsepTransportForMid(content_info.mid()); if (!transport) { LOG_AND_RETURN_ERROR( RTCErrorType::INVALID_PARAMETER, "Could not find transport for m= section with mid='" + - content_info.name + "'"); + content_info.mid() + "'"); } SetIceRole_n(DetermineIceRole(transport, transport_info, type, local)); @@ -771,7 +771,7 @@ RTCError JsepTransportController::ApplyDescription_n( LOG_AND_RETURN_ERROR( RTCErrorType::INVALID_PARAMETER, "Failed to apply the description for m= section with mid='" + - content_info.name + "': " + error.message()); + content_info.mid() + "': " + error.message()); } error = transport->RecordPayloadTypes(local, type, content_info); if (!error.ok()) { @@ -981,7 +981,7 @@ RTCError JsepTransportController::ValidateContent( !content_info.bundle_only && !content_info.media_description()->rtcp_mux()) { return RTCError(RTCErrorType::INVALID_PARAMETER, - "The m= section with mid='" + content_info.name + + "The m= section with mid='" + content_info.mid() + "' is invalid. RTCP-MUX is not " "enabled when it is required."); } @@ -994,9 +994,9 @@ void JsepTransportController::HandleRejectedContent( // BaseChannel/SctpTransport change the RtpTransport/DtlsTransport first, // then destroy the cricket::JsepTransport. cricket::ContentGroup* bundle_group = - bundles_.LookupGroupByMid(content_info.name); + bundles_.LookupGroupByMid(content_info.mid()); if (bundle_group && !bundle_group->content_names().empty() && - content_info.name == *bundle_group->FirstContentName()) { + content_info.mid() == *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()) { @@ -1005,10 +1005,10 @@ void JsepTransportController::HandleRejectedContent( // Delete the BUNDLE group. bundles_.DeleteGroup(bundle_group); } else { - transports_.RemoveTransportForMid(content_info.name); + transports_.RemoveTransportForMid(content_info.mid()); if (bundle_group) { // Remove the rejected content from the `bundle_group`. - bundles_.DeleteMid(bundle_group, content_info.name); + bundles_.DeleteMid(bundle_group, content_info.mid()); } } } @@ -1028,7 +1028,7 @@ bool JsepTransportController::HandleBundledContent( // because it means that we first create media transport and start // connecting it, and then we destroy it. We will need to address it before // video path is enabled. - return transports_.SetTransportForMid(content_info.name, jsep_transport); + return transports_.SetTransportForMid(content_info.mid(), jsep_transport); } cricket::JsepTransportDescription @@ -1081,7 +1081,7 @@ 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 group = bundles_.LookupGroupByMid(content_info.name); + auto group = bundles_.LookupGroupByMid(content_info.mid()); if (!group) continue; // Get or create list of IDs for the BUNDLE group. @@ -1150,13 +1150,14 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( bool local, const cricket::ContentInfo& content_info, const cricket::SessionDescription& description) { - cricket::JsepTransport* transport = GetJsepTransportByName(content_info.name); + cricket::JsepTransport* transport = + GetJsepTransportByName(content_info.mid()); if (transport) { return RTCError::OK(); } rtc::scoped_refptr ice = - CreateIceTransport(content_info.name, /*rtcp=*/false); + CreateIceTransport(content_info.mid(), /*rtcp=*/false); std::unique_ptr rtp_dtls_transport = CreateDtlsTransport(content_info, ice->internal()); @@ -1170,7 +1171,7 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( if (config_.rtcp_mux_policy != PeerConnectionInterface::kRtcpMuxPolicyRequire && content_info.type == cricket::MediaProtocolType::kRtp) { - rtcp_ice = CreateIceTransport(content_info.name, /*rtcp=*/true); + rtcp_ice = CreateIceTransport(content_info.mid(), /*rtcp=*/true); rtcp_dtls_transport = CreateDtlsTransport(content_info, rtcp_ice->internal()); } @@ -1179,11 +1180,13 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( RTC_LOG(LS_INFO) << "Creating UnencryptedRtpTransport, becayse encryption is disabled."; unencrypted_rtp_transport = CreateUnencryptedRtpTransport( - content_info.name, rtp_dtls_transport.get(), rtcp_dtls_transport.get()); + content_info.mid(), rtp_dtls_transport.get(), + rtcp_dtls_transport.get()); } else { RTC_LOG(LS_INFO) << "Creating DtlsSrtpTransport."; - dtls_srtp_transport = CreateDtlsSrtpTransport( - content_info.name, rtp_dtls_transport.get(), rtcp_dtls_transport.get()); + dtls_srtp_transport = + CreateDtlsSrtpTransport(content_info.mid(), rtp_dtls_transport.get(), + rtcp_dtls_transport.get()); } std::unique_ptr sctp_transport; @@ -1194,7 +1197,7 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( std::unique_ptr jsep_transport = std::make_unique( - content_info.name, certificate_, std::move(ice), std::move(rtcp_ice), + content_info.mid(), certificate_, std::move(ice), std::move(rtcp_ice), std::move(unencrypted_rtp_transport), std::move(sdes_transport), std::move(dtls_srtp_transport), std::move(rtp_dtls_transport), std::move(rtcp_dtls_transport), std::move(sctp_transport), @@ -1215,7 +1218,7 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( OnUnDemuxableRtpPacketReceived_n(packet); }); - transports_.RegisterTransport(content_info.name, std::move(jsep_transport)); + transports_.RegisterTransport(content_info.mid(), std::move(jsep_transport)); UpdateAggregateStates_n(); return RTCError::OK(); } diff --git a/pc/session_description.cc b/pc/session_description.cc index e1152eb107..df32f4018a 100644 --- a/pc/session_description.cc +++ b/pc/session_description.cc @@ -19,23 +19,21 @@ namespace cricket { namespace { ContentInfo* FindContentInfoByName(ContentInfos* contents, - const std::string& name) { + absl::string_view name) { RTC_DCHECK(contents); for (ContentInfo& content : *contents) { - if (content.name == name) { + if (content.mid() == name) { return &content; } } return nullptr; } -} // namespace - const ContentInfo* FindContentInfoByName(const ContentInfos& contents, - const std::string& name) { + absl::string_view name) { for (ContentInfos::const_iterator content = contents.begin(); content != contents.end(); ++content) { - if (content->name == name) { + if (content->mid() == name) { return &(*content); } } @@ -52,6 +50,8 @@ const ContentInfo* FindContentInfoByType(const ContentInfos& contents, return nullptr; } +} // namespace + ContentGroup::ContentGroup(const std::string& semantics) : semantics_(semantics) {} @@ -117,7 +117,7 @@ ContentInfo* SessionDescription::GetContentByName(const std::string& name) { } const MediaContentDescription* SessionDescription::GetContentDescriptionByName( - const std::string& name) const { + absl::string_view name) const { const ContentInfo* cinfo = FindContentInfoByName(contents_, name); if (cinfo == NULL) { return NULL; @@ -127,7 +127,7 @@ const MediaContentDescription* SessionDescription::GetContentDescriptionByName( } MediaContentDescription* SessionDescription::GetContentDescriptionByName( - const std::string& name) { + absl::string_view name) { ContentInfo* cinfo = FindContentInfoByName(&contents_, name); if (cinfo == NULL) { return NULL; @@ -149,9 +149,8 @@ void SessionDescription::AddContent( const std::string& name, MediaProtocolType type, std::unique_ptr description) { - ContentInfo content(type); + ContentInfo content(type, std::move(description)); content.name = name; - content.set_media_description(std::move(description)); AddContent(std::move(content)); } @@ -160,10 +159,9 @@ void SessionDescription::AddContent( MediaProtocolType type, bool rejected, std::unique_ptr description) { - ContentInfo content(type); + ContentInfo content(type, std::move(description)); content.name = name; content.rejected = rejected; - content.set_media_description(std::move(description)); AddContent(std::move(content)); } @@ -173,11 +171,10 @@ void SessionDescription::AddContent( bool rejected, bool bundle_only, std::unique_ptr description) { - ContentInfo content(type); + ContentInfo content(type, std::move(description)); content.name = name; content.rejected = rejected; content.bundle_only = bundle_only; - content.set_media_description(std::move(description)); AddContent(std::move(content)); } @@ -193,7 +190,7 @@ void SessionDescription::AddContent(ContentInfo&& content) { bool SessionDescription::RemoveContentByName(const std::string& name) { for (ContentInfos::iterator content = contents_.begin(); content != contents_.end(); ++content) { - if (content->name == name) { + if (content->mid() == name) { contents_.erase(content); return true; } @@ -291,15 +288,6 @@ ContentInfo::ContentInfo(const ContentInfo& o) bundle_only(o.bundle_only), description_(o.description_->Clone()) {} -ContentInfo& ContentInfo::operator=(const ContentInfo& o) { - name = o.name; - type = o.type; - rejected = o.rejected; - bundle_only = o.bundle_only; - description_ = o.description_->Clone(); - return *this; -} - const MediaContentDescription* ContentInfo::media_description() const { return description_.get(); } diff --git a/pc/session_description.h b/pc/session_description.h index a49e9e4dc3..0f49feb36e 100644 --- a/pc/session_description.h +++ b/pc/session_description.h @@ -399,26 +399,29 @@ enum class MediaProtocolType { class RTC_EXPORT ContentInfo { public: explicit ContentInfo(MediaProtocolType type) : type(type) {} + ContentInfo(MediaProtocolType type, + std::unique_ptr description) + : type(type), description_(std::move(description)) {} ~ContentInfo(); - // Copy + + // Copy ctor and assignment will clone `description_`. ContentInfo(const ContentInfo& o); - ContentInfo& operator=(const ContentInfo& o); + // Const ref assignment operator removed. Instead, use the explicit ctor. + ContentInfo& operator=(const ContentInfo& o) = delete; + ContentInfo(ContentInfo&& o) = default; ContentInfo& operator=(ContentInfo&& o) = default; // Alias for `name`. - std::string mid() const { return name; } - void set_mid(const std::string& mid) { this->name = mid; } + // TODO(tommi): change return type to string_view. + const std::string& mid() const { return name; } + void set_mid(absl::string_view mid) { name = std::string(mid); } // Alias for `description`. MediaContentDescription* media_description(); const MediaContentDescription* media_description() const; - void set_media_description(std::unique_ptr desc) { - description_ = std::move(desc); - } - - // TODO(bugs.webrtc.org/8620): Rename this to mid. + // TODO(bugs.webrtc.org/8620): Rename this to mid and make private. std::string name; MediaProtocolType type; bool rejected = false; @@ -462,11 +465,6 @@ class ContentGroup { typedef std::vector ContentInfos; typedef std::vector ContentGroups; -const ContentInfo* FindContentInfoByName(const ContentInfos& contents, - const std::string& name); -const ContentInfo* FindContentInfoByType(const ContentInfos& contents, - const std::string& type); - // Determines how the MSID will be signaled in the SDP. // These can be used as bit flags to indicate both or the special value none. enum MsidSignaling { @@ -502,8 +500,8 @@ class SessionDescription { const ContentInfo* GetContentByName(const std::string& name) const; ContentInfo* GetContentByName(const std::string& name); const MediaContentDescription* GetContentDescriptionByName( - const std::string& name) const; - MediaContentDescription* GetContentDescriptionByName(const std::string& name); + absl::string_view name) const; + MediaContentDescription* GetContentDescriptionByName(absl::string_view name); const ContentInfo* FirstContentByType(MediaProtocolType type) const; const ContentInfo* FirstContent() const;