sdp: parse a=msid:<stream_id> w/o msid-appdata
parse
a=msid:<stream_id>
since JSEP stipulates sending this syntax as track identifers
have become meaningless. The track id will be set to a random string.
a=msid:<stream_id> <track_id>
remains supported for backward compability.
BUG=webrtc:14729
Change-Id: I86c073eb97cd613324271125de18a773235fc79d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/285783
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#38814}
diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc
index e7ac752..a931500 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -2333,33 +2333,46 @@
std::vector<std::string>* stream_ids,
std::string* track_id,
SdpParseError* error) {
- // https://datatracker.ietf.org/doc/draft-ietf-mmusic-msid/16/
- // a=msid:<stream id> <track id>
+ // https://datatracker.ietf.org/doc/rfc8830/
+ // a=msid:<msid-value>
// msid-value = msid-id [ SP msid-appdata ]
// msid-id = 1*64token-char ; see RFC 4566
// msid-appdata = 1*64token-char ; see RFC 4566
- std::string field1;
- std::string new_stream_id;
- std::string new_track_id;
- if (!rtc::tokenize_first(line.substr(kLinePrefixLength),
- kSdpDelimiterSpaceChar, &field1, &new_track_id)) {
- const size_t expected_fields = 2;
- return ParseFailedExpectFieldNum(line, expected_fields, error);
+ // Note that JSEP stipulates not sending msid-appdata so
+ // a=msid:<stream id> <track id>
+ // is supported for backward compability reasons only.
+ std::vector<std::string> fields;
+ size_t num_fields = rtc::tokenize(line.substr(kLinePrefixLength),
+ kSdpDelimiterSpaceChar, &fields);
+ if (num_fields < 1 || num_fields > 2) {
+ return ParseFailed(line, "Expected a stream ID and optionally a track ID",
+ error);
}
+ if (num_fields == 1) {
+ if (line.back() == kSdpDelimiterSpaceChar) {
+ return ParseFailed(line, "Missing track ID in msid attribute.", error);
+ }
+ if (!track_id->empty()) {
+ fields.push_back(*track_id);
+ } else {
+ // Ending with an empty string track will cause a random track id
+ // to be generated later in the process.
+ fields.push_back("");
+ }
+ }
+ RTC_DCHECK_EQ(fields.size(), 2);
- if (new_track_id.empty()) {
- return ParseFailed(line, "Missing track ID in msid attribute.", error);
- }
// All track ids should be the same within an m section in a Unified Plan SDP.
- if (!track_id->empty() && new_track_id.compare(*track_id) != 0) {
+ if (!track_id->empty() && track_id->compare(fields[1]) != 0) {
return ParseFailed(
line, "Two different track IDs in msid attribute in one m= section",
error);
}
- *track_id = new_track_id;
+ *track_id = fields[1];
// msid:<msid-id>
- if (!GetValue(field1, kAttributeMsid, &new_stream_id, error)) {
+ std::string new_stream_id;
+ if (!GetValue(fields[0], kAttributeMsid, &new_stream_id, error)) {
return false;
}
if (new_stream_id.empty()) {
@@ -3330,6 +3343,10 @@
// still create a track. This isn't done for data media types because
// StreamParams aren't used for SCTP streams, and RTP data channels don't
// support unsignaled SSRCs.
+ // If track id was not specified, create a random one.
+ if (track_id.empty()) {
+ track_id = rtc::CreateRandomString(8);
+ }
CreateTrackWithNoSsrcs(stream_ids, track_id, send_rids, &tracks);
}
diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc
index 7880af0..772a97c 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -4073,6 +4073,72 @@
EXPECT_FALSE(SdpDeserialize(kSdpWithMissingTrackId, &jdesc_output));
}
+TEST_F(WebRtcSdpTest, DeserializeMsidAttributeWithoutAppData) {
+ static const char kSdpWithMissingStreamId[] =
+ "v=0\r\n"
+ "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "m=audio 9 RTP/SAVPF 111\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtpmap:111 opus/48000/2\r\n"
+ "a=msid:stream_id\r\n";
+
+ JsepSessionDescription jdesc_output(kDummyType);
+ EXPECT_TRUE(SdpDeserialize(kSdpWithMissingStreamId, &jdesc_output));
+}
+
+TEST_F(WebRtcSdpTest, DeserializeMsidAttributeWithoutAppDataTwoStreams) {
+ static const char kSdpWithMissingStreamId[] =
+ "v=0\r\n"
+ "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "m=audio 9 RTP/SAVPF 111\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtpmap:111 opus/48000/2\r\n"
+ "a=msid:stream_id\r\n"
+ "a=msid:stream_id2\r\n";
+
+ JsepSessionDescription jdesc_output(kDummyType);
+ EXPECT_TRUE(SdpDeserialize(kSdpWithMissingStreamId, &jdesc_output));
+}
+
+TEST_F(WebRtcSdpTest, DeserializeMsidAttributeWithoutAppDataDuplicate) {
+ static const char kSdpWithMissingStreamId[] =
+ "v=0\r\n"
+ "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "m=audio 9 RTP/SAVPF 111\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtpmap:111 opus/48000/2\r\n"
+ "a=msid:stream_id\r\n"
+ "a=msid:stream_id\r\n";
+
+ JsepSessionDescription jdesc_output(kDummyType);
+ // This is somewhat silly but accept it.
+ EXPECT_TRUE(SdpDeserialize(kSdpWithMissingStreamId, &jdesc_output));
+}
+
+TEST_F(WebRtcSdpTest, DeserializeMsidAttributeWithoutAppDataMixed) {
+ static const char kSdpWithMissingStreamId[] =
+ "v=0\r\n"
+ "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "m=audio 9 RTP/SAVPF 111\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtpmap:111 opus/48000/2\r\n"
+ "a=msid:stream_id\r\n"
+ "a=msid:stream_id track_id\r\n";
+
+ JsepSessionDescription jdesc_output(kDummyType);
+ // Mixing the syntax like this is not a good idea but we accept it
+ // and the result is the second track_id.
+ EXPECT_TRUE(SdpDeserialize(kSdpWithMissingStreamId, &jdesc_output));
+}
+
TEST_F(WebRtcSdpTest, DeserializeMsidAttributeWithMissingStreamId) {
static const char kSdpWithMissingStreamId[] =
"v=0\r\n"
