diff --git a/pc/media_session.cc b/pc/media_session.cc index 86b785c91b..bd36c378bd 100644 --- a/pc/media_session.cc +++ b/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::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); + cricket::kMsidSignalingMediaSection); } } else { // Plan B always signals MSID using a=ssrc lines. diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc index 536c71226f..3a6f510864 100644 --- a/pc/sdp_offer_answer_unittest.cc +++ b/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.