Adds support for multiple or no media stream ids.
With Unified Plan SDP semantics, this adds support for specifying
either no media stream ids or multiple media stream ids for a
transceiver/sender/receiver. This includes serializing/deserializing
SDPs with multiple a=msid lines in a m section, or an "a=msid:-
<appdata>" line to indicate the no stream case. Note that this does
not synchronize between multiple streams, this is still just supported
based upon the first media stream id.
Bug: webrtc:7932, webrtc:7933
Change-Id: Ib7433929af7b2925abe2824b485b360cec12f275
Reviewed-on: https://webrtc-review.googlesource.com/61341
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@{#22611}diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h
index cf9a1f5..6d60b4c 100644
--- a/api/peerconnectioninterface.h
+++ b/api/peerconnectioninterface.h
@@ -1152,13 +1152,6 @@
virtual void OnTrack(
rtc::scoped_refptr<RtpTransceiverInterface> transceiver) {}
- // TODO(hbos,deadbeef): Add |OnAssociatedStreamsUpdated| with |receiver| and
- // |streams| as arguments. This should be called when an existing receiver its
- // associated streams updated. https://crbug.com/webrtc/8315
- // This may be blocked on supporting multiple streams per sender or else
- // this may count as the removal and addition of a track?
- // https://crbug.com/webrtc/7932
-
// Called when a receiver is completely removed. This is current (Plan B SDP)
// behavior that occurs when processing the removal of a remote track, and is
// called when the receiver is removed and the track is muted. When Unified
diff --git a/api/rtpsenderinterface.h b/api/rtpsenderinterface.h
index 2ca2edc..01279a5 100644
--- a/api/rtpsenderinterface.h
+++ b/api/rtpsenderinterface.h
@@ -48,8 +48,9 @@
// to uniquely identify a receiver until we implement Unified Plan SDP.
virtual std::string id() const = 0;
- // Returns a list of streams associated with this sender's track. Although we
- // only support one track per stream, in theory the API allows for multiple.
+ // Returns a list of media stream ids associated with this sender's track.
+ // These are signalled in the SDP so that the remote side can associate
+ // tracks.
virtual std::vector<std::string> stream_ids() const = 0;
virtual RtpParameters GetParameters() const = 0;
diff --git a/api/rtptransceiverinterface.h b/api/rtptransceiverinterface.h
index 3ea75fd..c9a86e6 100644
--- a/api/rtptransceiverinterface.h
+++ b/api/rtptransceiverinterface.h
@@ -40,9 +40,6 @@
RtpTransceiverDirection direction = RtpTransceiverDirection::kSendRecv;
// The added RtpTransceiver will be added to these streams.
- // TODO(shampson): Change name to stream_id & update native wrapper's naming
- // as well.
- // TODO(bugs.webrtc.org/7600): Not implemented.
std::vector<std::string> stream_ids;
// TODO(bugs.webrtc.org/7600): Not implemented.
diff --git a/media/base/streamparams.cc b/media/base/streamparams.cc
index 421111a..2efa0d0 100644
--- a/media/base/streamparams.cc
+++ b/media/base/streamparams.cc
@@ -141,9 +141,15 @@
if (!cname.empty()) {
ost << "cname:" << cname << ";";
}
- if (!sync_label.empty()) {
- ost << "sync_label:" << sync_label;
+ ost << "stream_ids:";
+ for (std::vector<std::string>::const_iterator it = stream_ids_.begin();
+ it != stream_ids_.end(); ++it) {
+ if (it != stream_ids_.begin()) {
+ ost << ",";
+ }
+ ost << *it;
}
+ ost << ";";
ost << "}";
return ost.str();
}
@@ -200,22 +206,15 @@
}
std::vector<std::string> StreamParams::stream_ids() const {
- if (sync_label.empty()) {
- return {};
- }
- return {sync_label};
+ return stream_ids_;
}
void StreamParams::set_stream_ids(const std::vector<std::string>& stream_ids) {
- // TODO(bugs.webrtc.org/7932): Support an arbitrary number of stream ids.
- RTC_DCHECK_LE(stream_ids.size(), 1) << "set_stream_ids currently only "
- "supports exactly 0 or 1 stream "
- "id.";
- sync_label = (stream_ids.empty() ? "" : stream_ids[0]);
+ stream_ids_ = stream_ids;
}
std::string StreamParams::first_stream_id() const {
- return sync_label;
+ return stream_ids_.empty() ? "" : stream_ids_[0];
}
bool IsOneSsrcStream(const StreamParams& sp) {
diff --git a/media/base/streamparams.h b/media/base/streamparams.h
index 016c27a..22b98c7 100644
--- a/media/base/streamparams.h
+++ b/media/base/streamparams.h
@@ -72,7 +72,7 @@
return (groupid == other.groupid && id == other.id &&
ssrcs == other.ssrcs && ssrc_groups == other.ssrc_groups &&
type == other.type && display == other.display &&
- cname == other.cname && sync_label == other.sync_label);
+ cname == other.cname && stream_ids_ == other.stream_ids_);
}
bool operator!=(const StreamParams &other) const {
return !(*this == other);
@@ -165,8 +165,6 @@
// Friendly name describing stream
std::string display;
std::string cname; // RTCP CNAME
- // TODO(shampson): Move callers to |stream_ids()| and make private.
- std::string sync_label; // Friendly name of cname.
private:
bool AddSecondarySsrc(const std::string& semantics,
@@ -175,6 +173,8 @@
bool GetSecondarySsrc(const std::string& semantics,
uint32_t primary_ssrc,
uint32_t* secondary_ssrc) const;
+
+ std::vector<std::string> stream_ids_;
};
// A Stream can be selected by either groupid+id or ssrc.
diff --git a/media/base/streamparams_unittest.cc b/media/base/streamparams_unittest.cc
index 6e934ae..db37180 100644
--- a/media/base/streamparams_unittest.cc
+++ b/media/base/streamparams_unittest.cc
@@ -208,8 +208,11 @@
TEST(StreamParams, ToString) {
cricket::StreamParams sp =
CreateStreamParamsWithSsrcGroup("XYZ", kSsrcs2, arraysize(kSsrcs2));
- EXPECT_STREQ("{ssrcs:[1,2];ssrc_groups:{semantics:XYZ;ssrcs:[1,2]};}",
- sp.ToString().c_str());
+ sp.set_stream_ids({"stream_id"});
+ EXPECT_STREQ(
+ "{ssrcs:[1,2];ssrc_groups:{semantics:XYZ;ssrcs:[1,2]};stream_ids:stream_"
+ "id;}",
+ sp.ToString().c_str());
}
TEST(StreamParams, TestIsOneSsrcStream_LegacyStream) {
diff --git a/pc/mediasession.cc b/pc/mediasession.cc
index 8a9c1f8..474c0b4 100644
--- a/pc/mediasession.cc
+++ b/pc/mediasession.cc
@@ -476,8 +476,6 @@
}
}
stream_param.cname = rtcp_cname;
- // TODO(steveanton): Support any number of stream ids.
- RTC_CHECK(sender.stream_ids.size() == 1U);
stream_param.set_stream_ids(sender.stream_ids);
content_description->AddStream(stream_param);
@@ -488,8 +486,6 @@
// Use existing generated SSRCs/groups, but update the sync_label if
// necessary. This may be needed if a MediaStreamTrack was moved from one
// MediaStream to another.
- // TODO(steveanton): Support any number of stream ids.
- RTC_CHECK(sender.stream_ids.size() == 1U);
param->set_stream_ids(sender.stream_ids);
content_description->AddStream(*param);
}
diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index 6fb64fc..2d28b87 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -1141,12 +1141,6 @@
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
"Track has invalid kind: " + track->kind());
}
- // TODO(bugs.webrtc.org/7932): Support adding a track to multiple streams.
- if (stream_ids.size() > 1u) {
- LOG_AND_RETURN_ERROR(
- RTCErrorType::UNSUPPORTED_OPERATION,
- "AddTrack with more than one stream is not currently supported.");
- }
if (IsClosed()) {
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_STATE,
"PeerConnection is closed.");
@@ -1156,18 +1150,9 @@
RTCErrorType::INVALID_PARAMETER,
"Sender already exists for track " + track->id() + ".");
}
- // TODO(bugs.webrtc.org/7933): MediaSession expects the sender to have exactly
- // one stream. AddTrackInternal will return an error if there is more than one
- // stream, but if the caller specifies none then we need to generate a random
- // stream id.
- std::vector<std::string> adjusted_stream_ids = stream_ids;
- if (stream_ids.empty()) {
- adjusted_stream_ids.push_back(rtc::CreateRandomUuid());
- }
- RTC_DCHECK_EQ(1, adjusted_stream_ids.size());
auto sender_or_error =
- (IsUnifiedPlan() ? AddTrackUnifiedPlan(track, adjusted_stream_ids)
- : AddTrackPlanB(track, adjusted_stream_ids));
+ (IsUnifiedPlan() ? AddTrackUnifiedPlan(track, stream_ids)
+ : AddTrackPlanB(track, stream_ids));
if (sender_or_error.ok()) {
observer_->OnRenegotiationNeeded();
stats_->AddTrack(track);
@@ -1179,17 +1164,25 @@
PeerConnection::AddTrackPlanB(
rtc::scoped_refptr<MediaStreamTrackInterface> track,
const std::vector<std::string>& stream_ids) {
+ if (stream_ids.size() > 1u) {
+ LOG_AND_RETURN_ERROR(RTCErrorType::UNSUPPORTED_OPERATION,
+ "AddTrack with more than one stream is not "
+ "supported with Plan B semantics.");
+ }
+ std::vector<std::string> adjusted_stream_ids = stream_ids;
+ if (adjusted_stream_ids.empty()) {
+ adjusted_stream_ids.push_back(rtc::CreateRandomUuid());
+ }
cricket::MediaType media_type =
(track->kind() == MediaStreamTrackInterface::kAudioKind
? cricket::MEDIA_TYPE_AUDIO
: cricket::MEDIA_TYPE_VIDEO);
- auto new_sender = CreateSender(media_type, track, stream_ids);
+ auto new_sender = CreateSender(media_type, track, adjusted_stream_ids);
if (track->kind() == MediaStreamTrackInterface::kAudioKind) {
new_sender->internal()->SetVoiceMediaChannel(voice_media_channel());
GetAudioTransceiver()->internal()->AddSender(new_sender);
const RtpSenderInfo* sender_info =
- FindSenderInfo(local_audio_sender_infos_,
- new_sender->internal()->stream_id(), track->id());
+ FindSenderInfo(local_audio_sender_infos_, track->id());
if (sender_info) {
new_sender->internal()->SetSsrc(sender_info->first_ssrc);
}
@@ -1198,8 +1191,7 @@
new_sender->internal()->SetVideoMediaChannel(video_media_channel());
GetVideoTransceiver()->internal()->AddSender(new_sender);
const RtpSenderInfo* sender_info =
- FindSenderInfo(local_video_sender_infos_,
- new_sender->internal()->stream_id(), track->id());
+ FindSenderInfo(local_video_sender_infos_, track->id());
if (sender_info) {
new_sender->internal()->SetSsrc(sender_info->first_ssrc);
}
@@ -1370,15 +1362,7 @@
// TODO(bugs.webrtc.org/7600): Verify init.
- if (init.stream_ids.size() > 1u) {
- LOG_AND_RETURN_ERROR(RTCErrorType::UNSUPPORTED_PARAMETER,
- "More than one stream is not yet supported.");
- }
-
- std::vector<std::string> stream_ids = {
- !init.stream_ids.empty() ? init.stream_ids[0] : rtc::CreateRandomUuid()};
-
- auto sender = CreateSender(media_type, track, stream_ids);
+ auto sender = CreateSender(media_type, track, init.stream_ids);
auto receiver = CreateReceiver(media_type, rtc::CreateRandomUuid());
auto transceiver = CreateAndAddTransceiver(sender, receiver);
transceiver->internal()->set_direction(init.direction);
@@ -1485,8 +1469,12 @@
return nullptr;
}
+ // Internally we need to have one stream with Plan B semantics, so we
+ // generate a random stream ID if not specified.
std::vector<std::string> stream_ids;
- if (!stream_id.empty()) {
+ if (stream_id.empty()) {
+ stream_ids.push_back(rtc::CreateRandomUuid());
+ } else {
stream_ids.push_back(stream_id);
}
@@ -2346,28 +2334,33 @@
// the RTCSessionDescription: If direction is sendrecv or recvonly, and
// transceiver's current direction is neither sendrecv nor recvonly,
// process the addition of a remote track for the media description.
- //
- // Create a sync label in the case of an unsignalled msid.
- std::string sync_label = rtc::CreateRandomUuid();
+ 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()) {
- sync_label = stream_params.stream_ids()[0];
+ stream_ids = stream_params.stream_ids();
}
}
if (RtpTransceiverDirectionHasRecv(local_direction) &&
(!transceiver->current_direction() ||
!RtpTransceiverDirectionHasRecv(
*transceiver->current_direction()))) {
- rtc::scoped_refptr<MediaStreamInterface> stream =
- remote_streams_->find(sync_label);
- if (!stream) {
- stream = MediaStreamProxy::Create(rtc::Thread::Current(),
- MediaStream::Create(sync_label));
- remote_streams_->AddStream(stream);
- added_streams.push_back(stream);
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>> media_streams;
+ for (const std::string& stream_id : stream_ids) {
+ rtc::scoped_refptr<MediaStreamInterface> stream =
+ remote_streams_->find(stream_id);
+ if (!stream) {
+ stream = MediaStreamProxy::Create(rtc::Thread::Current(),
+ MediaStream::Create(stream_id));
+ remote_streams_->AddStream(stream);
+ added_streams.push_back(stream);
+ }
+ media_streams.push_back(stream);
}
- transceiver->internal()->receiver_internal()->SetStreams({stream});
+ transceiver->internal()->receiver_internal()->SetStreams(media_streams);
receiving_transceivers.push_back(transceiver);
}
// If direction is sendonly or inactive, and transceiver's current
@@ -3306,7 +3299,7 @@
if (sender) {
// We already have a sender for this track, so just change the stream_id
// so that it's correct in the next call to CreateOffer.
- sender->internal()->set_stream_id(stream->id());
+ sender->internal()->set_stream_ids({stream->id()});
return;
}
@@ -3322,7 +3315,7 @@
// session description is not changed and RemoveStream is called, and
// later AddStream is called again with the same stream.
const RtpSenderInfo* sender_info =
- FindSenderInfo(local_audio_sender_infos_, stream->id(), track->id());
+ FindSenderInfo(local_audio_sender_infos_, track->id());
if (sender_info) {
new_sender->internal()->SetSsrc(sender_info->first_ssrc);
}
@@ -3349,7 +3342,7 @@
if (sender) {
// We already have a sender for this track, so just change the stream_id
// so that it's correct in the next call to CreateOffer.
- sender->internal()->set_stream_id(stream->id());
+ sender->internal()->set_stream_ids({stream->id()});
return;
}
@@ -3359,7 +3352,7 @@
new_sender->internal()->SetVideoMediaChannel(video_media_channel());
GetVideoTransceiver()->internal()->AddSender(new_sender);
const RtpSenderInfo* sender_info =
- FindSenderInfo(local_video_sender_infos_, stream->id(), track->id());
+ FindSenderInfo(local_video_sender_infos_, track->id());
if (sender_info) {
new_sender->internal()->SetSsrc(sender_info->first_ssrc);
}
@@ -3980,6 +3973,8 @@
bool default_sender_needed,
cricket::MediaType media_type,
StreamCollection* new_streams) {
+ RTC_DCHECK(!IsUnifiedPlan());
+
std::vector<RtpSenderInfo>* current_senders =
GetRemoteSenderInfos(media_type);
@@ -3991,7 +3986,8 @@
const RtpSenderInfo& info = *sender_it;
const cricket::StreamParams* params =
cricket::GetStreamBySsrc(streams, info.first_ssrc);
- bool sender_exists = params && params->id == info.sender_id;
+ bool sender_exists = params && params->id == info.sender_id &&
+ params->first_stream_id() == info.stream_id;
// If this is a default track, and we still need it, don't remove it.
if ((info.stream_id == kDefaultStreamId && default_sender_needed) ||
sender_exists) {
@@ -4005,8 +4001,10 @@
// Find new and active senders.
for (const cricket::StreamParams& params : streams) {
// |params.id| is the sender id and the stream id uses the first of
- // |params.stream_ids|.
- // TODO(bugs.webrtc.org/7932): Add support for multiple stream ids.
+ // |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.
const std::string& stream_id =
(!params.stream_ids().empty() ? params.stream_ids()[0] : "");
const std::string& sender_id = params.id;
@@ -4023,7 +4021,7 @@
}
const RtpSenderInfo* sender_info =
- FindSenderInfo(*current_senders, stream_id, sender_id);
+ FindSenderInfo(*current_senders, sender_id);
if (!sender_info) {
current_senders->push_back(RtpSenderInfo(stream_id, sender_id, ssrc));
OnRemoteSenderAdded(current_senders->back(), media_type);
@@ -4045,7 +4043,7 @@
? kDefaultAudioSenderId
: kDefaultVideoSenderId;
const RtpSenderInfo* default_sender_info =
- FindSenderInfo(*current_senders, kDefaultStreamId, default_sender_id);
+ FindSenderInfo(*current_senders, default_sender_id);
if (!default_sender_info) {
current_senders->push_back(
RtpSenderInfo(kDefaultStreamId, default_sender_id, 0));
@@ -4145,7 +4143,7 @@
const std::string& sender_id = params.id;
uint32_t ssrc = params.first_ssrc();
const RtpSenderInfo* sender_info =
- FindSenderInfo(*current_senders, stream_id, sender_id);
+ FindSenderInfo(*current_senders, sender_id);
if (!sender_info) {
current_senders->push_back(RtpSenderInfo(stream_id, sender_id, ssrc));
OnLocalSenderAdded(current_senders->back(), media_type);
@@ -4155,6 +4153,7 @@
void PeerConnection::OnLocalSenderAdded(const RtpSenderInfo& sender_info,
cricket::MediaType media_type) {
+ RTC_DCHECK(!IsUnifiedPlan());
auto sender = FindSenderById(sender_info.sender_id);
if (!sender) {
RTC_LOG(LS_WARNING) << "An unknown RtpSender with id "
@@ -4169,7 +4168,7 @@
return;
}
- sender->internal()->set_stream_id(sender_info.stream_id);
+ sender->internal()->set_stream_ids({sender_info.stream_id});
sender->internal()->SetSsrc(sender_info.first_ssrc);
}
@@ -4504,11 +4503,9 @@
const PeerConnection::RtpSenderInfo* PeerConnection::FindSenderInfo(
const std::vector<PeerConnection::RtpSenderInfo>& infos,
- const std::string& stream_id,
const std::string sender_id) const {
for (const RtpSenderInfo& sender_info : infos) {
- if (sender_info.stream_id == stream_id &&
- sender_info.sender_id == sender_id) {
+ if (sender_info.sender_id == sender_id) {
return &sender_info;
}
}
diff --git a/pc/peerconnection.h b/pc/peerconnection.h
index d1ba212..1ca117b 100644
--- a/pc/peerconnection.h
+++ b/pc/peerconnection.h
@@ -635,7 +635,6 @@
std::vector<RtpSenderInfo>* GetLocalSenderInfos(
cricket::MediaType media_type);
const RtpSenderInfo* FindSenderInfo(const std::vector<RtpSenderInfo>& infos,
- const std::string& stream_id,
const std::string sender_id) const;
// Returns the specified SCTP DataChannel in sctp_data_channels_,
diff --git a/pc/peerconnection_jsep_unittest.cc b/pc/peerconnection_jsep_unittest.cc
index 84bd060..caa0ea0 100644
--- a/pc/peerconnection_jsep_unittest.cc
+++ b/pc/peerconnection_jsep_unittest.cc
@@ -1162,11 +1162,11 @@
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
- ASSERT_EQ(callee->observer()->add_track_events_.size(), 1u);
- EXPECT_EQ(kTrackLabel,
- callee->observer()->add_track_events_[0].receiver->track()->id());
- // TODO(bugs.webrtc.org/7933): Also verify that no stream was added to the
- // receiver.
+ const auto& track_events = callee->observer()->add_track_events_;
+ ASSERT_EQ(1u, track_events.size());
+ const auto& event = track_events[0];
+ EXPECT_EQ(kTrackLabel, event.receiver->track()->id());
+ EXPECT_EQ(0u, event.streams.size());
}
// Test that setting a remote offer with one track that has one stream fires off
@@ -1226,7 +1226,27 @@
UnorderedElementsAre(track1, track2));
}
-// TODO(bugs.webrtc.org/7932): Also test multi-stream case.
+// Test that setting a remote offer with one track that has two streams fires
+// off the correct OnAddTrack event.
+TEST_F(PeerConnectionJsepTest,
+ SetRemoteOfferWithOneTrackTwoStreamFiresOnAddTrack) {
+ const std::string kTrackLabel = "audio_track";
+ const std::string kStreamId1 = "audio_stream1";
+ const std::string kStreamId2 = "audio_stream2";
+
+ auto caller = CreatePeerConnection();
+ auto callee = CreatePeerConnection();
+ ASSERT_TRUE(caller->AddAudioTrack(kTrackLabel, {kStreamId1, kStreamId2}));
+
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
+
+ const auto& track_events = callee->observer()->add_track_events_;
+ ASSERT_EQ(1u, track_events.size());
+ const auto& event = track_events[0];
+ ASSERT_EQ(2u, event.streams.size());
+ EXPECT_EQ(kStreamId1, event.streams[0]->id());
+ EXPECT_EQ(kStreamId2, event.streams[1]->id());
+}
// Test that if an RtpTransceiver with a current_direction set is stopped, then
// current_direction is changed to null.
diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc
index 69f329c..8d055a8 100644
--- a/pc/peerconnection_rtp_unittest.cc
+++ b/pc/peerconnection_rtp_unittest.cc
@@ -115,8 +115,8 @@
static_cast<webrtc::RTCError*>(nullptr)));
ASSERT_EQ(callee->observer()->add_track_events_.size(), 1u);
- // TODO(hbos): When "no stream" is handled correctly we would expect
- // |add_track_events_[0].streams| to be empty. https://crbug.com/webrtc/7933
+ // Since we are not supporting the no stream case with Plan B, there should be
+ // a generated stream, even though we didn't set one with AddTrack.
auto& add_track_event = callee->observer()->add_track_events_[0];
ASSERT_EQ(add_track_event.streams.size(), 1u);
EXPECT_TRUE(add_track_event.streams[0]->FindAudioTrack("audio_track"));
@@ -219,15 +219,54 @@
callee->observer()->remove_track_events_);
}
+// Tests the edge case that if a stream ID changes for a given track that both
+// OnRemoveTrack and OnAddTrack is fired.
+TEST_F(PeerConnectionRtpCallbacksTest,
+ RemoteStreamIdChangesFiresOnRemoveAndOnAddTrack) {
+ auto caller = CreatePeerConnection();
+ auto callee = CreatePeerConnection();
+
+ 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)));
+ EXPECT_EQ(callee->observer()->add_track_events_.size(), 1u);
+
+ // Change the stream ID of the sender in the session description.
+ auto offer = caller->CreateOfferAndSetAsLocal();
+ auto audio_desc = offer->description()->GetContentDescriptionByName("audio");
+ 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)));
+
+ ASSERT_EQ(callee->observer()->add_track_events_.size(), 2u);
+ EXPECT_EQ(callee->observer()->add_track_events_[1].streams[0]->id(),
+ kStreamId2);
+ ASSERT_EQ(callee->observer()->remove_track_events_.size(), 1u);
+ EXPECT_EQ(callee->observer()->remove_track_events_[0]->streams()[0]->id(),
+ kStreamId1);
+}
+
// Tests that setting a remote description with sending transceivers will fire
// the OnTrack callback for each transceiver and setting a remote description
-// with receive only transceivers will not call OnTrack.
+// with receive only transceivers will not call OnTrack. One transceiver is
+// created without any stream_ids, while the other is created with multiple
+// stream_ids.
TEST_F(PeerConnectionRtpCallbacksTest, UnifiedPlanAddTransceiverCallsOnTrack) {
+ const std::string kStreamId1 = "video_stream1";
+ const std::string kStreamId2 = "video_stream2";
auto caller = CreatePeerConnectionWithUnifiedPlan();
auto callee = CreatePeerConnectionWithUnifiedPlan();
auto audio_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
- auto video_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO);
+ RtpTransceiverInit video_transceiver_init;
+ video_transceiver_init.stream_ids = {kStreamId1, kStreamId2};
+ auto video_transceiver =
+ caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, video_transceiver_init);
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
@@ -237,6 +276,14 @@
callee->pc()->GetTransceivers()[0]->mid());
EXPECT_EQ(video_transceiver->mid(),
callee->pc()->GetTransceivers()[1]->mid());
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>> audio_streams =
+ callee->pc()->GetTransceivers()[0]->receiver()->streams();
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>> video_streams =
+ callee->pc()->GetTransceivers()[1]->receiver()->streams();
+ ASSERT_EQ(0u, audio_streams.size());
+ ASSERT_EQ(2u, video_streams.size());
+ EXPECT_EQ(kStreamId1, video_streams[0]->id());
+ EXPECT_EQ(kStreamId2, video_streams[1]->id());
}
// Test that doing additional offer/answer exchanges with no changes to tracks
@@ -335,8 +382,8 @@
EXPECT_EQ(callee->pc()->GetReceivers().size(), 1u);
auto receiver_added = callee->pc()->GetReceivers()[0];
EXPECT_EQ("audio_track", receiver_added->track()->id());
- // TODO(hbos): When "no stream" is handled correctly we would expect
- // |receiver_added->streams()| to be empty. https://crbug.com/webrtc/7933
+ // Since we are not supporting the no stream case with Plan B, there should be
+ // a generated stream, even though we didn't set one with AddTrack.
EXPECT_EQ(receiver_added->streams().size(), 1u);
EXPECT_TRUE(receiver_added->streams()[0]->FindAudioTrack("audio_track"));
}
@@ -505,6 +552,38 @@
EXPECT_TRUE_WAIT(srd2_callback_called, kDefaultTimeout);
}
+// 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.
+TEST_F(PeerConnectionRtpObserverTest, RemoteStreamIdChangesUpdatesReceiver) {
+ auto caller = CreatePeerConnectionWithUnifiedPlan();
+ auto callee = CreatePeerConnectionWithUnifiedPlan();
+
+ 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)));
+ EXPECT_EQ(callee->observer()->add_track_events_.size(), 1u);
+
+ // Change the stream id of the sender in the session description.
+ auto offer = caller->CreateOfferAndSetAsLocal();
+ auto contents = offer->description()->contents();
+ ASSERT_EQ(contents.size(), 1u);
+ ASSERT_EQ(contents[0].media_description()->mutable_streams().size(), 1u);
+ contents[0].media_description()->mutable_streams()[0].set_stream_ids(
+ {kStreamId2});
+
+ // Set the remote description and verify that the stream was updated properly.
+ ASSERT_TRUE(
+ callee->SetRemoteDescription(CloneSessionDescription(offer.get()),
+ static_cast<webrtc::RTCError*>(nullptr)));
+ auto receivers = callee->pc()->GetReceivers();
+ ASSERT_EQ(receivers.size(), 1u);
+ ASSERT_EQ(receivers[0]->streams().size(), 1u);
+ EXPECT_EQ(receivers[0]->streams()[0]->id(), kStreamId2);
+}
// Tests for the legacy SetRemoteDescription() function signature.
class PeerConnectionRtpLegacyObserverTest : public PeerConnectionRtpTest {};
@@ -1068,6 +1147,39 @@
answer->description()->msid_signaling());
}
+// This tests that a Plan B endpoint appropriately sets the remote description
+// from a Unified Plan offer. When the Unified Plan offer contains a=msid lines
+// that signal no stream ids or multiple stream ids we expect that the Plan B
+// endpoint always has exactly one media stream per track.
+TEST_F(PeerConnectionMsidSignalingTest, UnifiedPlanToPlanBAnswer) {
+ const std::string kStreamId1 = "audio_stream_1";
+ const std::string kStreamId2 = "audio_stream_2";
+
+ auto caller = CreatePeerConnectionWithUnifiedPlan();
+ caller->AddAudioTrack("caller_audio", {kStreamId1, kStreamId2});
+ caller->AddVideoTrack("caller_video", {});
+ auto callee = CreatePeerConnectionWithPlanB();
+ callee->AddAudioTrack("callee_audio");
+ caller->AddVideoTrack("callee_video");
+
+ ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+
+ // Offer should have had both a=msid and a=ssrc MSID lines.
+ auto* offer = callee->pc()->remote_description();
+ EXPECT_EQ((cricket::kMsidSignalingMediaSection |
+ cricket::kMsidSignalingSsrcAttribute),
+ offer->description()->msid_signaling());
+
+ // Callee should always have 1 stream for all of it's receivers.
+ const auto& track_events = callee->observer()->add_track_events_;
+ ASSERT_EQ(2u, track_events.size());
+ ASSERT_EQ(1u, track_events[0].streams.size());
+ EXPECT_EQ(kStreamId1, track_events[0].streams[0]->id());
+ ASSERT_EQ(1u, track_events[1].streams.size());
+ // This autogenerated a stream id for the empty one signalled.
+ EXPECT_FALSE(track_events[0].streams[0]->id().empty());
+}
+
TEST_F(PeerConnectionMsidSignalingTest, PureUnifiedPlanToUs) {
auto caller = CreatePeerConnectionWithUnifiedPlan();
caller->AddAudioTrack("caller_audio");
diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc
index 62fd3bf..c906c23 100644
--- a/pc/peerconnectioninterface_unittest.cc
+++ b/pc/peerconnectioninterface_unittest.cc
@@ -1628,9 +1628,16 @@
EXPECT_EQ(audio_track, audio_sender->track());
EXPECT_EQ("video_track", video_sender->id());
EXPECT_EQ(video_track, video_sender->track());
- // If the ID is truly a random GUID, it should be infinitely unlikely they
- // will be the same.
- EXPECT_NE(video_sender->stream_ids(), audio_sender->stream_ids());
+ if (sdp_semantics_ == SdpSemantics::kPlanB) {
+ // If the ID is truly a random GUID, it should be infinitely unlikely they
+ // will be the same.
+ EXPECT_NE(video_sender->stream_ids(), audio_sender->stream_ids());
+ } else {
+ // We allows creating tracks without stream ids under Unified Plan
+ // semantics.
+ EXPECT_EQ(0u, video_sender->stream_ids().size());
+ EXPECT_EQ(0u, audio_sender->stream_ids().size());
+ }
}
// Test that we can call GetStats() after AddTrack but before connecting
diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc
index 170aa9c..626c850 100644
--- a/pc/rtpsender.cc
+++ b/pc/rtpsender.cc
@@ -79,9 +79,6 @@
sink_adapter_(new LocalAudioSinkAdapter()),
attachment_id_(track ? GenerateUniqueId() : 0) {
RTC_DCHECK(worker_thread);
- // TODO(bugs.webrtc.org/7932): Remove once zero or multiple streams are
- // supported.
- RTC_DCHECK_EQ(stream_ids.size(), 1u);
if (track_) {
track_->RegisterObserver(this);
track_->AddSink(sink_adapter_.get());
@@ -318,9 +315,6 @@
: VideoTrackInterface::ContentHint::kNone),
attachment_id_(track ? GenerateUniqueId() : 0) {
RTC_DCHECK(worker_thread);
- // TODO(bugs.webrtc.org/7932): Remove once zero or multiple streams are
- // supported.
- RTC_DCHECK_EQ(stream_ids.size(), 1u);
if (track_) {
track_->RegisterObserver(this);
}
diff --git a/pc/rtpsender.h b/pc/rtpsender.h
index 05e1131..27b1513 100644
--- a/pc/rtpsender.h
+++ b/pc/rtpsender.h
@@ -50,12 +50,6 @@
// description).
virtual void SetSsrc(uint32_t ssrc) = 0;
- // TODO(steveanton): With Unified Plan, a track/RTCRTPSender can be part of
- // multiple streams (or no stream at all). Replace these singular methods with
- // their corresponding plural methods.
- // Until these are removed, RtpSenders must have exactly one stream.
- virtual void set_stream_id(const std::string& stream_id) = 0;
- virtual std::string stream_id() const = 0;
virtual void set_stream_ids(const std::vector<std::string>& stream_ids) = 0;
virtual void Stop() = 0;
@@ -142,10 +136,6 @@
// RtpSenderInternal implementation.
void SetSsrc(uint32_t ssrc) override;
- void set_stream_id(const std::string& stream_id) override {
- stream_ids_ = {stream_id};
- }
- std::string stream_id() const override { return stream_ids_[0]; }
void set_stream_ids(const std::vector<std::string>& stream_ids) override {
stream_ids_ = stream_ids;
}
@@ -177,8 +167,6 @@
rtc::Thread* const worker_thread_;
const std::string id_;
- // TODO(steveanton): Until more Unified Plan work is done, this can only have
- // exactly one element.
std::vector<std::string> stream_ids_;
cricket::VoiceMediaChannel* media_channel_ = nullptr;
StatsCollector* stats_;
@@ -236,10 +224,6 @@
// RtpSenderInternal implementation.
void SetSsrc(uint32_t ssrc) override;
- void set_stream_id(const std::string& stream_id) override {
- stream_ids_ = {stream_id};
- }
- std::string stream_id() const override { return stream_ids_[0]; }
void set_stream_ids(const std::vector<std::string>& stream_ids) override {
stream_ids_ = stream_ids;
}
@@ -266,8 +250,6 @@
rtc::Thread* worker_thread_;
const std::string id_;
- // TODO(steveanton): Until more Unified Plan work is done, this can only have
- // exactly one element.
std::vector<std::string> stream_ids_;
cricket::VideoMediaChannel* media_channel_ = nullptr;
rtc::scoped_refptr<VideoTrackInterface> track_;
diff --git a/pc/test/mock_rtpsenderinternal.h b/pc/test/mock_rtpsenderinternal.h
index b76a9ac..5988c7b 100644
--- a/pc/test/mock_rtpsenderinternal.h
+++ b/pc/test/mock_rtpsenderinternal.h
@@ -37,8 +37,6 @@
MOCK_METHOD1(SetVoiceMediaChannel, void(cricket::VoiceMediaChannel*));
MOCK_METHOD1(SetVideoMediaChannel, void(cricket::VideoMediaChannel*));
MOCK_METHOD1(SetSsrc, void(uint32_t));
- MOCK_METHOD1(set_stream_id, void(const std::string&));
- MOCK_CONST_METHOD0(stream_id, std::string());
MOCK_METHOD1(set_stream_ids, void(const std::vector<std::string>&));
MOCK_METHOD0(Stop, void());
MOCK_CONST_METHOD0(AttachmentId, int());
diff --git a/pc/webrtcsdp.cc b/pc/webrtcsdp.cc
index 80ac8d5..a0887a6 100644
--- a/pc/webrtcsdp.cc
+++ b/pc/webrtcsdp.cc
@@ -120,10 +120,12 @@
static const char kAttributeExtmap[] = "extmap";
// draft-alvestrand-mmusic-msid-01
// a=msid-semantic: WMS
+// This is a legacy field supported only for Plan B semantics.
static const char kAttributeMsidSemantics[] = "msid-semantic";
static const char kMediaStreamSemantic[] = "WMS";
static const char kSsrcAttributeMsid[] = "msid";
static const char kDefaultMsid[] = "default";
+static const char kNoStreamMsid[] = "-";
static const char kSsrcAttributeMslabel[] = "mslabel";
static const char kSSrcAttributeLabel[] = "label";
static const char kAttributeSsrcGroup[] = "ssrc-group";
@@ -325,7 +327,7 @@
cricket::ConnectionRole* role,
SdpParseError* error);
static bool ParseMsidAttribute(const std::string& line,
- std::string* stream_id,
+ std::vector<std::string>* stream_ids,
std::string* track_id,
SdpParseError* error);
@@ -578,43 +580,50 @@
cricket::IsValidRtpPayloadType(*payload_type);
}
-// |msid_stream_id| and |msid_track_id| represent the stream/track ID from the
+// |msid_stream_idss| 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.
void CreateTracksFromSsrcInfos(const SsrcInfoVec& ssrc_infos,
- const std::string& msid_stream_id,
+ const std::vector<std::string>& msid_stream_ids,
const std::string& msid_track_id,
- StreamParamsVec* tracks) {
+ StreamParamsVec* tracks,
+ int msid_signaling) {
RTC_DCHECK(tracks != NULL);
- RTC_DCHECK(msid_stream_id.empty() == msid_track_id.empty());
for (SsrcInfoVec::const_iterator ssrc_info = ssrc_infos.begin();
ssrc_info != ssrc_infos.end(); ++ssrc_info) {
if (ssrc_info->cname.empty()) {
continue;
}
- std::string stream_id;
+ std::vector<std::string> stream_ids;
std::string track_id;
- if (ssrc_info->stream_id.empty() && !ssrc_info->mslabel.empty()) {
- // If there's no msid and there's mslabel, we consider this is a sdp from
- // a older version of client that doesn't support msid.
- // In that case, we use the mslabel and label to construct the track.
- stream_id = ssrc_info->mslabel;
- track_id = ssrc_info->label;
- } else if (ssrc_info->stream_id.empty() && !msid_stream_id.empty()) {
- // If there's no msid in the SSRC attributes, but there's a global one
- // (from a=msid), use that. This is the case with Unified Plan SDP.
- stream_id = msid_stream_id;
+ if (msid_signaling & cricket::kMsidSignalingMediaSection) {
+ // This is the case with Unified Plan SDP msid signaling.
+ stream_ids = msid_stream_ids;
track_id = msid_track_id;
- } else {
- stream_id = ssrc_info->stream_id;
+ } else if (msid_signaling & cricket::kMsidSignalingSsrcAttribute) {
+ // This is the case with Plan B SDP msid signaling.
+ stream_ids.push_back(ssrc_info->stream_id);
track_id = ssrc_info->track_id;
+ } else if (!ssrc_info->mslabel.empty()) {
+ // Since there's no a=msid or a=ssrc msid signaling, this is a sdp from
+ // an older version of client that doesn't support msid.
+ // In that case, we use the mslabel and label to construct the track.
+ stream_ids.push_back(ssrc_info->mslabel);
+ track_id = ssrc_info->label;
+ } else {
+ // Since no media streams isn't supported with older SDP signaling, we
+ // use a default a stream id.
+ stream_ids.push_back(kDefaultMsid);
}
- // If a stream/track ID wasn't populated from the SSRC attributes OR the
+ // If a track ID wasn't populated from the SSRC attributes OR the
// msid attribute, use default/random values.
- if (stream_id.empty()) {
- stream_id = kDefaultMsid;
- }
if (track_id.empty()) {
// TODO(ronghuawu): What should we do if the track id doesn't appear?
// Create random string (which will be used as track label later)?
@@ -634,7 +643,7 @@
}
track->add_ssrc(ssrc_info->ssrc_id);
track->cname = ssrc_info->cname;
- track->set_stream_ids({stream_id});
+ track->set_stream_ids(stream_ids);
track->id = track_id;
}
}
@@ -1472,18 +1481,26 @@
}
AddLine(os.str(), message);
- // Specified in https://tools.ietf.org/html/draft-ietf-mmusic-msid-16
- // a=msid:<stream id> <track id>
+ // Specified in https://datatracker.ietf.org/doc/draft-ietf-mmusic-msid/16/
+ // a=msid:<msid-id> <msid-appdata>
+ // The msid-id is a 1*64 token char representing the media stream id, and the
+ // msid-appdata is a 1*64 token char representing the track id. There is a
+ // line for every media stream, with a special msid-id value of "-"
+ // representing no streams. The value of "msid-appdata" MUST be identical for
+ // all lines.
if (msid_signaling & cricket::kMsidSignalingMediaSection) {
const StreamParamsVec& streams = media_desc->streams();
if (streams.size() == 1u) {
const StreamParams& track = streams[0];
- // TODO(bugs.webrtc.org/7932): Support serializing more than one stream
- // label.
- const std::string& stream_id = track.first_stream_id();
- InitAttrLine(kAttributeMsid, &os);
- os << kSdpDelimiterColon << stream_id << kSdpDelimiterSpace << track.id;
- AddLine(os.str(), message);
+ std::vector<std::string> stream_ids = track.stream_ids();
+ if (stream_ids.empty()) {
+ stream_ids.push_back(kNoStreamMsid);
+ }
+ for (const std::string& stream_id : stream_ids) {
+ InitAttrLine(kAttributeMsid, &os);
+ os << kSdpDelimiterColon << stream_id << kSdpDelimiterSpace << track.id;
+ AddLine(os.str(), message);
+ }
} else if (streams.size() > 1u) {
RTC_LOG(LS_WARNING)
<< "Trying to serialize Unified Plan SDP with more than "
@@ -1532,13 +1549,6 @@
for (StreamParamsVec::const_iterator track = media_desc->streams().begin();
track != media_desc->streams().end(); ++track) {
- // Require that the track belongs to a media stream. This extra check is
- // necessary since the MediaContentDescription always contains a
- // StreamParams with an ssrc even if no track or media stream have been
- // created.
- if (track->stream_ids().empty())
- continue;
-
// Build the ssrc-group lines.
for (size_t i = 0; i < track->ssrc_groups.size(); ++i) {
// RFC 5576
@@ -1568,8 +1578,9 @@
// a=ssrc:<ssrc-id> msid:identifier [appdata]
// The appdata consists of the "id" attribute of a MediaStreamTrack,
// which corresponds to the "id" attribute of StreamParams.
- // TODO(bugs.webrtc.org/7932): Support serializing more than one stream
- // label.
+ // Since a=ssrc msid signaling is used in Plan B SDP semantics, and
+ // multiple stream ids are not supported for Plan B, we are only adding
+ // a line for the first media stream id here.
const std::string& stream_id = track->first_stream_id();
InitAttrLine(kAttributeSsrc, &os);
os << kSdpDelimiterColon << ssrc << kSdpDelimiterSpace
@@ -1583,8 +1594,7 @@
// a=ssrc:<ssrc-id> mslabel:<value>
// The label isn't yet defined.
// a=ssrc:<ssrc-id> label:<value>
- AddSsrcLine(ssrc, kSsrcAttributeMslabel, track->first_stream_id(),
- message);
+ AddSsrcLine(ssrc, kSsrcAttributeMslabel, stream_id, message);
AddSsrcLine(ssrc, kSSrcAttributeLabel, track->id, message);
}
}
@@ -2197,32 +2207,45 @@
}
static bool ParseMsidAttribute(const std::string& line,
- std::string* stream_id,
+ std::vector<std::string>* stream_ids,
std::string* track_id,
SdpParseError* error) {
- // draft-ietf-mmusic-msid-11
+ // https://datatracker.ietf.org/doc/draft-ietf-mmusic-msid/16/
// a=msid:<stream id> <track id>
// msid-value = msid-id [ SP msid-appdata ]
// msid-id = 1*64token-char ; see RFC 4566
// msid-appdata = 1*64token-char ; see RFC 4566
std::string field1;
+ std::string new_stream_id;
+ std::string new_track_id;
if (!rtc::tokenize_first(line.substr(kLinePrefixLength), kSdpDelimiterSpace,
- &field1, track_id)) {
+ &field1, &new_track_id)) {
const size_t expected_fields = 2;
return ParseFailedExpectFieldNum(line, expected_fields, error);
}
- if (track_id->empty()) {
+ if (new_track_id.empty()) {
return ParseFailed(line, "Missing track ID in msid attribute.", error);
}
+ // All track ids should be the same within an m section in a Unified Plan SDP.
+ if (!track_id->empty() && new_track_id.compare(*track_id) != 0) {
+ return ParseFailed(
+ line, "Two different track IDs in msid attribute in one m= section",
+ error);
+ }
+ *track_id = new_track_id;
// msid:<msid-id>
- if (!GetValue(field1, kAttributeMsid, stream_id, error)) {
+ if (!GetValue(field1, kAttributeMsid, &new_stream_id, error)) {
return false;
}
- if (stream_id->empty()) {
+ if (new_stream_id.empty()) {
return ParseFailed(line, "Missing stream ID in msid attribute.", error);
}
+ // The special value "-" indicates "no MediaStream".
+ if (new_stream_id.compare(kNoStreamMsid) != 0) {
+ stream_ids->push_back(new_stream_id);
+ }
return true;
}
@@ -2706,7 +2729,7 @@
SsrcGroupVec ssrc_groups;
std::string maxptime_as_string;
std::string ptime_as_string;
- std::string stream_id;
+ std::vector<std::string> stream_ids;
std::string track_id;
// Loop until the next m line
@@ -2906,7 +2929,7 @@
if (flag_value.compare(kValueConference) == 0)
media_desc->set_conference_mode(true);
} else if (HasAttribute(line, kAttributeMsid)) {
- if (!ParseMsidAttribute(line, &stream_id, &track_id, error)) {
+ if (!ParseMsidAttribute(line, &stream_ids, &track_id, error)) {
return false;
}
*msid_signaling |= cricket::kMsidSignalingMediaSection;
@@ -2922,7 +2945,8 @@
// 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_id, track_id, &tracks);
+ CreateTracksFromSsrcInfos(ssrc_infos, stream_ids, track_id, &tracks,
+ *msid_signaling);
// Add the ssrc group to the track.
for (SsrcGroupVec::iterator ssrc_group = ssrc_groups.begin();
@@ -3038,7 +3062,7 @@
ssrc_info->cname = value;
} else if (attribute == kSsrcAttributeMsid) {
// draft-alvestrand-mmusic-msid-00
- // "msid:" identifier [ " " appdata ]
+ // msid:identifier [appdata]
std::vector<std::string> fields;
rtc::split(value, kSdpDelimiterSpace, &fields);
if (fields.size() < 1 || fields.size() > 2) {
diff --git a/pc/webrtcsdp_unittest.cc b/pc/webrtcsdp_unittest.cc
index 918dc30..9d6cfa3 100644
--- a/pc/webrtcsdp_unittest.cc
+++ b/pc/webrtcsdp_unittest.cc
@@ -480,91 +480,6 @@
"a=ssrc:6 mslabel:local_stream_2\r\n"
"a=ssrc:6 label:video_track_id_3\r\n";
-// Plan B SDP reference string, with 2 streams, 2 audio tracks and 3 video
-// tracks, but with the unified plan "a=msid" attribute.
-static const char kPlanBSdpFullStringWithMsid[] =
- "v=0\r\n"
- "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
- "s=-\r\n"
- "t=0 0\r\n"
- "a=msid-semantic: WMS local_stream_1 local_stream_2\r\n"
- "m=audio 2345 RTP/SAVPF 111 103 104\r\n"
- "c=IN IP4 74.125.127.126\r\n"
- "a=rtcp:2347 IN IP4 74.125.127.126\r\n"
- "a=candidate:a0+B/1 1 udp 2130706432 192.168.1.5 1234 typ host "
- "generation 2\r\n"
- "a=candidate:a0+B/1 2 udp 2130706432 192.168.1.5 1235 typ host "
- "generation 2\r\n"
- "a=candidate:a0+B/2 1 udp 2130706432 ::1 1238 typ host "
- "generation 2\r\n"
- "a=candidate:a0+B/2 2 udp 2130706432 ::1 1239 typ host "
- "generation 2\r\n"
- "a=candidate:a0+B/3 1 udp 2130706432 74.125.127.126 2345 typ srflx "
- "raddr 192.168.1.5 rport 2346 "
- "generation 2\r\n"
- "a=candidate:a0+B/3 2 udp 2130706432 74.125.127.126 2347 typ srflx "
- "raddr 192.168.1.5 rport 2348 "
- "generation 2\r\n"
- "a=ice-ufrag:ufrag_voice\r\na=ice-pwd:pwd_voice\r\n"
- "a=mid:audio_content_name\r\n"
- "a=msid:local_stream_1 audio_track_id_1\r\n"
- "a=sendrecv\r\n"
- "a=rtcp-mux\r\n"
- "a=rtcp-rsize\r\n"
- "a=crypto:1 AES_CM_128_HMAC_SHA1_32 "
- "inline:NzB4d1BINUAvLEw6UzF3WSJ+PSdFcGdUJShpX1Zj|2^20|1:32 "
- "dummy_session_params\r\n"
- "a=rtpmap:111 opus/48000/2\r\n"
- "a=rtpmap:103 ISAC/16000\r\n"
- "a=rtpmap:104 ISAC/32000\r\n"
- "a=ssrc:1 cname:stream_1_cname\r\n"
- "a=ssrc:1 msid:local_stream_1 audio_track_id_1\r\n"
- "a=ssrc:1 mslabel:local_stream_1\r\n"
- "a=ssrc:1 label:audio_track_id_1\r\n"
- "a=ssrc:4 cname:stream_2_cname\r\n"
- "a=ssrc:4 msid:local_stream_2 audio_track_id_2\r\n"
- "a=ssrc:4 mslabel:local_stream_2\r\n"
- "a=ssrc:4 label:audio_track_id_2\r\n"
- "m=video 3457 RTP/SAVPF 120\r\n"
- "c=IN IP4 74.125.224.39\r\n"
- "a=rtcp:3456 IN IP4 74.125.224.39\r\n"
- "a=candidate:a0+B/1 2 udp 2130706432 192.168.1.5 1236 typ host "
- "generation 2\r\n"
- "a=candidate:a0+B/1 1 udp 2130706432 192.168.1.5 1237 typ host "
- "generation 2\r\n"
- "a=candidate:a0+B/2 2 udp 2130706432 ::1 1240 typ host "
- "generation 2\r\n"
- "a=candidate:a0+B/2 1 udp 2130706432 ::1 1241 typ host "
- "generation 2\r\n"
- "a=candidate:a0+B/4 2 udp 2130706432 74.125.224.39 3456 typ relay "
- "generation 2\r\n"
- "a=candidate:a0+B/4 1 udp 2130706432 74.125.224.39 3457 typ relay "
- "generation 2\r\n"
- "a=ice-ufrag:ufrag_video\r\na=ice-pwd:pwd_video\r\n"
- "a=mid:video_content_name\r\n"
- "a=msid:local_stream_1 video_track_id_1\r\n"
- "a=sendrecv\r\n"
- "a=crypto:1 AES_CM_128_HMAC_SHA1_80 "
- "inline:d0RmdmcmVCspeEc3QGZiNWpVLFJhQX1cfHAwJSoj|2^20|1:32\r\n"
- "a=rtpmap:120 VP8/90000\r\n"
- "a=ssrc-group:FEC 2 3\r\n"
- "a=ssrc:2 cname:stream_1_cname\r\n"
- "a=ssrc:2 msid:local_stream_1 video_track_id_1\r\n"
- "a=ssrc:2 mslabel:local_stream_1\r\n"
- "a=ssrc:2 label:video_track_id_1\r\n"
- "a=ssrc:3 cname:stream_1_cname\r\n"
- "a=ssrc:3 msid:local_stream_1 video_track_id_1\r\n"
- "a=ssrc:3 mslabel:local_stream_1\r\n"
- "a=ssrc:3 label:video_track_id_1\r\n"
- "a=ssrc:5 cname:stream_2_cname\r\n"
- "a=ssrc:5 msid:local_stream_2 video_track_id_2\r\n"
- "a=ssrc:5 mslabel:local_stream_2\r\n"
- "a=ssrc:5 label:video_track_id_2\r\n"
- "a=ssrc:6 cname:stream_2_cname\r\n"
- "a=ssrc:6 msid:local_stream_2 video_track_id_3\r\n"
- "a=ssrc:6 mslabel:local_stream_2\r\n"
- "a=ssrc:6 label:video_track_id_3\r\n";
-
// Unified Plan SDP reference string, with 2 streams, 2 audio tracks and 3 video
// tracks.
static const char kUnifiedPlanSdpFullString[] =
@@ -672,6 +587,94 @@
"a=rtpmap:120 VP8/90000\r\n"
"a=ssrc:6 cname:stream_2_cname\r\n";
+// Unified Plan SDP reference string:
+// - audio track 1 has 1 a=msid lines
+// - audio track 2 has 2 a=msid lines
+// - audio track 3 has 1 a=msid line with the special "-" marker signifying that
+// there are 0 media stream ids.
+// This Unified Plan SDP represents a SDP that signals the msid using both
+// a=msid and a=ssrc msid semantics.
+static const char kUnifiedPlanSdpFullStringWithSpecialMsid[] =
+ "v=0\r\n"
+ "o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "a=msid-semantic: WMS local_stream_1\r\n"
+ // Audio track 1, with 1 stream id.
+ "m=audio 2345 RTP/SAVPF 111 103 104\r\n"
+ "c=IN IP4 74.125.127.126\r\n"
+ "a=rtcp:2347 IN IP4 74.125.127.126\r\n"
+ "a=candidate:a0+B/1 1 udp 2130706432 192.168.1.5 1234 typ host "
+ "generation 2\r\n"
+ "a=candidate:a0+B/1 2 udp 2130706432 192.168.1.5 1235 typ host "
+ "generation 2\r\n"
+ "a=candidate:a0+B/2 1 udp 2130706432 ::1 1238 typ host "
+ "generation 2\r\n"
+ "a=candidate:a0+B/2 2 udp 2130706432 ::1 1239 typ host "
+ "generation 2\r\n"
+ "a=candidate:a0+B/3 1 udp 2130706432 74.125.127.126 2345 typ srflx "
+ "raddr 192.168.1.5 rport 2346 "
+ "generation 2\r\n"
+ "a=candidate:a0+B/3 2 udp 2130706432 74.125.127.126 2347 typ srflx "
+ "raddr 192.168.1.5 rport 2348 "
+ "generation 2\r\n"
+ "a=ice-ufrag:ufrag_voice\r\na=ice-pwd:pwd_voice\r\n"
+ "a=mid:audio_content_name\r\n"
+ "a=msid:local_stream_1 audio_track_id_1\r\n"
+ "a=sendrecv\r\n"
+ "a=rtcp-mux\r\n"
+ "a=rtcp-rsize\r\n"
+ "a=crypto:1 AES_CM_128_HMAC_SHA1_32 "
+ "inline:NzB4d1BINUAvLEw6UzF3WSJ+PSdFcGdUJShpX1Zj|2^20|1:32 "
+ "dummy_session_params\r\n"
+ "a=rtpmap:111 opus/48000/2\r\n"
+ "a=rtpmap:103 ISAC/16000\r\n"
+ "a=rtpmap:104 ISAC/32000\r\n"
+ "a=ssrc:1 cname:stream_1_cname\r\n"
+ "a=ssrc:1 msid:local_stream_1 audio_track_id_1\r\n"
+ "a=ssrc:1 mslabel:local_stream_1\r\n"
+ "a=ssrc:1 label:audio_track_id_1\r\n"
+ // Audio track 2, with two stream ids.
+ "m=audio 9 RTP/SAVPF 111 103 104\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp:9 IN IP4 0.0.0.0\r\n"
+ "a=ice-ufrag:ufrag_voice_2\r\na=ice-pwd:pwd_voice_2\r\n"
+ "a=mid:audio_content_name_2\r\n"
+ "a=msid:local_stream_1 audio_track_id_2\r\n"
+ "a=msid:local_stream_2 audio_track_id_2\r\n"
+ "a=sendrecv\r\n"
+ "a=rtcp-mux\r\n"
+ "a=rtcp-rsize\r\n"
+ "a=crypto:1 AES_CM_128_HMAC_SHA1_32 "
+ "inline:NzB4d1BINUAvLEw6UzF3WSJ+PSdFcGdUJShpX1Zj|2^20|1:32 "
+ "dummy_session_params\r\n"
+ "a=rtpmap:111 opus/48000/2\r\n"
+ "a=rtpmap:103 ISAC/16000\r\n"
+ "a=rtpmap:104 ISAC/32000\r\n"
+ "a=ssrc:4 cname:stream_1_cname\r\n"
+ // The support for Plan B msid signaling only includes the
+ // first media stream id "local_stream_1."
+ "a=ssrc:4 msid:local_stream_1 audio_track_id_2\r\n"
+ "a=ssrc:4 mslabel:local_stream_1\r\n"
+ "a=ssrc:4 label:audio_track_id_2\r\n"
+ // Audio track 3, with no stream ids.
+ "m=audio 9 RTP/SAVPF 111 103 104\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp:9 IN IP4 0.0.0.0\r\n"
+ "a=ice-ufrag:ufrag_voice_3\r\na=ice-pwd:pwd_voice_3\r\n"
+ "a=mid:audio_content_name_3\r\n"
+ "a=msid:- audio_track_id_3\r\n"
+ "a=sendrecv\r\n"
+ "a=rtcp-mux\r\n"
+ "a=rtcp-rsize\r\n"
+ "a=crypto:1 AES_CM_128_HMAC_SHA1_32 "
+ "inline:NzB4d1BINUAvLEw6UzF3WSJ+PSdFcGdUJShpX1Zj|2^20|1:32 "
+ "dummy_session_params\r\n"
+ "a=rtpmap:111 opus/48000/2\r\n"
+ "a=rtpmap:103 ISAC/16000\r\n"
+ "a=rtpmap:104 ISAC/32000\r\n"
+ "a=ssrc:7 cname:stream_2_cname\r\n";
+
// One candidate reference string as per W3c spec.
// candidate:<blah> not a=candidate:<blah>CRLF
static const char kRawCandidate[] =
@@ -724,6 +727,8 @@
// Extra ufrags/passwords for extra unified plan m= sections.
static const char kUfragVoice2[] = "ufrag_voice_2";
static const char kPwdVoice2[] = "pwd_voice_2";
+static const char kUfragVoice3[] = "ufrag_voice_3";
+static const char kPwdVoice3[] = "pwd_voice_3";
static const char kUfragVideo2[] = "ufrag_video_2";
static const char kPwdVideo2[] = "pwd_video_2";
static const char kUfragVideo3[] = "ufrag_video_3";
@@ -736,6 +741,7 @@
// Extra content names for extra unified plan m= sections.
static const char kAudioContentName2[] = "audio_content_name_2";
+static const char kAudioContentName3[] = "audio_content_name_3";
static const char kVideoContentName2[] = "video_content_name_2";
static const char kVideoContentName3[] = "video_content_name_3";
@@ -757,6 +763,8 @@
static const uint32_t kVideoTrack2Ssrc = 5;
static const char kVideoTrackId3[] = "video_track_id_3";
static const uint32_t kVideoTrack3Ssrc = 6;
+static const char kAudioTrackId3[] = "audio_track_id_3";
+static const uint32_t kAudioTrack3Ssrc = 7;
// DataChannel
static const char kDataChannelLabel[] = "data_channel";
@@ -1010,12 +1018,7 @@
}
}
- // Turns the existing reference description into a description using
- // a=bundle-only. This means no transport attributes and a 0 port value on
- // the m= sections not associated with the BUNDLE-tag.
- void MakeBundleOnlyDescription() {
- // Remove video candidates. JsepSessionDescription doesn't make it
- // simple.
+ void RemoveVideoCandidates() {
const IceCandidateCollection* video_candidates_collection =
jdesc_.candidates(1);
ASSERT_NE(nullptr, video_candidates_collection);
@@ -1026,6 +1029,13 @@
video_candidates.push_back(c);
}
jdesc_.RemoveCandidates(video_candidates);
+ }
+
+ // Turns the existing reference description into a description using
+ // a=bundle-only. This means no transport attributes and a 0 port value on
+ // the m= sections not associated with the BUNDLE-tag.
+ void MakeBundleOnlyDescription() {
+ RemoveVideoCandidates();
// And the rest of the transport attributes.
desc_.transport_infos()[1].description.ice_ufrag.clear();
@@ -1142,6 +1152,45 @@
return audio;
}
+ // Turns the existing reference description into a unified plan description,
+ // with 3 audio MediaContentDescriptions with special StreamParams that
+ // contain 0 or multiple stream ids: - audio track 1 has 1 media stream id -
+ // audio track 2 has 2 media stream ids - audio track 3 has 0 media stream ids
+ void MakeUnifiedPlanDescriptionMultipleStreamIds() {
+ desc_.RemoveContentByName(kVideoContentName);
+ desc_.RemoveTransportInfoByName(kVideoContentName);
+ RemoveVideoCandidates();
+
+ // Audio track 2 has 2 media stream ids.
+ AudioContentDescription* audio_desc_2 = CreateAudioContentDescription();
+ StreamParams audio_track_2;
+ audio_track_2.id = kAudioTrackId2;
+ audio_track_2.cname = kStream1Cname;
+ audio_track_2.set_stream_ids({kStreamId1, kStreamId2});
+ audio_track_2.ssrcs.push_back(kAudioTrack2Ssrc);
+ audio_desc_2->AddStream(audio_track_2);
+ desc_.AddContent(kAudioContentName2, MediaProtocolType::kRtp, audio_desc_2);
+ EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
+ kAudioContentName2, TransportDescription(kUfragVoice2, kPwdVoice2))));
+
+ // Audio track 3 has no stream ids.
+ AudioContentDescription* audio_desc_3 = CreateAudioContentDescription();
+ StreamParams audio_track_3;
+ audio_track_3.id = kAudioTrackId3;
+ audio_track_3.cname = kStream2Cname;
+ audio_track_3.set_stream_ids({});
+ audio_track_3.ssrcs.push_back(kAudioTrack3Ssrc);
+ audio_desc_3->AddStream(audio_track_3);
+ desc_.AddContent(kAudioContentName3, MediaProtocolType::kRtp, audio_desc_3);
+ EXPECT_TRUE(desc_.AddTransportInfo(TransportInfo(
+ kAudioContentName3, TransportDescription(kUfragVoice3, kPwdVoice3))));
+ // Make sure to create both a=msid lines.
+ desc_.set_msid_signaling(cricket::kMsidSignalingMediaSection |
+ cricket::kMsidSignalingSsrcAttribute);
+ 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() {
@@ -3281,20 +3330,6 @@
TestSerialize(jdesc_);
}
-// Some WebRTC endpoints include the msid in both the Plan B and Unified Plan
-// ways, to make SDP that's compatible with both Plan B and Unified Plan (to
-// some extent). If we parse this, the Plan B msid attribute (which is more
-// specific, since it's at the SSRC level) should take priority.
-TEST_F(WebRtcSdpTest, DeserializePlanBSessionDescriptionWithMsid) {
- MakePlanBDescription();
-
- JsepSessionDescription deserialized_description(kDummyType);
- EXPECT_TRUE(
- SdpDeserialize(kPlanBSdpFullStringWithMsid, &deserialized_description));
-
- EXPECT_TRUE(CompareSessionDescription(jdesc_, deserialized_description));
-}
-
TEST_F(WebRtcSdpTest, DeserializeUnifiedPlanSessionDescription) {
MakeUnifiedPlanDescription();
@@ -3310,6 +3345,26 @@
TestSerialize(jdesc_);
}
+// This tests deserializing a Unified Plan SDP that is compatible with both
+// Unified Plan and Plan B style SDP. It tests the case for audio/video tracks
+// with no stream ids and multiple stream ids. For parsing this, the Unified
+// Plan a=msid lines should take priority, because the Plan B style a=ssrc msid
+// lines do not support multiple stream ids and no stream ids.
+TEST_F(WebRtcSdpTest, DeserializeUnifiedPlanSessionDescriptionSpecialMsid) {
+ MakeUnifiedPlanDescriptionMultipleStreamIds();
+
+ JsepSessionDescription deserialized_description(kDummyType);
+ EXPECT_TRUE(SdpDeserialize(kUnifiedPlanSdpFullStringWithSpecialMsid,
+ &deserialized_description));
+
+ EXPECT_TRUE(CompareSessionDescription(jdesc_, deserialized_description));
+}
+
+TEST_F(WebRtcSdpTest, SerializeUnifiedPlanSessionDescriptionSpecialMsid) {
+ MakeUnifiedPlanDescriptionMultipleStreamIds();
+ TestSerialize(jdesc_);
+}
+
TEST_F(WebRtcSdpTest, EmptyDescriptionHasNoMsidSignaling) {
JsepSessionDescription jsep_desc(kDummyType);
ASSERT_TRUE(SdpDeserialize(kSdpSessionString, &jsep_desc));
