admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Slight restriction of access to ContentInfo and prefer mid to name.

Browse Source 
As a first step, use .mid() instead of .name in JsepTransportController

Bug: webrtc:42233761
Change-Id: I23ab97609175f8dbfdf59ee41c4db42f21a9e9ad
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374660
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43756}
This commit is contained in:
Tommi 2025-01-17 12:31:07 +01:00 committed by WebRTC LUCI CQ
parent 88833e6d22
commit 762753d0a2
4 changed files with 69 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
pc/congestion_control_integrationtest.cc
View File

@ -53,6 +53,7 @@ TEST_F(PeerConnectionCongestionControlTest, ReceiveOfferSetsCcfbFlag) {
caller()->CreateAndSetAndSignalOffer(); caller()->CreateAndSetAndSignalOffer();
ASSERT_THAT(WaitUntil([&] { return SignalingStateStable(); }, IsTrue()), ASSERT_THAT(WaitUntil([&] { return SignalingStateStable(); }, IsTrue()),
IsRtcOk()); IsRtcOk());
{
// Check that the callee parsed it. // Check that the callee parsed it.
auto parsed_contents = auto parsed_contents =
callee()->pc()->remote_description()->description()->contents(); callee()->pc()->remote_description()->description()->contents();
@ -60,13 +61,17 @@ TEST_F(PeerConnectionCongestionControlTest, ReceiveOfferSetsCcfbFlag) {
for (const auto& content : parsed_contents) { for (const auto& content : parsed_contents) {
EXPECT_TRUE(content.media_description()->rtcp_fb_ack_ccfb()); EXPECT_TRUE(content.media_description()->rtcp_fb_ack_ccfb());
} }
}
{
// Check that the caller also parsed it. // Check that the caller also parsed it.
parsed_contents = auto parsed_contents =
caller()->pc()->remote_description()->description()->contents(); caller()->pc()->remote_description()->description()->contents();
EXPECT_FALSE(parsed_contents.empty()); EXPECT_FALSE(parsed_contents.empty());
for (const auto& content : parsed_contents) { for (const auto& content : parsed_contents) {
EXPECT_TRUE(content.media_description()->rtcp_fb_ack_ccfb()); EXPECT_TRUE(content.media_description()->rtcp_fb_ack_ccfb());
} }
}
// Check that the answer does not contain transport-cc // Check that the answer does not contain transport-cc
std::string answer_str = absl::StrCat(*caller()->pc()->remote_description()); std::string answer_str = absl::StrCat(*caller()->pc()->remote_description());
EXPECT_THAT(answer_str, Not(HasSubstr("transport-cc"))); EXPECT_THAT(answer_str, Not(HasSubstr("transport-cc")));

49
pc/jsep_transport_controller.cc
View File

@ -687,7 +687,7 @@ RTCError JsepTransportController::ApplyDescription_n(
for (const cricket::ContentInfo& content_info : description->contents()) { for (const cricket::ContentInfo& content_info : description->contents()) {
// Don't create transports for rejected m-lines and bundled m-lines. // Don't create transports for rejected m-lines and bundled m-lines.
if (content_info.rejected || if (content_info.rejected ||
!bundles_.IsFirstMidInGroup(content_info.name)) { !bundles_.IsFirstMidInGroup(content_info.mid())) {
continue; continue;
} }
error = MaybeCreateJsepTransport(local, content_info, *description); error = MaybeCreateJsepTransport(local, content_info, *description);
@ -710,17 +710,17 @@ RTCError JsepTransportController::ApplyDescription_n(
} }
const cricket::ContentGroup* established_bundle_group = 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), // 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. // configure their transport to be the same as the BUNDLE-tagged transport.
if (established_bundle_group && 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)) { if (!HandleBundledContent(content_info, *established_bundle_group)) {
return RTCError(RTCErrorType::INVALID_PARAMETER, return RTCError(RTCErrorType::INVALID_PARAMETER,
"Failed to process the bundled m= section with " "Failed to process the bundled m= section with "
"mid='" + "mid='" +
content_info.name + "'."); content_info.mid() + "'.");
} }
continue; continue;
} }
@ -733,7 +733,7 @@ RTCError JsepTransportController::ApplyDescription_n(
std::vector<int> extension_ids; std::vector<int> extension_ids;
// Is BUNDLE-tagged (first in the group)? // Is BUNDLE-tagged (first in the group)?
if (established_bundle_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( auto it = merged_encrypted_extension_ids_by_bundle.find(
established_bundle_group); established_bundle_group);
RTC_DCHECK(it != merged_encrypted_extension_ids_by_bundle.end()); RTC_DCHECK(it != merged_encrypted_extension_ids_by_bundle.end());
@ -746,12 +746,12 @@ RTCError JsepTransportController::ApplyDescription_n(
GetRtpAbsSendTimeHeaderExtensionId(content_info); GetRtpAbsSendTimeHeaderExtensionId(content_info);
cricket::JsepTransport* transport = cricket::JsepTransport* transport =
GetJsepTransportForMid(content_info.name); GetJsepTransportForMid(content_info.mid());
if (!transport) { if (!transport) {
LOG_AND_RETURN_ERROR( LOG_AND_RETURN_ERROR(
RTCErrorType::INVALID_PARAMETER, RTCErrorType::INVALID_PARAMETER,
"Could not find transport for m= section with mid='" + "Could not find transport for m= section with mid='" +
content_info.name + "'"); content_info.mid() + "'");
} }
SetIceRole_n(DetermineIceRole(transport, transport_info, type, local)); SetIceRole_n(DetermineIceRole(transport, transport_info, type, local));
@ -771,7 +771,7 @@ RTCError JsepTransportController::ApplyDescription_n(
LOG_AND_RETURN_ERROR( LOG_AND_RETURN_ERROR(
RTCErrorType::INVALID_PARAMETER, RTCErrorType::INVALID_PARAMETER,
"Failed to apply the description for m= section with mid='" + "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); error = transport->RecordPayloadTypes(local, type, content_info);
if (!error.ok()) { if (!error.ok()) {
@ -981,7 +981,7 @@ RTCError JsepTransportController::ValidateContent(
!content_info.bundle_only && !content_info.bundle_only &&
!content_info.media_description()->rtcp_mux()) { !content_info.media_description()->rtcp_mux()) {
return RTCError(RTCErrorType::INVALID_PARAMETER, 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 " "' is invalid. RTCP-MUX is not "
"enabled when it is required."); "enabled when it is required.");
} }
@ -994,9 +994,9 @@ void JsepTransportController::HandleRejectedContent(
// BaseChannel/SctpTransport change the RtpTransport/DtlsTransport first, // BaseChannel/SctpTransport change the RtpTransport/DtlsTransport first,
// then destroy the cricket::JsepTransport. // then destroy the cricket::JsepTransport.
cricket::ContentGroup* bundle_group = cricket::ContentGroup* bundle_group =
bundles_.LookupGroupByMid(content_info.name); bundles_.LookupGroupByMid(content_info.mid());
if (bundle_group && !bundle_group->content_names().empty() && 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 // Rejecting a BUNDLE group's first mid means we are rejecting the entire
// group. // group.
for (const auto& content_name : bundle_group->content_names()) { for (const auto& content_name : bundle_group->content_names()) {
@ -1005,10 +1005,10 @@ void JsepTransportController::HandleRejectedContent(
// Delete the BUNDLE group. // Delete the BUNDLE group.
bundles_.DeleteGroup(bundle_group); bundles_.DeleteGroup(bundle_group);
} else { } else {
transports_.RemoveTransportForMid(content_info.name); transports_.RemoveTransportForMid(content_info.mid());
if (bundle_group) { if (bundle_group) {
// Remove the rejected content from the `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 // 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 // connecting it, and then we destroy it. We will need to address it before
// video path is enabled. // video path is enabled.
return transports_.SetTransportForMid(content_info.name, jsep_transport); return transports_.SetTransportForMid(content_info.mid(), jsep_transport);
} }
cricket::JsepTransportDescription cricket::JsepTransportDescription
@ -1081,7 +1081,7 @@ JsepTransportController::MergeEncryptedHeaderExtensionIdsForBundles(
merged_encrypted_extension_ids_by_bundle; merged_encrypted_extension_ids_by_bundle;
// Union the encrypted header IDs in the group when bundle is enabled. // Union the encrypted header IDs in the group when bundle is enabled.
for (const cricket::ContentInfo& content_info : description->contents()) { for (const cricket::ContentInfo& content_info : description->contents()) {
auto group = bundles_.LookupGroupByMid(content_info.name); auto group = bundles_.LookupGroupByMid(content_info.mid());
if (!group) if (!group)
continue; continue;
// Get or create list of IDs for the BUNDLE group. // Get or create list of IDs for the BUNDLE group.
@ -1150,13 +1150,14 @@ RTCError JsepTransportController::MaybeCreateJsepTransport(
bool local, bool local,
const cricket::ContentInfo& content_info, const cricket::ContentInfo& content_info,
const cricket::SessionDescription& description) { const cricket::SessionDescription& description) {
cricket::JsepTransport* transport = GetJsepTransportByName(content_info.name); cricket::JsepTransport* transport =
GetJsepTransportByName(content_info.mid());
if (transport) { if (transport) {
return RTCError::OK(); return RTCError::OK();
} }
rtc::scoped_refptr<IceTransportInterface> ice = rtc::scoped_refptr<IceTransportInterface> ice =
CreateIceTransport(content_info.name, /*rtcp=*/false); CreateIceTransport(content_info.mid(), /*rtcp=*/false);
std::unique_ptr<cricket::DtlsTransportInternal> rtp_dtls_transport = std::unique_ptr<cricket::DtlsTransportInternal> rtp_dtls_transport =
CreateDtlsTransport(content_info, ice->internal()); CreateDtlsTransport(content_info, ice->internal());
@ -1170,7 +1171,7 @@ RTCError JsepTransportController::MaybeCreateJsepTransport(
if (config_.rtcp_mux_policy != if (config_.rtcp_mux_policy !=
PeerConnectionInterface::kRtcpMuxPolicyRequire && PeerConnectionInterface::kRtcpMuxPolicyRequire &&
content_info.type == cricket::MediaProtocolType::kRtp) { 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 = rtcp_dtls_transport =
CreateDtlsTransport(content_info, rtcp_ice->internal()); CreateDtlsTransport(content_info, rtcp_ice->internal());
} }
@ -1179,11 +1180,13 @@ RTCError JsepTransportController::MaybeCreateJsepTransport(
RTC_LOG(LS_INFO) RTC_LOG(LS_INFO)
<< "Creating UnencryptedRtpTransport, becayse encryption is disabled."; << "Creating UnencryptedRtpTransport, becayse encryption is disabled.";
unencrypted_rtp_transport = CreateUnencryptedRtpTransport( 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 { } else {
RTC_LOG(LS_INFO) << "Creating DtlsSrtpTransport."; RTC_LOG(LS_INFO) << "Creating DtlsSrtpTransport.";
dtls_srtp_transport = CreateDtlsSrtpTransport( dtls_srtp_transport =
content_info.name, rtp_dtls_transport.get(), rtcp_dtls_transport.get()); CreateDtlsSrtpTransport(content_info.mid(), rtp_dtls_transport.get(),
rtcp_dtls_transport.get());
} }
std::unique_ptr<cricket::SctpTransportInternal> sctp_transport; std::unique_ptr<cricket::SctpTransportInternal> sctp_transport;
@ -1194,7 +1197,7 @@ RTCError JsepTransportController::MaybeCreateJsepTransport(
std::unique_ptr<cricket::JsepTransport> jsep_transport = std::unique_ptr<cricket::JsepTransport> jsep_transport =
std::make_unique<cricket::JsepTransport>( std::make_unique<cricket::JsepTransport>(
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(unencrypted_rtp_transport), std::move(sdes_transport),
std::move(dtls_srtp_transport), std::move(rtp_dtls_transport), std::move(dtls_srtp_transport), std::move(rtp_dtls_transport),
std::move(rtcp_dtls_transport), std::move(sctp_transport), std::move(rtcp_dtls_transport), std::move(sctp_transport),
@ -1215,7 +1218,7 @@ RTCError JsepTransportController::MaybeCreateJsepTransport(
OnUnDemuxableRtpPacketReceived_n(packet); OnUnDemuxableRtpPacketReceived_n(packet);
}); });
transports_.RegisterTransport(content_info.name, std::move(jsep_transport)); transports_.RegisterTransport(content_info.mid(), std::move(jsep_transport));
UpdateAggregateStates_n(); UpdateAggregateStates_n();
return RTCError::OK(); return RTCError::OK();
} }

36
pc/session_description.cc
View File

@ -19,23 +19,21 @@ namespace cricket {
namespace { namespace {
ContentInfo* FindContentInfoByName(ContentInfos* contents, ContentInfo* FindContentInfoByName(ContentInfos* contents,
const std::string& name) { absl::string_view name) {
RTC_DCHECK(contents); RTC_DCHECK(contents);
for (ContentInfo& content : *contents) { for (ContentInfo& content : *contents) {
if (content.name == name) { if (content.mid() == name) {
return &content; return &content;
} }
} }
return nullptr; return nullptr;
} }
} // namespace
const ContentInfo* FindContentInfoByName(const ContentInfos& contents, const ContentInfo* FindContentInfoByName(const ContentInfos& contents,
const std::string& name) { absl::string_view name) {
for (ContentInfos::const_iterator content = contents.begin(); for (ContentInfos::const_iterator content = contents.begin();
content != contents.end(); ++content) { content != contents.end(); ++content) {
if (content->name == name) { if (content->mid() == name) {
return &(*content); return &(*content);
} }
} }
@ -52,6 +50,8 @@ const ContentInfo* FindContentInfoByType(const ContentInfos& contents,
return nullptr; return nullptr;
} }
} // namespace
ContentGroup::ContentGroup(const std::string& semantics) ContentGroup::ContentGroup(const std::string& semantics)
: semantics_(semantics) {} : semantics_(semantics) {}
@ -117,7 +117,7 @@ ContentInfo* SessionDescription::GetContentByName(const std::string& name) {
} }
const MediaContentDescription* SessionDescription::GetContentDescriptionByName( const MediaContentDescription* SessionDescription::GetContentDescriptionByName(
const std::string& name) const { absl::string_view name) const {
const ContentInfo* cinfo = FindContentInfoByName(contents_, name); const ContentInfo* cinfo = FindContentInfoByName(contents_, name);
if (cinfo == NULL) { if (cinfo == NULL) {
return NULL; return NULL;
@ -127,7 +127,7 @@ const MediaContentDescription* SessionDescription::GetContentDescriptionByName(
} }
MediaContentDescription* SessionDescription::GetContentDescriptionByName( MediaContentDescription* SessionDescription::GetContentDescriptionByName(
const std::string& name) { absl::string_view name) {
ContentInfo* cinfo = FindContentInfoByName(&contents_, name); ContentInfo* cinfo = FindContentInfoByName(&contents_, name);
if (cinfo == NULL) { if (cinfo == NULL) {
return NULL; return NULL;
@ -149,9 +149,8 @@ void SessionDescription::AddContent(
const std::string& name, const std::string& name,
MediaProtocolType type, MediaProtocolType type,
std::unique_ptr<MediaContentDescription> description) { std::unique_ptr<MediaContentDescription> description) {
ContentInfo content(type); ContentInfo content(type, std::move(description));
content.name = name; content.name = name;
content.set_media_description(std::move(description));
AddContent(std::move(content)); AddContent(std::move(content));
} }
@ -160,10 +159,9 @@ void SessionDescription::AddContent(
MediaProtocolType type, MediaProtocolType type,
bool rejected, bool rejected,
std::unique_ptr<MediaContentDescription> description) { std::unique_ptr<MediaContentDescription> description) {
ContentInfo content(type); ContentInfo content(type, std::move(description));
content.name = name; content.name = name;
content.rejected = rejected; content.rejected = rejected;
content.set_media_description(std::move(description));
AddContent(std::move(content)); AddContent(std::move(content));
} }
@ -173,11 +171,10 @@ void SessionDescription::AddContent(
bool rejected, bool rejected,
bool bundle_only, bool bundle_only,
std::unique_ptr<MediaContentDescription> description) { std::unique_ptr<MediaContentDescription> description) {
ContentInfo content(type); ContentInfo content(type, std::move(description));
content.name = name; content.name = name;
content.rejected = rejected; content.rejected = rejected;
content.bundle_only = bundle_only; content.bundle_only = bundle_only;
content.set_media_description(std::move(description));
AddContent(std::move(content)); AddContent(std::move(content));
} }
@ -193,7 +190,7 @@ void SessionDescription::AddContent(ContentInfo&& content) {
bool SessionDescription::RemoveContentByName(const std::string& name) { bool SessionDescription::RemoveContentByName(const std::string& name) {
for (ContentInfos::iterator content = contents_.begin(); for (ContentInfos::iterator content = contents_.begin();
content != contents_.end(); ++content) { content != contents_.end(); ++content) {
if (content->name == name) { if (content->mid() == name) {
contents_.erase(content); contents_.erase(content);
return true; return true;
} }
@ -291,15 +288,6 @@ ContentInfo::ContentInfo(const ContentInfo& o)
bundle_only(o.bundle_only), bundle_only(o.bundle_only),
description_(o.description_->Clone()) {} 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 { const MediaContentDescription* ContentInfo::media_description() const {
return description_.get(); return description_.get();
} }

30
pc/session_description.h
View File

@ -399,26 +399,29 @@ enum class MediaProtocolType {
class RTC_EXPORT ContentInfo { class RTC_EXPORT ContentInfo {
public: public:
explicit ContentInfo(MediaProtocolType type) : type(type) {} explicit ContentInfo(MediaProtocolType type) : type(type) {}
ContentInfo(MediaProtocolType type,
std::unique_ptr<MediaContentDescription> description)
: type(type), description_(std::move(description)) {}
~ContentInfo(); ~ContentInfo();
// Copy
// Copy ctor and assignment will clone `description_`.
ContentInfo(const ContentInfo& o); 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(ContentInfo&& o) = default;
ContentInfo& operator=(ContentInfo&& o) = default; ContentInfo& operator=(ContentInfo&& o) = default;
// Alias for `name`. // Alias for `name`.
std::string mid() const { return name; } // TODO(tommi): change return type to string_view.
void set_mid(const std::string& mid) { this->name = mid; } const std::string& mid() const { return name; }
void set_mid(absl::string_view mid) { name = std::string(mid); }
// Alias for `description`. // Alias for `description`.
MediaContentDescription* media_description(); MediaContentDescription* media_description();
const MediaContentDescription* media_description() const; const MediaContentDescription* media_description() const;
void set_media_description(std::unique_ptr<MediaContentDescription> desc) { // TODO(bugs.webrtc.org/8620): Rename this to mid and make private.
description_ = std::move(desc);
}
// TODO(bugs.webrtc.org/8620): Rename this to mid.
std::string name; std::string name;
MediaProtocolType type; MediaProtocolType type;
bool rejected = false; bool rejected = false;
@ -462,11 +465,6 @@ class ContentGroup {
typedef std::vector<ContentInfo> ContentInfos; typedef std::vector<ContentInfo> ContentInfos;
typedef std::vector<ContentGroup> ContentGroups; typedef std::vector<ContentGroup> 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. // 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. // These can be used as bit flags to indicate both or the special value none.
enum MsidSignaling { enum MsidSignaling {
@ -502,8 +500,8 @@ class SessionDescription {
const ContentInfo* GetContentByName(const std::string& name) const; const ContentInfo* GetContentByName(const std::string& name) const;
ContentInfo* GetContentByName(const std::string& name); ContentInfo* GetContentByName(const std::string& name);
const MediaContentDescription* GetContentDescriptionByName( const MediaContentDescription* GetContentDescriptionByName(
const std::string& name) const; absl::string_view name) const;
MediaContentDescription* GetContentDescriptionByName(const std::string& name); MediaContentDescription* GetContentDescriptionByName(absl::string_view name);
const ContentInfo* FirstContentByType(MediaProtocolType type) const; const ContentInfo* FirstContentByType(MediaProtocolType type) const;
const ContentInfo* FirstContent() const; const ContentInfo* FirstContent() const;