From 6f04b653aedc495121863296b8ce52a80b11b6bd Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Fri, 9 Oct 2020 11:42:17 +0000 Subject: [PATCH] Move the streams concept into sdp_offer_answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes it easier to see that the tying of tracks to streams affects only the SDP negotiation, and not what's sent on the wire. Bug: webrtc:11995 Change-Id: I8ca5adf0050e4a2be55d164a6d0e4d5811582476 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/187359 Reviewed-by: Henrik Boström Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#32368} --- pc/peer_connection.cc | 81 ++++---------------------- pc/peer_connection.h | 28 ++------- pc/sdp_offer_answer.cc | 127 ++++++++++++++++++++++++++++++++++++----- pc/sdp_offer_answer.h | 16 ++++++ 4 files changed, 146 insertions(+), 106 deletions(-) diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 6b7028e9e0..29ebd81eb5 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -93,20 +93,6 @@ static const char kDefaultVideoSenderId[] = "defaultv0"; static const int REPORT_USAGE_PATTERN_DELAY_MS = 60000; -// Check if we can send |new_stream| on a PeerConnection. -bool CanAddLocalMediaStream(webrtc::StreamCollectionInterface* current_streams, - webrtc::MediaStreamInterface* new_stream) { - if (!new_stream || !current_streams) { - return false; - } - if (current_streams->find(new_stream->id()) != nullptr) { - RTC_LOG(LS_ERROR) << "MediaStream with ID " << new_stream->id() - << " is already added."; - return false; - } - return true; -} - uint32_t ConvertIceTransportTypeToCandidateFilter( PeerConnectionInterface::IceTransportsType type) { @@ -355,8 +341,6 @@ PeerConnection::PeerConnection(PeerConnectionFactory* factory, : factory_(factory), event_log_(std::move(event_log)), event_log_ptr_(event_log_.get()), - local_streams_(StreamCollection::Create()), - remote_streams_(StreamCollection::Create()), call_(std::move(call)), call_ptr_(call_.get()), sdp_handler_(this), @@ -682,7 +666,7 @@ rtc::scoped_refptr PeerConnection::local_streams() { RTC_CHECK(!IsUnifiedPlan()) << "local_streams is not available with Unified " "Plan SdpSemantics. Please use GetSenders " "instead."; - return local_streams_; + return sdp_handler_.local_streams(); } rtc::scoped_refptr PeerConnection::remote_streams() { @@ -690,7 +674,7 @@ rtc::scoped_refptr PeerConnection::remote_streams() { RTC_CHECK(!IsUnifiedPlan()) << "remote_streams is not available with Unified " "Plan SdpSemantics. Please use GetReceivers " "instead."; - return remote_streams_; + return sdp_handler_.remote_streams(); } bool PeerConnection::AddStream(MediaStreamInterface* local_stream) { @@ -698,35 +682,7 @@ bool PeerConnection::AddStream(MediaStreamInterface* local_stream) { RTC_CHECK(!IsUnifiedPlan()) << "AddStream is not available with Unified Plan " "SdpSemantics. Please use AddTrack instead."; TRACE_EVENT0("webrtc", "PeerConnection::AddStream"); - if (IsClosed()) { - return false; - } - if (!CanAddLocalMediaStream(local_streams_, local_stream)) { - return false; - } - - local_streams_->AddStream(local_stream); - MediaStreamObserver* observer = new MediaStreamObserver(local_stream); - observer->SignalAudioTrackAdded.connect(this, - &PeerConnection::OnAudioTrackAdded); - observer->SignalAudioTrackRemoved.connect( - this, &PeerConnection::OnAudioTrackRemoved); - observer->SignalVideoTrackAdded.connect(this, - &PeerConnection::OnVideoTrackAdded); - observer->SignalVideoTrackRemoved.connect( - this, &PeerConnection::OnVideoTrackRemoved); - stream_observers_.push_back(std::unique_ptr(observer)); - - for (const auto& track : local_stream->GetAudioTracks()) { - AddAudioTrack(track.get(), local_stream); - } - for (const auto& track : local_stream->GetVideoTracks()) { - AddVideoTrack(track.get(), local_stream); - } - - stats_->AddStream(local_stream); - sdp_handler_.UpdateNegotiationNeeded(); - return true; + return sdp_handler_.AddStream(local_stream); } void PeerConnection::RemoveStream(MediaStreamInterface* local_stream) { @@ -735,27 +691,7 @@ void PeerConnection::RemoveStream(MediaStreamInterface* local_stream) { "Plan SdpSemantics. Please use RemoveTrack " "instead."; TRACE_EVENT0("webrtc", "PeerConnection::RemoveStream"); - if (!IsClosed()) { - for (const auto& track : local_stream->GetAudioTracks()) { - RemoveAudioTrack(track.get(), local_stream); - } - for (const auto& track : local_stream->GetVideoTracks()) { - RemoveVideoTrack(track.get(), local_stream); - } - } - local_streams_->RemoveStream(local_stream); - stream_observers_.erase( - std::remove_if( - stream_observers_.begin(), stream_observers_.end(), - [local_stream](const std::unique_ptr& observer) { - return observer->stream()->id().compare(local_stream->id()) == 0; - }), - stream_observers_.end()); - - if (IsClosed()) { - return; - } - sdp_handler_.UpdateNegotiationNeeded(); + sdp_handler_.RemoveStream(local_stream); } RTCErrorOr> PeerConnection::AddTrack( @@ -2027,6 +1963,7 @@ rtc::scoped_refptr PeerConnection::RemoveAndStopReceiver( void PeerConnection::AddAudioTrack(AudioTrackInterface* track, MediaStreamInterface* stream) { + RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK(!IsClosed()); RTC_DCHECK(track); RTC_DCHECK(stream); @@ -2060,6 +1997,7 @@ void PeerConnection::AddAudioTrack(AudioTrackInterface* track, // indefinitely, when we have unified plan SDP. void PeerConnection::RemoveAudioTrack(AudioTrackInterface* track, MediaStreamInterface* stream) { + RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK(!IsClosed()); auto sender = FindSenderForTrack(track); if (!sender) { @@ -2072,6 +2010,7 @@ void PeerConnection::RemoveAudioTrack(AudioTrackInterface* track, void PeerConnection::AddVideoTrack(VideoTrackInterface* track, MediaStreamInterface* stream) { + RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK(!IsClosed()); RTC_DCHECK(track); RTC_DCHECK(stream); @@ -2097,6 +2036,7 @@ void PeerConnection::AddVideoTrack(VideoTrackInterface* track, void PeerConnection::RemoveVideoTrack(VideoTrackInterface* track, MediaStreamInterface* stream) { + RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK(!IsClosed()); auto sender = FindSenderForTrack(track); if (!sender) { @@ -2262,13 +2202,13 @@ absl::optional PeerConnection::GetDataMid() const { } void PeerConnection::OnRemoteSenderAdded(const RtpSenderInfo& sender_info, + MediaStreamInterface* stream, cricket::MediaType media_type) { RTC_DCHECK_RUN_ON(signaling_thread()); RTC_LOG(LS_INFO) << "Creating " << cricket::MediaTypeToString(media_type) << " receiver for track_id=" << sender_info.sender_id << " and stream_id=" << sender_info.stream_id; - MediaStreamInterface* stream = remote_streams_->find(sender_info.stream_id); if (media_type == cricket::MEDIA_TYPE_AUDIO) { CreateAudioReceiver(stream, sender_info); } else if (media_type == cricket::MEDIA_TYPE_VIDEO) { @@ -2279,14 +2219,13 @@ void PeerConnection::OnRemoteSenderAdded(const RtpSenderInfo& sender_info, } void PeerConnection::OnRemoteSenderRemoved(const RtpSenderInfo& sender_info, + MediaStreamInterface* stream, cricket::MediaType media_type) { RTC_DCHECK_RUN_ON(signaling_thread()); RTC_LOG(LS_INFO) << "Removing " << cricket::MediaTypeToString(media_type) << " receiver for track_id=" << sender_info.sender_id << " and stream_id=" << sender_info.stream_id; - MediaStreamInterface* stream = remote_streams_->find(sender_info.stream_id); - rtc::scoped_refptr receiver; if (media_type == cricket::MEDIA_TYPE_AUDIO) { // When the MediaEngine audio channel is destroyed, the RemoteAudioSource diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 8698c30008..db3621bbd8 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -309,10 +309,6 @@ class PeerConnection : public PeerConnectionInternal, RTC_DCHECK_RUN_ON(signaling_thread()); return &configuration_; } - rtc::scoped_refptr remote_streams_internal() const { - RTC_DCHECK_RUN_ON(signaling_thread()); - return remote_streams_; - } absl::optional sctp_mid() { RTC_DCHECK_RUN_ON(signaling_thread()); return sctp_mid_s_; @@ -389,16 +385,12 @@ class PeerConnection : public PeerConnectionInternal, // May be called either by AddStream/RemoveStream, or when a track is // added/removed from a stream previously added via AddStream. - void AddAudioTrack(AudioTrackInterface* track, MediaStreamInterface* stream) - RTC_RUN_ON(signaling_thread()); + void AddAudioTrack(AudioTrackInterface* track, MediaStreamInterface* stream); void RemoveAudioTrack(AudioTrackInterface* track, - MediaStreamInterface* stream) - RTC_RUN_ON(signaling_thread()); - void AddVideoTrack(VideoTrackInterface* track, MediaStreamInterface* stream) - RTC_RUN_ON(signaling_thread()); + MediaStreamInterface* stream); + void AddVideoTrack(VideoTrackInterface* track, MediaStreamInterface* stream); void RemoveVideoTrack(VideoTrackInterface* track, - MediaStreamInterface* stream) - RTC_RUN_ON(signaling_thread()); + MediaStreamInterface* stream); // AddTrack implementation when Unified Plan is specified. RTCErrorOr> AddTrackUnifiedPlan( @@ -503,12 +495,14 @@ class PeerConnection : public PeerConnectionInternal, // session description. It creates a remote MediaStreamTrackInterface // implementation and triggers CreateAudioReceiver or CreateVideoReceiver. void OnRemoteSenderAdded(const RtpSenderInfo& sender_info, + MediaStreamInterface* stream, cricket::MediaType media_type); // Triggered when a remote sender has been removed from a remote session // description. It removes the remote sender with id |sender_id| from a remote // MediaStream and triggers DestroyAudioReceiver or DestroyVideoReceiver. void OnRemoteSenderRemoved(const RtpSenderInfo& sender_info, + MediaStreamInterface* stream, cricket::MediaType media_type); // Triggered when a local sender has been seen for the first time in a local @@ -764,16 +758,6 @@ class PeerConnection : public PeerConnectionInternal, tls_cert_verifier_; // TODO(bugs.webrtc.org/9987): Accessed on both // signaling and network thread. - // Streams added via AddStream. - const rtc::scoped_refptr local_streams_ - RTC_GUARDED_BY(signaling_thread()); - // Streams created as a result of SetRemoteDescription. - const rtc::scoped_refptr remote_streams_ - RTC_GUARDED_BY(signaling_thread()); - - std::vector> stream_observers_ - RTC_GUARDED_BY(signaling_thread()); - // These lists store sender info seen in local/remote descriptions. std::vector remote_audio_sender_infos_ RTC_GUARDED_BY(signaling_thread()); diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index c6432a6f34..022e5e5846 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -16,6 +16,7 @@ #include "api/media_stream_proxy.h" #include "api/uma_metrics.h" #include "pc/media_stream.h" +#include "pc/media_stream_observer.h" #include "pc/peer_connection.h" #include "pc/peer_connection_message_handler.h" #include "pc/rtp_media_utils.h" @@ -669,6 +670,20 @@ void AddRtpDataChannelOptions( } } +// Check if we can send |new_stream| on a PeerConnection. +bool CanAddLocalMediaStream(webrtc::StreamCollectionInterface* current_streams, + webrtc::MediaStreamInterface* new_stream) { + if (!new_stream || !current_streams) { + return false; + } + if (current_streams->find(new_stream->id()) != nullptr) { + RTC_LOG(LS_ERROR) << "MediaStream with ID " << new_stream->id() + << " is already added."; + return false; + } + return true; +} + } // namespace // Used by parameterless SetLocalDescription() to create an offer or answer. @@ -872,6 +887,8 @@ class SdpOfferAnswerHandler::LocalIceCredentialsToReplace { SdpOfferAnswerHandler::SdpOfferAnswerHandler(PeerConnection* pc) : pc_(pc), + local_streams_(StreamCollection::Create()), + remote_streams_(StreamCollection::Create()), operations_chain_(rtc::OperationsChain::Create()), rtcp_cname_(GenerateRtcpCname()), local_ice_credentials_to_replace_(new LocalIceCredentialsToReplace()), @@ -2088,11 +2105,11 @@ void SdpOfferAnswerHandler::SetAssociatedRemoteStreams( std::vector> media_streams; for (const std::string& stream_id : stream_ids) { rtc::scoped_refptr stream = - pc_->remote_streams_internal()->find(stream_id); + remote_streams_->find(stream_id); if (!stream) { stream = MediaStreamProxy::Create(rtc::Thread::Current(), MediaStream::Create(stream_id)); - pc_->remote_streams_internal()->AddStream(stream); + remote_streams_->AddStream(stream); added_streams->push_back(stream); } media_streams.push_back(stream); @@ -2394,6 +2411,88 @@ bool SdpOfferAnswerHandler::ShouldFireNegotiationNeededEvent( return true; } +rtc::scoped_refptr +SdpOfferAnswerHandler::local_streams() { + RTC_DCHECK_RUN_ON(signaling_thread()); + RTC_CHECK(!IsUnifiedPlan()) << "local_streams is not available with Unified " + "Plan SdpSemantics. Please use GetSenders " + "instead."; + return local_streams_; +} + +rtc::scoped_refptr +SdpOfferAnswerHandler::remote_streams() { + RTC_DCHECK_RUN_ON(signaling_thread()); + RTC_CHECK(!IsUnifiedPlan()) << "remote_streams is not available with Unified " + "Plan SdpSemantics. Please use GetReceivers " + "instead."; + return remote_streams_; +} + +bool SdpOfferAnswerHandler::AddStream(MediaStreamInterface* local_stream) { + RTC_DCHECK_RUN_ON(signaling_thread()); + RTC_CHECK(!IsUnifiedPlan()) << "AddStream is not available with Unified Plan " + "SdpSemantics. Please use AddTrack instead."; + if (pc_->IsClosed()) { + return false; + } + if (!CanAddLocalMediaStream(local_streams_, local_stream)) { + return false; + } + + local_streams_->AddStream(local_stream); + MediaStreamObserver* observer = new MediaStreamObserver(local_stream); + observer->SignalAudioTrackAdded.connect(pc_, + &PeerConnection::OnAudioTrackAdded); + observer->SignalAudioTrackRemoved.connect( + pc_, &PeerConnection::OnAudioTrackRemoved); + observer->SignalVideoTrackAdded.connect(pc_, + &PeerConnection::OnVideoTrackAdded); + observer->SignalVideoTrackRemoved.connect( + pc_, &PeerConnection::OnVideoTrackRemoved); + stream_observers_.push_back(std::unique_ptr(observer)); + + for (const auto& track : local_stream->GetAudioTracks()) { + pc_->AddAudioTrack(track.get(), local_stream); + } + for (const auto& track : local_stream->GetVideoTracks()) { + pc_->AddVideoTrack(track.get(), local_stream); + } + + pc_->stats()->AddStream(local_stream); + UpdateNegotiationNeeded(); + return true; +} + +void SdpOfferAnswerHandler::RemoveStream(MediaStreamInterface* local_stream) { + RTC_DCHECK_RUN_ON(signaling_thread()); + RTC_CHECK(!IsUnifiedPlan()) << "RemoveStream is not available with Unified " + "Plan SdpSemantics. Please use RemoveTrack " + "instead."; + TRACE_EVENT0("webrtc", "PeerConnection::RemoveStream"); + if (!pc_->IsClosed()) { + for (const auto& track : local_stream->GetAudioTracks()) { + pc_->RemoveAudioTrack(track.get(), local_stream); + } + for (const auto& track : local_stream->GetVideoTracks()) { + pc_->RemoveVideoTrack(track.get(), local_stream); + } + } + local_streams_->RemoveStream(local_stream); + stream_observers_.erase( + std::remove_if( + stream_observers_.begin(), stream_observers_.end(), + [local_stream](const std::unique_ptr& observer) { + return observer->stream()->id().compare(local_stream->id()) == 0; + }), + stream_observers_.end()); + + if (pc_->IsClosed()) { + return; + } + UpdateNegotiationNeeded(); +} + RTCError SdpOfferAnswerHandler::Rollback(SdpType desc_type) { auto state = signaling_state(); if (state != PeerConnectionInterface::kHaveLocalOffer && @@ -3752,7 +3851,7 @@ void SdpOfferAnswerHandler::RemoveRemoteStreamsIfEmpty( for (const auto& remote_stream : remote_streams) { if (remote_stream->GetAudioTracks().empty() && remote_stream->GetVideoTracks().empty()) { - pc_->remote_streams_internal()->RemoveStream(remote_stream); + remote_streams_->RemoveStream(remote_stream); removed_streams->push_back(remote_stream); } } @@ -3838,7 +3937,8 @@ void SdpOfferAnswerHandler::UpdateRemoteSendersList( sender_exists) { ++sender_it; } else { - pc_->OnRemoteSenderRemoved(info, media_type); + pc_->OnRemoteSenderRemoved(info, remote_streams_->find(info.stream_id), + media_type); sender_it = current_senders->erase(sender_it); } } @@ -3865,12 +3965,12 @@ void SdpOfferAnswerHandler::UpdateRemoteSendersList( uint32_t ssrc = params.first_ssrc(); rtc::scoped_refptr stream = - pc_->remote_streams_internal()->find(stream_id); + remote_streams_->find(stream_id); if (!stream) { // This is a new MediaStream. Create a new remote MediaStream. stream = MediaStreamProxy::Create(rtc::Thread::Current(), MediaStream::Create(stream_id)); - pc_->remote_streams_internal()->AddStream(stream); + remote_streams_->AddStream(stream); new_streams->AddStream(stream); } @@ -3879,19 +3979,19 @@ void SdpOfferAnswerHandler::UpdateRemoteSendersList( if (!sender_info) { current_senders->push_back( PeerConnection::RtpSenderInfo(stream_id, sender_id, ssrc)); - pc_->OnRemoteSenderAdded(current_senders->back(), media_type); + pc_->OnRemoteSenderAdded(current_senders->back(), stream, media_type); } } // Add default sender if necessary. if (default_sender_needed) { rtc::scoped_refptr default_stream = - pc_->remote_streams_internal()->find(kDefaultStreamId); + remote_streams_->find(kDefaultStreamId); if (!default_stream) { // Create the new default MediaStream. default_stream = MediaStreamProxy::Create( rtc::Thread::Current(), MediaStream::Create(kDefaultStreamId)); - pc_->remote_streams_internal()->AddStream(default_stream); + remote_streams_->AddStream(default_stream); new_streams->AddStream(default_stream); } std::string default_sender_id = (media_type == cricket::MEDIA_TYPE_AUDIO) @@ -3903,7 +4003,8 @@ void SdpOfferAnswerHandler::UpdateRemoteSendersList( if (!default_sender_info) { current_senders->push_back(PeerConnection::RtpSenderInfo( kDefaultStreamId, default_sender_id, /*ssrc=*/0)); - pc_->OnRemoteSenderAdded(current_senders->back(), media_type); + pc_->OnRemoteSenderAdded(current_senders->back(), default_stream, + media_type); } } } @@ -4114,15 +4215,15 @@ void SdpOfferAnswerHandler::ReportNegotiatedSdpSemantics( void SdpOfferAnswerHandler::UpdateEndedRemoteMediaStreams() { RTC_DCHECK_RUN_ON(signaling_thread()); std::vector> streams_to_remove; - for (size_t i = 0; i < pc_->remote_streams_internal()->count(); ++i) { - MediaStreamInterface* stream = pc_->remote_streams_internal()->at(i); + for (size_t i = 0; i < remote_streams_->count(); ++i) { + MediaStreamInterface* stream = remote_streams_->at(i); if (stream->GetAudioTracks().empty() && stream->GetVideoTracks().empty()) { streams_to_remove.push_back(stream); } } for (auto& stream : streams_to_remove) { - pc_->remote_streams_internal()->RemoveStream(stream); + remote_streams_->RemoveStream(stream); pc_->Observer()->OnRemoveStream(std::move(stream)); } } diff --git a/pc/sdp_offer_answer.h b/pc/sdp_offer_answer.h index 42e75b0752..bd49e3479b 100644 --- a/pc/sdp_offer_answer.h +++ b/pc/sdp_offer_answer.h @@ -127,6 +127,9 @@ class SdpOfferAnswerHandler { const std::vector& candidates); bool ShouldFireNegotiationNeededEvent(uint32_t event_id); + bool AddStream(MediaStreamInterface* local_stream); + void RemoveStream(MediaStreamInterface* local_stream); + absl::optional is_caller(); bool HasNewIceCredentials(); bool IceRestartPending(const std::string& content_name) const; @@ -148,6 +151,9 @@ class SdpOfferAnswerHandler { // Destroys all BaseChannels and destroys the SCTP data channel, if present. void DestroyAllChannels(); + rtc::scoped_refptr local_streams(); + rtc::scoped_refptr remote_streams(); + private: class ImplicitCreateSessionDescriptionObserver; friend class ImplicitCreateSessionDescriptionObserver; @@ -518,6 +524,16 @@ class SdpOfferAnswerHandler { // Whether this peer is the caller. Set when the local description is applied. absl::optional is_caller_ RTC_GUARDED_BY(signaling_thread()); + // Streams added via AddStream. + const rtc::scoped_refptr local_streams_ + RTC_GUARDED_BY(signaling_thread()); + // Streams created as a result of SetRemoteDescription. + const rtc::scoped_refptr remote_streams_ + RTC_GUARDED_BY(signaling_thread()); + + std::vector> stream_observers_ + RTC_GUARDED_BY(signaling_thread()); + // The operations chain is used by the offer/answer exchange methods to ensure // they are executed in the right order. For example, if // SetRemoteDescription() is invoked while CreateOffer() is still pending, the