diff --git a/pc/jsep_session_description_unittest.cc b/pc/jsep_session_description_unittest.cc index aca9dd80c2..d2fc6e5957 100644 --- a/pc/jsep_session_description_unittest.cc +++ b/pc/jsep_session_description_unittest.cc @@ -63,10 +63,12 @@ CreateCricketSessionDescription() { auto video = absl::make_unique(); audio->AddCodec(cricket::AudioCodec(103, "ISAC", 16000, 0, 0)); - desc->AddContent(cricket::CN_AUDIO, MediaProtocolType::kRtp, audio.release()); + desc->AddContent(cricket::CN_AUDIO, MediaProtocolType::kRtp, + std::move(audio)); video->AddCodec(cricket::VideoCodec(120, "VP8")); - desc->AddContent(cricket::CN_VIDEO, MediaProtocolType::kRtp, video.release()); + desc->AddContent(cricket::CN_VIDEO, MediaProtocolType::kRtp, + std::move(video)); desc->AddTransportInfo(cricket::TransportInfo( cricket::CN_AUDIO, diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 4f14e00af2..6adecb3be3 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -876,13 +876,12 @@ void JsepTransportController::RemoveTransportForMid(const std::string& mid) { cricket::JsepTransportDescription JsepTransportController::CreateJsepTransportDescription( - cricket::ContentInfo content_info, - cricket::TransportInfo transport_info, + const cricket::ContentInfo& content_info, + const cricket::TransportInfo& transport_info, const std::vector& encrypted_extension_ids, int rtp_abs_sendtime_extn_id) { const cricket::MediaContentDescription* content_desc = - static_cast( - content_info.description); + content_info.media_description(); RTC_DCHECK(content_desc); bool rtcp_mux_enabled = content_info.type == cricket::MediaProtocolType::kSctp ? true @@ -916,8 +915,7 @@ bool JsepTransportController::ShouldUpdateBundleGroup( std::vector JsepTransportController::GetEncryptedHeaderExtensionIds( const cricket::ContentInfo& content_info) { const cricket::MediaContentDescription* content_desc = - static_cast( - content_info.description); + content_info.media_description(); if (!config_.crypto_options.srtp.enable_encrypted_rtp_header_extensions) { return std::vector(); @@ -964,8 +962,7 @@ int JsepTransportController::GetRtpAbsSendTimeHeaderExtensionId( } const cricket::MediaContentDescription* content_desc = - static_cast( - content_info.description); + content_info.media_description(); const webrtc::RtpExtension* send_time_extension = webrtc::RtpExtension::FindHeaderExtensionByUri( @@ -1145,8 +1142,7 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( } const cricket::MediaContentDescription* content_desc = - static_cast( - content_info.description); + content_info.media_description(); if (certificate_ && !content_desc->cryptos().empty()) { return RTCError(RTCErrorType::INVALID_PARAMETER, "SDES and DTLS-SRTP cannot be enabled at the same time."); diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index ffc9515a60..aaaab4881e 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -261,8 +261,8 @@ class JsepTransportController : public sigslot::has_slots<> { void RemoveTransportForMid(const std::string& mid); cricket::JsepTransportDescription CreateJsepTransportDescription( - cricket::ContentInfo content_info, - cricket::TransportInfo transport_info, + const cricket::ContentInfo& content_info, + const cricket::TransportInfo& transport_info, const std::vector& encrypted_extension_ids, int rtp_abs_sendtime_extn_id); diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index d796b236d3..3bde0e7c0b 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -146,7 +146,7 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, // Set RTCP-mux to be true because the default policy is "mux required". audio->set_rtcp_mux(true); description->AddContent(mid, cricket::MediaProtocolType::kRtp, - /*rejected=*/false, audio.release()); + /*rejected=*/false, std::move(audio)); AddTransportInfo(description, mid, ufrag, pwd, ice_mode, conn_role, cert); } @@ -162,7 +162,7 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, // Set RTCP-mux to be true because the default policy is "mux required". video->set_rtcp_mux(true); description->AddContent(mid, cricket::MediaProtocolType::kRtp, - /*rejected=*/false, video.release()); + /*rejected=*/false, std::move(video)); AddTransportInfo(description, mid, ufrag, pwd, ice_mode, conn_role, cert); } @@ -179,7 +179,7 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, new cricket::SctpDataContentDescription()); data->set_rtcp_mux(true); description->AddContent(mid, protocol_type, - /*rejected=*/false, data.release()); + /*rejected=*/false, std::move(data)); AddTransportInfo(description, mid, ufrag, pwd, ice_mode, conn_role, cert); } diff --git a/pc/media_session.cc b/pc/media_session.cc index 1b3d015a79..7cc4b95f14 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -2122,7 +2122,7 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer( audio->set_direction(media_description_options.direction); desc->AddContent(media_description_options.mid, MediaProtocolType::kRtp, - media_description_options.stopped, audio.release()); + media_description_options.stopped, std::move(audio)); if (!AddTransportOffer(media_description_options.mid, media_description_options.transport_options, current_description, desc, ice_credentials)) { @@ -2203,7 +2203,7 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( video->set_direction(media_description_options.direction); desc->AddContent(media_description_options.mid, MediaProtocolType::kRtp, - media_description_options.stopped, video.release()); + media_description_options.stopped, std::move(video)); if (!AddTransportOffer(media_description_options.mid, media_description_options.transport_options, current_description, desc, ice_credentials)) { @@ -2249,7 +2249,7 @@ bool MediaSessionDescriptionFactory::AddSctpDataContentForOffer( } desc->AddContent(media_description_options.mid, MediaProtocolType::kSctp, - data.release()); + std::move(data)); if (!AddTransportOffer(media_description_options.mid, media_description_options.transport_options, current_description, desc, ice_credentials)) { @@ -2288,7 +2288,7 @@ bool MediaSessionDescriptionFactory::AddRtpDataContentForOffer( data->set_bandwidth(kDataMaxBandwidth); SetMediaProtocol(secure_transport, data.get()); desc->AddContent(media_description_options.mid, MediaProtocolType::kRtp, - media_description_options.stopped, data.release()); + media_description_options.stopped, std::move(data)); if (!AddTransportOffer(media_description_options.mid, media_description_options.transport_options, current_description, desc, ice_credentials)) { @@ -2443,7 +2443,7 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer( } answer->AddContent(media_description_options.mid, offer_content->type, - rejected, audio_answer.release()); + rejected, std::move(audio_answer)); return true; } @@ -2544,7 +2544,7 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( << "' being rejected in answer."; } answer->AddContent(media_description_options.mid, offer_content->type, - rejected, video_answer.release()); + rejected, std::move(video_answer)); return true; } @@ -2646,7 +2646,7 @@ bool MediaSessionDescriptionFactory::AddDataContentForAnswer( RTC_LOG(LS_INFO) << "Data is not supported in the answer."; } answer->AddContent(media_description_options.mid, offer_content->type, - rejected, data_answer.release()); + rejected, std::move(data_answer)); return true; } diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index b14e14c936..76a33d0745 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include #include "absl/algorithm/container.h" @@ -3268,14 +3269,22 @@ TEST(MediaSessionDescription, CopySessionDescription) { SessionDescription source; cricket::ContentGroup group(cricket::CN_AUDIO); source.AddGroup(group); - AudioContentDescription* acd(new AudioContentDescription()); + std::unique_ptr acd = + absl::make_unique(); acd->set_codecs(MAKE_VECTOR(kAudioCodecs1)); acd->AddLegacyStream(1); - source.AddContent(cricket::CN_AUDIO, MediaProtocolType::kRtp, acd); - VideoContentDescription* vcd(new VideoContentDescription()); + std::unique_ptr acd_passed = + absl::WrapUnique(acd->Copy()); + source.AddContent(cricket::CN_AUDIO, MediaProtocolType::kRtp, + std::move(acd_passed)); + std::unique_ptr vcd = + absl::make_unique(); vcd->set_codecs(MAKE_VECTOR(kVideoCodecs1)); vcd->AddLegacyStream(2); - source.AddContent(cricket::CN_VIDEO, MediaProtocolType::kRtp, vcd); + std::unique_ptr vcd_passed = + absl::WrapUnique(vcd->Copy()); + source.AddContent(cricket::CN_VIDEO, MediaProtocolType::kRtp, + std::move(vcd_passed)); std::unique_ptr copy = source.Clone(); ASSERT_TRUE(copy.get() != NULL); @@ -4184,7 +4193,7 @@ TEST_P(MediaProtocolTest, TestAudioVideoAcceptance) { std::unique_ptr offer = f1_.CreateOffer(opts, nullptr); ASSERT_TRUE(offer.get() != nullptr); // Set the protocol for all the contents. - for (auto content : offer.get()->contents()) { + for (auto& content : offer.get()->contents()) { content.media_description()->set_protocol(GetParam()); } std::unique_ptr answer = diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 0b7c9c8c63..82eb425d4d 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -652,11 +652,11 @@ void ReportSimulcastApiVersion(const char* name, bool has_legacy = false; bool has_spec_compliant = false; for (const ContentInfo& content : session.contents()) { - if (!content.description) { + if (!content.media_description()) { continue; } - has_spec_compliant |= content.description->HasSimulcast(); - for (const StreamParams& sp : content.description->streams()) { + has_spec_compliant |= content.media_description()->HasSimulcast(); + for (const StreamParams& sp : content.media_description()->streams()) { has_legacy |= sp.has_ssrc_group(cricket::kSimSsrcGroupSemantics); } } @@ -2435,12 +2435,13 @@ static absl::string_view GetDefaultMidForPlanB(cricket::MediaType media_type) { void PeerConnection::FillInMissingRemoteMids( cricket::SessionDescription* new_remote_description) { RTC_DCHECK(new_remote_description); + const cricket::ContentInfos no_infos; const cricket::ContentInfos& local_contents = (local_description() ? local_description()->description()->contents() - : cricket::ContentInfos()); + : no_infos); const cricket::ContentInfos& remote_contents = (remote_description() ? remote_description()->description()->contents() - : cricket::ContentInfos()); + : no_infos); for (size_t i = 0; i < new_remote_description->contents().size(); ++i) { cricket::ContentInfo& content = new_remote_description->contents()[i]; if (!content.name.empty()) { @@ -4469,12 +4470,13 @@ void PeerConnection::GetOptionsForUnifiedPlanOffer( // Rules for generating an offer are dictated by JSEP sections 5.2.1 (Initial // Offers) and 5.2.2 (Subsequent Offers). RTC_DCHECK_EQ(session_options->media_description_options.size(), 0); + const ContentInfos no_infos; const ContentInfos& local_contents = (local_description() ? local_description()->description()->contents() - : ContentInfos()); + : no_infos); const ContentInfos& remote_contents = (remote_description() ? remote_description()->description()->contents() - : ContentInfos()); + : no_infos); // The mline indices that can be recycled. New transceivers should reuse these // slots first. std::queue recycleable_mline_indices; @@ -6725,7 +6727,7 @@ RTCError PeerConnection::ValidateSessionDescription( // same type. With Unified Plan, there can only be at most one track per // media section. for (const ContentInfo& content : sdesc->description()->contents()) { - const MediaContentDescription& desc = *content.description; + const MediaContentDescription& desc = *content.media_description(); if ((desc.type() == cricket::MEDIA_TYPE_AUDIO || desc.type() == cricket::MEDIA_TYPE_VIDEO) && desc.streams().size() > 1u) { diff --git a/pc/peer_connection_bundle_unittest.cc b/pc/peer_connection_bundle_unittest.cc index 57d059c06e..7780ac6442 100644 --- a/pc/peer_connection_bundle_unittest.cc +++ b/pc/peer_connection_bundle_unittest.cc @@ -687,11 +687,13 @@ TEST_P(PeerConnectionBundleTest, ASSERT_GE(offer->description()->contents().size(), 2U); offer->description() ->contents()[0] - .description->mutable_streams()[0] + .media_description() + ->mutable_streams()[0] .ssrcs[0] = 1111222; offer->description() ->contents()[1] - .description->mutable_streams()[0] + .media_description() + ->mutable_streams()[0] .ssrcs[0] = 1111222; EXPECT_TRUE( caller->SetLocalDescription(CloneSessionDescription(offer.get()))); diff --git a/pc/peer_connection_media_unittest.cc b/pc/peer_connection_media_unittest.cc index e9e3a00a13..1ff9f07198 100644 --- a/pc/peer_connection_media_unittest.cc +++ b/pc/peer_connection_media_unittest.cc @@ -845,7 +845,7 @@ void ChangeMediaTypeAudioToVideo(cricket::SessionDescription* desc) { desc->RemoveContentByName(audio_mid); auto* video_content = cricket::GetFirstVideoContent(desc); desc->AddContent(audio_mid, video_content->type, - video_content->media_description()->Copy()); + video_content->media_description()->Clone()); } constexpr char kMLinesOutOfOrder[] = diff --git a/pc/session_description.cc b/pc/session_description.cc index df31163a03..476bf1af6a 100644 --- a/pc/session_description.cc +++ b/pc/session_description.cc @@ -91,21 +91,12 @@ SessionDescription::SessionDescription() = default; SessionDescription::SessionDescription(const SessionDescription&) = default; SessionDescription::~SessionDescription() { - for (ContentInfos::iterator content = contents_.begin(); - content != contents_.end(); ++content) { - delete content->description; - } } std::unique_ptr SessionDescription::Clone() const { - // Copy the non-special portions using the private copy constructor. - auto copy = absl::WrapUnique(new SessionDescription(*this)); - // Copy all ContentDescriptions. - for (ContentInfos::iterator content = copy->contents_.begin(); - content != copy->contents().end(); ++content) { - content->description = content->description->Copy(); - } - return copy; + // Copy using the private copy constructor. + // This will clone the descriptions using ContentInfo's copy constructor. + return absl::WrapUnique(new SessionDescription(*this)); } SessionDescription* SessionDescription::Copy() const { @@ -150,71 +141,79 @@ const ContentInfo* SessionDescription::FirstContent() const { return (contents_.empty()) ? NULL : &(*contents_.begin()); } -void SessionDescription::AddContent(const std::string& name, - MediaProtocolType type, - MediaContentDescription* description) { +void SessionDescription::AddContent( + const std::string& name, + MediaProtocolType type, + std::unique_ptr description) { ContentInfo content(type); content.name = name; - content.description = description; - AddContent(&content); + content.set_media_description(std::move(description)); + AddContent(std::move(content)); } -void SessionDescription::AddContent(const std::string& name, - MediaProtocolType type, - bool rejected, - MediaContentDescription* description) { +void SessionDescription::AddContent( + const std::string& name, + MediaProtocolType type, + bool rejected, + std::unique_ptr description) { ContentInfo content(type); content.name = name; content.rejected = rejected; - content.description = description; - AddContent(&content); + content.set_media_description(std::move(description)); + AddContent(std::move(content)); } -void SessionDescription::AddContent(const std::string& name, - MediaProtocolType type, - bool rejected, - bool bundle_only, - MediaContentDescription* description) { +void SessionDescription::AddContent( + const std::string& name, + MediaProtocolType type, + bool rejected, + bool bundle_only, + std::unique_ptr description) { ContentInfo content(type); content.name = name; content.rejected = rejected; content.bundle_only = bundle_only; - content.description = description; - AddContent(&content); + content.set_media_description(std::move(description)); + AddContent(std::move(content)); } -void SessionDescription::AddContent(ContentInfo* content) { +void SessionDescription::AddContent(ContentInfo&& content) { // Unwrap the as_data shim layer before using. - auto* description = content->media_description(); + auto* description = content.media_description(); bool should_delete = false; if (description->as_rtp_data()) { if (description->as_rtp_data() != description) { - content->set_media_description( - description->deprecated_as_data()->Unshim(&should_delete)); + auto* media_description = + description->deprecated_as_data()->Unshim(&should_delete); + // If should_delete was false, the media description passed to + // AddContent is referenced from elsewhere, and double deletion + // is going to result. Don't allow this. + RTC_CHECK(should_delete) + << "Non-owned shim description passed to AddContent"; + // Setting the media description will delete the old description. + content.set_media_description(absl::WrapUnique(media_description)); } - } - if (description->as_sctp()) { + } else if (description->as_sctp()) { if (description->as_sctp() != description) { - content->set_media_description( - description->deprecated_as_data()->Unshim(&should_delete)); + auto* media_description = + description->deprecated_as_data()->Unshim(&should_delete); + RTC_CHECK(should_delete) + << "Non-owned shim description passed to AddContent"; + content.set_media_description(absl::WrapUnique(media_description)); } } - if (should_delete) { - delete description; - } if (extmap_allow_mixed()) { // Mixed support on session level overrides setting on media level. - content->description->set_extmap_allow_mixed_enum( + content.media_description()->set_extmap_allow_mixed_enum( MediaContentDescription::kSession); } - contents_.push_back(std::move(*content)); + contents_.push_back(std::move(content)); } bool SessionDescription::RemoveContentByName(const std::string& name) { for (ContentInfos::iterator content = contents_.begin(); content != contents_.end(); ++content) { if (content->name == name) { - delete content->description; contents_.erase(content); return true; } @@ -701,4 +700,55 @@ void DataContentDescription::AddCodecs(const std::vector& codecs) { } } +ContentInfo::~ContentInfo() { + if (description_ && description_.get() != description) { + // If description_ is null, we assume that a move operator + // has been applied. + RTC_LOG(LS_ERROR) << "ContentInfo::description has been updated by " + << "assignment. This usage is deprecated."; + description_.reset(description); // ensure that it is destroyed. + } +} + +// Copy operator. +ContentInfo::ContentInfo(const ContentInfo& o) + : name(o.name), + type(o.type), + rejected(o.rejected), + bundle_only(o.bundle_only), + description_(o.description_->Clone()), + description(description_.get()) {} + +ContentInfo& ContentInfo::operator=(const ContentInfo& o) { + name = o.name; + type = o.type; + rejected = o.rejected; + bundle_only = o.bundle_only; + description_ = o.description_->Clone(); + description = description_.get(); + return *this; +} + +const MediaContentDescription* ContentInfo::media_description() const { + if (description_.get() != description) { + // Someone's updated |description|, or used a move operator + // on the record. + RTC_LOG(LS_ERROR) << "ContentInfo::description has been updated by " + << "assignment. This usage is deprecated."; + const_cast(this)->description_.reset(description); + } + return description_.get(); +} + +MediaContentDescription* ContentInfo::media_description() { + if (description_.get() != description) { + // Someone's updated |description|, or used a move operator + // on the record. + RTC_LOG(LS_ERROR) << "ContentInfo::description has been updated by " + << "assignment. This usage is deprecated."; + description_.reset(description); + } + return description_.get(); +} + } // namespace cricket diff --git a/pc/session_description.h b/pc/session_description.h index eb9401f0ba..0ff90a5340 100644 --- a/pc/session_description.h +++ b/pc/session_description.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include "absl/memory/memory.h" @@ -96,6 +97,9 @@ class MediaContentDescription { virtual bool has_codecs() const = 0; virtual MediaContentDescription* Copy() const = 0; + virtual std::unique_ptr Clone() const { + return absl::WrapUnique(Copy()); + } // |protocol| is the expected media transport protocol, such as RTP/AVPF, // RTP/SAVPF or SCTP/DTLS. @@ -526,22 +530,29 @@ constexpr MediaProtocolType NS_JINGLE_DRAFT_SCTP = MediaProtocolType::kSctp; // Represents a session description section. Most information about the section // is stored in the description, which is a subclass of MediaContentDescription. -struct ContentInfo { - friend class SessionDescription; - +// Owns the description. +class ContentInfo { + public: explicit ContentInfo(MediaProtocolType type) : type(type) {} + ~ContentInfo(); + // Copy + ContentInfo(const ContentInfo& o); + ContentInfo& operator=(const ContentInfo& o); + 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; } // Alias for |description|. - MediaContentDescription* media_description() { return description; } - const MediaContentDescription* media_description() const { - return description; - } - void set_media_description(MediaContentDescription* desc) { - description = desc; + MediaContentDescription* media_description(); + const MediaContentDescription* media_description() const; + + void set_media_description(std::unique_ptr desc) { + description_ = std::move(desc); + // For backwards compatibility only. + description = description_.get(); } // TODO(bugs.webrtc.org/8620): Rename this to mid. @@ -549,8 +560,13 @@ struct ContentInfo { MediaProtocolType type; bool rejected = false; bool bundle_only = false; - // TODO(bugs.webrtc.org/8620): Switch to the getter and setter, and make this - // private. + + private: + friend class SessionDescription; + std::unique_ptr description_; + + public: + // Kept for backwards compatibility only. MediaContentDescription* description = nullptr; }; @@ -629,17 +645,40 @@ class SessionDescription { // Adds a content to this description. Takes ownership of ContentDescription*. void AddContent(const std::string& name, MediaProtocolType type, - MediaContentDescription* description); + std::unique_ptr description); void AddContent(const std::string& name, MediaProtocolType type, bool rejected, - MediaContentDescription* description); + std::unique_ptr description); void AddContent(const std::string& name, MediaProtocolType type, bool rejected, bool bundle_only, - MediaContentDescription* description); - void AddContent(ContentInfo* content); + std::unique_ptr description); + void AddContent(ContentInfo&& content); + RTC_DEPRECATED void AddContent(const std::string& name, + MediaProtocolType type, + MediaContentDescription* description) { + AddContent(name, type, absl::WrapUnique(description)); + } + RTC_DEPRECATED void AddContent(const std::string& name, + MediaProtocolType type, + bool rejected, + MediaContentDescription* description) { + AddContent(name, type, rejected, absl::WrapUnique(description)); + } + RTC_DEPRECATED void AddContent(const std::string& name, + MediaProtocolType type, + bool rejected, + bool bundle_only, + MediaContentDescription* description) { + AddContent(name, type, rejected, bundle_only, + absl::WrapUnique(description)); + } + + RTC_DEPRECATED void AddContent(ContentInfo* content) { + AddContent(std::move(*content)); + } bool RemoveContentByName(const std::string& name); diff --git a/pc/session_description_unittest.cc b/pc/session_description_unittest.cc index d9426c8348..eda83a5a71 100644 --- a/pc/session_description_unittest.cc +++ b/pc/session_description_unittest.cc @@ -65,8 +65,10 @@ TEST(SessionDescriptionTest, SetExtmapAllowMixed) { TEST(SessionDescriptionTest, SetExtmapAllowMixedPropagatesToMediaLevel) { SessionDescription session_desc; - MediaContentDescription* video_desc = new VideoContentDescription(); - session_desc.AddContent("video", MediaProtocolType::kRtp, video_desc); + session_desc.AddContent("video", MediaProtocolType::kRtp, + absl::make_unique()); + MediaContentDescription* video_desc = + session_desc.GetContentDescriptionByName("video"); // Setting true on session level propagates to media level. session_desc.set_extmap_allow_mixed(true); @@ -104,29 +106,38 @@ TEST(SessionDescriptionTest, SetExtmapAllowMixedPropagatesToMediaLevel) { TEST(SessionDescriptionTest, AddContentTransfersExtmapAllowMixedSetting) { SessionDescription session_desc; session_desc.set_extmap_allow_mixed(false); - MediaContentDescription* audio_desc = new AudioContentDescription(); + std::unique_ptr audio_desc = + absl::make_unique(); audio_desc->set_extmap_allow_mixed_enum(MediaContentDescription::kMedia); // If session setting is false, media level setting is preserved when new // content is added. - session_desc.AddContent("audio", MediaProtocolType::kRtp, audio_desc); + session_desc.AddContent("audio", MediaProtocolType::kRtp, + std::move(audio_desc)); EXPECT_EQ(MediaContentDescription::kMedia, - audio_desc->extmap_allow_mixed_enum()); + session_desc.GetContentDescriptionByName("audio") + ->extmap_allow_mixed_enum()); // If session setting is true, it's transferred to media level when new // content is added. session_desc.set_extmap_allow_mixed(true); - MediaContentDescription* video_desc = new VideoContentDescription(); - session_desc.AddContent("video", MediaProtocolType::kRtp, video_desc); + std::unique_ptr video_desc = + absl::make_unique(); + session_desc.AddContent("video", MediaProtocolType::kRtp, + std::move(video_desc)); EXPECT_EQ(MediaContentDescription::kSession, - video_desc->extmap_allow_mixed_enum()); + session_desc.GetContentDescriptionByName("video") + ->extmap_allow_mixed_enum()); // Session level setting overrides media level when new content is added. - MediaContentDescription* data_desc = new RtpDataContentDescription; + std::unique_ptr data_desc = + absl::make_unique(); data_desc->set_extmap_allow_mixed_enum(MediaContentDescription::kMedia); - session_desc.AddContent("data", MediaProtocolType::kRtp, data_desc); + session_desc.AddContent("data", MediaProtocolType::kRtp, + std::move(data_desc)); EXPECT_EQ(MediaContentDescription::kSession, - data_desc->extmap_allow_mixed_enum()); + session_desc.GetContentDescriptionByName("data") + ->extmap_allow_mixed_enum()); } // The tests for DataContentDescription will be deleted soon. @@ -165,7 +176,7 @@ TEST(SessionDescriptionTest, DataContentDesriptionInSessionIsUnwrapped) { // Create a DTLS object behind the shim. description->set_protocol(kMediaProtocolUdpDtlsSctp); SessionDescription session; - session.AddContent("name", MediaProtocolType::kSctp, description.release()); + session.AddContent("name", MediaProtocolType::kSctp, std::move(description)); ContentInfo* content = &(session.contents()[0]); ASSERT_TRUE(content); ASSERT_TRUE(content->media_description()->type() == MEDIA_TYPE_DATA); diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index 96be3b223f..af8a5277de 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -2798,7 +2798,7 @@ bool ParseMediaDescription( desc->AddContent(content_name, cricket::IsDtlsSctp(protocol) ? MediaProtocolType::kSctp : MediaProtocolType::kRtp, - content_rejected, bundle_only, content.release()); + content_rejected, bundle_only, std::move(content)); // Create TransportInfo with the media level "ice-pwd" and "ice-ufrag". desc->AddTransportInfo(TransportInfo(content_name, transport)); } diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc index 9a99b6dcc7..4b7a6d9219 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -1018,7 +1018,8 @@ class WebRtcSdpTest : public ::testing::Test { audio_desc_->AddStream(audio_stream); rtc::SocketAddress audio_addr("74.125.127.126", 2345); audio_desc_->set_connection_address(audio_addr); - desc_.AddContent(kAudioContentName, MediaProtocolType::kRtp, audio_desc_); + desc_.AddContent(kAudioContentName, MediaProtocolType::kRtp, + absl::WrapUnique(audio_desc_)); // VideoContentDescription video_desc_ = CreateVideoContentDescription(); @@ -1033,7 +1034,8 @@ class WebRtcSdpTest : public ::testing::Test { video_desc_->AddStream(video_stream); rtc::SocketAddress video_addr("74.125.224.39", 3457); video_desc_->set_connection_address(video_addr); - desc_.AddContent(kVideoContentName, MediaProtocolType::kRtp, video_desc_); + desc_.AddContent(kVideoContentName, MediaProtocolType::kRtp, + absl::WrapUnique(video_desc_)); // TransportInfo desc_.AddTransportInfo(TransportInfo( @@ -1214,8 +1216,10 @@ class WebRtcSdpTest : public ::testing::Test { desc_.RemoveContentByName(kAudioContentName); desc_.RemoveContentByName(kVideoContentName); - desc_.AddContent(kAudioContentName, MediaProtocolType::kRtp, audio_desc_); - desc_.AddContent(kVideoContentName, MediaProtocolType::kRtp, video_desc_); + desc_.AddContent(kAudioContentName, MediaProtocolType::kRtp, + absl::WrapUnique(audio_desc_)); + desc_.AddContent(kVideoContentName, MediaProtocolType::kRtp, + absl::WrapUnique(video_desc_)); ASSERT_TRUE(jdesc_.Initialize(desc_.Clone(), jdesc_.session_id(), jdesc_.session_version())); @@ -1234,7 +1238,8 @@ class WebRtcSdpTest : public ::testing::Test { audio_track_2.ssrcs.push_back(kAudioTrack2Ssrc); } audio_desc_2->AddStream(audio_track_2); - desc_.AddContent(kAudioContentName2, MediaProtocolType::kRtp, audio_desc_2); + desc_.AddContent(kAudioContentName2, MediaProtocolType::kRtp, + absl::WrapUnique(audio_desc_2)); desc_.AddTransportInfo(TransportInfo( kAudioContentName2, TransportDescription(kUfragVoice2, kPwdVoice2))); // Video track 2, in stream 2. @@ -1247,7 +1252,8 @@ class WebRtcSdpTest : public ::testing::Test { video_track_2.ssrcs.push_back(kVideoTrack2Ssrc); } video_desc_2->AddStream(video_track_2); - desc_.AddContent(kVideoContentName2, MediaProtocolType::kRtp, video_desc_2); + desc_.AddContent(kVideoContentName2, MediaProtocolType::kRtp, + absl::WrapUnique(video_desc_2)); desc_.AddTransportInfo(TransportInfo( kVideoContentName2, TransportDescription(kUfragVideo2, kPwdVideo2))); @@ -1261,7 +1267,8 @@ class WebRtcSdpTest : public ::testing::Test { video_track_3.ssrcs.push_back(kVideoTrack3Ssrc); } video_desc_3->AddStream(video_track_3); - desc_.AddContent(kVideoContentName3, MediaProtocolType::kRtp, video_desc_3); + desc_.AddContent(kVideoContentName3, MediaProtocolType::kRtp, + absl::WrapUnique(video_desc_3)); desc_.AddTransportInfo(TransportInfo( kVideoContentName3, TransportDescription(kUfragVideo3, kPwdVideo3))); desc_.set_msid_signaling(cricket::kMsidSignalingMediaSection); @@ -1305,7 +1312,8 @@ class WebRtcSdpTest : public ::testing::Test { audio_track_2.set_stream_ids({kStreamId1, kStreamId2}); audio_track_2.ssrcs.push_back(kAudioTrack2Ssrc); audio_desc_2->AddStream(audio_track_2); - desc_.AddContent(kAudioContentName2, MediaProtocolType::kRtp, audio_desc_2); + desc_.AddContent(kAudioContentName2, MediaProtocolType::kRtp, + absl::WrapUnique(audio_desc_2)); desc_.AddTransportInfo(TransportInfo( kAudioContentName2, TransportDescription(kUfragVoice2, kPwdVoice2))); @@ -1317,7 +1325,8 @@ class WebRtcSdpTest : public ::testing::Test { audio_track_3.set_stream_ids({}); audio_track_3.ssrcs.push_back(kAudioTrack3Ssrc); audio_desc_3->AddStream(audio_track_3); - desc_.AddContent(kAudioContentName3, MediaProtocolType::kRtp, audio_desc_3); + desc_.AddContent(kAudioContentName3, MediaProtocolType::kRtp, + absl::WrapUnique(audio_desc_3)); desc_.AddTransportInfo(TransportInfo( kAudioContentName3, TransportDescription(kUfragVoice3, kPwdVoice3))); desc_.set_msid_signaling(msid_signaling); @@ -1339,7 +1348,8 @@ class WebRtcSdpTest : public ::testing::Test { audio_track.id = kAudioTrackId1; audio_track.set_stream_ids({kStreamId1}); audio_desc->AddStream(audio_track); - desc_.AddContent(kAudioContentName, MediaProtocolType::kRtp, audio_desc); + desc_.AddContent(kAudioContentName, MediaProtocolType::kRtp, + absl::WrapUnique(audio_desc)); // Enable signaling a=msid lines. desc_.set_msid_signaling(cricket::kMsidSignalingMediaSection); @@ -1363,7 +1373,7 @@ class WebRtcSdpTest : public ::testing::Test { template void CompareMediaContentDescription(const MCD* cd1, const MCD* cd2) { // type - EXPECT_EQ(cd1->type(), cd1->type()); + EXPECT_EQ(cd1->type(), cd2->type()); // content direction EXPECT_EQ(cd1->direction(), cd2->direction()); @@ -1685,8 +1695,10 @@ class WebRtcSdpTest : public ::testing::Test { RtpExtension(kExtmapUri, kExtmapId, encrypted)); desc_.RemoveContentByName(kAudioContentName); desc_.RemoveContentByName(kVideoContentName); - desc_.AddContent(kAudioContentName, MediaProtocolType::kRtp, audio_desc_); - desc_.AddContent(kVideoContentName, MediaProtocolType::kRtp, video_desc_); + desc_.AddContent(kAudioContentName, MediaProtocolType::kRtp, + absl::WrapUnique(audio_desc_)); + desc_.AddContent(kVideoContentName, MediaProtocolType::kRtp, + absl::WrapUnique(video_desc_)); } void RemoveCryptos() { @@ -1697,7 +1709,8 @@ class WebRtcSdpTest : public ::testing::Test { // Removes everything in StreamParams from the session description that is // used for a=ssrc lines. void RemoveSsrcSignalingFromStreamParams() { - for (cricket::ContentInfo content_info : jdesc_.description()->contents()) { + for (cricket::ContentInfo& content_info : + jdesc_.description()->contents()) { // With Unified Plan there should be one StreamParams per m= section. StreamParams& stream = content_info.media_description()->mutable_streams()[0]; @@ -1761,9 +1774,9 @@ class WebRtcSdpTest : public ::testing::Test { desc_.RemoveContentByName(kAudioContentName); desc_.RemoveContentByName(kVideoContentName); desc_.AddContent(kAudioContentName, MediaProtocolType::kRtp, audio_rejected, - audio_desc_); + absl::WrapUnique(audio_desc_)); desc_.AddContent(kVideoContentName, MediaProtocolType::kRtp, video_rejected, - video_desc_); + absl::WrapUnique(video_desc_)); SetIceUfragPwd(kAudioContentName, audio_rejected ? "" : kUfragVoice, audio_rejected ? "" : kPwdVoice); SetIceUfragPwd(kVideoContentName, video_rejected ? "" : kUfragVideo, @@ -1787,7 +1800,7 @@ class WebRtcSdpTest : public ::testing::Test { sctp_desc_->set_protocol(cricket::kMediaProtocolUdpDtlsSctp); sctp_desc_->set_port(kDefaultSctpPort); desc_.AddContent(kDataContentName, MediaProtocolType::kSctp, - data.release()); + std::move(data)); desc_.AddTransportInfo(TransportInfo( kDataContentName, TransportDescription(kUfragData, kPwdData))); } @@ -1808,7 +1821,8 @@ class WebRtcSdpTest : public ::testing::Test { CryptoParams(1, "AES_CM_128_HMAC_SHA1_80", "inline:FvLcvU2P3ZWmQxgPAgcDu7Zl9vftYElFOjEzhWs5", "")); data_desc_->set_protocol(cricket::kMediaProtocolSavpf); - desc_.AddContent(kDataContentName, MediaProtocolType::kRtp, data.release()); + desc_.AddContent(kDataContentName, MediaProtocolType::kRtp, + std::move(data)); desc_.AddTransportInfo(TransportInfo( kDataContentName, TransportDescription(kUfragData, kPwdData))); } @@ -1841,9 +1855,9 @@ class WebRtcSdpTest : public ::testing::Test { desc_.RemoveContentByName(kAudioContentName); desc_.RemoveContentByName(kVideoContentName); desc_.AddContent(kAudioContentName, MediaProtocolType::kRtp, audio_rejected, - audio_desc_); + absl::WrapUnique(audio_desc_)); desc_.AddContent(kVideoContentName, MediaProtocolType::kRtp, video_rejected, - video_desc_); + absl::WrapUnique(video_desc_)); SetIceUfragPwd(kAudioContentName, audio_rejected ? "" : kUfragVoice, audio_rejected ? "" : kPwdVoice); SetIceUfragPwd(kVideoContentName, video_rejected ? "" : kUfragVideo,