Revert "Adds support for multiple or no media stream ids."
This reverts commit 1550292efe680ac79a18004705c908b1cdca54cb.
Reason for revert:
webkit_layout_tests:fast/peerconnection/RTCPeerConnection-sdpSemantics.html is broken, see below. WebRTC roll isn't going through because of it. This CL looks the most suspicious within the 5 in the range.
https://chromium-review.googlesource.com/c/chromium/src/+/981899
https://webrtc.googlesource.com/src.git/+log/bb50ce5bb6d5..27f3bf512827
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_rel_ng/54616
Original change's description:
> 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}
TBR=steveanton@webrtc.org,deadbeef@webrtc.org,shampson@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:7932, webrtc:7933
Change-Id: I1d4e4332b518ec32b305c8af07679650059d02bb
Reviewed-on: https://webrtc-review.googlesource.com/65000
Reviewed-by: Emircan Uysaler <emircan@webrtc.org>
Commit-Queue: Emircan Uysaler <emircan@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22634}diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h
index 6d60b4c..cf9a1f5 100644
--- a/api/peerconnectioninterface.h
+++ b/api/peerconnectioninterface.h
@@ -1152,6 +1152,13 @@
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 01279a5..2ca2edc 100644
--- a/api/rtpsenderinterface.h
+++ b/api/rtpsenderinterface.h
@@ -48,9 +48,8 @@
// to uniquely identify a receiver until we implement Unified Plan SDP.
virtual std::string id() const = 0;
- // 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.
+ // 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.
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 c9a86e6..3ea75fd 100644
--- a/api/rtptransceiverinterface.h
+++ b/api/rtptransceiverinterface.h
@@ -40,6 +40,9 @@
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 2efa0d0..421111a 100644
--- a/media/base/streamparams.cc
+++ b/media/base/streamparams.cc
@@ -141,15 +141,9 @@
if (!cname.empty()) {
ost << "cname:" << cname << ";";
}
- 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;
+ if (!sync_label.empty()) {
+ ost << "sync_label:" << sync_label;
}
- ost << ";";
ost << "}";
return ost.str();
}
@@ -206,15 +200,22 @@
}
std::vector<std::string> StreamParams::stream_ids() const {
- return stream_ids_;
+ if (sync_label.empty()) {
+ return {};
+ }
+ return {sync_label};
}
void StreamParams::set_stream_ids(const std::vector<std::string>& stream_ids) {
- stream_ids_ = 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]);
}
std::string StreamParams::first_stream_id() const {
- return stream_ids_.empty() ? "" : stream_ids_[0];
+ return sync_label;
}
bool IsOneSsrcStream(const StreamParams& sp) {
diff --git a/media/base/streamparams.h b/media/base/streamparams.h
index 22b98c7..016c27a 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 && stream_ids_ == other.stream_ids_);
+ cname == other.cname && sync_label == other.sync_label);
}
bool operator!=(const StreamParams &other) const {
return !(*this == other);
@@ -165,6 +165,8 @@
// 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,
@@ -173,8 +175,6 @@
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 db37180..6e934ae 100644
--- a/media/base/streamparams_unittest.cc
+++ b/media/base/streamparams_unittest.cc
@@ -208,11 +208,8 @@
TEST(StreamParams, ToString) {
cricket::StreamParams sp =
CreateStreamParamsWithSsrcGroup("XYZ", kSsrcs2, arraysize(kSsrcs2));
- 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());
+ EXPECT_STREQ("{ssrcs:[1,2];ssrc_groups:{semantics:XYZ;ssrcs:[1,2]};}",
+ sp.ToString().c_str());
}
TEST(StreamParams, TestIsOneSsrcStream_LegacyStream) {
diff --git a/pc/mediasession.cc b/pc/mediasession.cc
index 474c0b4..8a9c1f8 100644
--- a/pc/mediasession.cc
+++ b/pc/mediasession.cc
@@ -476,6 +476,8 @@
}
}
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);
@@ -486,6 +488,8 @@
// 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 2816cb2..84376ce 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -1141,6 +1141,12 @@
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.");
@@ -1150,9 +1156,18 @@
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, stream_ids)
- : AddTrackPlanB(track, stream_ids));
+ (IsUnifiedPlan() ? AddTrackUnifiedPlan(track, adjusted_stream_ids)
+ : AddTrackPlanB(track, adjusted_stream_ids));
if (sender_or_error.ok()) {
observer_->OnRenegotiationNeeded();
stats_->AddTrack(track);
@@ -1164,25 +1179,17 @@
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, adjusted_stream_ids);
+ auto new_sender = CreateSender(media_type, track, 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_, track->id());
+ FindSenderInfo(local_audio_sender_infos_,
+ new_sender->internal()->stream_id(), track->id());
if (sender_info) {
new_sender->internal()->SetSsrc(sender_info->first_ssrc);
}
@@ -1191,7 +1198,8 @@
new_sender->internal()->SetVideoMediaChannel(video_media_channel());
GetVideoTransceiver()->internal()->AddSender(new_sender);
const RtpSenderInfo* sender_info =
- FindSenderInfo(local_video_sender_infos_, track->id());
+ FindSenderInfo(local_video_sender_infos_,
+ new_sender->internal()->stream_id(), track->id());
if (sender_info) {
new_sender->internal()->SetSsrc(sender_info->first_ssrc);
}
@@ -1362,7 +1370,15 @@
// TODO(bugs.webrtc.org/7600): Verify init.
- auto sender = CreateSender(media_type, track, init.stream_ids);
+ 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 receiver = CreateReceiver(media_type, rtc::CreateRandomUuid());
auto transceiver = CreateAndAddTransceiver(sender, receiver);
transceiver->internal()->set_direction(init.direction);
@@ -1469,12 +1485,8 @@
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()) {
- stream_ids.push_back(rtc::CreateRandomUuid());
- } else {
+ if (!stream_id.empty()) {
stream_ids.push_back(stream_id);
}
@@ -2334,33 +2346,28 @@
// 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.
- std::vector<std::string> stream_ids;
+ //
+ // Create a sync label in the case of an unsignalled msid.
+ std::string sync_label = rtc::CreateRandomUuid();
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();
+ sync_label = stream_params.stream_ids()[0];
}
}
if (RtpTransceiverDirectionHasRecv(local_direction) &&
(!transceiver->current_direction() ||
!RtpTransceiverDirectionHasRecv(
*transceiver->current_direction()))) {
- 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);
+ 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);
}
- transceiver->internal()->receiver_internal()->SetStreams(media_streams);
+ transceiver->internal()->receiver_internal()->SetStreams({stream});
receiving_transceivers.push_back(transceiver);
}
// If direction is sendonly or inactive, and transceiver's current
@@ -3299,7 +3306,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_ids({stream->id()});
+ sender->internal()->set_stream_id(stream->id());
return;
}
@@ -3315,7 +3322,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_, track->id());
+ FindSenderInfo(local_audio_sender_infos_, stream->id(), track->id());
if (sender_info) {
new_sender->internal()->SetSsrc(sender_info->first_ssrc);
}
@@ -3342,7 +3349,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_ids({stream->id()});
+ sender->internal()->set_stream_id(stream->id());
return;
}
@@ -3352,7 +3359,7 @@
new_sender->internal()->SetVideoMediaChannel(video_media_channel());
GetVideoTransceiver()->internal()->AddSender(new_sender);
const RtpSenderInfo* sender_info =
- FindSenderInfo(local_video_sender_infos_, track->id());
+ FindSenderInfo(local_video_sender_infos_, stream->id(), track->id());
if (sender_info) {
new_sender->internal()->SetSsrc(sender_info->first_ssrc);
}
@@ -3973,8 +3980,6 @@
bool default_sender_needed,
cricket::MediaType media_type,
StreamCollection* new_streams) {
- RTC_DCHECK(!IsUnifiedPlan());
-
std::vector<RtpSenderInfo>* current_senders =
GetRemoteSenderInfos(media_type);
@@ -3986,8 +3991,7 @@
const RtpSenderInfo& info = *sender_it;
const cricket::StreamParams* params =
cricket::GetStreamBySsrc(streams, info.first_ssrc);
- bool sender_exists = params && params->id == info.sender_id &&
- params->first_stream_id() == info.stream_id;
+ bool sender_exists = params && params->id == info.sender_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) {
@@ -4001,10 +4005,8 @@
// 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|. 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.
+ // |params.stream_ids|.
+ // TODO(bugs.webrtc.org/7932): Add support for multiple stream ids.
const std::string& stream_id =
(!params.stream_ids().empty() ? params.stream_ids()[0] : "");
const std::string& sender_id = params.id;
@@ -4021,7 +4023,7 @@
}
const RtpSenderInfo* sender_info =
- FindSenderInfo(*current_senders, sender_id);
+ FindSenderInfo(*current_senders, stream_id, sender_id);
if (!sender_info) {
current_senders->push_back(RtpSenderInfo(stream_id, sender_id, ssrc));
OnRemoteSenderAdded(current_senders->back(), media_type);
@@ -4043,7 +4045,7 @@
? kDefaultAudioSenderId
: kDefaultVideoSenderId;
const RtpSenderInfo* default_sender_info =
- FindSenderInfo(*current_senders, default_sender_id);
+ FindSenderInfo(*current_senders, kDefaultStreamId, default_sender_id);
if (!default_sender_info) {
current_senders->push_back(
RtpSenderInfo(kDefaultStreamId, default_sender_id, 0));
@@ -4143,7 +4145,7 @@
const std::string& sender_id = params.id;
uint32_t ssrc = params.first_ssrc();
const RtpSenderInfo* sender_info =
- FindSenderInfo(*current_senders, sender_id);
+ FindSenderInfo(*current_senders, stream_id, sender_id);
if (!sender_info) {
current_senders->push_back(RtpSenderInfo(stream_id, sender_id, ssrc));
OnLocalSenderAdded(current_senders->back(), media_type);
@@ -4153,7 +4155,6 @@
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 "
@@ -4168,7 +4169,7 @@
return;
}
- sender->internal()->set_stream_ids({sender_info.stream_id});
+ sender->internal()->set_stream_id(sender_info.stream_id);
sender->internal()->SetSsrc(sender_info.first_ssrc);
}
@@ -4503,9 +4504,11 @@
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.sender_id == sender_id) {
+ if (sender_info.stream_id == stream_id &&
+ sender_info.sender_id == sender_id) {
return &sender_info;
}
}
diff --git a/pc/peerconnection.h b/pc/peerconnection.h
index 1ca117b..d1ba212 100644
--- a/pc/peerconnection.h
+++ b/pc/peerconnection.h
@@ -635,6 +635,7 @@
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 caa0ea0..84bd060 100644
--- a/pc/peerconnection_jsep_unittest.cc
+++ b/pc/peerconnection_jsep_unittest.cc
@@ -1162,11 +1162,11 @@
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];
- EXPECT_EQ(kTrackLabel, event.receiver->track()->id());
- EXPECT_EQ(0u, event.streams.size());
+ 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.
}
// Test that setting a remote offer with one track that has one stream fires off
@@ -1226,27 +1226,7 @@
UnorderedElementsAre(track1, track2));
}
-// 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());
-}
+// TODO(bugs.webrtc.org/7932): Also test multi-stream case.
// 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 8d055a8..69f329c 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);
- // 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.
+ // TODO(hbos): When "no stream" is handled correctly we would expect
+ // |add_track_events_[0].streams| to be empty. https://crbug.com/webrtc/7933
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,54 +219,15 @@
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. One transceiver is
-// created without any stream_ids, while the other is created with multiple
-// stream_ids.
+// with receive only transceivers will not call OnTrack.
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);
- RtpTransceiverInit video_transceiver_init;
- video_transceiver_init.stream_ids = {kStreamId1, kStreamId2};
- auto video_transceiver =
- caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, video_transceiver_init);
+ auto video_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO);
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
@@ -276,14 +237,6 @@
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
@@ -382,8 +335,8 @@
EXPECT_EQ(callee->pc()->GetReceivers().size(), 1u);
auto receiver_added = callee->pc()->GetReceivers()[0];
EXPECT_EQ("audio_track", receiver_added->track()->id());
- // 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.
+ // TODO(hbos): When "no stream" is handled correctly we would expect
+ // |receiver_added->streams()| to be empty. https://crbug.com/webrtc/7933
EXPECT_EQ(receiver_added->streams().size(), 1u);
EXPECT_TRUE(receiver_added->streams()[0]->FindAudioTrack("audio_track"));
}
@@ -552,38 +505,6 @@
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 {};
@@ -1147,39 +1068,6 @@
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 c906c23..62fd3bf 100644
--- a/pc/peerconnectioninterface_unittest.cc
+++ b/pc/peerconnectioninterface_unittest.cc
@@ -1628,16 +1628,9 @@
EXPECT_EQ(audio_track, audio_sender->track());
EXPECT_EQ("video_track", video_sender->id());
EXPECT_EQ(video_track, video_sender->track());
- 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());
- }
+ // 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());
}
// Test that we can call GetStats() after AddTrack but before connecting
diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc
index 626c850..170aa9c 100644
--- a/pc/rtpsender.cc
+++ b/pc/rtpsender.cc
@@ -79,6 +79,9 @@
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());
@@ -315,6 +318,9 @@
: 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 27b1513..05e1131 100644
--- a/pc/rtpsender.h
+++ b/pc/rtpsender.h
@@ -50,6 +50,12 @@
// 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;
@@ -136,6 +142,10 @@
// 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;
}
@@ -167,6 +177,8 @@
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_;
@@ -224,6 +236,10 @@
// 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;
}
@@ -250,6 +266,8 @@
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 5988c7b..b76a9ac 100644
--- a/pc/test/mock_rtpsenderinternal.h
+++ b/pc/test/mock_rtpsenderinternal.h
@@ -37,6 +37,8 @@
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 a0887a6..80ac8d5 100644
--- a/pc/webrtcsdp.cc
+++ b/pc/webrtcsdp.cc
@@ -120,12 +120,10 @@
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";
@@ -327,7 +325,7 @@
cricket::ConnectionRole* role,
SdpParseError* error);
static bool ParseMsidAttribute(const std::string& line,
- std::vector<std::string>* stream_ids,
+ std::string* stream_id,
std::string* track_id,
SdpParseError* error);
@@ -580,50 +578,43 @@
cricket::IsValidRtpPayloadType(*payload_type);
}
-// |msid_stream_idss| and |msid_track_id| represent the stream/track ID from the
+// |msid_stream_id| 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::vector<std::string>& msid_stream_ids,
+ const std::string& msid_stream_id,
const std::string& msid_track_id,
- StreamParamsVec* tracks,
- int msid_signaling) {
+ StreamParamsVec* tracks) {
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::vector<std::string> stream_ids;
+ std::string stream_id;
std::string track_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 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.
+ 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_ids.push_back(ssrc_info->mslabel);
+ 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;
+ track_id = msid_track_id;
} else {
- // Since no media streams isn't supported with older SDP signaling, we
- // use a default a stream id.
- stream_ids.push_back(kDefaultMsid);
+ stream_id = ssrc_info->stream_id;
+ track_id = ssrc_info->track_id;
}
- // If a track ID wasn't populated from the SSRC attributes OR the
+ // If a stream/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)?
@@ -643,7 +634,7 @@
}
track->add_ssrc(ssrc_info->ssrc_id);
track->cname = ssrc_info->cname;
- track->set_stream_ids(stream_ids);
+ track->set_stream_ids({stream_id});
track->id = track_id;
}
}
@@ -1481,26 +1472,18 @@
}
AddLine(os.str(), message);
- // 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.
+ // Specified in https://tools.ietf.org/html/draft-ietf-mmusic-msid-16
+ // a=msid:<stream id> <track id>
if (msid_signaling & cricket::kMsidSignalingMediaSection) {
const StreamParamsVec& streams = media_desc->streams();
if (streams.size() == 1u) {
const StreamParams& track = streams[0];
- 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);
- }
+ // 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);
} else if (streams.size() > 1u) {
RTC_LOG(LS_WARNING)
<< "Trying to serialize Unified Plan SDP with more than "
@@ -1549,6 +1532,13 @@
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
@@ -1578,9 +1568,8 @@
// 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.
- // 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.
+ // TODO(bugs.webrtc.org/7932): Support serializing more than one stream
+ // label.
const std::string& stream_id = track->first_stream_id();
InitAttrLine(kAttributeSsrc, &os);
os << kSdpDelimiterColon << ssrc << kSdpDelimiterSpace
@@ -1594,7 +1583,8 @@
// a=ssrc:<ssrc-id> mslabel:<value>
// The label isn't yet defined.
// a=ssrc:<ssrc-id> label:<value>
- AddSsrcLine(ssrc, kSsrcAttributeMslabel, stream_id, message);
+ AddSsrcLine(ssrc, kSsrcAttributeMslabel, track->first_stream_id(),
+ message);
AddSsrcLine(ssrc, kSSrcAttributeLabel, track->id, message);
}
}
@@ -2207,45 +2197,32 @@
}
static bool ParseMsidAttribute(const std::string& line,
- std::vector<std::string>* stream_ids,
+ std::string* stream_id,
std::string* track_id,
SdpParseError* error) {
- // https://datatracker.ietf.org/doc/draft-ietf-mmusic-msid/16/
+ // draft-ietf-mmusic-msid-11
// 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, &new_track_id)) {
+ &field1, track_id)) {
const size_t expected_fields = 2;
return ParseFailedExpectFieldNum(line, expected_fields, error);
}
- if (new_track_id.empty()) {
+ if (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, &new_stream_id, error)) {
+ if (!GetValue(field1, kAttributeMsid, stream_id, error)) {
return false;
}
- if (new_stream_id.empty()) {
+ if (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;
}
@@ -2729,7 +2706,7 @@
SsrcGroupVec ssrc_groups;
std::string maxptime_as_string;
std::string ptime_as_string;
- std::vector<std::string> stream_ids;
+ std::string stream_id;
std::string track_id;
// Loop until the next m line
@@ -2929,7 +2906,7 @@
if (flag_value.compare(kValueConference) == 0)
media_desc->set_conference_mode(true);
} else if (HasAttribute(line, kAttributeMsid)) {
- if (!ParseMsidAttribute(line, &stream_ids, &track_id, error)) {
+ if (!ParseMsidAttribute(line, &stream_id, &track_id, error)) {
return false;
}
*msid_signaling |= cricket::kMsidSignalingMediaSection;
@@ -2945,8 +2922,7 @@
// 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);
+ CreateTracksFromSsrcInfos(ssrc_infos, stream_id, track_id, &tracks);
// Add the ssrc group to the track.
for (SsrcGroupVec::iterator ssrc_group = ssrc_groups.begin();
@@ -3062,7 +3038,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 9d6cfa3..918dc30 100644
--- a/pc/webrtcsdp_unittest.cc
+++ b/pc/webrtcsdp_unittest.cc
@@ -480,6 +480,91 @@
"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[] =
@@ -587,94 +672,6 @@
"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[] =
@@ -727,8 +724,6 @@
// 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";
@@ -741,7 +736,6 @@
// 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";
@@ -763,8 +757,6 @@
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";
@@ -1018,7 +1010,12 @@
}
}
- void RemoveVideoCandidates() {
+ // 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.
const IceCandidateCollection* video_candidates_collection =
jdesc_.candidates(1);
ASSERT_NE(nullptr, video_candidates_collection);
@@ -1029,13 +1026,6 @@
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();
@@ -1152,45 +1142,6 @@
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() {
@@ -3330,6 +3281,20 @@
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();
@@ -3345,26 +3310,6 @@
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));
