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}

audio/audio_send_stream.cc

@@ -204,25 +204,28 @@ void AudioSendStream::ConfigureStream(
     channel_proxy->SetSendAudioLevelIndicationStatus(new_ids.audio_level != 0,
                                                      new_ids.audio_level);
   }
-  // Transport sequence number
+  bool transport_seq_num_id_changed =
-  if (first_time ||
+      new_ids.transport_sequence_number != old_ids.transport_sequence_number;
-      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()
+      bandwidth_observer =
-                                            ->GetBitrateController()
+          stream->transport_->send_side_cc()->GetBandwidthObserver();
-                                            ->CreateRtcpBandwidthObserver());
     }
 
-    channel_proxy->RegisterSenderCongestionControlObjects(
+    channel_proxy->RegisterSenderCongestionControlObjects(stream->transport_,
-        stream->transport_, stream->bandwidth_observer_.get());
+                                                          bandwidth_observer);
   }
 
   if (!ReconfigureSendCodec(stream, new_config)) {

audio/audio_send_stream.h

@@ -111,7 +111,6 @@ class AudioSendStream final : public webrtc::AudioSendStream,
 
   BitrateAllocator* const bitrate_allocator_;
   RtpTransportControllerSendInterface* const transport_;
-  std::unique_ptr<RtcpBandwidthObserver> bandwidth_observer_;
 
   rtc::CriticalSection packet_loss_tracker_cs_;
   TransportFeedbackPacketLossTracker packet_loss_tracker_

modules/bitrate_controller/bitrate_controller_impl.h

@@ -39,7 +39,7 @@ class BitrateControllerImpl : public BitrateController {
 
   bool AvailableBandwidth(uint32_t* bandwidth) const override;
 
-  RtcpBandwidthObserver* CreateRtcpBandwidthObserver() override;
+  RTC_DEPRECATED RtcpBandwidthObserver* CreateRtcpBandwidthObserver() override;
 
   // Deprecated
   void SetStartBitrate(int start_bitrate_bps) override;

modules/bitrate_controller/bitrate_controller_unittest.cc

@@ -73,7 +73,7 @@ class BitrateControllerTest : public ::testing::Test {
     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 @@ class BitrateControllerTest : public ::testing::Test {
   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 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) {
   bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms);
   time_ms += 500;
 
-  RtcpBandwidthObserver* second_bandwidth_observer =
+  RtcpBandwidthObserver* second_bandwidth_observer = controller_.get();
-      controller_->CreateRtcpBandwidthObserver();
   report_blocks = {CreateReportBlock(kSenderSsrc2, kMediaSsrc2, 0, 21)};
   second_bandwidth_observer->OnReceivedRtcpReceiverReport(
       report_blocks, 100, time_ms);
@@ -273,7 +272,6 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) {
   // Min cap.
   bandwidth_observer_->OnReceivedEstimatedBitrate(1000);
   EXPECT_EQ(100000, bitrate_observer_.last_bitrate_);
-  delete second_bandwidth_observer;
 }
 
 TEST_F(BitrateControllerTest, OneBitrateObserverMultipleReportBlocks) {

modules/bitrate_controller/include/bitrate_controller.h

@@ -69,8 +69,10 @@ class BitrateController : public Module, public RtcpBandwidthObserver {
 
   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;

modules/congestion_controller/include/send_side_congestion_controller.h

@@ -87,6 +87,7 @@ class SendSideCongestionController : public CallStatsObserver,
   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;
 

modules/congestion_controller/send_side_congestion_controller.cc

@@ -212,6 +212,11 @@ BitrateController* SendSideCongestionController::GetBitrateController() const {
   return bitrate_controller_.get();
 }
 
+RtcpBandwidthObserver* SendSideCongestionController::GetBandwidthObserver()
+    const {
+  return bitrate_controller_.get();
+}
+
 RateLimiter* SendSideCongestionController::GetRetransmissionRateLimiter() {
   return retransmission_rate_limiter_.get();
 }

modules/congestion_controller/send_side_congestion_controller_unittest.cc

@@ -54,8 +54,7 @@ class SendSideCongestionControllerTest : public ::testing::Test {
     pacer_.reset(new NiceMock<MockPacedSender>());
     controller_.reset(new SendSideCongestionController(
         &clock_, &observer_, &event_log_, pacer_.get()));
-    bandwidth_observer_.reset(
+    bandwidth_observer_ = controller_->GetBandwidthObserver();
-        controller_->GetBitrateController()->CreateRtcpBandwidthObserver());
 
     // Set the initial bitrate estimate and expect the |observer| and |pacer_|
     // to be updated.
@@ -135,7 +134,7 @@ class SendSideCongestionControllerTest : public ::testing::Test {
   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_;

modules/remote_bitrate_estimator/test/estimators/remb.cc

@@ -28,7 +28,7 @@ RembBweSender::RembBweSender(int kbps, BitrateObserver* observer, Clock* clock)
           BitrateController::CreateBitrateController(clock,
                                                      observer,
                                                      &event_log_)),
-      feedback_observer_(bitrate_controller_->CreateRtcpBandwidthObserver()),
+      feedback_observer_(bitrate_controller_.get()),
       clock_(clock) {
   assert(kbps >= kMinBitrateKbps);
   assert(kbps <= kMaxBitrateKbps);

modules/remote_bitrate_estimator/test/estimators/remb.h

@@ -42,7 +42,7 @@ class RembBweSender : public BweSender {
 
  protected:
   std::unique_ptr<BitrateController> bitrate_controller_;
-  std::unique_ptr<RtcpBandwidthObserver> feedback_observer_;
+  RtcpBandwidthObserver* feedback_observer_;
 
  private:
   Clock* clock_;

modules/remote_bitrate_estimator/test/estimators/send_side.cc

@@ -33,7 +33,7 @@ SendSideBweSender::SendSideBweSender(int kbps,
       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),

modules/remote_bitrate_estimator/test/estimators/send_side.h

@@ -40,7 +40,7 @@ class SendSideBweSender : public BweSender, public RemoteBitrateObserver {
   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_;

video/video_send_stream.cc

@@ -356,7 +356,7 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver,
   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 @@ VideoSendStreamImpl::VideoSendStreamImpl(
                         config_->rtp.ssrcs,
                         video_stream_encoder),
       protection_bitrate_calculator_(Clock::GetRealTimeClock(), this),
-      bandwidth_observer_(transport->send_side_cc()
+      bandwidth_observer_(transport->send_side_cc()->GetBandwidthObserver()),
-                              ->GetBitrateController()
-                              ->CreateRtcpBandwidthObserver()),
       rtp_rtcp_modules_(CreateRtpRtcpModules(
           config_->send_transport,
           &encoder_feedback_,
-          bandwidth_observer_.get(),
+          bandwidth_observer_,
           transport,
           call_stats_->rtcp_rtt_stats(),
           flexfec_sender_.get(),