[Unified Plan] SRD: Always set associated remote streams.
This fixes a bug where the streams are not updated if the "msid" changes
without triggering "ontrack", such as if the streams associated with a
receiver changes while the receiver is active.
Bug: webrtc:10083, chromium:916934
Change-Id: Ic7b19ad5ef648ed6880cae4157bf49f8435467ae
Reviewed-on: https://webrtc-review.googlesource.com/c/114161
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26069}diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index 642ff49..a70ab2e 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -2532,51 +2532,32 @@
const MediaContentDescription* media_desc = content->media_description();
RtpTransceiverDirection local_direction =
RtpTransceiverDirectionReversed(media_desc->direction());
- // From the WebRTC specification, steps 2.2.8.5/6 of section 4.4.1.6 "Set
- // 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;
- if (!media_desc->streams().empty()) {
- // The remote description has signaled the stream IDs.
- stream_ids = media_desc->streams()[0].stream_ids();
- }
- if (RtpTransceiverDirectionHasRecv(local_direction) &&
- (!transceiver->fired_direction() ||
- !RtpTransceiverDirectionHasRecv(*transceiver->fired_direction()))) {
- RTC_LOG(LS_INFO) << "Processing the addition of a new track for MID="
- << 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);
+ // Roughly the same as steps 2.2.8.6 of section 4.4.1.6 "Set the
+ // RTCSessionDescription: Set the associated remote streams given
+ // transceiver.[[Receiver]], msids, addList, and removeList".
+ // https://w3c.github.io/webrtc-pc/#set-the-rtcsessiondescription
+ if (RtpTransceiverDirectionHasRecv(local_direction)) {
+ std::vector<std::string> stream_ids;
+ if (!media_desc->streams().empty()) {
+ // The remote description has signaled the stream IDs.
+ stream_ids = media_desc->streams()[0].stream_ids();
}
- // Special case: "a=msid" missing, use random stream ID.
- if (media_streams.empty() &&
- !(remote_description()->description()->msid_signaling() &
- cricket::kMsidSignalingMediaSection)) {
- if (!missing_msid_default_stream_) {
- missing_msid_default_stream_ = MediaStreamProxy::Create(
- rtc::Thread::Current(),
- MediaStream::Create(rtc::CreateRandomUuid()));
- added_streams.push_back(missing_msid_default_stream_);
- }
- media_streams.push_back(missing_msid_default_stream_);
+ RTC_LOG(LS_INFO) << "Processing the MSIDs for MID=" << content->name
+ << " (" << GetStreamIdsString(stream_ids) << ").";
+ SetAssociatedRemoteStreams(transceiver->internal()->receiver_internal(),
+ stream_ids, &added_streams,
+ &removed_streams);
+ // From the WebRTC specification, steps 2.2.8.5/6 of section 4.4.1.6
+ // "Set 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.
+ if (!transceiver->fired_direction() ||
+ !RtpTransceiverDirectionHasRecv(*transceiver->fired_direction())) {
+ RTC_LOG(LS_INFO)
+ << "Processing the addition of a remote track for MID="
+ << content->name << ".";
+ now_receiving_transceivers.push_back(transceiver);
}
- // This will add the remote track to the streams.
- // TODO(hbos): When we remove remote_streams(), use set_stream_ids()
- // instead. https://crbug.com/webrtc/9480
- transceiver->internal()->receiver_internal()->SetStreams(media_streams);
- now_receiving_transceivers.push_back(transceiver);
}
// 2.2.8.1.7: If direction is "sendonly" or "inactive", and transceiver's
// [[FiredDirection]] slot is either "sendrecv" or "recvonly", process the
@@ -2719,6 +2700,46 @@
return RTCError::OK();
}
+void PeerConnection::SetAssociatedRemoteStreams(
+ rtc::scoped_refptr<RtpReceiverInternal> receiver,
+ const std::vector<std::string>& stream_ids,
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>>* added_streams,
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>>* removed_streams) {
+ 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);
+ }
+ // Special case: "a=msid" missing, use random stream ID.
+ if (media_streams.empty() &&
+ !(remote_description()->description()->msid_signaling() &
+ cricket::kMsidSignalingMediaSection)) {
+ if (!missing_msid_default_stream_) {
+ missing_msid_default_stream_ = MediaStreamProxy::Create(
+ rtc::Thread::Current(), MediaStream::Create(rtc::CreateRandomUuid()));
+ added_streams->push_back(missing_msid_default_stream_);
+ }
+ media_streams.push_back(missing_msid_default_stream_);
+ }
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>> previous_streams =
+ receiver->streams();
+ // SetStreams() will add/remove the receiver's track to/from the streams. This
+ // differs from the spec - the spec uses an "addList" and "removeList" to
+ // update the stream-track relationships in a later step. We do this earlier,
+ // changing the order of things, but the end-result is the same.
+ // TODO(hbos): When we remove remote_streams(), use set_stream_ids()
+ // instead. https://crbug.com/webrtc/9480
+ receiver->SetStreams(media_streams);
+ RemoveRemoteStreamsIfEmpty(previous_streams, removed_streams);
+}
+
void PeerConnection::ProcessRemovalOfRemoteTrack(
rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>
transceiver,
@@ -2727,19 +2748,25 @@
RTC_DCHECK(transceiver->mid());
RTC_LOG(LS_INFO) << "Processing the removal of a track for MID="
<< *transceiver->mid();
- std::vector<rtc::scoped_refptr<MediaStreamInterface>> media_streams =
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>> previous_streams =
transceiver->internal()->receiver_internal()->streams();
// This will remove the remote track from the streams.
transceiver->internal()->receiver_internal()->set_stream_ids({});
remove_list->push_back(transceiver);
- // Remove any streams that no longer have tracks.
- // TODO(https://crbug.com/webrtc/9480): When we use stream IDs instead
- // of streams, see if the stream was removed by checking if this was the
- // last receiver with that stream ID.
- for (auto stream : media_streams) {
- if (stream->GetAudioTracks().empty() && stream->GetVideoTracks().empty()) {
- remote_streams_->RemoveStream(stream);
- removed_streams->push_back(stream);
+ RemoveRemoteStreamsIfEmpty(previous_streams, removed_streams);
+}
+
+void PeerConnection::RemoveRemoteStreamsIfEmpty(
+ const std::vector<rtc::scoped_refptr<MediaStreamInterface>>& remote_streams,
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>>* removed_streams) {
+ // TODO(https://crbug.com/webrtc/9480): When we use stream IDs instead of
+ // streams, see if the stream was removed by checking if this was the last
+ // receiver with that stream ID.
+ for (auto remote_stream : remote_streams) {
+ if (remote_stream->GetAudioTracks().empty() &&
+ remote_stream->GetVideoTracks().empty()) {
+ remote_streams_->RemoveStream(remote_stream);
+ removed_streams->push_back(remote_stream);
}
}
}
diff --git a/pc/peerconnection.h b/pc/peerconnection.h
index 7529543..e596f00 100644
--- a/pc/peerconnection.h
+++ b/pc/peerconnection.h
@@ -475,6 +475,14 @@
transceiver,
const SessionDescriptionInterface* sdesc) const;
+ // Runs the algorithm **set the associated remote streams** specified in
+ // https://w3c.github.io/webrtc-pc/#set-associated-remote-streams.
+ void SetAssociatedRemoteStreams(
+ rtc::scoped_refptr<RtpReceiverInternal> receiver,
+ const std::vector<std::string>& stream_ids,
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>>* added_streams,
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>>* removed_streams);
+
// Runs the algorithm **process the removal of a remote track** specified in
// the WebRTC specification.
// This method will update the following lists:
@@ -489,6 +497,11 @@
std::vector<rtc::scoped_refptr<RtpTransceiverInterface>>* remove_list,
std::vector<rtc::scoped_refptr<MediaStreamInterface>>* removed_streams);
+ void RemoveRemoteStreamsIfEmpty(
+ const std::vector<rtc::scoped_refptr<MediaStreamInterface>>&
+ remote_streams,
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>>* removed_streams);
+
void OnNegotiationNeeded();
bool IsClosed() const {
diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc
index c1f7656..1b78ec2 100644
--- a/pc/peerconnection_rtp_unittest.cc
+++ b/pc/peerconnection_rtp_unittest.cc
@@ -476,6 +476,33 @@
EXPECT_EQ(1u, callee->observer()->remove_track_events_.size());
}
+TEST_F(PeerConnectionRtpTestUnifiedPlan, ChangeMsidWhileReceiving) {
+ auto caller = CreatePeerConnection();
+ caller->AddAudioTrack("audio_track", {"stream1"});
+ auto callee = CreatePeerConnection();
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
+
+ ASSERT_EQ(1u, callee->observer()->on_track_transceivers_.size());
+ auto transceiver = callee->observer()->on_track_transceivers_[0];
+ ASSERT_EQ(1u, transceiver->receiver()->streams().size());
+ EXPECT_EQ("stream1", transceiver->receiver()->streams()[0]->id());
+
+ ASSERT_TRUE(callee->SetLocalDescription(callee->CreateAnswer()));
+
+ // Change the stream ID in the offer.
+ // TODO(https://crbug.com/webrtc/10129): When RtpSenderInterface::SetStreams
+ // is supported, this can use that instead of munging the SDP.
+ auto offer = caller->CreateOffer();
+ auto contents = offer->description()->contents();
+ ASSERT_EQ(1u, contents.size());
+ auto& stream_params = contents[0].media_description()->mutable_streams();
+ ASSERT_EQ(1u, stream_params.size());
+ stream_params[0].set_stream_ids({"stream2"});
+ ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer)));
+ ASSERT_EQ(1u, transceiver->receiver()->streams().size());
+ EXPECT_EQ("stream2", transceiver->receiver()->streams()[0]->id());
+}
+
// These tests examine the state of the peer connection as a result of
// performing SetRemoteDescription().
