From 280054f2e63a50cb56a79172f15fccabb09ec986 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Tue, 10 Nov 2020 13:12:53 +0000 Subject: [PATCH] Eliminate sigslot from RtpTransmissionManager at the cost of adding a WeakPointerFactory. Moves the RtpTransceiver "NegotiationNeeded" signal to a callback function that is passed as a constructor argument. Bug: webrtc:11943 Change-Id: I37b2027379acce38dbaf0f396daebdb3e579ee54 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/192540 Commit-Queue: Harald Alvestrand Reviewed-by: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#32575} --- pc/rtp_transceiver.cc | 10 ++++++---- pc/rtp_transceiver.h | 6 ++++-- pc/rtp_transceiver_unittest.cc | 6 ++++-- pc/rtp_transmission_manager.cc | 12 ++++++++---- pc/rtp_transmission_manager.h | 5 +++-- 5 files changed, 25 insertions(+), 14 deletions(-) diff --git a/pc/rtp_transceiver.cc b/pc/rtp_transceiver.cc index fd8ff81e4a..6b3032e27f 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc @@ -119,12 +119,14 @@ RtpTransceiver::RtpTransceiver( rtc::scoped_refptr> receiver, cricket::ChannelManager* channel_manager, - std::vector header_extensions_offered) + std::vector header_extensions_offered, + std::function on_negotiation_needed) : thread_(GetCurrentTaskQueueOrThread()), unified_plan_(true), media_type_(sender->media_type()), channel_manager_(channel_manager), - header_extensions_to_offer_(std::move(header_extensions_offered)) { + header_extensions_to_offer_(std::move(header_extensions_offered)), + on_negotiation_needed_(std::move(on_negotiation_needed)) { RTC_DCHECK(media_type_ == cricket::MEDIA_TYPE_AUDIO || media_type_ == cricket::MEDIA_TYPE_VIDEO); RTC_DCHECK_EQ(sender->media_type(), receiver->media_type()); @@ -314,7 +316,7 @@ RTCError RtpTransceiver::SetDirectionWithError( } direction_ = new_direction; - SignalNegotiationNeeded(); + on_negotiation_needed_(); return RTCError::OK(); } @@ -378,7 +380,7 @@ RTCError RtpTransceiver::StopStandard() { // 5. Stop sending and receiving given transceiver, and update the // negotiation-needed flag for connection. StopSendingAndReceiving(); - SignalNegotiationNeeded(); + on_negotiation_needed_(); return RTCError::OK(); } diff --git a/pc/rtp_transceiver.h b/pc/rtp_transceiver.h index 97e60a50d1..4d9716c89b 100644 --- a/pc/rtp_transceiver.h +++ b/pc/rtp_transceiver.h @@ -71,7 +71,8 @@ class RtpTransceiver final rtc::scoped_refptr> receiver, cricket::ChannelManager* channel_manager, - std::vector HeaderExtensionsToOffer); + std::vector HeaderExtensionsToOffer, + std::function on_negotiation_needed); ~RtpTransceiver() override; // Returns the Voice/VideoChannel set for this transceiver. May be null if @@ -183,7 +184,7 @@ class RtpTransceiver final // Fired when the RtpTransceiver state changes such that negotiation is now // needed (e.g., in response to a direction change). - sigslot::signal0<> SignalNegotiationNeeded; + // sigslot::signal0<> SignalNegotiationNeeded; // RtpTransceiverInterface implementation. cricket::MediaType media_type() const override; @@ -240,6 +241,7 @@ class RtpTransceiver final cricket::ChannelManager* channel_manager_ = nullptr; std::vector codec_preferences_; std::vector header_extensions_to_offer_; + const std::function on_negotiation_needed_; }; BEGIN_SIGNALING_PROXY_MAP(RtpTransceiver) diff --git a/pc/rtp_transceiver_unittest.cc b/pc/rtp_transceiver_unittest.cc index e4883f32f6..96e38b0b23 100644 --- a/pc/rtp_transceiver_unittest.cc +++ b/pc/rtp_transceiver_unittest.cc @@ -93,7 +93,8 @@ class RtpTransceiverUnifiedPlanTest : public ::testing::Test { rtc::Thread::Current(), new rtc::RefCountedObject()), &channel_manager_, - channel_manager_.GetSupportedAudioRtpHeaderExtensions()) {} + channel_manager_.GetSupportedAudioRtpHeaderExtensions(), + /* on_negotiation_needed= */ [] {}) {} cricket::ChannelManager channel_manager_; RtpTransceiver transceiver_; @@ -140,7 +141,8 @@ class RtpTransceiverTestForHeaderExtensions : public ::testing::Test { rtc::Thread::Current(), new rtc::RefCountedObject()), &channel_manager_, - extensions_) {} + extensions_, + /* on_negotiation_needed= */ [] {}) {} cricket::ChannelManager channel_manager_; std::vector extensions_; diff --git a/pc/rtp_transmission_manager.cc b/pc/rtp_transmission_manager.cc index a71ed53d4f..e796f9b1b1 100644 --- a/pc/rtp_transmission_manager.cc +++ b/pc/rtp_transmission_manager.cc @@ -48,7 +48,8 @@ RtpTransmissionManager::RtpTransmissionManager( usage_pattern_(usage_pattern), observer_(observer), stats_(stats), - on_negotiation_needed_(on_negotiation_needed) {} + on_negotiation_needed_(on_negotiation_needed), + weak_ptr_factory_(this) {} void RtpTransmissionManager::Close() { closed_ = true; @@ -269,10 +270,13 @@ RtpTransmissionManager::CreateAndAddTransceiver( sender, receiver, channel_manager(), sender->media_type() == cricket::MEDIA_TYPE_AUDIO ? channel_manager()->GetSupportedAudioRtpHeaderExtensions() - : channel_manager()->GetSupportedVideoRtpHeaderExtensions())); + : channel_manager()->GetSupportedVideoRtpHeaderExtensions(), + [this_weak_ptr = weak_ptr_factory_.GetWeakPtr()]() { + if (this_weak_ptr) { + this_weak_ptr->OnNegotiationNeeded(); + } + })); transceivers()->Add(transceiver); - transceiver->internal()->SignalNegotiationNeeded.connect( - this, &RtpTransmissionManager::OnNegotiationNeeded); return transceiver; } diff --git a/pc/rtp_transmission_manager.h b/pc/rtp_transmission_manager.h index de83fb2010..731c3b74dd 100644 --- a/pc/rtp_transmission_manager.h +++ b/pc/rtp_transmission_manager.h @@ -66,8 +66,7 @@ struct RtpSenderInfo { // The RtpTransmissionManager class is responsible for managing the lifetime // and relationships between objects of type RtpSender, RtpReceiver and // RtpTransceiver. -class RtpTransmissionManager : public RtpSenderBase::SetStreamsObserver, - public sigslot::has_slots<> { +class RtpTransmissionManager : public RtpSenderBase::SetStreamsObserver { public: RtpTransmissionManager(bool is_unified_plan, rtc::Thread* signaling_thread, @@ -259,6 +258,8 @@ class RtpTransmissionManager : public RtpSenderBase::SetStreamsObserver, PeerConnectionObserver* observer_; StatsCollectorInterface* const stats_; std::function on_negotiation_needed_; + rtc::WeakPtrFactory weak_ptr_factory_ + RTC_GUARDED_BY(signaling_thread()); }; } // namespace webrtc