diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index d6b441426e..0888acced2 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -1980,6 +1980,7 @@ RTCError PeerConnection::ApplyRemoteDescription( } if (IsUnifiedPlan()) { + std::vector track_events; for (auto transceiver : transceivers_) { const ContentInfo* content = FindMediaSectionForTransceiver(transceiver, remote_description()); @@ -1997,15 +1998,27 @@ RTCError PeerConnection::ApplyRemoteDescription( (!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 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 @@ RTCError PeerConnection::ApplyRemoteDescription( transceiver->Stop(); } } + for (auto event : track_events) { + observer_->OnAddTrack(event.receiver, event.streams); + } } const cricket::ContentInfo* audio_content = @@ -2760,8 +2776,6 @@ void PeerConnection::CreateAudioReceiver( new AudioRtpReceiver(worker_thread(), remote_sender_info.sender_id, streams, remote_sender_info.first_ssrc, voice_media_channel())); - stream->AddTrack( - static_cast(receiver->internal()->track().get())); GetAudioTransceiver()->internal()->AddReceiver(receiver); observer_->OnAddTrack(receiver, std::move(streams)); } @@ -2777,8 +2791,6 @@ void PeerConnection::CreateVideoReceiver( new VideoRtpReceiver(worker_thread(), remote_sender_info.sender_id, streams, remote_sender_info.first_ssrc, video_media_channel())); - stream->AddTrack( - static_cast(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 f2f833cc6b..d86f2975b3 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -314,6 +314,11 @@ class PeerConnection : public PeerConnectionInterface, uint32_t first_ssrc; }; + struct TrackEvent { + rtc::scoped_refptr receiver; + std::vector> streams; + }; + // Implements MessageHandler. void OnMessage(rtc::Message* msg) override; diff --git a/pc/peerconnection_jsep_unittest.cc b/pc/peerconnection_jsep_unittest.cc index edab0820e4..0ba96e1a3b 100644 --- a/pc/peerconnection_jsep_unittest.cc +++ b/pc/peerconnection_jsep_unittest.cc @@ -39,6 +39,7 @@ using RTCConfiguration = PeerConnectionInterface::RTCConfiguration; using ::testing::Values; using ::testing::Combine; using ::testing::ElementsAre; +using ::testing::UnorderedElementsAre; class PeerConnectionFactoryForJsepTest : public PeerConnectionFactory { public: @@ -961,4 +962,82 @@ TEST_F(PeerConnectionJsepTest, 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 35aebcce9b..21d154289f 100644 --- a/pc/peerconnectionwrapper.cc +++ b/pc/peerconnectionwrapper.cc @@ -277,14 +277,14 @@ rtc::scoped_refptr PeerConnectionWrapper::AddTrack( rtc::scoped_refptr PeerConnectionWrapper::AddAudioTrack( const std::string& track_label, - std::vector streams) { - return pc()->AddTrack(CreateAudioTrack(track_label), std::move(streams)); + const std::vector& stream_labels) { + return AddTrack(CreateAudioTrack(track_label), stream_labels); } rtc::scoped_refptr PeerConnectionWrapper::AddVideoTrack( const std::string& track_label, - std::vector streams) { - return pc()->AddTrack(CreateVideoTrack(track_label), std::move(streams)); + const std::vector& stream_labels) { + return AddTrack(CreateVideoTrack(track_label), stream_labels); } rtc::scoped_refptr diff --git a/pc/peerconnectionwrapper.h b/pc/peerconnectionwrapper.h index c20c1f259f..b5aa16342b 100644 --- a/pc/peerconnectionwrapper.h +++ b/pc/peerconnectionwrapper.h @@ -140,13 +140,13 @@ class PeerConnectionWrapper { // stream track not bound to any source. rtc::scoped_refptr AddAudioTrack( const std::string& track_label, - std::vector streams = {}); + const std::vector& stream_labels = {}); // Calls the underlying PeerConnection's AddTrack method with a video media // stream track fed by a fake video capturer. rtc::scoped_refptr AddVideoTrack( const std::string& track_label, - std::vector streams = {}); + const std::vector& stream_labels = {}); // Calls the underlying PeerConnection's CreateDataChannel method with default // initialization parameters. diff --git a/pc/rtpreceiver.cc b/pc/rtpreceiver.cc index 33ad899010..98c501c22b 100644 --- a/pc/rtpreceiver.cc +++ b/pc/rtpreceiver.cc @@ -24,7 +24,7 @@ namespace webrtc { AudioRtpReceiver::AudioRtpReceiver( rtc::Thread* worker_thread, const std::string& receiver_id, - std::vector> streams, + const std::vector>& streams, uint32_t ssrc, cricket::VoiceMediaChannel* media_channel) : worker_thread_(worker_thread), @@ -35,12 +35,12 @@ AudioRtpReceiver::AudioRtpReceiver( 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 @@ void AudioRtpReceiver::Stop() { stopped_ = true; } +void AudioRtpReceiver::SetStreams( + const std::vector>& 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 AudioRtpReceiver::GetSources() const { return worker_thread_->Invoke>( RTC_FROM_HERE, [&] { return media_channel_->GetSources(ssrc_); }); @@ -157,12 +190,12 @@ void AudioRtpReceiver::NotifyFirstPacketReceived() { VideoRtpReceiver::VideoRtpReceiver( rtc::Thread* worker_thread, - const std::string& track_id, - std::vector> streams, + const std::string& receiver_id, + const std::vector>& streams, uint32_t ssrc, cricket::VideoMediaChannel* media_channel) : worker_thread_(worker_thread), - id_(track_id), + id_(receiver_id), ssrc_(ssrc), source_(new RefCountedObject(&broadcaster_, true /* remote */)), @@ -170,13 +203,13 @@ VideoRtpReceiver::VideoRtpReceiver( 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 @@ void VideoRtpReceiver::Stop() { stopped_ = true; } +void VideoRtpReceiver::SetStreams( + const std::vector>& 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 0ce637337e..e3848a7c5b 100644 --- a/pc/rtpreceiver.h +++ b/pc/rtpreceiver.h @@ -33,6 +33,7 @@ namespace webrtc { 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 @@ class RtpReceiverInternal : public RtpReceiverInterface { // 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>& streams) = 0; }; class AudioRtpReceiver : public ObserverInterface, @@ -53,7 +60,7 @@ class AudioRtpReceiver : public ObserverInterface, AudioRtpReceiver( rtc::Thread* worker_thread, const std::string& receiver_id, - std::vector> streams, + const std::vector>& streams, uint32_t ssrc, cricket::VoiceMediaChannel* media_channel); virtual ~AudioRtpReceiver(); @@ -90,6 +97,8 @@ class AudioRtpReceiver : public ObserverInterface, void Stop() override; uint32_t ssrc() const override { return ssrc_; } void NotifyFirstPacketReceived() override; + void SetStreams(const std::vector>& + streams) override; void SetObserver(RtpReceiverObserverInterface* observer) override; @@ -122,8 +131,8 @@ class VideoRtpReceiver : public rtc::RefCountedObject { // sees. VideoRtpReceiver( rtc::Thread* worker_thread, - const std::string& track_id, - std::vector> streams, + const std::string& receiver_id, + const std::vector>& streams, uint32_t ssrc, cricket::VideoMediaChannel* media_channel); @@ -155,6 +164,8 @@ class VideoRtpReceiver : public rtc::RefCountedObject { void Stop() override; uint32_t ssrc() const override { return ssrc_; } void NotifyFirstPacketReceived() override; + void SetStreams(const std::vector>& + streams) override; void SetObserver(RtpReceiverObserverInterface* observer) override; @@ -166,7 +177,7 @@ class VideoRtpReceiver : public rtc::RefCountedObject { bool SetSink(rtc::VideoSinkInterface* 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 5eb29f4a55..86f22368d2 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 #include #include #include @@ -31,12 +32,29 @@ class MockPeerConnectionObserver : public PeerConnectionObserver { public: struct AddTrackEvent { explicit AddTrackEvent( - rtc::scoped_refptr receiver, - std::vector> streams) - : receiver(std::move(receiver)), streams(std::move(streams)) {} + rtc::scoped_refptr event_receiver, + std::vector> event_streams) + : receiver(std::move(event_receiver)), + streams(std::move(event_streams)) { + for (auto stream : streams) { + std::vector> 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 receiver; std::vector> streams; + // This map records the tracks present in each stream at the time the + // OnAddTrack callback was issued. + std::map, + std::vector>> + snapshotted_stream_tracks; }; MockPeerConnectionObserver() : remote_streams_(StreamCollection::Create()) {}