Fire OnAddTrack when Unified Plan is enabled
When Unified Plan semantics are set, PeerConnection will fire OnAddTrack
according to the WebRTC spec. OnRemoveTrack will never be fired and will
be deprecated in the future.
Bug: webrtc:7600
Change-Id: Idfaada65b795b5fb9fe4844cff036d52ea90da17
Reviewed-on: https://webrtc-review.googlesource.com/38122
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21564}diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index d6b4414..0888acc 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -1980,6 +1980,7 @@
}
if (IsUnifiedPlan()) {
+ std::vector<TrackEvent> track_events;
for (auto transceiver : transceivers_) {
const ContentInfo* content =
FindMediaSectionForTransceiver(transceiver, remote_description());
@@ -1997,15 +1998,27 @@
(!transceiver->current_direction() ||
!RtpTransceiverDirectionHasRecv(
*transceiver->current_direction()))) {
- // TODO(bugs.webrtc.org/7600): Process the addition of a remote track.
+ const std::string& sync_label = media_desc->streams()[0].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);
+ }
+ transceiver->internal()->receiver_internal()->SetStreams({stream});
+ TrackEvent track_event;
+ track_event.receiver = transceiver->receiver();
+ track_event.streams = transceiver->receiver()->streams();
+ track_events.push_back(std::move(track_event));
}
// If direction is sendonly or inactive, and transceiver's current
// direction is neither sendonly nor inactive, process the removal of a
// remote track for the media description.
if (!RtpTransceiverDirectionHasRecv(local_direction) &&
- (!transceiver->current_direction() ||
+ (transceiver->current_direction() &&
RtpTransceiverDirectionHasRecv(*transceiver->current_direction()))) {
- // TODO(bugs.webrtc.org/7600): Process the removal of a remote track.
+ transceiver->internal()->receiver_internal()->SetStreams({});
}
if (type == SdpType::kPrAnswer || type == SdpType::kAnswer) {
transceiver->internal()->set_current_direction(local_direction);
@@ -2014,6 +2027,9 @@
transceiver->Stop();
}
}
+ for (auto event : track_events) {
+ observer_->OnAddTrack(event.receiver, event.streams);
+ }
}
const cricket::ContentInfo* audio_content =
@@ -2760,8 +2776,6 @@
new AudioRtpReceiver(worker_thread(), remote_sender_info.sender_id,
streams, remote_sender_info.first_ssrc,
voice_media_channel()));
- stream->AddTrack(
- static_cast<AudioTrackInterface*>(receiver->internal()->track().get()));
GetAudioTransceiver()->internal()->AddReceiver(receiver);
observer_->OnAddTrack(receiver, std::move(streams));
}
@@ -2777,8 +2791,6 @@
new VideoRtpReceiver(worker_thread(), remote_sender_info.sender_id,
streams, remote_sender_info.first_ssrc,
video_media_channel()));
- stream->AddTrack(
- static_cast<VideoTrackInterface*>(receiver->internal()->track().get()));
GetVideoTransceiver()->internal()->AddReceiver(receiver);
observer_->OnAddTrack(receiver, std::move(streams));
}
diff --git a/pc/peerconnection.h b/pc/peerconnection.h
index f2f833c..d86f297 100644
--- a/pc/peerconnection.h
+++ b/pc/peerconnection.h
@@ -314,6 +314,11 @@
uint32_t first_ssrc;
};
+ struct TrackEvent {
+ rtc::scoped_refptr<RtpReceiverInterface> receiver;
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams;
+ };
+
// Implements MessageHandler.
void OnMessage(rtc::Message* msg) override;
diff --git a/pc/peerconnection_jsep_unittest.cc b/pc/peerconnection_jsep_unittest.cc
index edab082..0ba96e1 100644
--- a/pc/peerconnection_jsep_unittest.cc
+++ b/pc/peerconnection_jsep_unittest.cc
@@ -39,6 +39,7 @@
using ::testing::Values;
using ::testing::Combine;
using ::testing::ElementsAre;
+using ::testing::UnorderedElementsAre;
class PeerConnectionFactoryForJsepTest : public PeerConnectionFactory {
public:
@@ -961,4 +962,82 @@
EXPECT_EQ(receiver_id, receiver->id());
}
+// Test that setting a remote offer with one track that has no streams fires off
+// the correct OnAddTrack event.
+TEST_F(PeerConnectionJsepTest,
+ SetRemoteOfferWithOneTrackNoStreamFiresOnAddTrack) {
+ const std::string kTrackLabel = "audio_track";
+
+ auto caller = CreatePeerConnection();
+ auto callee = CreatePeerConnection();
+ ASSERT_TRUE(caller->AddAudioTrack(kTrackLabel));
+
+ 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.
+}
+
+// Test that setting a remote offer with one track that has one stream fires off
+// the correct OnAddTrack event.
+TEST_F(PeerConnectionJsepTest,
+ SetRemoteOfferWithOneTrackOneStreamFiresOnAddTrack) {
+ const std::string kTrackLabel = "audio_track";
+ const std::string kStreamLabel = "audio_stream";
+
+ auto caller = CreatePeerConnection();
+ auto callee = CreatePeerConnection();
+ ASSERT_TRUE(caller->AddAudioTrack(kTrackLabel, {kStreamLabel}));
+
+ 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(1u, event.streams.size());
+ auto stream = event.streams[0];
+ EXPECT_EQ(kStreamLabel, stream->label());
+ EXPECT_THAT(track_events[0].snapshotted_stream_tracks.at(stream),
+ ElementsAre(event.receiver->track()));
+ EXPECT_EQ(event.receiver->streams(), track_events[0].streams);
+}
+
+// Test that setting a remote offer with two tracks that share the same stream
+// fires off two OnAddTrack events, both with the same stream that has both
+// tracks present at the time of firing. This is to ensure that track events are
+// not fired until SetRemoteDescription has finished processing all the media
+// sections.
+TEST_F(PeerConnectionJsepTest,
+ SetRemoteOfferWithTwoTracksSameStreamFiresOnAddTrack) {
+ const std::string kTrack1Label = "audio_track1";
+ const std::string kTrack2Label = "audio_track2";
+ const std::string kSharedStreamLabel = "stream";
+
+ auto caller = CreatePeerConnection();
+ auto callee = CreatePeerConnection();
+ ASSERT_TRUE(caller->AddAudioTrack(kTrack1Label, {kSharedStreamLabel}));
+ ASSERT_TRUE(caller->AddAudioTrack(kTrack2Label, {kSharedStreamLabel}));
+
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
+
+ const auto& track_events = callee->observer()->add_track_events_;
+ ASSERT_EQ(2u, track_events.size());
+ const auto& event1 = track_events[0];
+ const auto& event2 = track_events[1];
+ ASSERT_EQ(1u, event1.streams.size());
+ auto stream = event1.streams[0];
+ ASSERT_THAT(event2.streams, ElementsAre(stream));
+ auto track1 = event1.receiver->track();
+ auto track2 = event2.receiver->track();
+ EXPECT_THAT(event1.snapshotted_stream_tracks.at(stream),
+ UnorderedElementsAre(track1, track2));
+ EXPECT_THAT(event2.snapshotted_stream_tracks.at(stream),
+ UnorderedElementsAre(track1, track2));
+}
+
+// TODO(bugs.webrtc.org/7932): Also test multi-stream case.
+
} // namespace webrtc
diff --git a/pc/peerconnectionwrapper.cc b/pc/peerconnectionwrapper.cc
index 35aebcc..21d1542 100644
--- a/pc/peerconnectionwrapper.cc
+++ b/pc/peerconnectionwrapper.cc
@@ -277,14 +277,14 @@
rtc::scoped_refptr<RtpSenderInterface> PeerConnectionWrapper::AddAudioTrack(
const std::string& track_label,
- std::vector<MediaStreamInterface*> streams) {
- return pc()->AddTrack(CreateAudioTrack(track_label), std::move(streams));
+ const std::vector<std::string>& stream_labels) {
+ return AddTrack(CreateAudioTrack(track_label), stream_labels);
}
rtc::scoped_refptr<RtpSenderInterface> PeerConnectionWrapper::AddVideoTrack(
const std::string& track_label,
- std::vector<MediaStreamInterface*> streams) {
- return pc()->AddTrack(CreateVideoTrack(track_label), std::move(streams));
+ const std::vector<std::string>& stream_labels) {
+ return AddTrack(CreateVideoTrack(track_label), stream_labels);
}
rtc::scoped_refptr<DataChannelInterface>
diff --git a/pc/peerconnectionwrapper.h b/pc/peerconnectionwrapper.h
index c20c1f2..b5aa163 100644
--- a/pc/peerconnectionwrapper.h
+++ b/pc/peerconnectionwrapper.h
@@ -140,13 +140,13 @@
// stream track not bound to any source.
rtc::scoped_refptr<RtpSenderInterface> AddAudioTrack(
const std::string& track_label,
- std::vector<MediaStreamInterface*> streams = {});
+ const std::vector<std::string>& stream_labels = {});
// Calls the underlying PeerConnection's AddTrack method with a video media
// stream track fed by a fake video capturer.
rtc::scoped_refptr<RtpSenderInterface> AddVideoTrack(
const std::string& track_label,
- std::vector<MediaStreamInterface*> streams = {});
+ const std::vector<std::string>& stream_labels = {});
// Calls the underlying PeerConnection's CreateDataChannel method with default
// initialization parameters.
diff --git a/pc/rtpreceiver.cc b/pc/rtpreceiver.cc
index 33ad899..98c501c 100644
--- a/pc/rtpreceiver.cc
+++ b/pc/rtpreceiver.cc
@@ -24,7 +24,7 @@
AudioRtpReceiver::AudioRtpReceiver(
rtc::Thread* worker_thread,
const std::string& receiver_id,
- std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams,
+ const std::vector<rtc::scoped_refptr<MediaStreamInterface>>& streams,
uint32_t ssrc,
cricket::VoiceMediaChannel* media_channel)
: worker_thread_(worker_thread),
@@ -35,12 +35,12 @@
AudioTrack::Create(
receiver_id,
RemoteAudioSource::Create(worker_thread, media_channel, ssrc)))),
- streams_(std::move(streams)),
cached_track_enabled_(track_->enabled()) {
RTC_DCHECK(worker_thread_);
RTC_DCHECK(track_->GetSource()->remote());
track_->RegisterObserver(this);
track_->GetSource()->RegisterAudioObserver(this);
+ SetStreams(streams);
SetMediaChannel(media_channel);
Reconfigure();
}
@@ -118,6 +118,39 @@
stopped_ = true;
}
+void AudioRtpReceiver::SetStreams(
+ const std::vector<rtc::scoped_refptr<MediaStreamInterface>>& streams) {
+ // Remove remote track from any streams that are going away.
+ for (auto existing_stream : streams_) {
+ bool removed = true;
+ for (auto stream : streams) {
+ if (existing_stream->label() == stream->label()) {
+ RTC_DCHECK_EQ(existing_stream.get(), stream.get());
+ removed = false;
+ break;
+ }
+ }
+ if (removed) {
+ existing_stream->RemoveTrack(track_);
+ }
+ }
+ // Add remote track to any streams that are new.
+ for (auto stream : streams) {
+ bool added = true;
+ for (auto existing_stream : streams_) {
+ if (stream->label() == existing_stream->label()) {
+ RTC_DCHECK_EQ(stream.get(), existing_stream.get());
+ added = false;
+ break;
+ }
+ }
+ if (added) {
+ stream->AddTrack(track_);
+ }
+ }
+ streams_ = streams;
+}
+
std::vector<RtpSource> AudioRtpReceiver::GetSources() const {
return worker_thread_->Invoke<std::vector<RtpSource>>(
RTC_FROM_HERE, [&] { return media_channel_->GetSources(ssrc_); });
@@ -157,12 +190,12 @@
VideoRtpReceiver::VideoRtpReceiver(
rtc::Thread* worker_thread,
- const std::string& track_id,
- std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams,
+ const std::string& receiver_id,
+ const std::vector<rtc::scoped_refptr<MediaStreamInterface>>& streams,
uint32_t ssrc,
cricket::VideoMediaChannel* media_channel)
: worker_thread_(worker_thread),
- id_(track_id),
+ id_(receiver_id),
ssrc_(ssrc),
source_(new RefCountedObject<VideoTrackSource>(&broadcaster_,
true /* remote */)),
@@ -170,13 +203,13 @@
rtc::Thread::Current(),
worker_thread,
VideoTrack::Create(
- track_id,
+ receiver_id,
VideoTrackSourceProxy::Create(rtc::Thread::Current(),
worker_thread,
source_),
- worker_thread))),
- streams_(std::move(streams)) {
+ worker_thread))) {
RTC_DCHECK(worker_thread_);
+ SetStreams(streams);
source_->SetState(MediaSourceInterface::kLive);
SetMediaChannel(media_channel);
}
@@ -229,6 +262,39 @@
stopped_ = true;
}
+void VideoRtpReceiver::SetStreams(
+ const std::vector<rtc::scoped_refptr<MediaStreamInterface>>& streams) {
+ // Remove remote track from any streams that are going away.
+ for (auto existing_stream : streams_) {
+ bool removed = true;
+ for (auto stream : streams) {
+ if (existing_stream->label() == stream->label()) {
+ RTC_DCHECK_EQ(existing_stream.get(), stream.get());
+ removed = false;
+ break;
+ }
+ }
+ if (removed) {
+ existing_stream->RemoveTrack(track_);
+ }
+ }
+ // Add remote track to any streams that are new.
+ for (auto stream : streams) {
+ bool added = true;
+ for (auto existing_stream : streams_) {
+ if (stream->label() == existing_stream->label()) {
+ RTC_DCHECK_EQ(stream.get(), existing_stream.get());
+ added = false;
+ break;
+ }
+ }
+ if (added) {
+ stream->AddTrack(track_);
+ }
+ }
+ streams_ = streams;
+}
+
void VideoRtpReceiver::SetObserver(RtpReceiverObserverInterface* observer) {
observer_ = observer;
// Deliver any notifications the observer may have missed by being set late.
diff --git a/pc/rtpreceiver.h b/pc/rtpreceiver.h
index 0ce6373..e3848a7 100644
--- a/pc/rtpreceiver.h
+++ b/pc/rtpreceiver.h
@@ -33,6 +33,7 @@
class RtpReceiverInternal : public RtpReceiverInterface {
public:
virtual void Stop() = 0;
+
// This SSRC is used as an identifier for the receiver between the API layer
// and the WebRtcVideoEngine, WebRtcVoiceEngine layer.
virtual uint32_t ssrc() const = 0;
@@ -40,6 +41,12 @@
// Call this to notify the RtpReceiver when the first packet has been received
// on the corresponding channel.
virtual void NotifyFirstPacketReceived() = 0;
+
+ // Set the associated remote media streams for this receiver. The remote track
+ // will be removed from any streams that are no longer present and added to
+ // any new streams.
+ virtual void SetStreams(
+ const std::vector<rtc::scoped_refptr<MediaStreamInterface>>& streams) = 0;
};
class AudioRtpReceiver : public ObserverInterface,
@@ -53,7 +60,7 @@
AudioRtpReceiver(
rtc::Thread* worker_thread,
const std::string& receiver_id,
- std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams,
+ const std::vector<rtc::scoped_refptr<MediaStreamInterface>>& streams,
uint32_t ssrc,
cricket::VoiceMediaChannel* media_channel);
virtual ~AudioRtpReceiver();
@@ -90,6 +97,8 @@
void Stop() override;
uint32_t ssrc() const override { return ssrc_; }
void NotifyFirstPacketReceived() override;
+ void SetStreams(const std::vector<rtc::scoped_refptr<MediaStreamInterface>>&
+ streams) override;
void SetObserver(RtpReceiverObserverInterface* observer) override;
@@ -122,8 +131,8 @@
// sees.
VideoRtpReceiver(
rtc::Thread* worker_thread,
- const std::string& track_id,
- std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams,
+ const std::string& receiver_id,
+ const std::vector<rtc::scoped_refptr<MediaStreamInterface>>& streams,
uint32_t ssrc,
cricket::VideoMediaChannel* media_channel);
@@ -155,6 +164,8 @@
void Stop() override;
uint32_t ssrc() const override { return ssrc_; }
void NotifyFirstPacketReceived() override;
+ void SetStreams(const std::vector<rtc::scoped_refptr<MediaStreamInterface>>&
+ streams) override;
void SetObserver(RtpReceiverObserverInterface* observer) override;
@@ -166,7 +177,7 @@
bool SetSink(rtc::VideoSinkInterface<VideoFrame>* sink);
rtc::Thread* const worker_thread_;
- std::string id_;
+ const std::string id_;
uint32_t ssrc_;
cricket::VideoMediaChannel* media_channel_ = nullptr;
// |broadcaster_| is needed since the decoder can only handle one sink.
diff --git a/pc/test/mockpeerconnectionobservers.h b/pc/test/mockpeerconnectionobservers.h
index 5eb29f4..86f2236 100644
--- a/pc/test/mockpeerconnectionobservers.h
+++ b/pc/test/mockpeerconnectionobservers.h
@@ -14,6 +14,7 @@
#ifndef PC_TEST_MOCKPEERCONNECTIONOBSERVERS_H_
#define PC_TEST_MOCKPEERCONNECTIONOBSERVERS_H_
+#include <map>
#include <memory>
#include <string>
#include <utility>
@@ -31,12 +32,29 @@
public:
struct AddTrackEvent {
explicit AddTrackEvent(
- rtc::scoped_refptr<RtpReceiverInterface> receiver,
- std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams)
- : receiver(std::move(receiver)), streams(std::move(streams)) {}
+ rtc::scoped_refptr<RtpReceiverInterface> event_receiver,
+ std::vector<rtc::scoped_refptr<MediaStreamInterface>> event_streams)
+ : receiver(std::move(event_receiver)),
+ streams(std::move(event_streams)) {
+ for (auto stream : streams) {
+ std::vector<rtc::scoped_refptr<MediaStreamTrackInterface>> tracks;
+ for (auto audio_track : stream->GetAudioTracks()) {
+ tracks.push_back(audio_track);
+ }
+ for (auto video_track : stream->GetVideoTracks()) {
+ tracks.push_back(video_track);
+ }
+ snapshotted_stream_tracks[stream] = tracks;
+ }
+ }
rtc::scoped_refptr<RtpReceiverInterface> receiver;
std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams;
+ // This map records the tracks present in each stream at the time the
+ // OnAddTrack callback was issued.
+ std::map<rtc::scoped_refptr<MediaStreamInterface>,
+ std::vector<rtc::scoped_refptr<MediaStreamTrackInterface>>>
+ snapshotted_stream_tracks;
};
MockPeerConnectionObserver() : remote_streams_(StreamCollection::Create()) {}
