sdp: answer with spec msid when msid support is unknown
this removes the reliance on the no-longer-spec a=msid-semantic lines
in case the offer did not signal any msid. Endpoints not supporting
msid should silently ignore the resulting a=msid: line. This also changes behavior such that a "legacy" offer without msid-semantic
line will be responded to with both msid-semantic and msid for any tracks present.
Plan-B ssrc-specific msid attributes are not signalled in that case.
See https://datatracker.ietf.org/doc/html/rfc8829#section-5.3.1
which includes it in the answer depending on the transceiver direction
but not if and only if the offer signalled a msid.
This also avoids recreating the stream and changing the SSRC
which could happen if the answer object was serialized to SDP
(which most unit tests do not do)
BUG=chromium:328522463
Change-Id: Id2f890b7756721d7c50460359950826d392483ae
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/346741
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#42237}
diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc
index 536c712..3a6f510 100644
--- a/pc/sdp_offer_answer_unittest.cc
+++ b/pc/sdp_offer_answer_unittest.cc
@@ -1052,6 +1052,64 @@
EXPECT_NE(std::string::npos, sdp.find("a=msid:- audio_track\r\n"));
}
+// Regression test for crbug.com/328522463
+// where the stream parameters got recreated which changed the ssrc.
+TEST_F(SdpOfferAnswerTest, MsidSignalingUnknownRespondsWithMsidAndKeepsSsrc) {
+ auto pc = CreatePeerConnection();
+ pc->AddAudioTrack("audio_track", {"default"});
+ 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=msid-semantic: WMS *\r\n"
+ "a=ice-ufrag:ETEn\r\n"
+ "a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\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"
+ "m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp:9 IN IP4 0.0.0.0\r\n"
+ "a=recvonly\r\n"
+ "a=rtcp-mux\r\n"
+ "a=mid:0\r\n"
+ "a=rtpmap:111 opus/48000/2\r\n";
+
+ auto offer = CreateSessionDescription(SdpType::kOffer, sdp);
+ EXPECT_TRUE(pc->SetRemoteDescription(std::move(offer)));
+ auto first_transceiver = pc->pc()->GetTransceivers()[0];
+ EXPECT_TRUE(first_transceiver
+ ->SetDirectionWithError(RtpTransceiverDirection::kSendOnly)
+ .ok());
+ // Check the generated *serialized* SDP.
+ auto answer = pc->CreateAnswer();
+ const auto& answer_contents = answer->description()->contents();
+ ASSERT_EQ(answer_contents.size(), 1u);
+ auto answer_streams = answer_contents[0].media_description()->streams();
+ ASSERT_EQ(answer_streams.size(), 1u);
+ std::string first_stream_serialized = answer_streams[0].ToString();
+ uint32_t first_ssrc = answer_contents[0].media_description()->first_ssrc();
+
+ answer->ToString(&sdp);
+ EXPECT_TRUE(
+ pc->SetLocalDescription(CreateSessionDescription(SdpType::kAnswer, sdp)));
+
+ auto reoffer = pc->CreateOffer();
+ const auto& offer_contents = reoffer->description()->contents();
+ ASSERT_EQ(offer_contents.size(), 1u);
+
+ auto offer_streams = offer_contents[0].media_description()->streams();
+ ASSERT_EQ(offer_streams.size(), 1u);
+ std::string second_stream_serialized = offer_streams[0].ToString();
+ uint32_t second_ssrc = offer_contents[0].media_description()->first_ssrc();
+
+ EXPECT_EQ(first_ssrc, second_ssrc);
+ EXPECT_EQ(first_stream_serialized, second_stream_serialized);
+ EXPECT_TRUE(pc->SetLocalDescription(std::move(reoffer)));
+}
+
// Test variant with boolean order for audio-video and video-audio.
class SdpOfferAnswerShuffleMediaTypes
: public SdpOfferAnswerTest,
@@ -1182,39 +1240,6 @@
EXPECT_EQ(answer_contents[1].rejected, true);
}
-TEST_F(SdpOfferAnswerTest,
- OfferWithNoMsidSemanticYieldsAnswerWithoutMsidSemantic) {
- auto pc = CreatePeerConnection();
- // An offer with no msid-semantic line. The answer should not add one.
- 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=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=audio 9 RTP/SAVPF 111\r\n"
- "c=IN IP4 0.0.0.0\r\n"
- "a=sendrecv\r\n"
- "a=rtpmap:111 opus/48000/2\r\n"
- "a=rtcp-mux\r\n";
-
- auto desc = CreateSessionDescription(SdpType::kOffer, sdp);
- ASSERT_NE(desc, nullptr);
- EXPECT_EQ(desc->description()->msid_signaling(),
- cricket::kMsidSignalingNotUsed);
- RTCError error;
- pc->SetRemoteDescription(std::move(desc), &error);
- EXPECT_TRUE(error.ok());
-
- auto answer = pc->CreateAnswer();
- EXPECT_EQ(answer->description()->msid_signaling(),
- cricket::kMsidSignalingNotUsed);
-}
-
TEST_F(SdpOfferAnswerTest, OfferWithRejectedMlineWithoutFingerprintIsAccepted) {
auto pc = CreatePeerConnection();
// A rejected m-line without fingerprint.
