Reland "Reland "Adds support for multiple or no media stream ids.""
This is a reland of f351c3408a0c7f695447a2a9f4e6a1719a0d6a26
Reland history:
The original CL broke tests in chromium which were manually tested in
the first reland. Another small fix was added to the reland to fix a
downstream bug, which caused separate tests to fail in chromium.
These were not caught because the chromium trybot was down. These
are temporarily disabled in chrome to allow this change to roll in.
Original change's description:
> Reland "Adds support for multiple or no media stream ids."
>
> This is a reland of 1550292efe680ac79a18004705c908b1cdca54cb
>
> 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}
>
> Bug: webrtc:7932, webrtc:7933
> Change-Id: Ica272ac18088103e65cccf6b96a6d3ecccb178ed
> Reviewed-on: https://webrtc-review.googlesource.com/65560
> 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@{#22687}
TBR=deadbeef@webrtc.org
Bug: webrtc:7932, webrtc:7933
Change-Id: Ideb30219b2f952dd51428cd4e8bd43ef49df5b17
Reviewed-on: https://webrtc-review.googlesource.com/66280
Commit-Queue: Seth Hampson <shampson@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22699}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 f2bc958..52a9441 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 bad0926..f1e5ac2 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -599,6 +599,17 @@
return oss.str();
}
+std::string GetStreamIdsString(rtc::ArrayView<const std::string> stream_ids) {
+ std::string output = "streams=[";
+ const char* separator = "";
+ for (const auto& stream_id : stream_ids) {
+ output.append(separator).append(stream_id);
+ separator = ", ";
+ }
+ output.append("]");
+ return output;
+}
+
rtc::Optional<int> RTCConfigurationToIceConfigOptionalInt(
int rtc_configuration_parameter) {
if (rtc_configuration_parameter ==
@@ -1153,12 +1164,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.");
@@ -1168,18 +1173,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);
@@ -1191,17 +1187,26 @@
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());
+ new_sender->internal()->stream_ids()[0], track->id());
if (sender_info) {
new_sender->internal()->SetSsrc(sender_info->first_ssrc);
}
@@ -1211,7 +1216,7 @@
GetVideoTransceiver()->internal()->AddSender(new_sender);
const RtpSenderInfo* sender_info =
FindSenderInfo(local_video_sender_infos_,
- new_sender->internal()->stream_id(), track->id());
+ new_sender->internal()->stream_ids()[0], track->id());
if (sender_info) {
new_sender->internal()->SetSsrc(sender_info->first_ssrc);
}
@@ -1387,17 +1392,9 @@
// 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()};
-
RTC_LOG(LS_INFO) << "Adding " << cricket::MediaTypeToString(media_type)
<< " transceiver in response to a call to AddTransceiver.";
- 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);
@@ -1504,8 +1501,15 @@
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());
+ RTC_LOG(LS_INFO)
+ << "No stream_id specified for sender. Generated stream ID: "
+ << stream_ids[0];
+ } else {
stream_ids.push_back(stream_id);
}
@@ -2380,13 +2384,14 @@
// 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) &&
@@ -2394,17 +2399,22 @@
!RtpTransceiverDirectionHasRecv(
*transceiver->current_direction()))) {
RTC_LOG(LS_INFO) << "Processing the addition of a new track for MID="
- << content->name << " (added to stream=" << sync_label
- << ").";
- 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);
+ << content->name << " (added to "
+ << GetStreamIdsString(stream_ids) << ").";
+
+ 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
@@ -3351,7 +3361,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;
}
@@ -3394,7 +3404,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;
}
@@ -4025,6 +4035,8 @@
bool default_sender_needed,
cricket::MediaType media_type,
StreamCollection* new_streams) {
+ RTC_DCHECK(!IsUnifiedPlan());
+
std::vector<RtpSenderInfo>* current_senders =
GetRemoteSenderInfos(media_type);
@@ -4036,7 +4048,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) {
@@ -4050,10 +4063,13 @@
// 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] : "");
+ (!params.stream_ids().empty() ? params.stream_ids()[0]
+ : kDefaultStreamId);
const std::string& sender_id = params.id;
uint32_t ssrc = params.first_ssrc();
@@ -4203,6 +4219,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 "
@@ -4217,7 +4234,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);
}
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..bcfe0d8 100644
--- a/pc/peerconnection_rtp_unittest.cc
+++ b/pc/peerconnection_rtp_unittest.cc
@@ -17,6 +17,7 @@
#include "api/mediastreaminterface.h"
#include "api/peerconnectioninterface.h"
#include "api/umametrics.h"
+#include "pc/mediasession.h"
#include "pc/mediastream.h"
#include "pc/mediastreamtrack.h"
#include "pc/peerconnectionwrapper.h"
@@ -115,8 +116,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 +220,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 +277,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 +383,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 +553,74 @@
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);
+}
+
+// This tests a regression caught by a downstream client, that occured when
+// applying a remote description with a SessionDescription object that
+// contained StreamParams that didn't have ids. Although there were multiple
+// remote audio senders, FindSenderInfo didn't find them as unique, because
+// it looked up by StreamParam.id, which none had. This meant only one
+// AudioRtpReceiver was created, as opposed to one for each remote sender.
+TEST_F(PeerConnectionRtpObserverTest,
+ MultipleRemoteSendersWithoutStreamParamIdAddsMultipleReceivers) {
+ auto caller = CreatePeerConnection();
+ auto callee = CreatePeerConnection();
+
+ const char kStreamId1[] = "stream1";
+ const char kStreamId2[] = "stream2";
+ caller->AddAudioTrack("audio_track1", {kStreamId1});
+ caller->AddAudioTrack("audio_track2", {kStreamId2});
+
+ auto offer = caller->CreateOfferAndSetAsLocal();
+ auto mutable_streams =
+ cricket::GetFirstAudioContentDescription(offer->description())
+ ->mutable_streams();
+ ASSERT_EQ(mutable_streams.size(), 2u);
+ // Clear the IDs in the StreamParams.
+ mutable_streams[0].id.clear();
+ mutable_streams[1].id.clear();
+ ASSERT_TRUE(
+ callee->SetRemoteDescription(CloneSessionDescription(offer.get())));
+
+ auto receivers = callee->pc()->GetReceivers();
+ ASSERT_EQ(receivers.size(), 2u);
+ ASSERT_EQ(receivers[0]->streams().size(), 1u);
+ EXPECT_EQ(kStreamId1, receivers[0]->streams()[0]->id());
+ ASSERT_EQ(receivers[1]->streams().size(), 1u);
+ EXPECT_EQ(kStreamId2, receivers[1]->streams()[0]->id());
+}
+
// Tests for the legacy SetRemoteDescription() function signature.
class PeerConnectionRtpLegacyObserverTest : public PeerConnectionRtpTest {};
@@ -1068,6 +1184,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[1].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 ad85d68..9242eea 100644
--- a/pc/peerconnectioninterface_unittest.cc
+++ b/pc/peerconnectioninterface_unittest.cc
@@ -1617,9 +1617,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
@@ -3040,6 +3047,33 @@
EXPECT_EQ(0u, observer_.remote_streams()->count());
}
+// This tests that when a Plan B endpoint receives an SDP that signals no media
+// stream IDs indicated by the special character "-" in the a=msid line, that
+// a default stream ID will be used for the MediaStream ID. This can occur
+// when a Unified Plan endpoint signals no media stream IDs, but signals both
+// a=ssrc msid and a=msid lines for interop signaling with Plan B.
+TEST_F(PeerConnectionInterfaceTestPlanB,
+ SdpWithEmptyMsidAndSsrcCreatesDefaultStreamId) {
+ FakeConstraints constraints;
+ constraints.AddMandatory(webrtc::MediaConstraintsInterface::kEnableDtlsSrtp,
+ true);
+ CreatePeerConnection(&constraints);
+ // Add a a=msid line to the SDP. This is prioritized when parsing the SDP, so
+ // the sender's stream ID will be interpreted as no stream IDs.
+ std::string sdp_string = kSdpStringWithStream1AudioTrackOnly;
+ sdp_string.append("a=msid:- audiotrack0\n");
+
+ CreateAndSetRemoteOffer(sdp_string);
+
+ ASSERT_EQ(1u, observer_.remote_streams()->count());
+ // Because SSRCs are signaled the track ID will be what was signaled in the
+ // a=msid line.
+ EXPECT_EQ("audiotrack0", observer_.last_added_track_label_);
+ MediaStreamInterface* remote_stream = observer_.remote_streams()->at(0);
+ EXPECT_EQ("default", remote_stream->id());
+ ASSERT_EQ(1u, remote_stream->GetAudioTracks().size());
+}
+
// This tests that an RtpSender is created when the local description is set
// after adding a local stream.
// TODO(deadbeef): This test and the one below it need to be updated when
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));