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}

pc/media_session.cc

@@ -1713,23 +1713,27 @@ MediaSessionDescriptionFactory::CreateAnswerOrError(
       // a=msid.
       answer->set_msid_signaling(cricket::kMsidSignalingSemantic |
                                  cricket::kMsidSignalingMediaSection);
-    } else if (msid_signaling == cricket::kMsidSignalingSemantic) {
+    } else if (msid_signaling == (cricket::kMsidSignalingSemantic |
+                                  cricket::kMsidSignalingSsrcAttribute) ||
+               msid_signaling == cricket::kMsidSignalingSsrcAttribute) {
+      // If only a=ssrc MSID signaling method was used, we're probably talking
+      // to a Plan B endpoint so respond with just a=ssrc MSID.
+      answer->set_msid_signaling(cricket::kMsidSignalingSemantic |
+                                 cricket::kMsidSignalingSsrcAttribute);
+    } else {
       // We end up here in one of three cases:
       // 1. An empty offer. We'll reply with an empty answer so it doesn't
       //    matter what we pick here.
       // 2. A data channel only offer. We won't add any MSIDs to the answer so
       //    it also doesn't matter what we pick here.
-      // 3. Media that's either sendonly or inactive from the remote endpoint.
+      // 3. Media that's either recvonly or inactive from the remote point of
+      // view.
       //    We don't have any information to say whether the endpoint is Plan B
-      //    or Unified Plan, so be conservative and send both.
+      //    or Unified Plan. Since plan-b is obsolete, do not respond with it.
+      //    We assume that endpoints not supporting MSID will silently ignore
+      //    the a=msid lines they do not understand.
       answer->set_msid_signaling(cricket::kMsidSignalingSemantic |
-                                 cricket::kMsidSignalingMediaSection |
+                                 cricket::kMsidSignalingMediaSection);
-                                 cricket::kMsidSignalingSsrcAttribute);
-    } else {
-      // Otherwise, it's clear which method the offerer is using so repeat that
-      // back to them. This includes the case where the msid-semantic line is
-      // not included.
-      answer->set_msid_signaling(msid_signaling);
     }
   } else {
     // Plan B always signals MSID using a=ssrc lines.

pc/sdp_offer_answer_unittest.cc

@@ -1052,6 +1052,64 @@ TEST_F(SdpOfferAnswerTest, MsidSignalingInSubsequentOfferAnswer) {
   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 @@ TEST_F(SdpOfferAnswerTest, OfferWithNoCompatibleCodecsIsRejectedInAnswer) {
   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.