Let SessionDescription take ownership of MediaDescription
This documents in the API what is already true in the
implementation - that SessionDescription will eventually
delete MediaDescription objects passed to it.
The old API is preserved for backwards compatibility, but
marked as RTC_DEPRECATED.
Bug: webrtc:10701
Change-Id: I9a822b20cf3e58c5945fa51dbf6082960a332de8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/139880
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28144}
diff --git a/pc/jsep_session_description_unittest.cc b/pc/jsep_session_description_unittest.cc
index aca9dd8..d2fc6e5 100644
--- a/pc/jsep_session_description_unittest.cc
+++ b/pc/jsep_session_description_unittest.cc
@@ -63,10 +63,12 @@
auto video = absl::make_unique<cricket::VideoContentDescription>();
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 4f14e00..6adecb3 100644
--- a/pc/jsep_transport_controller.cc
+++ b/pc/jsep_transport_controller.cc
@@ -876,13 +876,12 @@
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<int>& encrypted_extension_ids,
int rtp_abs_sendtime_extn_id) {
const cricket::MediaContentDescription* content_desc =
- static_cast<const cricket::MediaContentDescription*>(
- 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 @@
std::vector<int> JsepTransportController::GetEncryptedHeaderExtensionIds(
const cricket::ContentInfo& content_info) {
const cricket::MediaContentDescription* content_desc =
- static_cast<const cricket::MediaContentDescription*>(
- content_info.description);
+ content_info.media_description();
if (!config_.crypto_options.srtp.enable_encrypted_rtp_header_extensions) {
return std::vector<int>();
@@ -964,8 +962,7 @@
}
const cricket::MediaContentDescription* content_desc =
- static_cast<const cricket::MediaContentDescription*>(
- content_info.description);
+ content_info.media_description();
const webrtc::RtpExtension* send_time_extension =
webrtc::RtpExtension::FindHeaderExtensionByUri(
@@ -1145,8 +1142,7 @@
}
const cricket::MediaContentDescription* content_desc =
- static_cast<const cricket::MediaContentDescription*>(
- 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 ffc9515..aaaab48 100644
--- a/pc/jsep_transport_controller.h
+++ b/pc/jsep_transport_controller.h
@@ -261,8 +261,8 @@
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<int>& 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 d796b23..3bde0e7 100644
--- a/pc/jsep_transport_controller_unittest.cc
+++ b/pc/jsep_transport_controller_unittest.cc
@@ -146,7 +146,7 @@
// 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 @@
// 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 @@
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 1b3d015..7cc4b95 100644
--- a/pc/media_session.cc
+++ b/pc/media_session.cc
@@ -2122,7 +2122,7 @@
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 @@
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 @@
}
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 @@
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 @@
}
answer->AddContent(media_description_options.mid, offer_content->type,
- rejected, audio_answer.release());
+ rejected, std::move(audio_answer));
return true;
}
@@ -2544,7 +2544,7 @@
<< "' 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 @@
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 b14e14c..76a33d0 100644
--- a/pc/media_session_unittest.cc
+++ b/pc/media_session_unittest.cc
@@ -11,6 +11,7 @@
#include <algorithm>
#include <memory>
#include <string>
+#include <utility>
#include <vector>
#include "absl/algorithm/container.h"
@@ -3268,14 +3269,22 @@
SessionDescription source;
cricket::ContentGroup group(cricket::CN_AUDIO);
source.AddGroup(group);
- AudioContentDescription* acd(new AudioContentDescription());
+ std::unique_ptr<AudioContentDescription> acd =
+ absl::make_unique<AudioContentDescription>();
acd->set_codecs(MAKE_VECTOR(kAudioCodecs1));
acd->AddLegacyStream(1);
- source.AddContent(cricket::CN_AUDIO, MediaProtocolType::kRtp, acd);
- VideoContentDescription* vcd(new VideoContentDescription());
+ std::unique_ptr<AudioContentDescription> acd_passed =
+ absl::WrapUnique(acd->Copy());
+ source.AddContent(cricket::CN_AUDIO, MediaProtocolType::kRtp,
+ std::move(acd_passed));
+ std::unique_ptr<VideoContentDescription> vcd =
+ absl::make_unique<VideoContentDescription>();
vcd->set_codecs(MAKE_VECTOR(kVideoCodecs1));
vcd->AddLegacyStream(2);
- source.AddContent(cricket::CN_VIDEO, MediaProtocolType::kRtp, vcd);
+ std::unique_ptr<VideoContentDescription> vcd_passed =
+ absl::WrapUnique(vcd->Copy());
+ source.AddContent(cricket::CN_VIDEO, MediaProtocolType::kRtp,
+ std::move(vcd_passed));
std::unique_ptr<SessionDescription> copy = source.Clone();
ASSERT_TRUE(copy.get() != NULL);
@@ -4184,7 +4193,7 @@
std::unique_ptr<SessionDescription> 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<SessionDescription> answer =
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index 0b7c9c8..82eb425 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -652,11 +652,11 @@
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 @@
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 @@
// 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<size_t> recycleable_mline_indices;
@@ -6725,7 +6727,7 @@
// 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 57d059c..7780ac6 100644
--- a/pc/peer_connection_bundle_unittest.cc
+++ b/pc/peer_connection_bundle_unittest.cc
@@ -687,11 +687,13 @@
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 e9e3a00..1ff9f07 100644
--- a/pc/peer_connection_media_unittest.cc
+++ b/pc/peer_connection_media_unittest.cc
@@ -845,7 +845,7 @@
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 df31163..476bf1a 100644
--- a/pc/session_description.cc
+++ b/pc/session_description.cc
@@ -91,21 +91,12 @@
SessionDescription::SessionDescription(const SessionDescription&) = default;
SessionDescription::~SessionDescription() {
- for (ContentInfos::iterator content = contents_.begin();
- content != contents_.end(); ++content) {
- delete content->description;
- }
}
std::unique_ptr<SessionDescription> 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 @@
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<MediaContentDescription> 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<MediaContentDescription> 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<MediaContentDescription> 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 @@
}
}
+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<ContentInfo*>(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 eb9401f..0ff90a5 100644
--- a/pc/session_description.h
+++ b/pc/session_description.h
@@ -16,6 +16,7 @@
#include <iosfwd>
#include <memory>
#include <string>
+#include <utility>
#include <vector>
#include "absl/memory/memory.h"
@@ -96,6 +97,9 @@
virtual bool has_codecs() const = 0;
virtual MediaContentDescription* Copy() const = 0;
+ virtual std::unique_ptr<MediaContentDescription> 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 @@
// 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<MediaContentDescription> 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 @@
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<MediaContentDescription> description_;
+
+ public:
+ // Kept for backwards compatibility only.
MediaContentDescription* description = nullptr;
};
@@ -629,17 +645,40 @@
// Adds a content to this description. Takes ownership of ContentDescription*.
void AddContent(const std::string& name,
MediaProtocolType type,
- MediaContentDescription* description);
+ std::unique_ptr<MediaContentDescription> description);
void AddContent(const std::string& name,
MediaProtocolType type,
bool rejected,
- MediaContentDescription* description);
+ std::unique_ptr<MediaContentDescription> description);
void AddContent(const std::string& name,
MediaProtocolType type,
bool rejected,
bool bundle_only,
- MediaContentDescription* description);
- void AddContent(ContentInfo* content);
+ std::unique_ptr<MediaContentDescription> 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 d9426c8..eda83a5 100644
--- a/pc/session_description_unittest.cc
+++ b/pc/session_description_unittest.cc
@@ -65,8 +65,10 @@
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<VideoContentDescription>());
+ 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, AddContentTransfersExtmapAllowMixedSetting) {
SessionDescription session_desc;
session_desc.set_extmap_allow_mixed(false);
- MediaContentDescription* audio_desc = new AudioContentDescription();
+ std::unique_ptr<MediaContentDescription> audio_desc =
+ absl::make_unique<AudioContentDescription>();
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<MediaContentDescription> video_desc =
+ absl::make_unique<VideoContentDescription>();
+ 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<MediaContentDescription> data_desc =
+ absl::make_unique<RtpDataContentDescription>();
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 @@
// 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 96be3b2..af8a527 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -2798,7 +2798,7 @@
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 9a99b6d..4b7a6d9 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -1018,7 +1018,8 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
template <class MCD>
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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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,
