From e831b8c94d15e550084b5207af7522c6054f45e1 Mon Sep 17 00:00:00 2001 From: Steve Anton Date: Thu, 1 Feb 2018 12:22:16 -0800 Subject: [PATCH] Add MSID signaling compatibility for Unified Plan endpoints This is intended to ensure compatibility between Plan B and Unified Plan endpoints for the single audio - single video case. If Unified Plan is the offerer, it will add a=msid and a=ssrc MSID entries to its offer. If Unified Plan is the answerer, it will use whatever MSID signaling mechanism was used in the offer (either a=msid or a=ssrc). Bug: webrtc:7600 Change-Id: I6192dec19123fbb56f5d04540d2175c7fb30b9b6 Reviewed-on: https://webrtc-review.googlesource.com/44162 Commit-Queue: Steve Anton Reviewed-by: Taylor Brandstetter Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#21859} --- pc/jsepsessiondescription.cc | 2 +- pc/mediasession.cc | 47 +++++++++++ pc/mediasession.h | 1 + pc/peerconnection.cc | 2 + pc/peerconnection_rtp_unittest.cc | 76 ++++++++++++++++++ pc/sessiondescription.h | 19 +++++ pc/webrtcsdp.cc | 112 ++++++++++++++------------ pc/webrtcsdp.h | 3 +- pc/webrtcsdp_unittest.cc | 129 ++++++++++++++++++++++-------- 9 files changed, 304 insertions(+), 87 deletions(-) diff --git a/pc/jsepsessiondescription.cc b/pc/jsepsessiondescription.cc index 91840e3654..ecd2ee2160 100644 --- a/pc/jsepsessiondescription.cc +++ b/pc/jsepsessiondescription.cc @@ -273,7 +273,7 @@ bool JsepSessionDescription::ToString(std::string* out) const { if (!description_ || !out) { return false; } - *out = SdpSerialize(*this, false); + *out = SdpSerialize(*this); return !out->empty(); } diff --git a/pc/mediasession.cc b/pc/mediasession.cc index 3f750d18ba..a2f47dcb27 100644 --- a/pc/mediasession.cc +++ b/pc/mediasession.cc @@ -1375,6 +1375,20 @@ SessionDescription* MediaSessionDescriptionFactory::CreateOffer( return nullptr; } } + + // The following determines how to signal MSIDs to ensure compatibility with + // older endpoints (in particular, older Plan B endpoints). + if (session_options.is_unified_plan) { + // Be conservative and signal using both a=msid and a=ssrc lines. Unified + // Plan answerers will look at a=msid and Plan B answerers will look at the + // a=ssrc MSID line. + offer->set_msid_signaling(cricket::kMsidSignalingMediaSection | + cricket::kMsidSignalingSsrcAttribute); + } else { + // Plan B always signals MSID using a=ssrc lines. + offer->set_msid_signaling(cricket::kMsidSignalingSsrcAttribute); + } + return offer.release(); } @@ -1500,6 +1514,39 @@ SessionDescription* MediaSessionDescriptionFactory::CreateAnswer( } } + // The following determines how to signal MSIDs to ensure compatibility with + // older endpoints (in particular, older Plan B endpoints). + if (session_options.is_unified_plan) { + // Unified Plan needs to look at what the offer included to find the most + // compatible answer. + if (offer->msid_signaling() == 0) { + // We end up here in one of three cases: + // 1. An empty offer. We'll reply with an empty answer so it doesn't + // matter what we pick here. + // 2. A data channel only offer. We won't add any MSIDs to the answer so + // it also doesn't matter what we pick here. + // 3. Media that's either sendonly or inactive from the remote endpoint. + // We don't have any information to say whether the endpoint is Plan B + // or Unified Plan, so be conservative and send both. + answer->set_msid_signaling(cricket::kMsidSignalingMediaSection | + cricket::kMsidSignalingSsrcAttribute); + } else if (offer->msid_signaling() == + (cricket::kMsidSignalingMediaSection | + cricket::kMsidSignalingSsrcAttribute)) { + // If both a=msid and a=ssrc MSID signaling methods were used, we're + // probably talking to a Unified Plan endpoint so respond with just + // a=msid. + answer->set_msid_signaling(cricket::kMsidSignalingMediaSection); + } else { + // Otherwise, it's clear which method the offerer is using so repeat that + // back to them. + answer->set_msid_signaling(offer->msid_signaling()); + } + } else { + // Plan B always signals MSID using a=ssrc lines. + answer->set_msid_signaling(cricket::kMsidSignalingSsrcAttribute); + } + return answer.release(); } diff --git a/pc/mediasession.h b/pc/mediasession.h index 02205c68af..9b118e1ef1 100644 --- a/pc/mediasession.h +++ b/pc/mediasession.h @@ -94,6 +94,7 @@ struct MediaSessionOptions { bool vad_enabled = true; // When disabled, removes all CN codecs from SDP. bool rtcp_mux_enabled = true; bool bundle_enabled = false; + bool is_unified_plan = false; std::string rtcp_cname = kDefaultRtcpCname; rtc::CryptoOptions crypto_options; // List of media description options in the same order that the media diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index c361468c68..ca97e811ad 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -3199,6 +3199,7 @@ void PeerConnection::GetOptionsForOffer( session_options->rtcp_cname = rtcp_cname_; session_options->crypto_options = factory_->options().crypto_options; + session_options->is_unified_plan = IsUnifiedPlan(); } void PeerConnection::GetOptionsForPlanBOffer( @@ -3457,6 +3458,7 @@ void PeerConnection::GetOptionsForAnswer( session_options->rtcp_cname = rtcp_cname_; session_options->crypto_options = factory_->options().crypto_options; + session_options->is_unified_plan = IsUnifiedPlan(); } void PeerConnection::GetOptionsForPlanBAnswer( diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc index 12e58b7e65..f19388fe4b 100644 --- a/pc/peerconnection_rtp_unittest.cc +++ b/pc/peerconnection_rtp_unittest.cc @@ -19,6 +19,7 @@ #include "pc/mediastream.h" #include "pc/mediastreamtrack.h" #include "pc/peerconnectionwrapper.h" +#include "pc/sdputils.h" #include "pc/test/fakeaudiocapturemodule.h" #include "pc/test/mockpeerconnectionobservers.h" #include "rtc_base/checks.h" @@ -74,6 +75,12 @@ class PeerConnectionRtpTest : public testing::Test { return CreatePeerConnection(RTCConfiguration()); } + std::unique_ptr CreatePeerConnectionWithPlanB() { + RTCConfiguration config; + config.sdp_semantics = SdpSemantics::kPlanB; + return CreatePeerConnection(config); + } + std::unique_ptr CreatePeerConnectionWithUnifiedPlan() { RTCConfiguration config; config.sdp_semantics = SdpSemantics::kUnifiedPlan; @@ -866,6 +873,75 @@ TEST_F(PeerConnectionRtpUnifiedPlanTest, EXPECT_FALSE(caller->observer()->negotiation_needed()); } +// Test MSID signaling between Unified Plan and Plan B endpoints. There are two +// options for this kind of signaling: media section based (a=msid) and ssrc +// based (a=ssrc MSID). While JSEP only specifies media section MSID signaling, +// we want to ensure compatibility with older Plan B endpoints that might expect +// ssrc based MSID signaling. Thus we test here that Unified Plan offers both +// types but answers with the same type as the offer. + +class PeerConnectionMsidSignalingTest : public PeerConnectionRtpTest {}; + +TEST_F(PeerConnectionMsidSignalingTest, UnifiedPlanTalkingToOurself) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + caller->AddAudioTrack("caller_audio"); + auto callee = CreatePeerConnectionWithUnifiedPlan(); + callee->AddAudioTrack("callee_audio"); + + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + + // Offer should have had both a=msid and a=ssrc MSID lines. + auto* offer = callee->pc()->remote_description(); + EXPECT_EQ((cricket::kMsidSignalingMediaSection | + cricket::kMsidSignalingSsrcAttribute), + offer->description()->msid_signaling()); + + // Answer should have had only a=msid lines. + auto* answer = caller->pc()->remote_description(); + EXPECT_EQ(cricket::kMsidSignalingMediaSection, + answer->description()->msid_signaling()); +} + +TEST_F(PeerConnectionMsidSignalingTest, PlanBOfferToUnifiedPlanAnswer) { + auto caller = CreatePeerConnectionWithPlanB(); + caller->AddAudioTrack("caller_audio"); + auto callee = CreatePeerConnectionWithUnifiedPlan(); + callee->AddAudioTrack("callee_audio"); + + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + + // Offer should have only a=ssrc MSID lines. + auto* offer = callee->pc()->remote_description(); + EXPECT_EQ(cricket::kMsidSignalingSsrcAttribute, + offer->description()->msid_signaling()); + + // Answer should have only a=ssrc MSID lines to match the offer. + auto* answer = caller->pc()->remote_description(); + EXPECT_EQ(cricket::kMsidSignalingSsrcAttribute, + answer->description()->msid_signaling()); +} + +TEST_F(PeerConnectionMsidSignalingTest, PureUnifiedPlanToUs) { + auto caller = CreatePeerConnectionWithUnifiedPlan(); + caller->AddAudioTrack("caller_audio"); + auto callee = CreatePeerConnectionWithUnifiedPlan(); + callee->AddAudioTrack("callee_audio"); + + auto offer = caller->CreateOffer(); + // Simulate a pure Unified Plan offerer by setting the MSID signaling to media + // section only. + offer->description()->set_msid_signaling(cricket::kMsidSignalingMediaSection); + + ASSERT_TRUE( + caller->SetLocalDescription(CloneSessionDescription(offer.get()))); + ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer))); + + // Answer should have only a=msid to match the offer. + auto answer = callee->CreateAnswer(); + EXPECT_EQ(cricket::kMsidSignalingMediaSection, + answer->description()->msid_signaling()); +} + // Sender setups in a call. class PeerConnectionSenderTest : public PeerConnectionRtpTest {}; diff --git a/pc/sessiondescription.h b/pc/sessiondescription.h index b2cdee9482..645ebc54f8 100644 --- a/pc/sessiondescription.h +++ b/pc/sessiondescription.h @@ -363,6 +363,15 @@ const ContentInfo* FindContentInfoByName(const ContentInfos& contents, const ContentInfo* FindContentInfoByType(const ContentInfos& contents, const std::string& type); +// Determines how the MSID will be signaled in the SDP. These can be used as +// flags to indicate both or none. +enum MsidSignaling { + // Signal MSID with one a=msid line in the media section. + kMsidSignalingMediaSection = 0x1, + // Signal MSID with a=ssrc: msid lines in the media section. + kMsidSignalingSsrcAttribute = 0x2 +}; + // Describes a collection of contents, each with its own name and // type. Analogous to a or stanza. Assumes that // contents are unique be name, but doesn't enforce that. @@ -439,6 +448,13 @@ class SessionDescription { void set_msid_supported(bool supported) { msid_supported_ = supported; } bool msid_supported() const { return msid_supported_; } + // Determines how the MSIDs were/will be signaled. Flag value composed of + // MsidSignaling bits (see enum above). + void set_msid_signaling(int msid_signaling) { + msid_signaling_ = msid_signaling; + } + int msid_signaling() const { return msid_signaling_; } + private: SessionDescription(const SessionDescription&); @@ -446,6 +462,9 @@ class SessionDescription { TransportInfos transport_infos_; ContentGroups content_groups_; bool msid_supported_ = true; + // Default to what Plan B would do. + // TODO(bugs.webrtc.org/8530): Change default to kMsidSignalingMediaSection. + int msid_signaling_ = kMsidSignalingSsrcAttribute; }; // Indicates whether a session description was sent by the local client or diff --git a/pc/webrtcsdp.cc b/pc/webrtcsdp.cc index 56f4c991ca..535ed23082 100644 --- a/pc/webrtcsdp.cc +++ b/pc/webrtcsdp.cc @@ -232,14 +232,14 @@ static void BuildMediaDescription(const ContentInfo* content_info, const TransportInfo* transport_info, const MediaType media_type, const std::vector& candidates, - bool unified_plan_sdp, + int msid_signaling, std::string* message); static void BuildSctpContentAttributes(std::string* message, int sctp_port, bool use_sctpmap); static void BuildRtpContentAttributes(const MediaContentDescription* media_desc, const MediaType media_type, - bool unified_plan_sdp, + int msid_signaling, std::string* message); static void BuildRtpMap(const MediaContentDescription* media_desc, const MediaType media_type, @@ -280,12 +280,14 @@ static bool ParseContent(const std::string& message, size_t* pos, std::string* content_name, bool* bundle_only, + int* msid_signaling, MediaContentDescription* media_desc, TransportDescription* transport, std::vector* candidates, SdpParseError* error); static bool ParseSsrcAttribute(const std::string& line, SsrcInfoVec* ssrc_infos, + int* msid_signaling, SdpParseError* error); static bool ParseSsrcGroupAttribute(const std::string& line, SsrcGroupVec* ssrc_groups, @@ -601,7 +603,7 @@ void CreateTracksFromSsrcInfos(const SsrcInfoVec& ssrc_infos, track_id = ssrc_info->label; } else if (ssrc_info->stream_id.empty() && !msid_stream_id.empty()) { // If there's no msid in the SSRC attributes, but there's a global one - // (from a=msid), use that. This is the case with unified plan SDP. + // (from a=msid), use that. This is the case with Unified Plan SDP. stream_id = msid_stream_id; track_id = msid_track_id; } else { @@ -757,8 +759,7 @@ static bool IsValidPort(int port) { return port >= 0 && port <= 65535; } -std::string SdpSerialize(const JsepSessionDescription& jdesc, - bool unified_plan_sdp) { +std::string SdpSerialize(const JsepSessionDescription& jdesc) { const cricket::SessionDescription* desc = jdesc.description(); if (!desc) { return ""; @@ -829,7 +830,7 @@ std::string SdpSerialize(const JsepSessionDescription& jdesc, std::vector candidates; GetCandidatesByMindex(jdesc, ++mline_index, &candidates); BuildMediaDescription(&*it, desc->GetTransportInfoByName(it->name), - mdesc->type(), candidates, unified_plan_sdp, + mdesc->type(), candidates, desc->msid_signaling(), &message); } return message; @@ -1199,7 +1200,7 @@ void BuildMediaDescription(const ContentInfo* content_info, const TransportInfo* transport_info, const MediaType media_type, const std::vector& candidates, - bool unified_plan_sdp, + int msid_signaling, std::string* message) { RTC_DCHECK(message != NULL); if (content_info == NULL || message == NULL) { @@ -1405,8 +1406,7 @@ void BuildMediaDescription(const ContentInfo* content_info, bool use_sctpmap = data_desc->use_sctpmap(); BuildSctpContentAttributes(message, sctp_port, use_sctpmap); } else if (IsRtp(media_desc->protocol())) { - BuildRtpContentAttributes(media_desc, media_type, unified_plan_sdp, - message); + BuildRtpContentAttributes(media_desc, media_type, msid_signaling, message); } } @@ -1431,10 +1431,9 @@ void BuildSctpContentAttributes(std::string* message, AddLine(os.str(), message); } -// If unified_plan_sdp is true, will use "a=msid". void BuildRtpContentAttributes(const MediaContentDescription* media_desc, const MediaType media_type, - bool unified_plan_sdp, + int msid_signaling, std::string* message) { std::ostringstream os; // RFC 5285 @@ -1471,19 +1470,20 @@ void BuildRtpContentAttributes(const MediaContentDescription* media_desc, } AddLine(os.str(), message); - // draft-ietf-mmusic-msid-11 + // Specified in https://tools.ietf.org/html/draft-ietf-mmusic-msid-16 // a=msid: - if (unified_plan_sdp && !media_desc->streams().empty()) { - if (media_desc->streams().size() > 1u) { - RTC_LOG(LS_WARNING) - << "Trying to serialize unified plan SDP with more than " - << "one track in a media section. Omitting 'a=msid'."; - } else { - auto track = media_desc->streams().begin(); - const std::string& stream_id = track->sync_label; + if (msid_signaling & cricket::kMsidSignalingMediaSection) { + const StreamParamsVec& streams = media_desc->streams(); + if (streams.size() == 1u) { + const StreamParams& track = streams[0]; + const std::string& stream_id = track.sync_label; InitAttrLine(kAttributeMsid, &os); - os << kSdpDelimiterColon << stream_id << kSdpDelimiterSpace << track->id; + os << kSdpDelimiterColon << stream_id << kSdpDelimiterSpace << track.id; AddLine(os.str(), message); + } else if (streams.size() > 1u) { + RTC_LOG(LS_WARNING) + << "Trying to serialize Unified Plan SDP with more than " + << "one track in a media section. Omitting 'a=msid'."; } } @@ -1558,25 +1558,27 @@ void BuildRtpContentAttributes(const MediaContentDescription* media_desc, AddSsrcLine(ssrc, kSsrcAttributeCname, track->cname, message); - // draft-alvestrand-mmusic-msid-00 - // a=ssrc: msid:identifier [appdata] - // The appdata consists of the "id" attribute of a MediaStreamTrack, - // which corresponds to the "id" attribute of StreamParams. - const std::string& stream_id = track->sync_label; - InitAttrLine(kAttributeSsrc, &os); - os << kSdpDelimiterColon << ssrc << kSdpDelimiterSpace - << kSsrcAttributeMsid << kSdpDelimiterColon << stream_id - << kSdpDelimiterSpace << track->id; - AddLine(os.str(), message); + if (msid_signaling & cricket::kMsidSignalingSsrcAttribute) { + // draft-alvestrand-mmusic-msid-00 + // a=ssrc: msid:identifier [appdata] + // The appdata consists of the "id" attribute of a MediaStreamTrack, + // which corresponds to the "id" attribute of StreamParams. + const std::string& stream_id = track->sync_label; + InitAttrLine(kAttributeSsrc, &os); + os << kSdpDelimiterColon << ssrc << kSdpDelimiterSpace + << kSsrcAttributeMsid << kSdpDelimiterColon << stream_id + << kSdpDelimiterSpace << track->id; + AddLine(os.str(), message); - // TODO(ronghuawu): Remove below code which is for backward - // compatibility. - // draft-alvestrand-rtcweb-mid-01 - // a=ssrc: mslabel: - // The label isn't yet defined. - // a=ssrc: label: - AddSsrcLine(ssrc, kSsrcAttributeMslabel, track->sync_label, message); - AddSsrcLine(ssrc, kSSrcAttributeLabel, track->id, message); + // TODO(ronghuawu): Remove below code which is for backward + // compatibility. + // draft-alvestrand-rtcweb-mid-01 + // a=ssrc: mslabel: + // The label isn't yet defined. + // a=ssrc: label: + AddSsrcLine(ssrc, kSsrcAttributeMslabel, track->sync_label, message); + AddSsrcLine(ssrc, kSSrcAttributeLabel, track->id, message); + } } } } @@ -2294,6 +2296,7 @@ static C* ParseContentDescription(const std::string& message, size_t* pos, std::string* content_name, bool* bundle_only, + int* msid_signaling, TransportDescription* transport, std::vector* candidates, webrtc::SdpParseError* error) { @@ -2313,8 +2316,8 @@ static C* ParseContentDescription(const std::string& message, break; } if (!ParseContent(message, media_type, mline_index, protocol, payload_types, - pos, content_name, bundle_only, media_desc, transport, - candidates, error)) { + pos, content_name, bundle_only, msid_signaling, media_desc, + transport, candidates, error)) { delete media_desc; return nullptr; } @@ -2348,6 +2351,7 @@ bool ParseMediaDescription(const std::string& message, RTC_DCHECK(desc != NULL); std::string line; int mline_index = -1; + int msid_signaling = 0; // Zero or more media descriptions // RFC 4566 @@ -2405,22 +2409,23 @@ bool ParseMediaDescription(const std::string& message, std::unique_ptr content; std::string content_name; bool bundle_only = false; + int section_msid_signaling = 0; if (HasAttribute(line, kMediaTypeVideo)) { content.reset(ParseContentDescription( message, cricket::MEDIA_TYPE_VIDEO, mline_index, protocol, - payload_types, pos, &content_name, &bundle_only, &transport, - candidates, error)); + payload_types, pos, &content_name, &bundle_only, + §ion_msid_signaling, &transport, candidates, error)); } else if (HasAttribute(line, kMediaTypeAudio)) { content.reset(ParseContentDescription( message, cricket::MEDIA_TYPE_AUDIO, mline_index, protocol, - payload_types, pos, &content_name, &bundle_only, &transport, - candidates, error)); + payload_types, pos, &content_name, &bundle_only, + §ion_msid_signaling, &transport, candidates, error)); } else if (HasAttribute(line, kMediaTypeData)) { DataContentDescription* data_desc = ParseContentDescription( message, cricket::MEDIA_TYPE_DATA, mline_index, protocol, - payload_types, pos, &content_name, &bundle_only, &transport, - candidates, error); + payload_types, pos, &content_name, &bundle_only, + §ion_msid_signaling, &transport, candidates, error); content.reset(data_desc); if (data_desc && IsDtlsSctp(protocol)) { @@ -2442,6 +2447,8 @@ bool ParseMediaDescription(const std::string& message, return false; } + msid_signaling |= section_msid_signaling; + bool content_rejected = false; // A port of 0 is not interpreted as a rejected m= section when it's // used along with a=bundle-only. @@ -2500,6 +2507,8 @@ bool ParseMediaDescription(const std::string& message, } } + desc->set_msid_signaling(msid_signaling); + size_t end_of_message = message.size(); if (mline_index == -1 && *pos != end_of_message) { ParseFailed(message, *pos, "Expects m line.", error); @@ -2665,6 +2674,7 @@ bool ParseContent(const std::string& message, size_t* pos, std::string* content_name, bool* bundle_only, + int* msid_signaling, MediaContentDescription* media_desc, TransportDescription* transport, std::vector* candidates, @@ -2839,7 +2849,7 @@ bool ParseContent(const std::string& message, return false; } } else if (HasAttribute(line, kAttributeSsrc)) { - if (!ParseSsrcAttribute(line, &ssrc_infos, error)) { + if (!ParseSsrcAttribute(line, &ssrc_infos, msid_signaling, error)) { return false; } } else if (HasAttribute(line, kAttributeCrypto)) { @@ -2891,6 +2901,7 @@ bool ParseContent(const std::string& message, if (!ParseMsidAttribute(line, &stream_id, &track_id, error)) { return false; } + *msid_signaling |= cricket::kMsidSignalingMediaSection; } } else { // Only parse lines that we are interested of. @@ -2963,7 +2974,9 @@ bool ParseContent(const std::string& message, return true; } -bool ParseSsrcAttribute(const std::string& line, SsrcInfoVec* ssrc_infos, +bool ParseSsrcAttribute(const std::string& line, + SsrcInfoVec* ssrc_infos, + int* msid_signaling, SdpParseError* error) { RTC_DCHECK(ssrc_infos != NULL); // RFC 5576 @@ -3029,6 +3042,7 @@ bool ParseSsrcAttribute(const std::string& line, SsrcInfoVec* ssrc_infos, if (fields.size() == 2) { ssrc_info->track_id = fields[1]; } + *msid_signaling |= cricket::kMsidSignalingSsrcAttribute; } else if (attribute == kSsrcAttributeMslabel) { // draft-alvestrand-rtcweb-mid-01 // mslabel: diff --git a/pc/webrtcsdp.h b/pc/webrtcsdp.h index 83cc7688a3..2d842a1d5d 100644 --- a/pc/webrtcsdp.h +++ b/pc/webrtcsdp.h @@ -38,8 +38,7 @@ struct SdpParseError; // jdesc - The JsepSessionDescription object to be serialized. // unified_plan_sdp - If set to true, include "a=msid" lines where appropriate. // return - SDP string serialized from the arguments. -std::string SdpSerialize(const JsepSessionDescription& jdesc, - bool unified_plan_sdp); +std::string SdpSerialize(const JsepSessionDescription& jdesc); // Serializes the passed in IceCandidateInterface to a SDP string. // candidate - The candidate to be serialized. diff --git a/pc/webrtcsdp_unittest.cc b/pc/webrtcsdp_unittest.cc index 831d23655a..c268b04ae1 100644 --- a/pc/webrtcsdp_unittest.cc +++ b/pc/webrtcsdp_unittest.cc @@ -1119,6 +1119,7 @@ class WebRtcSdpTest : public testing::Test { desc_.AddContent(kVideoContentName3, MediaProtocolType::kRtp, video_desc_3); EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo( kVideoContentName3, TransportDescription(kUfragVideo3, kPwdVideo3)))); + desc_.set_msid_signaling(cricket::kMsidSignalingMediaSection); ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(), jdesc_.session_version())); @@ -1464,7 +1465,7 @@ class WebRtcSdpTest : public testing::Test { jdesc_.session_version())) { return false; } - std::string message = webrtc::SdpSerialize(jdesc_, false); + std::string message = webrtc::SdpSerialize(jdesc_); EXPECT_EQ(new_sdp, message); return true; } @@ -1489,7 +1490,7 @@ class WebRtcSdpTest : public testing::Test { JsepSessionDescription jdesc_no_candidates(kDummyType); MakeDescriptionWithoutCandidates(&jdesc_no_candidates); - std::string message = webrtc::SdpSerialize(jdesc_no_candidates, false); + std::string message = webrtc::SdpSerialize(jdesc_no_candidates); EXPECT_EQ(new_sdp, message); return true; } @@ -1759,9 +1760,8 @@ class WebRtcSdpTest : public testing::Test { // no order. If deserializer has already been tested, serializing then // deserializing and comparing JsepSessionDescription will test // the serializer sufficiently. - void TestSerialize(const JsepSessionDescription& jdesc, - bool unified_plan_sdp) { - std::string message = webrtc::SdpSerialize(jdesc, unified_plan_sdp); + void TestSerialize(const JsepSessionDescription& jdesc) { + std::string message = webrtc::SdpSerialize(jdesc); JsepSessionDescription jdesc_output_des(kDummyType); SdpParseError error; EXPECT_TRUE(webrtc::SdpDeserialize(message, &jdesc_output_des, &error)); @@ -1806,13 +1806,13 @@ void TestMismatch(const std::string& string1, const std::string& string2) { TEST_F(WebRtcSdpTest, SerializeSessionDescription) { // SessionDescription with desc and candidates. - std::string message = webrtc::SdpSerialize(jdesc_, false); + std::string message = webrtc::SdpSerialize(jdesc_); TestMismatch(std::string(kSdpFullString), message); } TEST_F(WebRtcSdpTest, SerializeSessionDescriptionEmpty) { JsepSessionDescription jdesc_empty(kDummyType); - EXPECT_EQ("", webrtc::SdpSerialize(jdesc_empty, false)); + EXPECT_EQ("", webrtc::SdpSerialize(jdesc_empty)); } // This tests serialization of SDP with a=crypto and a=fingerprint, as would be @@ -1821,7 +1821,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithFingerprint) { AddFingerprint(); JsepSessionDescription jdesc_with_fingerprint(kDummyType); MakeDescriptionWithoutCandidates(&jdesc_with_fingerprint); - std::string message = webrtc::SdpSerialize(jdesc_with_fingerprint, false); + std::string message = webrtc::SdpSerialize(jdesc_with_fingerprint); std::string sdp_with_fingerprint = kSdpString; InjectAfter(kAttributeIcePwdVoice, @@ -1839,7 +1839,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithFingerprintNoCryptos) { RemoveCryptos(); JsepSessionDescription jdesc_with_fingerprint(kDummyType); MakeDescriptionWithoutCandidates(&jdesc_with_fingerprint); - std::string message = webrtc::SdpSerialize(jdesc_with_fingerprint, false); + std::string message = webrtc::SdpSerialize(jdesc_with_fingerprint); std::string sdp_with_fingerprint = kSdpString; Replace(kAttributeCryptoVoice, "", &sdp_with_fingerprint); @@ -1856,7 +1856,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithoutCandidates) { // JsepSessionDescription with desc but without candidates. JsepSessionDescription jdesc_no_candidates(kDummyType); MakeDescriptionWithoutCandidates(&jdesc_no_candidates); - std::string message = webrtc::SdpSerialize(jdesc_no_candidates, false); + std::string message = webrtc::SdpSerialize(jdesc_no_candidates); EXPECT_EQ(std::string(kSdpString), message); } @@ -1868,7 +1868,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithBundle) { ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(), jdesc_.session_version())); - std::string message = webrtc::SdpSerialize(jdesc_, false); + std::string message = webrtc::SdpSerialize(jdesc_); std::string sdp_with_bundle = kSdpFullString; InjectAfter(kSessionTime, "a=group:BUNDLE audio_content_name video_content_name\r\n", @@ -1884,7 +1884,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithBandwidth) { ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(), jdesc_.session_version())); - std::string message = webrtc::SdpSerialize(jdesc_, false); + std::string message = webrtc::SdpSerialize(jdesc_); std::string sdp_with_bandwidth = kSdpFullString; InjectAfter("c=IN IP4 74.125.224.39\r\n", "b=AS:100\r\n", @@ -1907,7 +1907,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithIceOptions) { ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(), jdesc_.session_version())); - std::string message = webrtc::SdpSerialize(jdesc_, false); + std::string message = webrtc::SdpSerialize(jdesc_); std::string sdp_with_ice_options = kSdpFullString; InjectAfter(kAttributeIcePwdVoice, "a=ice-options:iceoption1 iceoption3\r\n", @@ -1947,7 +1947,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithRtpDataChannel) { JsepSessionDescription jsep_desc(kDummyType); MakeDescriptionWithoutCandidates(&jsep_desc); - std::string message = webrtc::SdpSerialize(jsep_desc, false); + std::string message = webrtc::SdpSerialize(jsep_desc); std::string expected_sdp = kSdpString; expected_sdp.append(kSdpRtpDataChannelString); @@ -1960,7 +1960,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithSctpDataChannel) { JsepSessionDescription jsep_desc(kDummyType); MakeDescriptionWithoutCandidates(&jsep_desc); - std::string message = webrtc::SdpSerialize(jsep_desc, false); + std::string message = webrtc::SdpSerialize(jsep_desc); std::string expected_sdp = kSdpString; expected_sdp.append(kSdpSctpDataChannelString); @@ -1983,7 +1983,7 @@ TEST_F(WebRtcSdpTest, SerializeWithSctpDataChannelAndNewPort) { codec.SetParam(cricket::kCodecParamPort, kNewPort); dcdesc->AddOrReplaceCodec(codec); - std::string message = webrtc::SdpSerialize(jsep_desc, false); + std::string message = webrtc::SdpSerialize(jsep_desc); std::string expected_sdp = kSdpString; expected_sdp.append(kSdpSctpDataChannelString); @@ -2005,7 +2005,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithDataChannelAndBandwidth) { AddRtpDataChannel(); data_desc_->set_bandwidth(100*1000); MakeDescriptionWithoutCandidates(&jsep_desc); - std::string message = webrtc::SdpSerialize(jsep_desc, false); + std::string message = webrtc::SdpSerialize(jsep_desc); std::string expected_sdp = kSdpString; expected_sdp.append(kSdpRtpDataChannelString); @@ -2021,7 +2021,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithExtmap) { AddExtmap(encrypted); JsepSessionDescription desc_with_extmap(kDummyType); MakeDescriptionWithoutCandidates(&desc_with_extmap); - std::string message = webrtc::SdpSerialize(desc_with_extmap, false); + std::string message = webrtc::SdpSerialize(desc_with_extmap); std::string sdp_with_extmap = kSdpString; InjectAfter("a=mid:audio_content_name\r\n", @@ -2038,7 +2038,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithExtmapEncrypted) { JsepSessionDescription desc_with_extmap(kDummyType); ASSERT_TRUE(desc_with_extmap.Initialize(desc_.Copy(), kSessionId, kSessionVersion)); - TestSerialize(desc_with_extmap, false); + TestSerialize(desc_with_extmap); } TEST_F(WebRtcSdpTest, SerializeCandidates) { @@ -2090,7 +2090,7 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithH264) { jdesc_.Initialize(desc_.Copy(), kSessionId, kSessionVersion); - std::string message = webrtc::SdpSerialize(jdesc_, false); + std::string message = webrtc::SdpSerialize(jdesc_); size_t after_pt = message.find(" H264/90000"); ASSERT_NE(after_pt, std::string::npos); size_t before_pt = message.rfind("a=rtpmap:", after_pt); @@ -2774,7 +2774,7 @@ TEST_F(WebRtcSdpTest, SerializeSdpWithConferenceFlag) { // We tested deserialization already above, so just test that if we serialize // and deserialize the flag doesn't disappear. EXPECT_TRUE(SdpDeserialize(kSdpConferenceString, &jdesc)); - std::string reserialized = webrtc::SdpSerialize(jdesc, false); + std::string reserialized = webrtc::SdpSerialize(jdesc); EXPECT_TRUE(SdpDeserialize(reserialized, &jdesc)); // Verify. @@ -2912,21 +2912,21 @@ TEST_F(WebRtcSdpTest, DeserializeSerializeCodecParams) { params.useinband = 1; params.maxaveragebitrate = 128000; TestDeserializeCodecParams(params, &jdesc_output); - TestSerialize(jdesc_output, false); + TestSerialize(jdesc_output); } TEST_F(WebRtcSdpTest, DeserializeSerializeRtcpFb) { const bool kUseWildcard = false; JsepSessionDescription jdesc_output(kDummyType); TestDeserializeRtcpFb(&jdesc_output, kUseWildcard); - TestSerialize(jdesc_output, false); + TestSerialize(jdesc_output); } TEST_F(WebRtcSdpTest, DeserializeSerializeRtcpFbWildcard) { const bool kUseWildcard = true; JsepSessionDescription jdesc_output(kDummyType); TestDeserializeRtcpFb(&jdesc_output, kUseWildcard); - TestSerialize(jdesc_output, false); + TestSerialize(jdesc_output); } TEST_F(WebRtcSdpTest, DeserializeVideoFmtp) { @@ -3039,7 +3039,7 @@ TEST_F(WebRtcSdpTest, SerializeAudioFmtpWithUnknownParameter) { ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(), jdesc_.session_version())); - std::string message = webrtc::SdpSerialize(jdesc_, false); + std::string message = webrtc::SdpSerialize(jdesc_); std::string sdp_with_fmtp = kSdpFullString; InjectAfter("a=rtpmap:111 opus/48000/2\r\n", "a=fmtp:111 unknown-future-parameter=SomeFutureValue\r\n", @@ -3057,7 +3057,7 @@ TEST_F(WebRtcSdpTest, SerializeAudioFmtpWithKnownFmtpParameter) { ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(), jdesc_.session_version())); - std::string message = webrtc::SdpSerialize(jdesc_, false); + std::string message = webrtc::SdpSerialize(jdesc_); std::string sdp_with_fmtp = kSdpFullString; InjectAfter("a=rtpmap:111 opus/48000/2\r\n", "a=fmtp:111 stereo=1\r\n", @@ -3076,7 +3076,7 @@ TEST_F(WebRtcSdpTest, SerializeAudioFmtpWithPTimeAndMaxPTime) { ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(), jdesc_.session_version())); - std::string message = webrtc::SdpSerialize(jdesc_, false); + std::string message = webrtc::SdpSerialize(jdesc_); std::string sdp_with_fmtp = kSdpFullString; InjectAfter("a=rtpmap:104 ISAC/32000\r\n", "a=maxptime:120\r\n" // No comma here. String merging! @@ -3095,7 +3095,7 @@ TEST_F(WebRtcSdpTest, SerializeVideoFmtp) { ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(), jdesc_.session_version())); - std::string message = webrtc::SdpSerialize(jdesc_, false); + std::string message = webrtc::SdpSerialize(jdesc_); std::string sdp_with_fmtp = kSdpFullString; InjectAfter("a=rtpmap:120 VP8/90000\r\n", "a=fmtp:120 x-google-min-bitrate=10\r\n", @@ -3135,7 +3135,7 @@ TEST_F(WebRtcSdpTest, RoundTripSdpWithSctpDataChannelsWithCandidates) { JsepSessionDescription jdesc_output(kDummyType); EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output)); - EXPECT_EQ(sdp_with_data, webrtc::SdpSerialize(jdesc_output, false)); + EXPECT_EQ(sdp_with_data, webrtc::SdpSerialize(jdesc_output)); } TEST_F(WebRtcSdpTest, SerializeDtlsSetupAttribute) { @@ -3163,7 +3163,7 @@ TEST_F(WebRtcSdpTest, SerializeDtlsSetupAttribute) { ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(), jdesc_.session_version())); - std::string message = webrtc::SdpSerialize(jdesc_, false); + std::string message = webrtc::SdpSerialize(jdesc_); std::string sdp_with_dtlssetup = kSdpFullString; // Fingerprint attribute is necessary to add DTLS setup attribute. @@ -3234,7 +3234,7 @@ TEST_F(WebRtcSdpTest, MediaContentOrderMaintainedRoundTrip) { EXPECT_EQ(media_types[media_content_in_sdp[i]], mdesc->type()); } - std::string serialized_sdp = webrtc::SdpSerialize(jdesc, false); + std::string serialized_sdp = webrtc::SdpSerialize(jdesc); EXPECT_EQ(sdp_string, serialized_sdp); } } @@ -3264,7 +3264,7 @@ TEST_F(WebRtcSdpTest, IgnoreBundleOnlyWithNonzeroPort) { TEST_F(WebRtcSdpTest, SerializeBundleOnlyAttribute) { MakeBundleOnlyDescription(); - TestSerialize(jdesc_, false); + TestSerialize(jdesc_); } TEST_F(WebRtcSdpTest, DeserializePlanBSessionDescription) { @@ -3278,7 +3278,7 @@ TEST_F(WebRtcSdpTest, DeserializePlanBSessionDescription) { TEST_F(WebRtcSdpTest, SerializePlanBSessionDescription) { MakePlanBDescription(); - TestSerialize(jdesc_, false); + TestSerialize(jdesc_); } // Some WebRTC endpoints include the msid in both the Plan B and Unified Plan @@ -3307,7 +3307,66 @@ TEST_F(WebRtcSdpTest, DeserializeUnifiedPlanSessionDescription) { TEST_F(WebRtcSdpTest, SerializeUnifiedPlanSessionDescription) { MakeUnifiedPlanDescription(); - TestSerialize(jdesc_, true); + TestSerialize(jdesc_); +} + +TEST_F(WebRtcSdpTest, EmptyDescriptionHasNoMsidSignaling) { + JsepSessionDescription jsep_desc(kDummyType); + ASSERT_TRUE(SdpDeserialize(kSdpSessionString, &jsep_desc)); + EXPECT_EQ(0, jsep_desc.description()->msid_signaling()); +} + +TEST_F(WebRtcSdpTest, DataChannelOnlyHasNoMsidSignaling) { + JsepSessionDescription jsep_desc(kDummyType); + std::string sdp = kSdpSessionString; + sdp += kSdpSctpDataChannelString; + ASSERT_TRUE(SdpDeserialize(sdp, &jsep_desc)); + EXPECT_EQ(0, jsep_desc.description()->msid_signaling()); +} + +TEST_F(WebRtcSdpTest, PlanBHasSsrcAttributeMsidSignaling) { + JsepSessionDescription jsep_desc(kDummyType); + ASSERT_TRUE(SdpDeserialize(kPlanBSdpFullString, &jsep_desc)); + EXPECT_EQ(cricket::kMsidSignalingSsrcAttribute, + jsep_desc.description()->msid_signaling()); +} + +TEST_F(WebRtcSdpTest, UnifiedPlanHasMediaSectionMsidSignaling) { + JsepSessionDescription jsep_desc(kDummyType); + ASSERT_TRUE(SdpDeserialize(kUnifiedPlanSdpFullString, &jsep_desc)); + EXPECT_EQ(cricket::kMsidSignalingMediaSection, + jsep_desc.description()->msid_signaling()); +} + +const char kMediaSectionMsidLine[] = "a=msid:local_stream_1 audio_track_id_1"; +const char kSsrcAttributeMsidLine[] = + "a=ssrc:1 msid:local_stream_1 audio_track_id_1"; + +TEST_F(WebRtcSdpTest, SerializeOnlyMediaSectionMsid) { + jdesc_.description()->set_msid_signaling(cricket::kMsidSignalingMediaSection); + std::string sdp = webrtc::SdpSerialize(jdesc_); + + EXPECT_NE(std::string::npos, sdp.find(kMediaSectionMsidLine)); + EXPECT_EQ(std::string::npos, sdp.find(kSsrcAttributeMsidLine)); +} + +TEST_F(WebRtcSdpTest, SerializeOnlySsrcAttributeMsid) { + jdesc_.description()->set_msid_signaling( + cricket::kMsidSignalingSsrcAttribute); + std::string sdp = webrtc::SdpSerialize(jdesc_); + + EXPECT_EQ(std::string::npos, sdp.find(kMediaSectionMsidLine)); + EXPECT_NE(std::string::npos, sdp.find(kSsrcAttributeMsidLine)); +} + +TEST_F(WebRtcSdpTest, SerializeBothMediaSectionAndSsrcAttributeMsid) { + jdesc_.description()->set_msid_signaling( + cricket::kMsidSignalingMediaSection | + cricket::kMsidSignalingSsrcAttribute); + std::string sdp = webrtc::SdpSerialize(jdesc_); + + EXPECT_NE(std::string::npos, sdp.find(kMediaSectionMsidLine)); + EXPECT_NE(std::string::npos, sdp.find(kSsrcAttributeMsidLine)); } // Regression test for heap overflow bug: @@ -3559,7 +3618,7 @@ TEST_F(WebRtcSdpTest, SerializeAndDeserializeWithConnectionAddress) { JsepSessionDescription expected_jsep(kDummyType); MakeDescriptionWithoutCandidates(&expected_jsep); // Serialization. - std::string message = webrtc::SdpSerialize(expected_jsep, false); + std::string message = webrtc::SdpSerialize(expected_jsep); // Deserialization. JsepSessionDescription jdesc(kDummyType); EXPECT_TRUE(SdpDeserialize(message, &jdesc));