diff --git a/webrtc/api/webrtcsdp.cc b/webrtc/api/webrtcsdp.cc index 69a1af28ad..43fdc68836 100644 --- a/webrtc/api/webrtcsdp.cc +++ b/webrtc/api/webrtcsdp.cc @@ -108,6 +108,7 @@ static const char kLineTypeAttributes = 'a'; static const char kAttributeGroup[] = "group"; static const char kAttributeMid[] = "mid"; static const char kAttributeMsid[] = "msid"; +static const char kAttributeBundleOnly[] = "bundle-only"; static const char kAttributeRtcpMux[] = "rtcp-mux"; static const char kAttributeRtcpReducedSize[] = "rtcp-rsize"; static const char kAttributeSsrc[] = "ssrc"; @@ -275,6 +276,7 @@ static bool ParseContent(const std::string& message, const std::vector& payload_types, size_t* pos, std::string* content_name, + bool* bundle_only, MediaContentDescription* media_desc, TransportDescription* transport, std::vector* candidates, @@ -1282,13 +1284,19 @@ void BuildMediaDescription(const ContentInfo* content_info, fmt = " 0"; } - // The port number in the m line will be updated later when associate with + // The port number in the m line will be updated later when associated with // the candidates. + // + // A port value of 0 indicates that the m= section is rejected. // RFC 3264 // To reject an offered stream, the port number in the corresponding stream in // the answer MUST be set to zero. - const std::string& port = content_info->rejected ? - kMediaPortRejected : kDummyPort; + // + // However, the BUNDLE draft adds a new meaning to port zero, when used along + // with a=bundle-only. + const std::string& port = + (content_info->rejected || content_info->bundle_only) ? kMediaPortRejected + : kDummyPort; rtc::SSLFingerprint* fp = (transport_info) ? transport_info->description.identity_fingerprint.get() : NULL; @@ -1307,6 +1315,12 @@ void BuildMediaDescription(const ContentInfo* content_info, AddLine(os.str(), message); } + // Add the a=bundle-only line. + if (content_info->bundle_only) { + InitAttrLine(kAttributeBundleOnly, &os); + AddLine(os.str(), message); + } + // Add the a=rtcp line. if (IsRtp(media_desc->protocol())) { std::string rtcp_line = GetRtcpLine(candidates); @@ -2210,6 +2224,7 @@ static C* ParseContentDescription(const std::string& message, const std::vector& payload_types, size_t* pos, std::string* content_name, + bool* bundle_only, TransportDescription* transport, std::vector* candidates, webrtc::SdpParseError* error) { @@ -2229,8 +2244,8 @@ static C* ParseContentDescription(const std::string& message, break; } if (!ParseContent(message, media_type, mline_index, protocol, payload_types, - pos, content_name, media_desc, transport, candidates, - error)) { + pos, content_name, bundle_only, media_desc, transport, + candidates, error)) { delete media_desc; return NULL; } @@ -2277,12 +2292,12 @@ bool ParseMediaDescription(const std::string& message, if (fields.size() < expected_min_fields) { return ParseFailedExpectMinFieldNum(line, expected_min_fields, error); } - bool rejected = false; + bool port_rejected = false; // RFC 3264 // To reject an offered stream, the port number in the corresponding stream // in the answer MUST be set to zero. if (fields[1] == kMediaPortRejected) { - rejected = true; + port_rejected = true; } std::string protocol = fields[2]; @@ -2314,19 +2329,23 @@ bool ParseMediaDescription(const std::string& message, std::unique_ptr content; std::string content_name; + bool bundle_only = false; if (HasAttribute(line, kMediaTypeVideo)) { content.reset(ParseContentDescription( message, cricket::MEDIA_TYPE_VIDEO, mline_index, protocol, - payload_types, pos, &content_name, &transport, candidates, error)); + payload_types, pos, &content_name, &bundle_only, &transport, + candidates, error)); } else if (HasAttribute(line, kMediaTypeAudio)) { content.reset(ParseContentDescription( message, cricket::MEDIA_TYPE_AUDIO, mline_index, protocol, - payload_types, pos, &content_name, &transport, candidates, error)); + payload_types, pos, &content_name, &bundle_only, &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, &transport, candidates, error); + payload_types, pos, &content_name, &bundle_only, &transport, + candidates, error); content.reset(data_desc); int p; @@ -2343,6 +2362,22 @@ bool ParseMediaDescription(const std::string& message, return false; } + bool content_rejected = false; + if (bundle_only) { + // A port of 0 is not interpreted as a rejected m= section when it's + // used along with a=bundle-only. + if (!port_rejected) { + return ParseFailed( + "a=bundle-only", + "a=bundle-only MUST only be used in combination with a 0 port.", + error); + } + } else { + // If not using bundle-only, interpret port 0 in the normal way; the m= + // section is being rejected. + content_rejected = port_rejected; + } + if (IsRtp(protocol)) { // Set the extmap. if (!session_extmaps.empty() && @@ -2358,10 +2393,9 @@ bool ParseMediaDescription(const std::string& message, } content->set_protocol(protocol); desc->AddContent(content_name, - IsDtlsSctp(protocol) ? cricket::NS_JINGLE_DRAFT_SCTP : - cricket::NS_JINGLE_RTP, - rejected, - content.release()); + IsDtlsSctp(protocol) ? cricket::NS_JINGLE_DRAFT_SCTP + : cricket::NS_JINGLE_RTP, + content_rejected, bundle_only, content.release()); // Create TransportInfo with the media level "ice-pwd" and "ice-ufrag". TransportInfo transport_info(content_name, transport); @@ -2537,6 +2571,7 @@ bool ParseContent(const std::string& message, const std::vector& payload_types, size_t* pos, std::string* content_name, + bool* bundle_only, MediaContentDescription* media_desc, TransportDescription* transport, std::vector* candidates, @@ -2619,6 +2654,8 @@ bool ParseContent(const std::string& message, return false; } *content_name = mline_id; + } else if (HasAttribute(line, kAttributeBundleOnly)) { + *bundle_only = true; } else if (HasAttribute(line, kAttributeCandidate)) { Candidate candidate; if (!ParseCandidate(line, &candidate, error, false)) { diff --git a/webrtc/api/webrtcsdp_unittest.cc b/webrtc/api/webrtcsdp_unittest.cc index 7941323d97..a2d839beb8 100644 --- a/webrtc/api/webrtcsdp_unittest.cc +++ b/webrtc/api/webrtcsdp_unittest.cc @@ -331,6 +331,65 @@ static const char kSdpVideoString[] = "a=ssrc:2 mslabel:local_stream\r\n" "a=ssrc:2 label:video_track_id_1\r\n"; +// Reference sdp string using bundle-only. +static const char kBundleOnlySdpFullString[] = + "v=0\r\n" + "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "a=group:BUNDLE audio_content_name video_content_name\r\n" + "a=msid-semantic: WMS local_stream_1\r\n" + "m=audio 2345 RTP/SAVPF 111 103 104\r\n" + "c=IN IP4 74.125.127.126\r\n" + "a=rtcp:2347 IN IP4 74.125.127.126\r\n" + "a=candidate:a0+B/1 1 udp 2130706432 192.168.1.5 1234 typ host " + "generation 2\r\n" + "a=candidate:a0+B/1 2 udp 2130706432 192.168.1.5 1235 typ host " + "generation 2\r\n" + "a=candidate:a0+B/2 1 udp 2130706432 ::1 1238 typ host " + "generation 2\r\n" + "a=candidate:a0+B/2 2 udp 2130706432 ::1 1239 typ host " + "generation 2\r\n" + "a=candidate:a0+B/3 1 udp 2130706432 74.125.127.126 2345 typ srflx " + "raddr 192.168.1.5 rport 2346 " + "generation 2\r\n" + "a=candidate:a0+B/3 2 udp 2130706432 74.125.127.126 2347 typ srflx " + "raddr 192.168.1.5 rport 2348 " + "generation 2\r\n" + "a=ice-ufrag:ufrag_voice\r\na=ice-pwd:pwd_voice\r\n" + "a=mid:audio_content_name\r\n" + "a=sendrecv\r\n" + "a=rtcp-mux\r\n" + "a=rtcp-rsize\r\n" + "a=crypto:1 AES_CM_128_HMAC_SHA1_32 " + "inline:NzB4d1BINUAvLEw6UzF3WSJ+PSdFcGdUJShpX1Zj|2^20|1:32 " + "dummy_session_params\r\n" + "a=rtpmap:111 opus/48000/2\r\n" + "a=rtpmap:103 ISAC/16000\r\n" + "a=rtpmap:104 ISAC/32000\r\n" + "a=ssrc:1 cname:stream_1_cname\r\n" + "a=ssrc:1 msid:local_stream_1 audio_track_id_1\r\n" + "a=ssrc:1 mslabel:local_stream_1\r\n" + "a=ssrc:1 label:audio_track_id_1\r\n" + "m=video 0 RTP/SAVPF 120\r\n" + "c=IN IP4 0.0.0.0\r\n" + "a=rtcp:9 IN IP4 0.0.0.0\r\n" + "a=bundle-only\r\n" + "a=mid:video_content_name\r\n" + "a=sendrecv\r\n" + "a=crypto:1 AES_CM_128_HMAC_SHA1_80 " + "inline:d0RmdmcmVCspeEc3QGZiNWpVLFJhQX1cfHAwJSoj|2^20|1:32\r\n" + "a=rtpmap:120 VP8/90000\r\n" + "a=ssrc-group:FEC 2 3\r\n" + "a=ssrc:2 cname:stream_1_cname\r\n" + "a=ssrc:2 msid:local_stream_1 video_track_id_1\r\n" + "a=ssrc:2 mslabel:local_stream_1\r\n" + "a=ssrc:2 label:video_track_id_1\r\n" + "a=ssrc:3 cname:stream_1_cname\r\n" + "a=ssrc:3 msid:local_stream_1 video_track_id_1\r\n" + "a=ssrc:3 mslabel:local_stream_1\r\n" + "a=ssrc:3 label:video_track_id_1\r\n"; + // Plan B SDP reference string, with 2 streams, 2 audio tracks and 3 video // tracks. static const char kPlanBSdpFullString[] = @@ -941,6 +1000,42 @@ class WebRtcSdpTest : public testing::Test { } } + // Turns the existing reference description into a description using + // a=bundle-only. This means no transport attributes and a 0 port value on + // the m= sections not associated with the BUNDLE-tag. + void MakeBundleOnlyDescription() { + // Remove video candidates. JsepSessionDescription doesn't make it + // simple. + const IceCandidateCollection* video_candidates_collection = + jdesc_.candidates(1); + ASSERT_NE(nullptr, video_candidates_collection); + std::vector video_candidates; + for (size_t i = 0; i < video_candidates_collection->count(); ++i) { + cricket::Candidate c = video_candidates_collection->at(i)->candidate(); + c.set_transport_name("video_content_name"); + video_candidates.push_back(c); + } + jdesc_.RemoveCandidates(video_candidates); + + // And the rest of the transport attributes. + desc_.transport_infos()[1].description.ice_ufrag.clear(); + desc_.transport_infos()[1].description.ice_pwd.clear(); + desc_.transport_infos()[1].description.connection_role = + cricket::CONNECTIONROLE_NONE; + + // Set bundle-only flag. + desc_.contents()[1].bundle_only = true; + + // Add BUNDLE group. + ContentGroup group(cricket::GROUP_TYPE_BUNDLE); + group.AddContentName(kAudioContentName); + group.AddContentName(kVideoContentName); + desc_.AddGroup(group); + + ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(), + jdesc_.session_version())); + } + // Turns the existing reference description into a plan B description, // with 2 audio tracks and 3 video tracks. void MakePlanBDescription() { @@ -1125,11 +1220,11 @@ class WebRtcSdpTest : public testing::Test { for (size_t i = 0 ; i < desc1.contents().size(); ++i) { const cricket::ContentInfo& c1 = desc1.contents().at(i); const cricket::ContentInfo& c2 = desc2.contents().at(i); - // content name + // ContentInfo properties. EXPECT_EQ(c1.name, c2.name); - // content type - // Note, ASSERT will return from the function, but will not stop the test. - ASSERT_EQ(c1.type, c2.type); + EXPECT_EQ(c1.type, c2.type); + EXPECT_EQ(c1.rejected, c2.rejected); + EXPECT_EQ(c1.bundle_only, c2.bundle_only); ASSERT_EQ(IsAudioContent(&c1), IsAudioContent(&c2)); if (IsAudioContent(&c1)) { @@ -1227,8 +1322,10 @@ class WebRtcSdpTest : public testing::Test { for (size_t i = 0; i < desc1.number_of_mediasections(); ++i) { const IceCandidateCollection* cc1 = desc1.candidates(i); const IceCandidateCollection* cc2 = desc2.candidates(i); - if (cc1->count() != cc2->count()) + if (cc1->count() != cc2->count()) { + ADD_FAILURE(); return false; + } for (size_t j = 0; j < cc1->count(); ++j) { const IceCandidateInterface* c1 = cc1->at(j); const IceCandidateInterface* c2 = cc2->at(j); @@ -3139,6 +3236,27 @@ TEST_F(WebRtcSdpTest, MediaContentOrderMaintainedRoundTrip) { } } +TEST_F(WebRtcSdpTest, DeserializeBundleOnlyAttribute) { + MakeBundleOnlyDescription(); + JsepSessionDescription deserialized_description(kDummyString); + EXPECT_TRUE( + SdpDeserialize(kBundleOnlySdpFullString, &deserialized_description)); + EXPECT_TRUE(CompareSessionDescription(jdesc_, deserialized_description)); +} + +// "a=bundle-only" should only be used in combination with a 0 port on the m= +// line. We should fail to parse anything else. +TEST_F(WebRtcSdpTest, FailToDeserializeBundleOnlyWithNonzeroPort) { + std::string bad_sdp = kBundleOnlySdpFullString; + Replace("m=video 0", "m=video 9", &bad_sdp); + ExpectParseFailure(bad_sdp, "a=bundle-only"); +} + +TEST_F(WebRtcSdpTest, SerializeBundleOnlyAttribute) { + MakeBundleOnlyDescription(); + TestSerialize(jdesc_, false); +} + TEST_F(WebRtcSdpTest, DeserializePlanBSessionDescription) { MakePlanBDescription(); diff --git a/webrtc/p2p/base/sessiondescription.cc b/webrtc/p2p/base/sessiondescription.cc index 5320b0596a..1e69c83e76 100644 --- a/webrtc/p2p/base/sessiondescription.cc +++ b/webrtc/p2p/base/sessiondescription.cc @@ -132,6 +132,15 @@ void SessionDescription::AddContent(const std::string& name, contents_.push_back(ContentInfo(name, type, rejected, description)); } +void SessionDescription::AddContent(const std::string& name, + const std::string& type, + bool rejected, + bool bundle_only, + ContentDescription* description) { + contents_.push_back( + ContentInfo(name, type, rejected, bundle_only, description)); +} + bool SessionDescription::RemoveContentByName(const std::string& name) { for (ContentInfos::iterator content = contents_.begin(); content != contents_.end(); ++content) { diff --git a/webrtc/p2p/base/sessiondescription.h b/webrtc/p2p/base/sessiondescription.h index 7880167569..a20f7b7132 100644 --- a/webrtc/p2p/base/sessiondescription.h +++ b/webrtc/p2p/base/sessiondescription.h @@ -32,20 +32,31 @@ class ContentDescription { // name = name of // type = xmlns of struct ContentInfo { - ContentInfo() : description(NULL) {} + ContentInfo() {} ContentInfo(const std::string& name, const std::string& type, - ContentDescription* description) : - name(name), type(type), rejected(false), description(description) {} + ContentDescription* description) + : name(name), type(type), description(description) {} ContentInfo(const std::string& name, const std::string& type, bool rejected, ContentDescription* description) : name(name), type(type), rejected(rejected), description(description) {} + ContentInfo(const std::string& name, + const std::string& type, + bool rejected, + bool bundle_only, + ContentDescription* description) + : name(name), + type(type), + rejected(rejected), + bundle_only(bundle_only), + description(description) {} std::string name; std::string type; - bool rejected; - ContentDescription* description; + bool rejected = false; + bool bundle_only = false; + ContentDescription* description = nullptr; }; typedef std::vector ContentNames; @@ -127,6 +138,11 @@ class SessionDescription { const std::string& type, bool rejected, ContentDescription* description); + void AddContent(const std::string& name, + const std::string& type, + bool rejected, + bool bundle_only, + ContentDescription* description); bool RemoveContentByName(const std::string& name); // Transport accessors.