From 5b1477839d8569291b88dfe950089d0ebf34bc8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Tue, 4 Dec 2018 11:25:05 +0100 Subject: [PATCH] [Unified Plan] If "a=msid" is missing, create default stream. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this CL, if the "a=msid" attribute was missing it was treated the same as if "no streams" were explicitly signaled (a=msid:-); the receivers would not be associated with any streams. In order to support legacy endpoints that don't recognize "a=msid" that assume the Plan B behavior of a stream being created anyway, this CL creates a stream with a random ID in such cases. For background, see https://github.com/web-platform-tests/wpt/pull/14054. Bug: chromium:907508 Change-Id: I9d9dd0e4ba8f9941f8652f4d7873adc560777cd9 Reviewed-on: https://webrtc-review.googlesource.com/c/112900 Reviewed-by: Seth Hampson Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/master@{#25901} --- api/rtpreceiverinterface.h | 1 + pc/peerconnection.cc | 12 +++++++++++ pc/peerconnection.h | 5 +++++ pc/peerconnection_integrationtest.cc | 32 ++++++++++++++++++++++++++++ pc/webrtcsdp_unittest.cc | 5 +++++ 5 files changed, 55 insertions(+) diff --git a/api/rtpreceiverinterface.h b/api/rtpreceiverinterface.h index 266f6ac7dd..2619735ebe 100644 --- a/api/rtpreceiverinterface.h +++ b/api/rtpreceiverinterface.h @@ -146,6 +146,7 @@ class RtpReceiverInterface : public rtc::RefCountInterface { BEGIN_SIGNALING_PROXY_MAP(RtpReceiver) PROXY_SIGNALING_THREAD_DESTRUCTOR() PROXY_CONSTMETHOD0(rtc::scoped_refptr, track) +PROXY_CONSTMETHOD0(std::vector, stream_ids) PROXY_CONSTMETHOD0(std::vector>, streams) PROXY_CONSTMETHOD0(cricket::MediaType, media_type) diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index c81749451c..4a5bd92d14 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -2468,6 +2468,18 @@ RTCError PeerConnection::ApplyRemoteDescription( } media_streams.push_back(stream); } + // Special case: "a=msid" missing, use random stream ID. + if (media_streams.empty() && + !(remote_description()->description()->msid_signaling() & + cricket::kMsidSignalingMediaSection)) { + if (!missing_msid_default_stream_) { + missing_msid_default_stream_ = MediaStreamProxy::Create( + rtc::Thread::Current(), + MediaStream::Create(rtc::CreateRandomUuid())); + added_streams.push_back(missing_msid_default_stream_); + } + media_streams.push_back(missing_msid_default_stream_); + } // This will add the remote track to the streams. // TODO(hbos): When we remove remote_streams(), use set_stream_ids() // instead. https://crbug.com/webrtc/9480 diff --git a/pc/peerconnection.h b/pc/peerconnection.h index f63f4a2a03..b17881c862 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -1033,6 +1033,11 @@ class PeerConnection : public PeerConnectionInternal, std::vector< rtc::scoped_refptr>> transceivers_; + // In Unified Plan, if we encounter remote SDP that does not contain an a=msid + // line we create and use a stream with a random ID for our receivers. This is + // to support legacy endpoints that do not support the a=msid attribute (as + // opposed to streamless tracks with "a=msid:-"). + rtc::scoped_refptr missing_msid_default_stream_; // MIDs that have been seen either by SetLocalDescription or // SetRemoteDescription over the life of the PeerConnection. std::set seen_mids_; diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index dd69aebf56..f0184f1327 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -151,6 +151,7 @@ void RemoveSsrcsAndMsids(cricket::SessionDescription* desc) { content.media_description()->mutable_streams().clear(); } desc->set_msid_supported(false); + desc->set_msid_signaling(0); } // Removes all stream information besides the stream ids, simulating an @@ -2451,6 +2452,37 @@ TEST_F(PeerConnectionIntegrationTestUnifiedPlan, EXPECT_TRUE(ExpectNewFrames(media_expectations)); } +TEST_F(PeerConnectionIntegrationTestUnifiedPlan, NoStreamsMsidLinePresent) { + ASSERT_TRUE(CreatePeerConnectionWrappers()); + ConnectFakeSignaling(); + caller()->AddAudioTrack(); + caller()->AddVideoTrack(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + auto callee_receivers = callee()->pc()->GetReceivers(); + ASSERT_EQ(2u, callee_receivers.size()); + EXPECT_TRUE(callee_receivers[0]->stream_ids().empty()); + EXPECT_TRUE(callee_receivers[1]->stream_ids().empty()); +} + +TEST_F(PeerConnectionIntegrationTestUnifiedPlan, NoStreamsMsidLineMissing) { + ASSERT_TRUE(CreatePeerConnectionWrappers()); + ConnectFakeSignaling(); + caller()->AddAudioTrack(); + caller()->AddVideoTrack(); + callee()->SetReceivedSdpMunger(RemoveSsrcsAndMsids); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + auto callee_receivers = callee()->pc()->GetReceivers(); + ASSERT_EQ(2u, callee_receivers.size()); + ASSERT_EQ(1u, callee_receivers[0]->stream_ids().size()); + ASSERT_EQ(1u, callee_receivers[1]->stream_ids().size()); + EXPECT_EQ(callee_receivers[0]->stream_ids()[0], + callee_receivers[1]->stream_ids()[0]); + EXPECT_EQ(callee_receivers[0]->streams()[0], + callee_receivers[1]->streams()[0]); +} + // Test that if two video tracks are sent (from caller to callee, in this test), // they're transmitted correctly end-to-end. TEST_P(PeerConnectionIntegrationTest, EndToEndCallWithTwoVideoTracks) { diff --git a/pc/webrtcsdp_unittest.cc b/pc/webrtcsdp_unittest.cc index d8b6811b7d..8a3c28fe5b 100644 --- a/pc/webrtcsdp_unittest.cc +++ b/pc/webrtcsdp_unittest.cc @@ -2501,6 +2501,8 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutMsid) { EXPECT_TRUE(SdpDeserialize(sdp_without_msid, &jdesc)); // Verify EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc)); + EXPECT_FALSE(jdesc.description()->msid_signaling() & + ~cricket::kMsidSignalingSsrcAttribute); } TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithExtmapAllowMixed) { @@ -3497,6 +3499,9 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionSpecialMsid) { &deserialized_description)); EXPECT_TRUE(CompareSessionDescription(jdesc_, deserialized_description)); + EXPECT_EQ(cricket::kMsidSignalingMediaSection | + cricket::kMsidSignalingSsrcAttribute, + deserialized_description.description()->msid_signaling()); } // Tests the serialization of a Unified Plan SDP that is compatible for both