Deprecated BitrateController::CreateRtcpBandwidthObserver.
The RtcpBandwidthObserverImpl did not provide any features that a raw pointer does not have. deprecating it to simplify further refactoring down the line. Preferring raw pointer usage as it provides equivalent functionality in less code.
Bug: webrtc:8415
Change-Id: Id2c4c73a331835f124da8d308615ca2ce34b2d1b
Reviewed-on: https://webrtc-review.googlesource.com/22500
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20712}diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index d0ec6dc..fc893e4 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -204,25 +204,28 @@
channel_proxy->SetSendAudioLevelIndicationStatus(new_ids.audio_level != 0,
new_ids.audio_level);
}
- // Transport sequence number
- if (first_time ||
- new_ids.transport_sequence_number != old_ids.transport_sequence_number) {
+ bool transport_seq_num_id_changed =
+ new_ids.transport_sequence_number != old_ids.transport_sequence_number;
+ if (first_time || transport_seq_num_id_changed) {
if (!first_time) {
channel_proxy->ResetSenderCongestionControlObjects();
- stream->bandwidth_observer_.reset();
}
- if (new_ids.transport_sequence_number != 0) {
+ RtcpBandwidthObserver* bandwidth_observer = nullptr;
+ bool has_transport_sequence_number = new_ids.transport_sequence_number != 0;
+ if (has_transport_sequence_number) {
channel_proxy->EnableSendTransportSequenceNumber(
new_ids.transport_sequence_number);
+ // Probing in application limited region is only used in combination with
+ // send side congestion control, wich depends on feedback packets which
+ // requires transport sequence numbers to be enabled.
stream->transport_->send_side_cc()->EnablePeriodicAlrProbing(true);
- stream->bandwidth_observer_.reset(stream->transport_->send_side_cc()
- ->GetBitrateController()
- ->CreateRtcpBandwidthObserver());
+ bandwidth_observer =
+ stream->transport_->send_side_cc()->GetBandwidthObserver();
}
- channel_proxy->RegisterSenderCongestionControlObjects(
- stream->transport_, stream->bandwidth_observer_.get());
+ channel_proxy->RegisterSenderCongestionControlObjects(stream->transport_,
+ bandwidth_observer);
}
if (!ReconfigureSendCodec(stream, new_config)) {
diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h
index a46dc3b..ef89269 100644
--- a/audio/audio_send_stream.h
+++ b/audio/audio_send_stream.h
@@ -111,7 +111,6 @@
BitrateAllocator* const bitrate_allocator_;
RtpTransportControllerSendInterface* const transport_;
- std::unique_ptr<RtcpBandwidthObserver> bandwidth_observer_;
rtc::CriticalSection packet_loss_tracker_cs_;
TransportFeedbackPacketLossTracker packet_loss_tracker_
diff --git a/modules/bitrate_controller/bitrate_controller_impl.h b/modules/bitrate_controller/bitrate_controller_impl.h
index 1bcb6ea..6a67808 100644
--- a/modules/bitrate_controller/bitrate_controller_impl.h
+++ b/modules/bitrate_controller/bitrate_controller_impl.h
@@ -39,7 +39,7 @@
bool AvailableBandwidth(uint32_t* bandwidth) const override;
- RtcpBandwidthObserver* CreateRtcpBandwidthObserver() override;
+ RTC_DEPRECATED RtcpBandwidthObserver* CreateRtcpBandwidthObserver() override;
// Deprecated
void SetStartBitrate(int start_bitrate_bps) override;
diff --git a/modules/bitrate_controller/bitrate_controller_unittest.cc b/modules/bitrate_controller/bitrate_controller_unittest.cc
index ab9928f..66ca5b9 100644
--- a/modules/bitrate_controller/bitrate_controller_unittest.cc
+++ b/modules/bitrate_controller/bitrate_controller_unittest.cc
@@ -73,7 +73,7 @@
EXPECT_EQ(kStartBitrateBps, bitrate_observer_.last_bitrate_);
controller_->SetMinMaxBitrate(kMinBitrateBps, kMaxBitrateBps);
EXPECT_EQ(kStartBitrateBps, bitrate_observer_.last_bitrate_);
- bandwidth_observer_.reset(controller_->CreateRtcpBandwidthObserver());
+ bandwidth_observer_ = controller_.get();
}
virtual void TearDown() {
@@ -89,7 +89,7 @@
webrtc::SimulatedClock clock_;
TestBitrateObserver bitrate_observer_;
std::unique_ptr<BitrateController> controller_;
- std::unique_ptr<RtcpBandwidthObserver> bandwidth_observer_;
+ RtcpBandwidthObserver* bandwidth_observer_;
testing::NiceMock<webrtc::MockRtcEventLog> event_log_;
};
@@ -195,8 +195,7 @@
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms);
time_ms += 500;
- RtcpBandwidthObserver* second_bandwidth_observer =
- controller_->CreateRtcpBandwidthObserver();
+ RtcpBandwidthObserver* second_bandwidth_observer = controller_.get();
report_blocks = {CreateReportBlock(kSenderSsrc2, kMediaSsrc2, 0, 21)};
second_bandwidth_observer->OnReceivedRtcpReceiverReport(
report_blocks, 100, time_ms);
@@ -273,7 +272,6 @@
// Min cap.
bandwidth_observer_->OnReceivedEstimatedBitrate(1000);
EXPECT_EQ(100000, bitrate_observer_.last_bitrate_);
- delete second_bandwidth_observer;
}
TEST_F(BitrateControllerTest, OneBitrateObserverMultipleReportBlocks) {
diff --git a/modules/bitrate_controller/include/bitrate_controller.h b/modules/bitrate_controller/include/bitrate_controller.h
index 52347a3..5aaf88b 100644
--- a/modules/bitrate_controller/include/bitrate_controller.h
+++ b/modules/bitrate_controller/include/bitrate_controller.h
@@ -69,8 +69,10 @@
virtual ~BitrateController() {}
+ // Deprecated, use raw pointer to BitrateController instance instead.
// Creates RtcpBandwidthObserver caller responsible to delete.
- virtual RtcpBandwidthObserver* CreateRtcpBandwidthObserver() = 0;
+ RTC_DEPRECATED virtual RtcpBandwidthObserver*
+ CreateRtcpBandwidthObserver() = 0;
// Deprecated
virtual void SetStartBitrate(int start_bitrate_bps) = 0;
diff --git a/modules/congestion_controller/include/send_side_congestion_controller.h b/modules/congestion_controller/include/send_side_congestion_controller.h
index e3e081c..5ed5fd4 100644
--- a/modules/congestion_controller/include/send_side_congestion_controller.h
+++ b/modules/congestion_controller/include/send_side_congestion_controller.h
@@ -87,6 +87,7 @@
virtual void SetTransportOverhead(size_t transport_overhead_bytes_per_packet);
virtual BitrateController* GetBitrateController() const;
+ virtual RtcpBandwidthObserver* GetBandwidthObserver() const;
virtual int64_t GetPacerQueuingDelayMs() const;
virtual int64_t GetFirstPacketTimeMs() const;
diff --git a/modules/congestion_controller/send_side_congestion_controller.cc b/modules/congestion_controller/send_side_congestion_controller.cc
index 6c44b3eb..4ba0062 100644
--- a/modules/congestion_controller/send_side_congestion_controller.cc
+++ b/modules/congestion_controller/send_side_congestion_controller.cc
@@ -212,6 +212,11 @@
return bitrate_controller_.get();
}
+RtcpBandwidthObserver* SendSideCongestionController::GetBandwidthObserver()
+ const {
+ return bitrate_controller_.get();
+}
+
RateLimiter* SendSideCongestionController::GetRetransmissionRateLimiter() {
return retransmission_rate_limiter_.get();
}
diff --git a/modules/congestion_controller/send_side_congestion_controller_unittest.cc b/modules/congestion_controller/send_side_congestion_controller_unittest.cc
index d0ca6ae..6c22f3f 100644
--- a/modules/congestion_controller/send_side_congestion_controller_unittest.cc
+++ b/modules/congestion_controller/send_side_congestion_controller_unittest.cc
@@ -54,8 +54,7 @@
pacer_.reset(new NiceMock<MockPacedSender>());
controller_.reset(new SendSideCongestionController(
&clock_, &observer_, &event_log_, pacer_.get()));
- bandwidth_observer_.reset(
- controller_->GetBitrateController()->CreateRtcpBandwidthObserver());
+ bandwidth_observer_ = controller_->GetBandwidthObserver();
// Set the initial bitrate estimate and expect the |observer| and |pacer_|
// to be updated.
@@ -135,7 +134,7 @@
StrictMock<MockCongestionObserver> observer_;
TargetBitrateObserver target_bitrate_observer_;
NiceMock<MockRtcEventLog> event_log_;
- std::unique_ptr<RtcpBandwidthObserver> bandwidth_observer_;
+ RtcpBandwidthObserver* bandwidth_observer_;
PacketRouter packet_router_;
std::unique_ptr<NiceMock<MockPacedSender>> pacer_;
std::unique_ptr<SendSideCongestionController> controller_;
diff --git a/modules/remote_bitrate_estimator/test/estimators/remb.cc b/modules/remote_bitrate_estimator/test/estimators/remb.cc
index 53fb757..ae96389 100644
--- a/modules/remote_bitrate_estimator/test/estimators/remb.cc
+++ b/modules/remote_bitrate_estimator/test/estimators/remb.cc
@@ -28,7 +28,7 @@
BitrateController::CreateBitrateController(clock,
observer,
&event_log_)),
- feedback_observer_(bitrate_controller_->CreateRtcpBandwidthObserver()),
+ feedback_observer_(bitrate_controller_.get()),
clock_(clock) {
assert(kbps >= kMinBitrateKbps);
assert(kbps <= kMaxBitrateKbps);
diff --git a/modules/remote_bitrate_estimator/test/estimators/remb.h b/modules/remote_bitrate_estimator/test/estimators/remb.h
index 1b30327..f90b926 100644
--- a/modules/remote_bitrate_estimator/test/estimators/remb.h
+++ b/modules/remote_bitrate_estimator/test/estimators/remb.h
@@ -42,7 +42,7 @@
protected:
std::unique_ptr<BitrateController> bitrate_controller_;
- std::unique_ptr<RtcpBandwidthObserver> feedback_observer_;
+ RtcpBandwidthObserver* feedback_observer_;
private:
Clock* clock_;
diff --git a/modules/remote_bitrate_estimator/test/estimators/send_side.cc b/modules/remote_bitrate_estimator/test/estimators/send_side.cc
index 4f67f30..6e03eee 100644
--- a/modules/remote_bitrate_estimator/test/estimators/send_side.cc
+++ b/modules/remote_bitrate_estimator/test/estimators/send_side.cc
@@ -33,7 +33,7 @@
acknowledged_bitrate_estimator_(
rtc::MakeUnique<AcknowledgedBitrateEstimator>()),
bwe_(new DelayBasedBwe(nullptr, clock)),
- feedback_observer_(bitrate_controller_->CreateRtcpBandwidthObserver()),
+ feedback_observer_(bitrate_controller_.get()),
clock_(clock),
send_time_history_(clock_, 10000),
has_received_ack_(false),
diff --git a/modules/remote_bitrate_estimator/test/estimators/send_side.h b/modules/remote_bitrate_estimator/test/estimators/send_side.h
index 89150c5..6816223 100644
--- a/modules/remote_bitrate_estimator/test/estimators/send_side.h
+++ b/modules/remote_bitrate_estimator/test/estimators/send_side.h
@@ -40,7 +40,7 @@
std::unique_ptr<BitrateController> bitrate_controller_;
std::unique_ptr<AcknowledgedBitrateEstimator> acknowledged_bitrate_estimator_;
std::unique_ptr<DelayBasedBwe> bwe_;
- std::unique_ptr<RtcpBandwidthObserver> feedback_observer_;
+ RtcpBandwidthObserver* feedback_observer_;
private:
Clock* const clock_;
diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc
index d0d9125..37e6f6e 100644
--- a/video/video_send_stream.cc
+++ b/video/video_send_stream.cc
@@ -356,7 +356,7 @@
EncoderRtcpFeedback encoder_feedback_;
ProtectionBitrateCalculator protection_bitrate_calculator_;
- const std::unique_ptr<RtcpBandwidthObserver> bandwidth_observer_;
+ RtcpBandwidthObserver* const bandwidth_observer_;
// RtpRtcp modules, declared here as they use other members on construction.
const std::vector<RtpRtcp*> rtp_rtcp_modules_;
PayloadRouter payload_router_;
@@ -716,13 +716,11 @@
config_->rtp.ssrcs,
video_stream_encoder),
protection_bitrate_calculator_(Clock::GetRealTimeClock(), this),
- bandwidth_observer_(transport->send_side_cc()
- ->GetBitrateController()
- ->CreateRtcpBandwidthObserver()),
+ bandwidth_observer_(transport->send_side_cc()->GetBandwidthObserver()),
rtp_rtcp_modules_(CreateRtpRtcpModules(
config_->send_transport,
&encoder_feedback_,
- bandwidth_observer_.get(),
+ bandwidth_observer_,
transport,
call_stats_->rtcp_rtt_stats(),
flexfec_sender_.get(),
