Move assignment of a streams random-msid
move this a bit later in the process since the current handling will consider two ssrc-lines with a cname in the same RTX FID ssrc-group to be part of separate streams due to the different randomly assigned msids. This leads to a misdetection as plan-b SDP.
BUG=None
Change-Id: Ie8acce9c2c7fb9eabda479b90e8cc7406dcb1696
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/318820
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#40701}
diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc
index 72ea4f9..03bea2a 100644
--- a/pc/sdp_offer_answer_unittest.cc
+++ b/pc/sdp_offer_answer_unittest.cc
@@ -930,4 +930,33 @@
EXPECT_FALSE(pc->SetLocalDescription(std::move(modified_offer)));
}
+TEST_F(SdpOfferAnswerTest, OfferWithRtxAndNoMsidIsNotRejected) {
+ auto pc = CreatePeerConnection();
+ std::string sdp =
+ "v=0\r\n"
+ "o=- 0 3 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "a=group:BUNDLE 0\r\n"
+ "a=fingerprint:sha-1 "
+ "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
+ "a=setup:actpass\r\n"
+ "a=ice-ufrag:ETEn\r\n"
+ "a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
+ "m=video 9 UDP/TLS/RTP/SAVPF 96 97\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp-mux\r\n"
+ "a=sendonly\r\n"
+ "a=mid:0\r\n"
+ // "a=msid:stream obsoletetrack\r\n"
+ "a=rtpmap:96 VP8/90000\r\n"
+ "a=rtpmap:97 rtx/90000\r\n"
+ "a=fmtp:97 apt=96\r\n"
+ "a=ssrc-group:FID 1 2\r\n"
+ "a=ssrc:1 cname:test\r\n"
+ "a=ssrc:2 cname:test\r\n";
+ auto offer = CreateSessionDescription(SdpType::kOffer, sdp);
+ EXPECT_TRUE(pc->SetRemoteDescription(std::move(offer)));
+}
+
} // namespace webrtc
diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc
index ecba22a..71cd18c 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -706,7 +706,7 @@
absl::string_view msid_track_id,
StreamParamsVec* tracks,
int msid_signaling) {
- RTC_DCHECK(tracks != NULL);
+ RTC_DCHECK(tracks);
for (const SsrcInfo& ssrc_info : ssrc_infos) {
// According to https://tools.ietf.org/html/rfc5576#section-6.1, the CNAME
// attribute is mandatory, but we relax that restriction.
@@ -726,16 +726,9 @@
track_id = ssrc_info.track_id;
} else {
// Since no media streams isn't supported with older SDP signaling, we
- // use a default a stream id.
+ // use a default stream id.
stream_ids.push_back(kDefaultMsid);
}
- // If a track ID wasn't populated from the SSRC attributes OR the
- // msid attribute, use default/random values.
- if (track_id.empty()) {
- // TODO(ronghuawu): What should we do if the track id doesn't appear?
- // Create random string (which will be used as track label later)?
- track_id = rtc::CreateRandomString(8);
- }
auto track_it = absl::c_find_if(
*tracks,
@@ -751,6 +744,14 @@
track.set_stream_ids(stream_ids);
track.id = track_id;
}
+ for (StreamParams& stream : *tracks) {
+ // If a track ID wasn't populated from the SSRC attributes OR the
+ // msid attribute, use default/random values. This happens after
+ // deduplication.
+ if (stream.id.empty()) {
+ stream.id = rtc::CreateRandomString(8);
+ }
+ }
}
void GetMediaStreamIds(const ContentInfo* content,
