[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}
This commit is contained in:
Henrik Boström 2018-12-20 11:06:02 +01:00 committed by Commit Bot
parent 02c4f150a8
commit afa07dda42
3 changed files with 119 additions and 52 deletions

View File

@ -2532,51 +2532,32 @@ RTCError PeerConnection::ApplyRemoteDescription(
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 @@ RTCError PeerConnection::ApplyRemoteDescription(
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 @@ void PeerConnection::ProcessRemovalOfRemoteTrack(
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);
}
}
}

View File

@ -475,6 +475,14 @@ class PeerConnection : public PeerConnectionInternal,
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 @@ class PeerConnection : public PeerConnectionInternal,
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 {

View File

@ -476,6 +476,33 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan,
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().