Adds support for "-" to a=ssrc msid lines.
Currently with in Unified Plan an initial offer will include both
"a=ssrc:... msid:..." lines and "a=msid:... ..." lines. The a=ssrc line
is added in order to support signaling to a Plan B endpoint. Although if
no stream is associated to a given track it will only be signaled in the
"a=msid" line with "-". The "a=ssrc msid" line will simply put an empty
string for the msid, which does not interoperate with FF. This change
adds support so that both lines will signal a "-".
Bug: webrtc:9880
Change-Id: I73655ce3c11a924b508616d820555bf24aae1bd3
Reviewed-on: https://webrtc-review.googlesource.com/c/106605
Commit-Queue: Seth Hampson <shampson@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25241}diff --git a/pc/webrtcsdp_unittest.cc b/pc/webrtcsdp_unittest.cc
index e3ccd06..bba10f2 100644
--- a/pc/webrtcsdp_unittest.cc
+++ b/pc/webrtcsdp_unittest.cc
@@ -623,8 +623,8 @@
"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=msid:local_stream_1 audio_track_id_1\r\n"
"a=sendrecv\r\n"
+ "a=msid:local_stream_1 audio_track_id_1\r\n"
"a=rtcp-mux\r\n"
"a=rtcp-rsize\r\n"
"a=crypto:1 AES_CM_128_HMAC_SHA1_32 "
@@ -643,9 +643,9 @@
"a=rtcp:9 IN IP4 0.0.0.0\r\n"
"a=ice-ufrag:ufrag_voice_2\r\na=ice-pwd:pwd_voice_2\r\n"
"a=mid:audio_content_name_2\r\n"
+ "a=sendrecv\r\n"
"a=msid:local_stream_1 audio_track_id_2\r\n"
"a=msid:local_stream_2 audio_track_id_2\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 "
@@ -666,8 +666,8 @@
"a=rtcp:9 IN IP4 0.0.0.0\r\n"
"a=ice-ufrag:ufrag_voice_3\r\na=ice-pwd:pwd_voice_3\r\n"
"a=mid:audio_content_name_3\r\n"
- "a=msid:- audio_track_id_3\r\n"
"a=sendrecv\r\n"
+ "a=msid:- audio_track_id_3\r\n"
"a=rtcp-mux\r\n"
"a=rtcp-rsize\r\n"
"a=crypto:1 AES_CM_128_HMAC_SHA1_32 "
@@ -676,7 +676,10 @@
"a=rtpmap:111 opus/48000/2\r\n"
"a=rtpmap:103 ISAC/16000\r\n"
"a=rtpmap:104 ISAC/32000\r\n"
- "a=ssrc:7 cname:stream_2_cname\r\n";
+ "a=ssrc:7 cname:stream_2_cname\r\n"
+ "a=ssrc:7 msid:- audio_track_id_3\r\n"
+ "a=ssrc:7 mslabel:-\r\n"
+ "a=ssrc:7 label:audio_track_id_3\r\n";
// One candidate reference string as per W3c spec.
// candidate:<blah> not a=candidate:<blah>CRLF
@@ -1162,7 +1165,7 @@
// with 3 audio MediaContentDescriptions with special StreamParams that
// contain 0 or multiple stream ids: - audio track 1 has 1 media stream id -
// audio track 2 has 2 media stream ids - audio track 3 has 0 media stream ids
- void MakeUnifiedPlanDescriptionMultipleStreamIds() {
+ void MakeUnifiedPlanDescriptionMultipleStreamIds(const int msid_signaling) {
desc_.RemoveContentByName(kVideoContentName);
desc_.RemoveTransportInfoByName(kVideoContentName);
RemoveVideoCandidates();
@@ -1190,9 +1193,7 @@
desc_.AddContent(kAudioContentName3, MediaProtocolType::kRtp, audio_desc_3);
EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
kAudioContentName3, TransportDescription(kUfragVoice3, kPwdVoice3))));
- // Make sure to create both a=msid lines.
- desc_.set_msid_signaling(cricket::kMsidSignalingMediaSection |
- cricket::kMsidSignalingSsrcAttribute);
+ desc_.set_msid_signaling(msid_signaling);
ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(),
jdesc_.session_version()));
}
@@ -1547,6 +1548,27 @@
}
}
+ // Removes all a=ssrc lines from the SDP string, except for the
+ // "a=ssrc:... cname:..." lines.
+ void RemoveSsrcMsidLinesFromSdpString(std::string* sdp_string) {
+ const char kAttributeSsrc[] = "a=ssrc";
+ const char kAttributeCname[] = "cname";
+ size_t ssrc_line_pos = sdp_string->find(kAttributeSsrc);
+ while (ssrc_line_pos != std::string::npos) {
+ size_t beg_line_pos = sdp_string->rfind('\n', ssrc_line_pos);
+ size_t end_line_pos = sdp_string->find('\n', ssrc_line_pos);
+ size_t cname_pos = sdp_string->find(kAttributeCname, ssrc_line_pos);
+ if (cname_pos == std::string::npos || cname_pos > end_line_pos) {
+ // Only erase a=ssrc lines that don't contain "cname".
+ sdp_string->erase(beg_line_pos, end_line_pos - beg_line_pos);
+ ssrc_line_pos = sdp_string->find(kAttributeSsrc, beg_line_pos);
+ } else {
+ // Skip the "a=ssrc:... cname" line and find the next "a=ssrc" line.
+ ssrc_line_pos = sdp_string->find(kAttributeSsrc, end_line_pos);
+ }
+ }
+ }
+
// Removes all a=ssrc lines from the SDP string.
void RemoveSsrcLinesFromSdpString(std::string* sdp_string) {
const char kAttributeSsrc[] = "a=ssrc";
@@ -3434,12 +3456,16 @@
}
// This tests deserializing a Unified Plan SDP that is compatible with both
-// Unified Plan and Plan B style SDP. It tests the case for audio/video tracks
+// Unified Plan and Plan B style SDP, meaning that it contains both "a=ssrc
+// msid" lines and "a=msid " lines. It tests the case for audio/video tracks
// with no stream ids and multiple stream ids. For parsing this, the Unified
// Plan a=msid lines should take priority, because the Plan B style a=ssrc msid
// lines do not support multiple stream ids and no stream ids.
-TEST_F(WebRtcSdpTest, DeserializeUnifiedPlanSessionDescriptionSpecialMsid) {
- MakeUnifiedPlanDescriptionMultipleStreamIds();
+TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionSpecialMsid) {
+ // Create both msid lines for Plan B and Unified Plan support.
+ MakeUnifiedPlanDescriptionMultipleStreamIds(
+ cricket::kMsidSignalingMediaSection |
+ cricket::kMsidSignalingSsrcAttribute);
JsepSessionDescription deserialized_description(kDummyType);
EXPECT_TRUE(SdpDeserialize(kUnifiedPlanSdpFullStringWithSpecialMsid,
@@ -3448,8 +3474,50 @@
EXPECT_TRUE(CompareSessionDescription(jdesc_, deserialized_description));
}
-TEST_F(WebRtcSdpTest, SerializeUnifiedPlanSessionDescriptionSpecialMsid) {
- MakeUnifiedPlanDescriptionMultipleStreamIds();
+// Tests the serialization of a Unified Plan SDP that is compatible for both
+// Unified Plan and Plan B style SDPs, meaning that it contains both "a=ssrc
+// msid" lines and "a=msid " lines. It tests the case for no stream ids and
+// multiple stream ids.
+TEST_F(WebRtcSdpTest, SerializeSessionDescriptionSpecialMsid) {
+ // Create both msid lines for Plan B and Unified Plan support.
+ MakeUnifiedPlanDescriptionMultipleStreamIds(
+ cricket::kMsidSignalingMediaSection |
+ cricket::kMsidSignalingSsrcAttribute);
+ std::string serialized_sdp = webrtc::SdpSerialize(jdesc_);
+ // We explicitly test that the serialized SDP string is equal to the hard
+ // coded SDP string. This is necessary, because in the parser "a=msid" lines
+ // take priority over "a=ssrc msid" lines. This means if we just used
+ // TestSerialize(), it could serialize an SDP that omits "a=ssrc msid" lines,
+ // and still pass, because the deserialized version would be the same.
+ EXPECT_EQ(kUnifiedPlanSdpFullStringWithSpecialMsid, serialized_sdp);
+}
+
+// Tests that a Unified Plan style SDP (does not contain "a=ssrc msid" lines
+// that signal stream IDs) is deserialized appropriately. It tests the case for
+// no stream ids and multiple stream ids.
+TEST_F(WebRtcSdpTest, UnifiedPlanDeserializeSessionDescriptionSpecialMsid) {
+ // Only create a=msid lines for strictly Unified Plan stream ID support.
+ MakeUnifiedPlanDescriptionMultipleStreamIds(
+ cricket::kMsidSignalingMediaSection);
+
+ JsepSessionDescription deserialized_description(kDummyType);
+ std::string unified_plan_sdp_string =
+ kUnifiedPlanSdpFullStringWithSpecialMsid;
+ RemoveSsrcMsidLinesFromSdpString(&unified_plan_sdp_string);
+ EXPECT_TRUE(
+ SdpDeserialize(unified_plan_sdp_string, &deserialized_description));
+
+ EXPECT_TRUE(CompareSessionDescription(jdesc_, deserialized_description));
+}
+
+// Tests that a Unified Plan style SDP (does not contain "a=ssrc msid" lines
+// that signal stream IDs) is serialized appropriately. It tests the case for no
+// stream ids and multiple stream ids.
+TEST_F(WebRtcSdpTest, UnifiedPlanSerializeSessionDescriptionSpecialMsid) {
+ // Only create a=msid lines for strictly Unified Plan stream ID support.
+ MakeUnifiedPlanDescriptionMultipleStreamIds(
+ cricket::kMsidSignalingMediaSection);
+
TestSerialize(jdesc_);
}
