From 23425f9068aed6ba93d947f0ace17e55f2e1247d Mon Sep 17 00:00:00 2001 From: nisse Date: Mon, 3 Apr 2017 04:54:25 -0700 Subject: [PATCH] Add methods to register congestion controller observer after construction. The point of this change is to make it possible to create the congestion controller as part of creating RtpTransportController, later pass it to the constructor of Call, and then let Call register itself as an observer. BUG=webrtc:7135 Review-Url: https://codereview.webrtc.org/2795643002 Cr-Commit-Position: refs/heads/master@{#17504} --- .../include/send_side_congestion_controller.h | 8 +++++- .../send_side_congestion_controller.cc | 27 ++++++++++++++----- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h b/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h index 8130feb01e..58eba25a7f 100644 --- a/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h @@ -57,6 +57,7 @@ class SendSideCongestionController : public CallStatsObserver, protected: virtual ~Observer() {} }; + // TODO(nisse): Consider deleting the |observer| argument to constructors. SendSideCongestionController(const Clock* clock, Observer* observer, RtcEventLog* event_log, @@ -70,6 +71,10 @@ class SendSideCongestionController : public CallStatsObserver, void RegisterPacketFeedbackObserver(PacketFeedbackObserver* observer); void DeRegisterPacketFeedbackObserver(PacketFeedbackObserver* observer); + // Currently, there can be at most one observer. + void RegisterNetworkObserver(Observer* observer); + void DeRegisterNetworkObserver(Observer* observer); + virtual void SetBweBitrates(int min_bitrate_bps, int start_bitrate_bps, int max_bitrate_bps); @@ -130,7 +135,8 @@ class SendSideCongestionController : public CallStatsObserver, uint8_t fraction_loss, int64_t rtt); const Clock* const clock_; - Observer* const observer_; + rtc::CriticalSection observer_lock_; + Observer* observer_ GUARDED_BY(observer_lock_); RtcEventLog* const event_log_; const std::unique_ptr pacer_; const std::unique_ptr bitrate_controller_; diff --git a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc index e0918d7b76..e3f84d0e53 100644 --- a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc +++ b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc @@ -93,6 +93,19 @@ void SendSideCongestionController::DeRegisterPacketFeedbackObserver( transport_feedback_adapter_.DeRegisterPacketFeedbackObserver(observer); } +void SendSideCongestionController::RegisterNetworkObserver(Observer* observer) { + rtc::CritScope cs(&observer_lock_); + RTC_DCHECK(observer_ == nullptr); + observer_ = observer; +} + +void SendSideCongestionController::DeRegisterNetworkObserver( + Observer* observer) { + rtc::CritScope cs(&observer_lock_); + RTC_DCHECK_EQ(observer_, observer); + observer_ = nullptr; +} + void SendSideCongestionController::SetBweBitrates(int min_bitrate_bps, int start_bitrate_bps, int max_bitrate_bps) { @@ -245,11 +258,6 @@ SendSideCongestionController::GetTransportFeedbackVector() const { } void SendSideCongestionController::MaybeTriggerOnNetworkChanged() { - // TODO(perkj): |observer_| can be nullptr if the ctor that accepts a - // BitrateObserver is used. Remove this check once the ctor is removed. - if (!observer_) - return; - uint32_t bitrate_bps; uint8_t fraction_loss; int64_t rtt; @@ -269,8 +277,13 @@ void SendSideCongestionController::MaybeTriggerOnNetworkChanged() { rtc::CritScope cs(&bwe_lock_); probing_interval_ms = delay_based_bwe_->GetProbingIntervalMs(); } - observer_->OnNetworkChanged(bitrate_bps, fraction_loss, rtt, - probing_interval_ms); + { + rtc::CritScope cs(&observer_lock_); + if (observer_) { + observer_->OnNetworkChanged(bitrate_bps, fraction_loss, rtt, + probing_interval_ms); + } + } } }