Adds support for signaling a=msid lines without a=ssrc lines.
Currently in the SDP we require an a=ssrc line in the m= section in
order for a StreamParams object to be created with that
MediaContentDescription. This change creates a StreamParams object
without ssrcs in the case that a=msid lines are signaled, but ssrcs
are not. When the remote description is set, this allows us to store
the "unsignaled" StreamParams object in the media channel to later
be used when the first packet is received and we create the
receive stream.
Bug: webrtc:7932, webrtc:7933
Change-Id: Ib6734abeee62b8ed688a8208722c402134c074ef
Reviewed-on: https://webrtc-review.googlesource.com/63141
Commit-Queue: Seth Hampson <shampson@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22712}diff --git a/pc/channel.cc b/pc/channel.cc
index 2aab718..a1869c4 100644
--- a/pc/channel.cc
+++ b/pc/channel.cc
@@ -555,7 +555,7 @@
bool ret = true;
for (StreamParamsVec::const_iterator it = local_streams_.begin();
it != local_streams_.end(); ++it) {
- if (!GetStreamBySsrc(streams, it->first_ssrc())) {
+ if (it->has_ssrcs() && !GetStreamBySsrc(streams, it->first_ssrc())) {
if (!media_channel()->RemoveSendStream(it->first_ssrc())) {
std::ostringstream desc;
desc << "Failed to remove send stream with ssrc "
@@ -568,7 +568,7 @@
// Check for new streams.
for (StreamParamsVec::const_iterator it = streams.begin();
it != streams.end(); ++it) {
- if (!GetStreamBySsrc(local_streams_, it->first_ssrc())) {
+ if (it->has_ssrcs() && !GetStreamBySsrc(local_streams_, it->first_ssrc())) {
if (media_channel()->AddSendStream(*it)) {
RTC_LOG(LS_INFO) << "Add send stream ssrc: " << it->ssrcs[0];
} else {
@@ -591,7 +591,10 @@
bool ret = true;
for (StreamParamsVec::const_iterator it = remote_streams_.begin();
it != remote_streams_.end(); ++it) {
- if (!GetStreamBySsrc(streams, it->first_ssrc())) {
+ // If we no longer have an unsignaled stream, we would like to remove
+ // the unsignaled stream params that are cached.
+ if ((!it->has_ssrcs() && !HasStreamWithNoSsrcs(streams)) ||
+ !GetStreamBySsrc(streams, it->first_ssrc())) {
if (!RemoveRecvStream_w(it->first_ssrc())) {
std::ostringstream desc;
desc << "Failed to remove remote stream with ssrc "
@@ -604,9 +607,13 @@
// Check for new streams.
for (StreamParamsVec::const_iterator it = streams.begin();
it != streams.end(); ++it) {
- if (!GetStreamBySsrc(remote_streams_, it->first_ssrc())) {
+ // We allow a StreamParams with an empty list of SSRCs, in which case the
+ // MediaChannel will cache the parameters and use them for any unsignaled
+ // stream received later.
+ if ((!it->has_ssrcs() && !HasStreamWithNoSsrcs(remote_streams_)) ||
+ !GetStreamBySsrc(remote_streams_, it->first_ssrc())) {
if (AddRecvStream_w(*it)) {
- RTC_LOG(LS_INFO) << "Add remote ssrc: " << it->ssrcs[0];
+ RTC_LOG(LS_INFO) << "Add remote ssrc: " << it->first_ssrc();
} else {
std::ostringstream desc;
desc << "Failed to add remote stream ssrc: " << it->first_ssrc();
diff --git a/pc/mediasession.h b/pc/mediasession.h
index 72a756a..4f30b7c 100644
--- a/pc/mediasession.h
+++ b/pc/mediasession.h
@@ -35,8 +35,6 @@
// Options for an RtpSender contained with an media description/"m=" section.
struct SenderOptions {
std::string track_id;
- // TODO(steveanton): As part of work towards Unified Plan, this has been
- // changed to be a vector. But for now this can only have exactly one.
std::vector<std::string> stream_ids;
int num_sim_layers;
};
diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index 74f8872..304ab67 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -2386,13 +2386,8 @@
// process the addition of a remote track for the media description.
std::vector<std::string> stream_ids;
if (!media_desc->streams().empty()) {
- // TODO(bugs.webrtc.org/7932): Currently stream ids are all populated
- // within StreamParams. When they are updated to be stored within the
- // MediaContentDescription, change the logic here.
- const cricket::StreamParams& stream_params = media_desc->streams()[0];
- if (!stream_params.stream_ids().empty()) {
- stream_ids = stream_params.stream_ids();
- }
+ // The remote description has signaled the stream IDs.
+ stream_ids = media_desc->streams()[0].stream_ids();
}
if (RtpTransceiverDirectionHasRecv(local_direction) &&
(!transceiver->current_direction() ||
@@ -2439,7 +2434,8 @@
RtpTransceiverDirectionHasRecv(local_direction)) {
// Set ssrc to 0 in the case of an unsignalled ssrc.
uint32_t ssrc = 0;
- if (!media_desc->streams().empty()) {
+ if (!media_desc->streams().empty() &&
+ media_desc->streams()[0].has_ssrcs()) {
ssrc = media_desc->streams()[0].first_ssrc();
}
transceiver->internal()->receiver_internal()->SetupMediaChannel(ssrc);
@@ -4062,11 +4058,19 @@
// Find new and active senders.
for (const cricket::StreamParams& params : streams) {
+ if (!params.has_ssrcs()) {
+ // The remote endpoint has streams, but didn't signal ssrcs. For an active
+ // sender, this means it is coming from a Unified Plan endpoint,so we just
+ // create a default.
+ default_sender_needed = true;
+ break;
+ }
+
// |params.id| is the sender id and the stream id uses the first of
// |params.stream_ids|. The remote description could come from a Unified
- // Plan endpoint, with multiple or no stream_ids() signalled. Since this is
- // not supported in Plan B, we just take the first here and create an empty
- // stream id if none is specified.
+ // Plan endpoint, with multiple or no stream_ids() signaled. Since this is
+ // not supported in Plan B, we just take the first here and create the
+ // default stream ID if none is specified.
const std::string& stream_id =
(!params.stream_ids().empty() ? params.stream_ids()[0]
: kDefaultStreamId);
diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc
index a249d79..d9cad53 100644
--- a/pc/peerconnection_integrationtest.cc
+++ b/pc/peerconnection_integrationtest.cc
@@ -136,6 +136,21 @@
desc->set_msid_supported(false);
}
+// Removes all stream information besides the stream ids, simulating an
+// endpoint that only signals a=msid lines to convey stream_ids.
+void RemoveSsrcsAndKeepMsids(cricket::SessionDescription* desc) {
+ for (ContentInfo& content : desc->contents()) {
+ std::vector<std::string> stream_ids;
+ if (!content.media_description()->streams().empty()) {
+ stream_ids = content.media_description()->streams()[0].stream_ids();
+ }
+ content.media_description()->mutable_streams().clear();
+ cricket::StreamParams new_stream;
+ new_stream.set_stream_ids(stream_ids);
+ content.media_description()->AddStream(new_stream);
+ }
+}
+
int FindFirstMediaStatsIndexByKind(
const std::string& kind,
const std::vector<const webrtc::RTCMediaStreamTrackStats*>&
@@ -2181,6 +2196,27 @@
ASSERT_TRUE(ExpectNewFrames(media_expectations));
}
+// Basic end-to-end test, without SSRC signaling. This means that the track
+// was created properly and frames are delivered when the MSIDs are communicated
+// with a=msid lines and no a=ssrc lines.
+TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
+ EndToEndCallWithoutSsrcSignaling) {
+ const char kStreamId[] = "streamId";
+ ASSERT_TRUE(CreatePeerConnectionWrappers());
+ ConnectFakeSignaling();
+ // Add just audio tracks.
+ caller()->AddTrack(caller()->CreateLocalAudioTrack(), {kStreamId});
+ callee()->AddAudioTrack();
+
+ // Remove SSRCs from the received offer SDP.
+ callee()->SetReceivedSdpMunger(RemoveSsrcsAndKeepMsids);
+ caller()->CreateAndSetAndSignalOffer();
+ ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
+ MediaExpectations media_expectations;
+ media_expectations.ExpectBidirectionalAudio();
+ ASSERT_TRUE(ExpectNewFrames(media_expectations));
+}
+
// 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/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc
index bcfe0d8..5f4f34e 100644
--- a/pc/peerconnection_rtp_unittest.cc
+++ b/pc/peerconnection_rtp_unittest.cc
@@ -111,9 +111,7 @@
auto callee = CreatePeerConnection();
ASSERT_TRUE(caller->AddTrack(caller->CreateAudioTrack("audio_track")));
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_EQ(callee->observer()->add_track_events_.size(), 1u);
// Since we are not supporting the no stream case with Plan B, there should be
@@ -130,9 +128,7 @@
ASSERT_TRUE(caller->AddTrack(caller->CreateAudioTrack("audio_track"),
{"audio_stream"}));
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_EQ(callee->observer()->add_track_events_.size(), 1u);
auto& add_track_event = callee->observer()->add_track_events_[0];
@@ -148,14 +144,10 @@
auto callee = CreatePeerConnection();
auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"), {});
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_EQ(callee->observer()->add_track_events_.size(), 1u);
EXPECT_TRUE(caller->pc()->RemoveTrack(sender));
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_EQ(callee->observer()->add_track_events_.size(), 1u);
EXPECT_EQ(callee->observer()->GetAddTrackReceivers(),
@@ -169,14 +161,10 @@
auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"),
{"audio_stream"});
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_EQ(callee->observer()->add_track_events_.size(), 1u);
EXPECT_TRUE(caller->pc()->RemoveTrack(sender));
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_EQ(callee->observer()->add_track_events_.size(), 1u);
EXPECT_EQ(callee->observer()->GetAddTrackReceivers(),
@@ -193,17 +181,13 @@
{kSharedStreamId});
auto sender2 = caller->AddTrack(caller->CreateAudioTrack("audio_track2"),
{kSharedStreamId});
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_EQ(callee->observer()->add_track_events_.size(), 2u);
// Remove "audio_track1".
EXPECT_TRUE(caller->pc()->RemoveTrack(sender1));
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_EQ(callee->observer()->add_track_events_.size(), 2u);
EXPECT_EQ(
std::vector<rtc::scoped_refptr<RtpReceiverInterface>>{
@@ -212,9 +196,7 @@
// Remove "audio_track2".
EXPECT_TRUE(caller->pc()->RemoveTrack(sender2));
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_EQ(callee->observer()->add_track_events_.size(), 2u);
EXPECT_EQ(callee->observer()->GetAddTrackReceivers(),
callee->observer()->remove_track_events_);
@@ -230,9 +212,7 @@
const char kStreamId1[] = "stream1";
const char kStreamId2[] = "stream2";
caller->AddTrack(caller->CreateAudioTrack("audio_track1"), {kStreamId1});
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
EXPECT_EQ(callee->observer()->add_track_events_.size(), 1u);
// Change the stream ID of the sender in the session description.
@@ -241,8 +221,7 @@
ASSERT_EQ(audio_desc->mutable_streams().size(), 1u);
audio_desc->mutable_streams()[0].set_stream_ids({kStreamId2});
ASSERT_TRUE(
- callee->SetRemoteDescription(CloneSessionDescription(offer.get()),
- static_cast<webrtc::RTCError*>(nullptr)));
+ callee->SetRemoteDescription(CloneSessionDescription(offer.get())));
ASSERT_EQ(callee->observer()->add_track_events_.size(), 2u);
EXPECT_EQ(callee->observer()->add_track_events_[1].streams[0]->id(),
@@ -376,9 +355,7 @@
auto callee = CreatePeerConnection();
ASSERT_TRUE(caller->AddTrack(caller->CreateAudioTrack("audio_track"), {}));
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
EXPECT_EQ(callee->pc()->GetReceivers().size(), 1u);
auto receiver_added = callee->pc()->GetReceivers()[0];
@@ -395,9 +372,7 @@
ASSERT_TRUE(caller->AddTrack(caller->CreateAudioTrack("audio_track"),
{"audio_stream"}));
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
EXPECT_EQ(callee->pc()->GetReceivers().size(), 1u);
auto receiver_added = callee->pc()->GetReceivers()[0];
@@ -414,15 +389,11 @@
auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"), {});
ASSERT_TRUE(sender);
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_EQ(callee->pc()->GetReceivers().size(), 1u);
auto receiver = callee->pc()->GetReceivers()[0];
ASSERT_TRUE(caller->pc()->RemoveTrack(sender));
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
// TODO(hbos): When we implement Unified Plan, receivers will not be removed.
// Instead, the transceiver owning the receiver will become inactive.
@@ -436,15 +407,11 @@
auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"),
{"audio_stream"});
ASSERT_TRUE(sender);
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_EQ(callee->pc()->GetReceivers().size(), 1u);
auto receiver = callee->pc()->GetReceivers()[0];
ASSERT_TRUE(caller->pc()->RemoveTrack(sender));
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
// TODO(hbos): When we implement Unified Plan, receivers will not be removed.
// Instead, the transceiver owning the receiver will become inactive.
@@ -461,9 +428,7 @@
{kSharedStreamId});
auto sender2 = caller->AddTrack(caller->CreateAudioTrack("audio_track2"),
{kSharedStreamId});
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_EQ(callee->pc()->GetReceivers().size(), 2u);
rtc::scoped_refptr<webrtc::RtpReceiverInterface> receiver1;
@@ -480,9 +445,7 @@
// Remove "audio_track1".
EXPECT_TRUE(caller->pc()->RemoveTrack(sender1));
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
// Only |receiver2| should remain.
// TODO(hbos): When we implement Unified Plan, receivers will not be removed.
// Instead, the transceiver owning the receiver will become inactive.
@@ -492,9 +455,7 @@
// Remove "audio_track2".
EXPECT_TRUE(caller->pc()->RemoveTrack(sender2));
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
// TODO(hbos): When we implement Unified Plan, receivers will not be removed.
// Instead, the transceiver owning the receiver will become inactive.
EXPECT_EQ(callee->pc()->GetReceivers().size(), 0u);
@@ -553,6 +514,40 @@
EXPECT_TRUE_WAIT(srd2_callback_called, kDefaultTimeout);
}
+// Tests that a remote track is created with the signaled MSIDs when they are
+// communicated with a=msid and no SSRCs are signaled at all (i.e., no a=ssrc
+// lines).
+TEST_F(PeerConnectionRtpObserverTest, UnsignaledSsrcCreatesReceiverStreams) {
+ auto caller = CreatePeerConnectionWithUnifiedPlan();
+ auto callee = CreatePeerConnectionWithUnifiedPlan();
+ const char kStreamId1[] = "stream1";
+ const char kStreamId2[] = "stream2";
+ caller->AddTrack(caller->CreateAudioTrack("audio_track1"),
+ {kStreamId1, kStreamId2});
+
+ auto offer = caller->CreateOfferAndSetAsLocal();
+ // Munge the offer to take out everything but the stream_ids.
+ auto contents = offer->description()->contents();
+ ASSERT_TRUE(!contents.empty());
+ ASSERT_TRUE(!contents[0].media_description()->streams().empty());
+ std::vector<std::string> stream_ids =
+ contents[0].media_description()->streams()[0].stream_ids();
+ contents[0].media_description()->mutable_streams().clear();
+ cricket::StreamParams new_stream;
+ new_stream.set_stream_ids(stream_ids);
+ contents[0].media_description()->AddStream(new_stream);
+
+ // Set the remote description and verify that the streams were added to the
+ // receiver correctly.
+ ASSERT_TRUE(
+ callee->SetRemoteDescription(CloneSessionDescription(offer.get())));
+ auto receivers = callee->pc()->GetReceivers();
+ ASSERT_EQ(receivers.size(), 1u);
+ ASSERT_EQ(receivers[0]->streams().size(), 2u);
+ EXPECT_EQ(receivers[0]->streams()[0]->id(), kStreamId1);
+ EXPECT_EQ(receivers[0]->streams()[1]->id(), kStreamId2);
+}
+
// Tests that with Unified Plan if the the stream id changes for a track when
// when setting a new remote description, that the media stream is updated
// appropriately for the receiver.
@@ -563,9 +558,7 @@
const char kStreamId1[] = "stream1";
const char kStreamId2[] = "stream2";
caller->AddTrack(caller->CreateAudioTrack("audio_track1"), {kStreamId1});
- ASSERT_TRUE(
- callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
- static_cast<webrtc::RTCError*>(nullptr)));
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
EXPECT_EQ(callee->observer()->add_track_events_.size(), 1u);
// Change the stream id of the sender in the session description.
@@ -578,8 +571,7 @@
// Set the remote description and verify that the stream was updated properly.
ASSERT_TRUE(
- callee->SetRemoteDescription(CloneSessionDescription(offer.get()),
- static_cast<webrtc::RTCError*>(nullptr)));
+ callee->SetRemoteDescription(CloneSessionDescription(offer.get())));
auto receivers = callee->pc()->GetReceivers();
ASSERT_EQ(receivers.size(), 1u);
ASSERT_EQ(receivers[0]->streams().size(), 1u);
diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc
index c040353..4728819 100644
--- a/pc/peerconnectioninterface_unittest.cc
+++ b/pc/peerconnectioninterface_unittest.cc
@@ -3076,6 +3076,27 @@
EXPECT_EQ(0u, observer_.remote_streams()->count());
}
+// This tests that a default MediaStream is created if a remote SDP comes from
+// an endpoint that doesn't signal SSRCs, but signals media stream IDs.
+TEST_F(PeerConnectionInterfaceTestPlanB,
+ SdpWithMsidWithoutSsrcCreatesDefaultStream) {
+ FakeConstraints constraints;
+ constraints.AddMandatory(webrtc::MediaConstraintsInterface::kEnableDtlsSrtp,
+ true);
+ CreatePeerConnection(&constraints);
+ std::string sdp_string = kSdpStringWithoutStreamsAudioOnly;
+ // Add a=msid lines to simulate a Unified Plan endpoint that only
+ // signals stream IDs with a=msid lines.
+ sdp_string.append("a=msid:audio_stream_id audio_track_id\n");
+
+ CreateAndSetRemoteOffer(sdp_string);
+
+ ASSERT_EQ(1u, observer_.remote_streams()->count());
+ MediaStreamInterface* remote_stream = observer_.remote_streams()->at(0);
+ EXPECT_EQ("default", remote_stream->id());
+ ASSERT_EQ(1u, remote_stream->GetAudioTracks().size());
+}
+
// This tests that when a Plan B endpoint receives an SDP that signals no media
// stream IDs indicated by the special character "-" in the a=msid line, that
// a default stream ID will be used for the MediaStream ID. This can occur
diff --git a/pc/webrtcsdp.cc b/pc/webrtcsdp.cc
index a0887a6..2c34728 100644
--- a/pc/webrtcsdp.cc
+++ b/pc/webrtcsdp.cc
@@ -580,15 +580,26 @@
cricket::IsValidRtpPayloadType(*payload_type);
}
-// |msid_stream_idss| and |msid_track_id| represent the stream/track ID from the
+// Creates a StreamParams track in the case when no SSRC lines are signaled.
+// This is a track that does not contain SSRCs and only contains
+// stream_ids/track_id if it's signaled with a=msid lines.
+void CreateTrackWithNoSsrcs(const std::vector<std::string>& msid_stream_ids,
+ const std::string& msid_track_id,
+ StreamParamsVec* tracks) {
+ StreamParams track;
+ if (msid_track_id.empty() || msid_stream_ids.empty()) {
+ // We only create an unsignaled track if a=msid lines were signaled.
+ return;
+ }
+ track.set_stream_ids(msid_stream_ids);
+ track.id = msid_track_id;
+ tracks->push_back(track);
+}
+
+// Creates the StreamParams tracks, for the case when SSRC lines are signaled.
+// |msid_stream_ids| and |msid_track_id| represent the stream/track ID from the
// "a=msid" attribute, if it exists. They are empty if the attribute does not
-// exist.
-// TODO(bugs.webrtc.org/7932): Currently we require that an a=ssrc line is
-// signalled in order to add the appropriate stream_ids. Update here to add both
-// StreamParams objects for the a=ssrc msid lines, and add the
-// stream_id/track_id, to the MediaContentDescription for the a=msid lines. This
-// way the logic will get pushed out to peerconnection.cc for interpreting the
-// media stream ids.
+// exist. We prioritize getting stream_ids/track_ids signaled in a=msid lines.
void CreateTracksFromSsrcInfos(const SsrcInfoVec& ssrc_infos,
const std::vector<std::string>& msid_stream_ids,
const std::string& msid_track_id,
@@ -2945,8 +2956,17 @@
// If the stream_id/track_id for all SSRCS are identical, one StreamParams
// will be created in CreateTracksFromSsrcInfos, containing all the SSRCs from
// the m= section.
- CreateTracksFromSsrcInfos(ssrc_infos, stream_ids, track_id, &tracks,
- *msid_signaling);
+ if (!ssrc_infos.empty()) {
+ CreateTracksFromSsrcInfos(ssrc_infos, stream_ids, track_id, &tracks,
+ *msid_signaling);
+ } else if (media_type != cricket::MEDIA_TYPE_DATA &&
+ (*msid_signaling & cricket::kMsidSignalingMediaSection)) {
+ // If the stream_ids/track_id was signaled but SSRCs were unsignaled we
+ // still create a track. This isn't done for data media types because
+ // StreamParams aren't used for SCTP streams, and RTP data channels don't
+ // support unsignaled SSRCs.
+ CreateTrackWithNoSsrcs(stream_ids, track_id, &tracks);
+ }
// Add the ssrc group to the track.
for (SsrcGroupVec::iterator ssrc_group = ssrc_groups.begin();
diff --git a/pc/webrtcsdp_unittest.cc b/pc/webrtcsdp_unittest.cc
index 9d6cfa3..7a974c4 100644
--- a/pc/webrtcsdp_unittest.cc
+++ b/pc/webrtcsdp_unittest.cc
@@ -1191,6 +1191,28 @@
jdesc_.session_version()));
}
+ // Turns the existing reference description into a unified plan description
+ // with one audio MediaContentDescription that contains one StreamParams with
+ // 0 ssrcs.
+ void MakeUnifiedPlanDescriptionNoSsrcSignaling() {
+ desc_.RemoveContentByName(kVideoContentName);
+ desc_.RemoveContentByName(kAudioContentName);
+ desc_.RemoveTransportInfoByName(kVideoContentName);
+ RemoveVideoCandidates();
+
+ AudioContentDescription* audio_desc = CreateAudioContentDescription();
+ StreamParams audio_track;
+ audio_track.id = kAudioTrackId1;
+ audio_track.set_stream_ids({kStreamId1});
+ audio_desc->AddStream(audio_track);
+ desc_.AddContent(kAudioContentName, MediaProtocolType::kRtp, audio_desc);
+
+ // Enable signaling a=msid lines.
+ desc_.set_msid_signaling(cricket::kMsidSignalingMediaSection);
+ ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), jdesc_.session_id(),
+ jdesc_.session_version()));
+ }
+
// Creates a video content description with no streams, and some default
// configuration.
VideoContentDescription* CreateVideoContentDescription() {
@@ -1503,6 +1525,30 @@
video_desc_->set_cryptos(std::vector<CryptoParams>());
}
+ // Removes everything in StreamParams from the session description that is
+ // used for a=ssrc lines.
+ void RemoveSsrcSignalingFromStreamParams() {
+ for (cricket::ContentInfo content_info : jdesc_.description()->contents()) {
+ // With Unified Plan there should be one StreamParams per m= section.
+ StreamParams& stream =
+ content_info.media_description()->mutable_streams()[0];
+ stream.ssrcs.clear();
+ stream.ssrc_groups.clear();
+ stream.cname.clear();
+ }
+ }
+
+ // Removes all a=ssrc lines from the SDP string.
+ void RemoveSsrcLinesFromSdpString(std::string* sdp_string) {
+ const char kAttributeSsrc[] = "a=ssrc";
+ while (sdp_string->find(kAttributeSsrc) != std::string::npos) {
+ size_t pos_ssrc_attribute = sdp_string->find(kAttributeSsrc);
+ size_t beg_line_pos = sdp_string->rfind('\n', pos_ssrc_attribute);
+ size_t end_line_pos = sdp_string->find('\n', pos_ssrc_attribute);
+ sdp_string->erase(beg_line_pos, end_line_pos - beg_line_pos);
+ }
+ }
+
bool TestSerializeDirection(RtpTransceiverDirection direction) {
audio_desc_->set_direction(direction);
video_desc_->set_direction(direction);
@@ -3365,6 +3411,30 @@
TestSerialize(jdesc_);
}
+// This tests that a Unified Plan SDP with no a=ssrc lines is
+// serialized/deserialized appropriately. In this case the
+// MediaContentDescription will contain a StreamParams object that doesn't have
+// any SSRCs. Vice versa, this will be created upon deserializing an SDP with no
+// SSRC lines.
+TEST_F(WebRtcSdpTest, DeserializeUnifiedPlanSessionDescriptionNoSsrcSignaling) {
+ MakeUnifiedPlanDescription();
+ RemoveSsrcSignalingFromStreamParams();
+ std::string unified_plan_sdp_string = kUnifiedPlanSdpFullString;
+ RemoveSsrcLinesFromSdpString(&unified_plan_sdp_string);
+
+ JsepSessionDescription deserialized_description(kDummyType);
+ EXPECT_TRUE(
+ SdpDeserialize(unified_plan_sdp_string, &deserialized_description));
+ EXPECT_TRUE(CompareSessionDescription(jdesc_, deserialized_description));
+}
+
+TEST_F(WebRtcSdpTest, SerializeUnifiedPlanSessionDescriptionNoSsrcSignaling) {
+ MakeUnifiedPlanDescription();
+ RemoveSsrcSignalingFromStreamParams();
+
+ TestSerialize(jdesc_);
+}
+
TEST_F(WebRtcSdpTest, EmptyDescriptionHasNoMsidSignaling) {
JsepSessionDescription jsep_desc(kDummyType);
ASSERT_TRUE(SdpDeserialize(kSdpSessionString, &jsep_desc));